Cleanup CreateUIChild node creation

This CL cleans up some of the code around creating the XML nodes related
to the UI widgets.

Change-Id: Ib91364439ab039f46e44690e92cc0cb93a8da203
Reviewed-on: https://pdfium-review.googlesource.com/23770
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

12 files changed, 99 insertions, 83 deletions

diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp
index 46af66ae28..d663fcce70 100644
--- a/fxjs/xfa/cjx_object.cpp
+++ b/fxjs/xfa/cjx_object.cpp
@@ -377,7 +377,7 @@ Optional<int32_t> CJX_Object::TryInteger(XFA_Attribute eAttr,
 }
 
 Optional<XFA_AttributeEnum> CJX_Object::TryEnum(XFA_Attribute eAttr,
-                                                bool bUseDefault) {
+                                                bool bUseDefault) const {
   void* pKey = GetMapKey_Element(GetXFAObject()->GetElementType(), eAttr);
   void* pValue = nullptr;
   if (GetMapModuleValue(pKey, pValue)) {
@@ -402,7 +402,7 @@ bool CJX_Object::SetEnum(XFA_Attribute eAttr,
   return true;
 }
 
-XFA_AttributeEnum CJX_Object::GetEnum(XFA_Attribute eAttr) {
+XFA_AttributeEnum CJX_Object::GetEnum(XFA_Attribute eAttr) const {
   return TryEnum(eAttr, true).value_or(XFA_AttributeEnum::Unknown);
 }
 
@@ -969,8 +969,8 @@ void CJX_Object::SetMapModuleValue(void* pKey, void* pValue) {
   CreateMapModuleData()->m_ValueMap[pKey] = pValue;
 }
 
-bool CJX_Object::GetMapModuleValue(void* pKey, void*& pValue) {
-  for (CXFA_Node* pNode = ToNode(GetXFAObject()); pNode;
+bool CJX_Object::GetMapModuleValue(void* pKey, void*& pValue) const {
+  for (const CXFA_Node* pNode = ToNode(GetXFAObject()); pNode;
        pNode = pNode->GetTemplateNodeIfExists()) {
     XFA_MAPMODULEDATA* pModule = pNode->JSObject()->GetMapModuleData();
     if (pModule) {
diff --git a/fxjs/xfa/cjx_object.h b/fxjs/xfa/cjx_object.h
index c252b45036..deedebc8e5 100644
--- a/fxjs/xfa/cjx_object.h
+++ b/fxjs/xfa/cjx_object.h
@@ -187,9 +187,10 @@ class CJX_Object {
                 bool bScriptModify);
   WideString GetCData(XFA_Attribute eAttr);
 
-  Optional<XFA_AttributeEnum> TryEnum(XFA_Attribute eAttr, bool bUseDefault);
+  Optional<XFA_AttributeEnum> TryEnum(XFA_Attribute eAttr,
+                                      bool bUseDefault) const;
   bool SetEnum(XFA_Attribute eAttr, XFA_AttributeEnum eValue, bool bNotify);
-  XFA_AttributeEnum GetEnum(XFA_Attribute eAttr);
+  XFA_AttributeEnum GetEnum(XFA_Attribute eAttr) const;
 
   Optional<bool> TryBoolean(XFA_Attribute eAttr, bool bUseDefault);
   bool SetBoolean(XFA_Attribute eAttr, bool bValue, bool bNotify);
@@ -255,7 +256,7 @@ class CJX_Object {
   XFA_MAPMODULEDATA* CreateMapModuleData();
   XFA_MAPMODULEDATA* GetMapModuleData() const;
   void SetMapModuleValue(void* pKey, void* pValue);
-  bool GetMapModuleValue(void* pKey, void*& pValue);
+  bool GetMapModuleValue(void* pKey, void*& pValue) const;
   bool GetMapModuleString(void* pKey, WideStringView& wsValue);
   void SetMapModuleBuffer(void* pKey,
                           void* pValue,
diff --git a/xfa/fxfa/parser/cxfa_choicelist.cpp b/xfa/fxfa/parser/cxfa_choicelist.cpp
index 33f868316f..4bd8387917 100644
--- a/xfa/fxfa/parser/cxfa_choicelist.cpp
+++ b/xfa/fxfa/parser/cxfa_choicelist.cpp
@@ -42,3 +42,10 @@ CXFA_ChoiceList::CXFA_ChoiceList(CXFA_Document* doc, XFA_PacketType packet)
                 pdfium::MakeUnique<CJX_ChoiceList>(this)) {}
 
 CXFA_ChoiceList::~CXFA_ChoiceList() {}
+
+XFA_Element CXFA_ChoiceList::GetValueNodeType() const {
+  return JSObject()->GetEnum(XFA_Attribute::Open) ==
+                 XFA_AttributeEnum::MultiSelect
+             ? XFA_Element::ExData
+             : XFA_Element::Text;
+}
diff --git a/xfa/fxfa/parser/cxfa_choicelist.h b/xfa/fxfa/parser/cxfa_choicelist.h
index 9e948807a2..b3384cbbd9 100644
--- a/xfa/fxfa/parser/cxfa_choicelist.h
+++ b/xfa/fxfa/parser/cxfa_choicelist.h
@@ -13,6 +13,8 @@ class CXFA_ChoiceList : public CXFA_Node {
  public:
   CXFA_ChoiceList(CXFA_Document* doc, XFA_PacketType packet);
   ~CXFA_ChoiceList() override;
+
+  XFA_Element GetValueNodeType() const override;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_CHOICELIST_H_
diff --git a/xfa/fxfa/parser/cxfa_datetimeedit.cpp b/xfa/fxfa/parser/cxfa_datetimeedit.cpp
index 5c9a8040ec..0869ce5297 100644
--- a/xfa/fxfa/parser/cxfa_datetimeedit.cpp
+++ b/xfa/fxfa/parser/cxfa_datetimeedit.cpp
@@ -42,3 +42,7 @@ CXFA_DateTimeEdit::CXFA_DateTimeEdit(CXFA_Document* doc, XFA_PacketType packet)
                 pdfium::MakeUnique<CJX_DateTimeEdit>(this)) {}
 
 CXFA_DateTimeEdit::~CXFA_DateTimeEdit() {}
+
+XFA_Element CXFA_DateTimeEdit::GetValueNodeType() const {
+  return XFA_Element::DateTime;
+}
diff --git a/xfa/fxfa/parser/cxfa_datetimeedit.h b/xfa/fxfa/parser/cxfa_datetimeedit.h
index bac7879cdb..802ff77dfd 100644
--- a/xfa/fxfa/parser/cxfa_datetimeedit.h
+++ b/xfa/fxfa/parser/cxfa_datetimeedit.h
@@ -13,6 +13,8 @@ class CXFA_DateTimeEdit : public CXFA_Node {
  public:
   CXFA_DateTimeEdit(CXFA_Document* doc, XFA_PacketType packet);
   ~CXFA_DateTimeEdit() override;
+
+  XFA_Element GetValueNodeType() const override;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_DATETIMEEDIT_H_
diff --git a/xfa/fxfa/parser/cxfa_imageedit.cpp b/xfa/fxfa/parser/cxfa_imageedit.cpp
index 6f2108504e..f8c56cbada 100644
--- a/xfa/fxfa/parser/cxfa_imageedit.cpp
+++ b/xfa/fxfa/parser/cxfa_imageedit.cpp
@@ -39,3 +39,7 @@ CXFA_ImageEdit::CXFA_ImageEdit(CXFA_Document* doc, XFA_PacketType packet)
                 pdfium::MakeUnique<CJX_ImageEdit>(this)) {}
 
 CXFA_ImageEdit::~CXFA_ImageEdit() {}
+
+XFA_Element CXFA_ImageEdit::GetValueNodeType() const {
+  return XFA_Element::Image;
+}
diff --git a/xfa/fxfa/parser/cxfa_imageedit.h b/xfa/fxfa/parser/cxfa_imageedit.h
index 379750cd0f..c41d4d3428 100644
--- a/xfa/fxfa/parser/cxfa_imageedit.h
+++ b/xfa/fxfa/parser/cxfa_imageedit.h
@@ -13,6 +13,8 @@ class CXFA_ImageEdit : public CXFA_Node {
  public:
   CXFA_ImageEdit(CXFA_Document* doc, XFA_PacketType packet);
   ~CXFA_ImageEdit() override;
+
+  XFA_Element GetValueNodeType() const override;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_IMAGEEDIT_H_
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 9a07ef3cac..1f98a27bfa 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2530,38 +2530,43 @@ std::pair<XFA_Element, CXFA_Node*> CXFA_Node::CreateUIChild() {
       JSObject()->GetOrCreateProperty<CXFA_Value>(0, XFA_Element::Value);
   ASSERT(value);
 
-  XFA_Element eValueType = value->GetChildValueClassID();
-  switch (eValueType) {
-    case XFA_Element::Boolean:
-      eUIType = XFA_Element::CheckButton;
-      break;
-    case XFA_Element::Integer:
-    case XFA_Element::Decimal:
-    case XFA_Element::Float:
-      eUIType = XFA_Element::NumericEdit;
-      break;
-    case XFA_Element::ExData:
-    case XFA_Element::Text:
-      eUIType = XFA_Element::TextEdit;
-      eWidgetType = XFA_Element::Text;
-      break;
-    case XFA_Element::Date:
-    case XFA_Element::Time:
-    case XFA_Element::DateTime:
-      eUIType = XFA_Element::DateTimeEdit;
-      break;
-    case XFA_Element::Image:
-      eUIType = XFA_Element::ImageEdit;
-      eWidgetType = XFA_Element::Image;
-      break;
-    case XFA_Element::Arc:
-    case XFA_Element::Line:
-    case XFA_Element::Rectangle:
-      eUIType = XFA_Element::DefaultUi;
-      eWidgetType = eValueType;
-      break;
-    default:
-      break;
+  // The Value nodes only have One-Of children. So, if we have a first child
+  // that child must be the type we want to use.
+  CXFA_Node* child = value->GetFirstChild();
+  if (child) {
+    switch (child->GetElementType()) {
+      case XFA_Element::Boolean:
+        eUIType = XFA_Element::CheckButton;
+        break;
+      case XFA_Element::Integer:
+      case XFA_Element::Decimal:
+      case XFA_Element::Float:
+        eUIType = XFA_Element::NumericEdit;
+        break;
+      case XFA_Element::ExData:
+      case XFA_Element::Text:
+        eUIType = XFA_Element::TextEdit;
+        eWidgetType = XFA_Element::Text;
+        break;
+      case XFA_Element::Date:
+      case XFA_Element::Time:
+      case XFA_Element::DateTime:
+        eUIType = XFA_Element::DateTimeEdit;
+        break;
+      case XFA_Element::Image:
+        eUIType = XFA_Element::ImageEdit;
+        eWidgetType = XFA_Element::Image;
+        break;
+      case XFA_Element::Arc:
+      case XFA_Element::Line:
+      case XFA_Element::Rectangle:
+        eUIType = XFA_Element::DefaultUi;
+        eWidgetType = child->GetElementType();
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
   }
 
   // Both Field and Draw have a UI property. We should always be able to
@@ -2607,57 +2612,37 @@ std::pair<XFA_Element, CXFA_Node*> CXFA_Node::CreateUIChild() {
   }
 
   if (!pUIChild) {
-    if (eUIType == XFA_Element::Unknown) {
+    if (eUIType == XFA_Element::Unknown)
       eUIType = XFA_Element::TextEdit;
-      value->JSObject()->GetOrCreateProperty<CXFA_Text>(0, XFA_Element::Text);
-    }
-    return {eWidgetType,
-            pUI->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eUIType)};
+    pUIChild = pUI->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eUIType);
   }
 
-  if (eUIType != XFA_Element::Unknown)
-    return {eWidgetType, pUIChild};
+  CreateValueNodeIfNeeded(value, pUIChild);
+  return {eWidgetType, pUIChild};
+}
 
-  switch (pUIChild->GetElementType()) {
-    case XFA_Element::CheckButton: {
-      eValueType = XFA_Element::Text;
-      if (CXFA_Items* pItems =
-              GetChild<CXFA_Items>(0, XFA_Element::Items, false)) {
-        if (CXFA_Node* pItem =
-                pItems->GetChild<CXFA_Node>(0, XFA_Element::Unknown, false)) {
-          eValueType = pItem->GetElementType();
-        }
-      }
-      break;
-    }
-    case XFA_Element::DateTimeEdit:
-      eValueType = XFA_Element::DateTime;
-      break;
-    case XFA_Element::ImageEdit:
-      eValueType = XFA_Element::Image;
-      break;
-    case XFA_Element::NumericEdit:
-      eValueType = XFA_Element::Float;
-      break;
-    case XFA_Element::ChoiceList: {
-      eValueType = (pUIChild->JSObject()->GetEnum(XFA_Attribute::Open) ==
-                    XFA_AttributeEnum::MultiSelect)
-                       ? XFA_Element::ExData
-                       : XFA_Element::Text;
-      break;
+void CXFA_Node::CreateValueNodeIfNeeded(CXFA_Value* value,
+                                        CXFA_Node* pUIChild) {
+  // Value nodes only have one child. If we have one already we're done.
+  if (value->GetFirstChild() != nullptr)
+    return;
+
+  // Create the Value node for our UI if needed.
+  XFA_Element valueType = pUIChild->GetValueNodeType();
+  if (pUIChild->GetElementType() == XFA_Element::CheckButton) {
+    CXFA_Items* pItems = GetChild<CXFA_Items>(0, XFA_Element::Items, false);
+    if (pItems) {
+      CXFA_Node* pItem =
+          pItems->GetChild<CXFA_Node>(0, XFA_Element::Unknown, false);
+      if (pItem)
+        valueType = pItem->GetElementType();
     }
-    case XFA_Element::Barcode:
-    case XFA_Element::Button:
-    case XFA_Element::PasswordEdit:
-    case XFA_Element::Signature:
-    case XFA_Element::TextEdit:
-    default:
-      eValueType = XFA_Element::Text;
-      break;
   }
-  value->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eValueType);
+  value->JSObject()->GetOrCreateProperty<CXFA_Node>(0, valueType);
+}
 
-  return {eWidgetType, pUIChild};
+XFA_Element CXFA_Node::GetValueNodeType() const {
+  return XFA_Element::Text;
 }
 
 CXFA_Node* CXFA_Node::GetUIChild() {
diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h
index 451e90c15b..5e1a31c63b 100644
--- a/xfa/fxfa/parser/cxfa_node.h
+++ b/xfa/fxfa/parser/cxfa_node.h
@@ -437,6 +437,8 @@ class CXFA_Node : public CXFA_Object {
                           int32_t iLead,
                           int32_t iTread) const;
 
+  virtual XFA_Element GetValueNodeType() const;
+
  protected:
   CXFA_Node(CXFA_Document* pDoc,
             XFA_PacketType ePacket,
@@ -515,6 +517,7 @@ class CXFA_Node : public CXFA_Object {
   WideString FormatNumStr(const WideString& wsValue, IFX_Locale* pLocale);
   void GetItemLabel(const WideStringView& wsValue, WideString& wsLabel);
   std::pair<XFA_Element, CXFA_Node*> CreateUIChild();
+  void CreateValueNodeIfNeeded(CXFA_Value* value, CXFA_Node* pUIChild);
 
   const PropertyData* const m_Properties;
   const AttributeData* const m_Attributes;
diff --git a/xfa/fxfa/parser/cxfa_numericedit.cpp b/xfa/fxfa/parser/cxfa_numericedit.cpp
index 5a65c763b2..0c282db88f 100644
--- a/xfa/fxfa/parser/cxfa_numericedit.cpp
+++ b/xfa/fxfa/parser/cxfa_numericedit.cpp
@@ -40,3 +40,7 @@ CXFA_NumericEdit::CXFA_NumericEdit(CXFA_Document* doc, XFA_PacketType packet)
                 pdfium::MakeUnique<CJX_NumericEdit>(this)) {}
 
 CXFA_NumericEdit::~CXFA_NumericEdit() {}
+
+XFA_Element CXFA_NumericEdit::GetValueNodeType() const {
+  return XFA_Element::Float;
+}
diff --git a/xfa/fxfa/parser/cxfa_numericedit.h b/xfa/fxfa/parser/cxfa_numericedit.h
index d9cfd7e57d..a2a10eb0d0 100644
--- a/xfa/fxfa/parser/cxfa_numericedit.h
+++ b/xfa/fxfa/parser/cxfa_numericedit.h
@@ -13,6 +13,8 @@ class CXFA_NumericEdit : public CXFA_Node {
  public:
   CXFA_NumericEdit(CXFA_Document* doc, XFA_PacketType packet);
   ~CXFA_NumericEdit() override;
+
+  XFA_Element GetValueNodeType() const override;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_NUMERICEDIT_H_