commit 59a3738a809a2ca20d74f9591a36ebf08bfea5ba
author Lei Zhang <thestig@chromium.org> Thu Sep 24 21:03:36 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Sep 24 21:03:36 2020 +0000
tree b6b308e9f9fe57c7689b55dfaaeebc61f6e98aa8
parent a16f7079957e9c073a6efa2c6789e3a16ddff6e2

Clean up CXFA_Node::AddBindItem() and RemoveBindItem().

- Simplify AddBindItem().
- Make AddBindItem() return void, since no one checks the result.
- Make RemoveBindItem() return bool, since that is what the callers care
  about.

Change-Id: I91ab6c9d183dde19a1adf973db790625667f339e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/74051
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

xfa/fxfa/parser/cxfa_node.cpp

diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index c675844..8b9de5c 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -1289,38 +1289,30 @@
   return std::vector<CXFA_Node*>(binding_nodes_.begin(), binding_nodes_.end());
 }
 
-int32_t CXFA_Node::AddBindItem(CXFA_Node* pFormNode) {
+void CXFA_Node::AddBindItem(CXFA_Node* pFormNode) {
   ASSERT(pFormNode);
 
   if (BindsFormItems()) {
-    bool found = false;
-    for (const auto& v : binding_nodes_) {
-      if (v == pFormNode) {
-        found = true;
-        break;
-      }
-    }
-    if (!found)
+    if (!pdfium::Contains(binding_nodes_, pFormNode))
       binding_nodes_.emplace_back(pFormNode);
-    return pdfium::CollectionSize<int32_t>(binding_nodes_);
+    return;
   }
 
   CXFA_Node* pOldFormItem = GetBindingNode();
   if (!pOldFormItem) {
     SetBindingNode(pFormNode);
-    return 1;
+    return;
   }
   if (pOldFormItem == pFormNode)
-    return 1;
+    return;
 
   binding_nodes_.clear();
   binding_nodes_.push_back(pOldFormItem);
   binding_nodes_.push_back(pFormNode);
   m_uNodeFlags |= XFA_NodeFlag_BindFormItems;
-  return 2;
 }
 
-int32_t CXFA_Node::RemoveBindItem(CXFA_Node* pFormNode) {
+bool CXFA_Node::RemoveBindItem(CXFA_Node* pFormNode) {
   if (BindsFormItems()) {
     auto it =
         std::find(binding_nodes_.begin(), binding_nodes_.end(), pFormNode);
@@ -1329,17 +1321,17 @@
 
     if (binding_nodes_.size() == 1) {
       m_uNodeFlags &= ~XFA_NodeFlag_BindFormItems;
-      return 1;
+      return true;
     }
-    return pdfium::CollectionSize<int32_t>(binding_nodes_);
+    return !binding_nodes_.empty();
   }
 
   CXFA_Node* pOldFormItem = GetBindingNode();
   if (pOldFormItem != pFormNode)
-    return pOldFormItem ? 1 : 0;
+    return !!pOldFormItem;
 
   SetBindingNode(nullptr);
-  return 0;
+  return false;
 }
 
 bool CXFA_Node::HasBindItem() const {
@@ -1983,7 +1975,7 @@
     if (!pDataNode)
       continue;
 
-    if (pDataNode->RemoveBindItem(pFormNode) == 0) {
+    if (!pDataNode->RemoveBindItem(pFormNode)) {
       if (CXFA_Node* pDataParent = pDataNode->GetParent()) {
         pDataParent->RemoveChildAndNotify(pDataNode, true);
       }

xfa/fxfa/parser/cxfa_node.h

diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h
index fdea32a..f32cfae 100644
--- a/xfa/fxfa/parser/cxfa_node.h
+++ b/xfa/fxfa/parser/cxfa_node.h
@@ -192,8 +192,9 @@
   CXFA_Node* GetBindData();
   bool HasBindItems() const { return !binding_nodes_.empty(); }
   std::vector<CXFA_Node*> GetBindItemsCopy() const;
-  int32_t AddBindItem(CXFA_Node* pFormNode);
-  int32_t RemoveBindItem(CXFA_Node* pFormNode);
+  void AddBindItem(CXFA_Node* pFormNode);
+  // Returns true if there are still more items.
+  bool RemoveBindItem(CXFA_Node* pFormNode);
   bool HasBindItem() const;
   CXFA_Node* GetContainerNode();
   LocaleIface* GetLocale();