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>

10 files changed, 91 insertions, 76 deletions

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_