Make methods which create nodes more obvious

This CL converts the various methods Get methods which take a boolean
value to explicit Get* and GetOrCreate* methods to make the usage
clearer.

Change-Id: I2af68448b1b69b95713e739bf7fe14a4336d2b65
Reviewed-on: https://pdfium-review.googlesource.com/22590
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

5 files changed, 66 insertions, 46 deletions

diff --git a/fxjs/cfxjse_engine.cpp b/fxjs/cfxjse_engine.cpp
index 179866b555..535ec41160 100644
--- a/fxjs/cfxjse_engine.cpp
+++ b/fxjs/cfxjse_engine.cpp
@@ -351,10 +351,12 @@ void CFXJSE_Engine::NormalPropertySetter(CFXJSE_Value* pOriginalValue,
     CXFA_Node* pNode = ToNode(pObject);
     CXFA_Node* pPropOrChild = nullptr;
     XFA_Element eType = CXFA_Node::NameToElement(wsPropName);
-    if (eType != XFA_Element::Unknown)
-      pPropOrChild = pNode->JSObject()->GetProperty<CXFA_Node>(0, eType, true);
-    else
+    if (eType != XFA_Element::Unknown) {
+      pPropOrChild =
+          pNode->JSObject()->GetOrCreateProperty<CXFA_Node>(0, eType);
+    } else {
       pPropOrChild = pNode->GetFirstChildByName(wsPropName.AsStringView());
+    }
 
     if (pPropOrChild) {
       const XFA_SCRIPTATTRIBUTEINFO* lpAttrInfo = XFA_GetScriptAttributeByName(
diff --git a/fxjs/cfxjse_resolveprocessor.cpp b/fxjs/cfxjse_resolveprocessor.cpp
index beb0541a49..b602db24ec 100644
--- a/fxjs/cfxjse_resolveprocessor.cpp
+++ b/fxjs/cfxjse_resolveprocessor.cpp
@@ -346,14 +346,16 @@ bool CFXJSE_ResolveProcessor::ResolveNormal(CFXJSE_ResolveNodeData& rnd) {
       CXFA_Node* pInstanceManager =
           curNode->AsNode()->GetInstanceMgrOfSubform();
       if (pInstanceManager) {
-        pProp = pInstanceManager->JSObject()->GetProperty<CXFA_Occur>(
-            0, XFA_Element::Occur, true);
+        pProp = pInstanceManager->JSObject()->GetOrCreateProperty<CXFA_Occur>(
+            0, XFA_Element::Occur);
       }
     } else {
       XFA_Element eType = CXFA_Node::NameToElement(wsName);
-      if (eType != XFA_Element::Unknown) {
-        pProp = curNode->AsNode()->JSObject()->GetProperty<CXFA_Node>(
-            0, eType, eType != XFA_Element::PageSet);
+      if (eType == XFA_Element::PageSet) {
+        pProp = curNode->AsNode()->JSObject()->GetProperty<CXFA_Node>(0, eType);
+      } else if (eType != XFA_Element::Unknown) {
+        pProp = curNode->AsNode()->JSObject()->GetOrCreateProperty<CXFA_Node>(
+            0, eType);
       }
     }
     if (pProp) {
diff --git a/fxjs/xfa/cjx_node.cpp b/fxjs/xfa/cjx_node.cpp
index c0634767cf..581d3281ef 100644
--- a/fxjs/xfa/cjx_node.cpp
+++ b/fxjs/xfa/cjx_node.cpp
@@ -163,8 +163,8 @@ CJS_Return CJX_Node::getElement(
   WideString expression = runtime->ToWideString(params[0]);
   int32_t iValue = params.size() >= 2 ? runtime->ToInt32(params[1]) : 0;
 
-  CXFA_Node* pNode = GetProperty<CXFA_Node>(
-      iValue, CXFA_Node::NameToElement(expression), true);
+  CXFA_Node* pNode = GetOrCreateProperty<CXFA_Node>(
+      iValue, CXFA_Node::NameToElement(expression));
   CFXJSE_Value* value =
       GetDocument()->GetScriptContext()->GetJSValueFromMap(pNode);
   if (!value)
@@ -186,12 +186,12 @@ CJS_Return CJX_Node::isPropertySpecified(
   bool bParent = params.size() < 2 || runtime->ToBoolean(params[1]);
   int32_t iIndex = params.size() == 3 ? runtime->ToInt32(params[2]) : 0;
   XFA_Element eType = CXFA_Node::NameToElement(expression);
-  bool bHas = !!GetProperty<CXFA_Node>(iIndex, eType, true);
+  bool bHas = !!GetOrCreateProperty<CXFA_Node>(iIndex, eType);
   if (!bHas && bParent && GetXFANode()->GetParent()) {
     // Also check on the parent.
     auto* jsnode = GetXFANode()->GetParent()->JSObject();
     bHas = jsnode->HasAttribute(attr) ||
-           !!jsnode->GetProperty<CXFA_Node>(iIndex, eType, true);
+           !!jsnode->GetOrCreateProperty<CXFA_Node>(iIndex, eType);
   }
   return CJS_Return(runtime->NewBoolean(bHas));
 }
diff --git a/fxjs/xfa/cjx_object.cpp b/fxjs/xfa/cjx_object.cpp
index 0528bd5edd..45089f68da 100644
--- a/fxjs/xfa/cjx_object.cpp
+++ b/fxjs/xfa/cjx_object.cpp
@@ -629,7 +629,7 @@ bool CJX_Object::SetContent(const WideString& wsContent,
     case XFA_ObjectType::ContainerNode: {
       if (XFA_FieldIsMultiListBox(ToNode(GetXFAObject()))) {
         CXFA_Value* pValue =
-            GetProperty<CXFA_Value>(0, XFA_Element::Value, true);
+            GetOrCreateProperty<CXFA_Value>(0, XFA_Element::Value);
         if (!pValue)
           break;
 
@@ -714,7 +714,7 @@ bool CJX_Object::SetContent(const WideString& wsContent,
         pNode = ToNode(GetXFAObject());
       } else {
         CXFA_Value* pValue =
-            GetProperty<CXFA_Value>(0, XFA_Element::Value, true);
+            GetOrCreateProperty<CXFA_Value>(0, XFA_Element::Value);
         if (!pValue)
           break;
 
@@ -908,33 +908,42 @@ Optional<WideString> CJX_Object::TryNamespace() {
   return {static_cast<CFX_XMLElement*>(pXMLNode)->GetNamespaceURI()};
 }
 
-CXFA_Node* CJX_Object::GetPropertyInternal(int32_t index,
-                                           XFA_Element eProperty,
-                                           bool bCreateProperty) {
-  if (index < 0 ||
-      index >= ToNode(GetXFAObject())->PropertyOccuranceCount(eProperty)) {
-    return nullptr;
-  }
+std::pair<CXFA_Node*, int32_t> CJX_Object::GetPropertyInternal(
+    int32_t index,
+    XFA_Element eProperty) const {
+  const CXFA_Node* xfaNode = ToNode(GetXFAObject());
+  if (index < 0 || index >= xfaNode->PropertyOccuranceCount(eProperty))
+    return {nullptr, 0};
 
   int32_t iCount = 0;
-  for (CXFA_Node* pNode = ToNode(GetXFAObject())->GetChildNode(); pNode;
+  for (CXFA_Node* pNode = xfaNode->GetChildNode(); pNode;
        pNode = pNode->GetNodeItem(XFA_NODEITEM_NextSibling)) {
     if (pNode->GetElementType() == eProperty) {
       iCount++;
       if (iCount > index)
-        return pNode;
+        return {pNode, iCount};
     }
   }
-  if (!bCreateProperty)
+  return {nullptr, iCount};
+}
+
+CXFA_Node* CJX_Object::GetOrCreatePropertyInternal(int32_t index,
+                                                   XFA_Element eProperty) {
+  CXFA_Node* xfaNode = ToNode(GetXFAObject());
+  if (index < 0 || index >= xfaNode->PropertyOccuranceCount(eProperty))
     return nullptr;
 
-  if (ToNode(GetXFAObject())
-          ->HasPropertyFlags(eProperty, XFA_PROPERTYFLAG_OneOf)) {
-    for (CXFA_Node* pNode = ToNode(GetXFAObject())->GetChildNode(); pNode;
+  int32_t iCount = 0;
+  CXFA_Node* node;
+  std::tie(node, iCount) = GetPropertyInternal(index, eProperty);
+  if (node)
+    return node;
+
+  if (xfaNode->HasPropertyFlags(eProperty, XFA_PROPERTYFLAG_OneOf)) {
+    for (CXFA_Node* pNode = xfaNode->GetChildNode(); pNode;
          pNode = pNode->GetNodeItem(XFA_NODEITEM_NextSibling)) {
-      if (ToNode(GetXFAObject())
-              ->HasPropertyFlags(pNode->GetElementType(),
-                                 XFA_PROPERTYFLAG_OneOf)) {
+      if (xfaNode->HasPropertyFlags(pNode->GetElementType(),
+                                    XFA_PROPERTYFLAG_OneOf)) {
         return nullptr;
       }
     }
@@ -942,11 +951,11 @@ CXFA_Node* CJX_Object::GetPropertyInternal(int32_t index,
 
   CXFA_Node* pNewNode = nullptr;
   for (; iCount <= index; ++iCount) {
-    pNewNode = GetDocument()->CreateNode(
-        ToNode(GetXFAObject())->GetPacketType(), eProperty);
+    pNewNode = GetDocument()->CreateNode(xfaNode->GetPacketType(), eProperty);
     if (!pNewNode)
       return nullptr;
-    ToNode(GetXFAObject())->InsertChild(pNewNode, nullptr);
+
+    xfaNode->InsertChild(pNewNode, nullptr);
     pNewNode->SetFlag(XFA_NodeFlag_Initialized, true);
   }
   return pNewNode;
@@ -1304,7 +1313,7 @@ void CJX_Object::Script_Attribute_Integer(CFXJSE_Value* pValue,
 void CJX_Object::Script_Som_FontColor(CFXJSE_Value* pValue,
                                       bool bSetting,
                                       XFA_Attribute eAttribute) {
-  CXFA_Font* font = ToNode(object_.Get())->GetFont(true);
+  CXFA_Font* font = ToNode(object_.Get())->GetOrCreateFont();
   if (!font)
     return;
 
@@ -1329,8 +1338,8 @@ void CJX_Object::Script_Som_FontColor(CFXJSE_Value* pValue,
 void CJX_Object::Script_Som_FillColor(CFXJSE_Value* pValue,
                                       bool bSetting,
                                       XFA_Attribute eAttribute) {
-  CXFA_Border* border = ToNode(object_.Get())->GetBorder(true);
-  CXFA_Fill* borderfill = border->GetFill(true);
+  CXFA_Border* border = ToNode(object_.Get())->GetOrCreateBorder();
+  CXFA_Fill* borderfill = border->GetOrCreateFill();
   if (!borderfill)
     return;
 
@@ -1357,7 +1366,7 @@ void CJX_Object::Script_Som_FillColor(CFXJSE_Value* pValue,
 void CJX_Object::Script_Som_BorderColor(CFXJSE_Value* pValue,
                                         bool bSetting,
                                         XFA_Attribute eAttribute) {
-  CXFA_Border* border = ToNode(object_.Get())->GetBorder(true);
+  CXFA_Border* border = ToNode(object_.Get())->GetOrCreateBorder();
   int32_t iSize = border->CountEdges();
   if (bSetting) {
     int32_t r = 0;
@@ -1384,7 +1393,7 @@ void CJX_Object::Script_Som_BorderColor(CFXJSE_Value* pValue,
 void CJX_Object::Script_Som_BorderWidth(CFXJSE_Value* pValue,
                                         bool bSetting,
                                         XFA_Attribute eAttribute) {
-  CXFA_Border* border = ToNode(object_.Get())->GetBorder(true);
+  CXFA_Border* border = ToNode(object_.Get())->GetOrCreateBorder();
   if (bSetting) {
     CXFA_Measurement thickness = border->GetEdge(0)->GetMSThickness();
     pValue->SetString(thickness.ToString().UTF8Encode().AsStringView());
@@ -1402,9 +1411,9 @@ void CJX_Object::Script_Som_Message(CFXJSE_Value* pValue,
                                     bool bSetting,
                                     XFA_SOM_MESSAGETYPE iMessageType) {
   bool bNew = false;
-  CXFA_Validate* validate = ToNode(object_.Get())->GetValidate(false);
+  CXFA_Validate* validate = ToNode(object_.Get())->GetValidate();
   if (!validate) {
-    validate = ToNode(object_.Get())->GetValidate(true);
+    validate = ToNode(object_.Get())->GetOrCreateValidate();
     bNew = true;
   }
 
@@ -1586,7 +1595,7 @@ void CJX_Object::Script_Som_DataNode(CFXJSE_Value* pValue,
 void CJX_Object::Script_Som_Mandatory(CFXJSE_Value* pValue,
                                       bool bSetting,
                                       XFA_Attribute eAttribute) {
-  CXFA_Validate* validate = ToNode(object_.Get())->GetValidate(true);
+  CXFA_Validate* validate = ToNode(object_.Get())->GetOrCreateValidate();
   if (!validate)
     return;
 
diff --git a/fxjs/xfa/cjx_object.h b/fxjs/xfa/cjx_object.h
index 87a164ff8b..c252b45036 100644
--- a/fxjs/xfa/cjx_object.h
+++ b/fxjs/xfa/cjx_object.h
@@ -96,8 +96,15 @@ class CJX_Object {
   WideString GetContent(bool bScriptModify);
 
   template <typename T>
-  T* GetProperty(int32_t index, XFA_Element eType, bool bCreateProperty) {
-    return static_cast<T*>(GetPropertyInternal(index, eType, bCreateProperty));
+  T* GetProperty(int32_t index, XFA_Element eType) const {
+    CXFA_Node* node;
+    int32_t count;
+    std::tie(node, count) = GetPropertyInternal(index, eType);
+    return static_cast<T*>(node);
+  }
+  template <typename T>
+  T* GetOrCreateProperty(int32_t index, XFA_Element eType) {
+    return static_cast<T*>(GetOrCreatePropertyInternal(index, eType));
   }
 
   void SetAttributeValue(const WideString& wsValue,
@@ -227,9 +234,9 @@ class CJX_Object {
                                  bool bSetting,
                                  XFA_Attribute eAttribute);
 
-  CXFA_Node* GetPropertyInternal(int32_t index,
-                                 XFA_Element eType,
-                                 bool bCreateProperty);
+  std::pair<CXFA_Node*, int32_t> GetPropertyInternal(int32_t index,
+                                                     XFA_Element eType) const;
+  CXFA_Node* GetOrCreatePropertyInternal(int32_t index, XFA_Element eType);
 
   void OnChanged(XFA_Attribute eAttr, bool bNotify, bool bScriptModify);
   void OnChanging(XFA_Attribute eAttr, bool bNotify);