Cleanup CreateUIChild pUI usage

This CL cleans up the usage of pUI in CreateUIChild. The node can't be
null per spec so ASSERT that's true. Cleanup the search for children to
make it clearer what is happening.

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

3 files changed, 22 insertions, 14 deletions

diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 970459f6ed..9a07ef3cac 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -2564,19 +2564,17 @@ std::pair<XFA_Element, CXFA_Node*> CXFA_Node::CreateUIChild() {
       break;
   }
 
-  CXFA_Node* pUIChild = nullptr;
+  // Both Field and Draw have a UI property. We should always be able to
+  // retrieve or create the UI element. If we can't something is wrong.
   CXFA_Ui* pUI = JSObject()->GetOrCreateProperty<CXFA_Ui>(0, XFA_Element::Ui);
-  CXFA_Node* pChild = pUI ? pUI->GetFirstChild() : nullptr;
-  for (; pChild; pChild = pChild->GetNextSibling()) {
-    XFA_Element eChildType = pChild->GetElementType();
-    if (eChildType == XFA_Element::Extras ||
-        eChildType == XFA_Element::Picture) {
-      continue;
-    }
+  ASSERT(pUI);
 
-    auto node = CXFA_Node::Create(pChild->GetDocument(), XFA_Element::Ui,
-                                  XFA_PacketType::Form);
-    if (node && node->HasPropertyFlags(eChildType, XFA_PROPERTYFLAG_OneOf)) {
+  CXFA_Node* pUIChild = nullptr;
+  // Search through the children of the UI node to see if we have any of our
+  // One-Of entries. If so, that is the node associated with our UI.
+  for (CXFA_Node* pChild = pUI->GetFirstChild(); pChild;
+       pChild = pChild->GetNextSibling()) {
+    if (pUI->IsAOneOfChild(pChild)) {
       pUIChild = pChild;
       break;
     }
@@ -2614,8 +2612,7 @@ std::pair<XFA_Element, CXFA_Node*> CXFA_Node::CreateUIChild() {
       value->JSObject()->GetOrCreateProperty<CXFA_Text>(0, XFA_Element::Text);
     }
     return {eWidgetType,
-            pUI ? pUI->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eUIType)
-                : nullptr};
+            pUI->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eUIType)};
   }
 
   if (eUIType != XFA_Element::Unknown)
diff --git a/xfa/fxfa/parser/cxfa_ui.cpp b/xfa/fxfa/parser/cxfa_ui.cpp
index 883e79ac59..e6a3963334 100644
--- a/xfa/fxfa/parser/cxfa_ui.cpp
+++ b/xfa/fxfa/parser/cxfa_ui.cpp
@@ -24,7 +24,6 @@ const CXFA_Node::PropertyData kPropertyData[] = {
     {XFA_Element::NumericEdit, 1, XFA_PROPERTYFLAG_OneOf},
     {XFA_Element::Signature, 1, XFA_PROPERTYFLAG_OneOf},
     {XFA_Element::TextEdit, 1, XFA_PROPERTYFLAG_OneOf},
-    {XFA_Element::ExObject, 1, XFA_PROPERTYFLAG_OneOf},
     {XFA_Element::Extras, 1, 0},
     {XFA_Element::Unknown, 0, 0}};
 const CXFA_Node::AttributeData kAttributeData[] = {
@@ -49,3 +48,13 @@ CXFA_Ui::CXFA_Ui(CXFA_Document* doc, XFA_PacketType packet)
                 pdfium::MakeUnique<CJX_Ui>(this)) {}
 
 CXFA_Ui::~CXFA_Ui() {}
+
+bool CXFA_Ui::IsAOneOfChild(CXFA_Node* child) const {
+  for (auto& prop : kPropertyData) {
+    if (prop.property != child->GetElementType())
+      continue;
+    if (!!(prop.flags & XFA_PROPERTYFLAG_OneOf))
+      return true;
+  }
+  return false;
+}
diff --git a/xfa/fxfa/parser/cxfa_ui.h b/xfa/fxfa/parser/cxfa_ui.h
index 0824d6b88b..a4c46f493c 100644
--- a/xfa/fxfa/parser/cxfa_ui.h
+++ b/xfa/fxfa/parser/cxfa_ui.h
@@ -13,6 +13,8 @@ class CXFA_Ui : public CXFA_Node {
  public:
   CXFA_Ui(CXFA_Document* doc, XFA_PacketType packet);
   ~CXFA_Ui() override;
+
+  bool IsAOneOfChild(CXFA_Node* child) const;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_UI_H_