Use sets of members in CFXA_Node node shuffling code.

This is not essential for the first versions of cppgc, since we
are not GCing while the stack is still active.

-- avoid some indirection while at it.

Change-Id: I1bacdb3b21d6cd9a5bbc5b3f19666d8b22594a3c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73851
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@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 212d7b0..c675844 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -546,12 +546,19 @@
   return true;
 }
 
-std::vector<CXFA_Node*> NodesSortedByDocumentIdx(
-    const std::set<CXFA_Node*>& rgNodeSet) {
-  if (rgNodeSet.empty())
-    return std::vector<CXFA_Node*>();
+// Stack allocated. Using containers of members would be correct here
+// if advanced GC worked with STL.
+using NodeSet = std::set<cppgc::Member<CXFA_Node>>;
+using NodeSetPair = std::pair<NodeSet, NodeSet>;
+using NodeSetPairMap = std::map<uint32_t, NodeSetPair>;
+using NodeSetPairMapMap = std::map<CXFA_Node*, NodeSetPairMap>;
+using NodeVector = std::vector<cppgc::Member<CXFA_Node>>;
 
-  std::vector<CXFA_Node*> rgNodeArray;
+NodeVector NodesSortedByDocumentIdx(const NodeSet& rgNodeSet) {
+  if (rgNodeSet.empty())
+    return NodeVector();
+
+  NodeVector rgNodeArray;
   CXFA_Node* pCommonParent = (*rgNodeSet.begin())->GetParent();
   for (CXFA_Node* pNode = pCommonParent->GetFirstChild(); pNode;
        pNode = pNode->GetNextSibling()) {
@@ -561,40 +568,26 @@
   return rgNodeArray;
 }
 
-using CXFA_NodeSetPair = std::pair<std::set<CXFA_Node*>, std::set<CXFA_Node*>>;
-using CXFA_NodeSetPairMap =
-    std::map<uint32_t, std::unique_ptr<CXFA_NodeSetPair>>;
-using CXFA_NodeSetPairMapMap =
-    std::map<CXFA_Node*, std::unique_ptr<CXFA_NodeSetPairMap>>;
-
-CXFA_NodeSetPair* NodeSetPairForNode(CXFA_Node* pNode,
-                                     CXFA_NodeSetPairMapMap* pMap) {
+NodeSetPair* NodeSetPairForNode(CXFA_Node* pNode, NodeSetPairMapMap* pMap) {
   CXFA_Node* pParentNode = pNode->GetParent();
   uint32_t dwNameHash = pNode->GetNameHash();
   if (!pParentNode || !dwNameHash)
     return nullptr;
 
-  if (!(*pMap)[pParentNode])
-    (*pMap)[pParentNode] = std::make_unique<CXFA_NodeSetPairMap>();
-
-  CXFA_NodeSetPairMap* pNodeSetPairMap = (*pMap)[pParentNode].get();
-  if (!(*pNodeSetPairMap)[dwNameHash])
-    (*pNodeSetPairMap)[dwNameHash] = std::make_unique<CXFA_NodeSetPair>();
-
-  return (*pNodeSetPairMap)[dwNameHash].get();
+  return &((*pMap)[pParentNode][dwNameHash]);
 }
 
-void ReorderDataNodes(const std::set<CXFA_Node*>& sSet1,
-                      const std::set<CXFA_Node*>& sSet2,
+void ReorderDataNodes(const NodeSet& sSet1,
+                      const NodeSet& sSet2,
                       bool bInsertBefore) {
-  CXFA_NodeSetPairMapMap rgMap;
+  NodeSetPairMapMap rgMap;
   for (CXFA_Node* pNode : sSet1) {
-    CXFA_NodeSetPair* pNodeSetPair = NodeSetPairForNode(pNode, &rgMap);
+    NodeSetPair* pNodeSetPair = NodeSetPairForNode(pNode, &rgMap);
     if (pNodeSetPair)
       pNodeSetPair->first.insert(pNode);
   }
   for (CXFA_Node* pNode : sSet2) {
-    CXFA_NodeSetPair* pNodeSetPair = NodeSetPairForNode(pNode, &rgMap);
+    NodeSetPair* pNodeSetPair = NodeSetPairForNode(pNode, &rgMap);
     if (pNodeSetPair) {
       if (pdfium::Contains(pNodeSetPair->first, pNode))
         pNodeSetPair->first.erase(pNode);
@@ -602,19 +595,13 @@
         pNodeSetPair->second.insert(pNode);
     }
   }
-  for (const auto& iter1 : rgMap) {
-    CXFA_NodeSetPairMap* pNodeSetPairMap = iter1.second.get();
-    if (!pNodeSetPairMap)
-      continue;
-
-    for (const auto& iter2 : *pNodeSetPairMap) {
-      CXFA_NodeSetPair* pNodeSetPair = iter2.second.get();
-      if (!pNodeSetPair)
-        continue;
+  for (auto& iter1 : rgMap) {
+    NodeSetPairMap* pNodeSetPairMap = &iter1.second;
+    for (auto& iter2 : *pNodeSetPairMap) {
+      NodeSetPair* pNodeSetPair = &iter2.second;
       if (!pNodeSetPair->first.empty() && !pNodeSetPair->second.empty()) {
-        std::vector<CXFA_Node*> rgNodeArray1 =
-            NodesSortedByDocumentIdx(pNodeSetPair->first);
-        std::vector<CXFA_Node*> rgNodeArray2 =
+        NodeVector rgNodeArray1 = NodesSortedByDocumentIdx(pNodeSetPair->first);
+        NodeVector rgNodeArray2 =
             NodesSortedByDocumentIdx(pNodeSetPair->second);
         CXFA_Node* pParentNode = nullptr;
         CXFA_Node* pBeforeNode = nullptr;
@@ -626,7 +613,7 @@
           pParentNode = pLastNode->GetParent();
           pBeforeNode = pLastNode->GetNextSibling();
         }
-        for (auto* pCurNode : rgNodeArray1) {
+        for (auto& pCurNode : rgNodeArray1) {
           pParentNode->RemoveChildAndNotify(pCurNode, true);
           pParentNode->InsertChildAndNotify(pCurNode, pBeforeNode);
         }
@@ -1926,29 +1913,25 @@
         iCount > 0 ? item->GetNextSibling() : GetNextSibling();
     GetParent()->InsertChildAndNotify(pNewInstance, pNextSibling);
     if (bMoveDataBindingNodes) {
-      std::set<CXFA_Node*> sNew;
-      std::set<CXFA_Node*> sAfter;
+      NodeSet sNew;
       CXFA_NodeIteratorTemplate<CXFA_Node,
                                 CXFA_TraverseStrategy_XFAContainerNode>
           sIteratorNew(pNewInstance);
       for (CXFA_Node* pNode = sIteratorNew.GetCurrent(); pNode;
            pNode = sIteratorNew.MoveToNext()) {
         CXFA_Node* pDataNode = pNode->GetBindData();
-        if (!pDataNode)
-          continue;
-
-        sNew.insert(pDataNode);
+        if (pDataNode)
+          sNew.insert(pDataNode);
       }
+      NodeSet sAfter;
       CXFA_NodeIteratorTemplate<CXFA_Node,
                                 CXFA_TraverseStrategy_XFAContainerNode>
           sIteratorAfter(pNextSibling);
       for (CXFA_Node* pNode = sIteratorAfter.GetCurrent(); pNode;
            pNode = sIteratorAfter.MoveToNext()) {
         CXFA_Node* pDataNode = pNode->GetBindData();
-        if (!pDataNode)
-          continue;
-
-        sAfter.insert(pDataNode);
+        if (pDataNode)
+          sAfter.insert(pDataNode);
       }
       ReorderDataNodes(sNew, sAfter, false);
     }
@@ -1961,29 +1944,25 @@
 
     GetParent()->InsertChildAndNotify(pNewInstance, pBeforeInstance);
     if (bMoveDataBindingNodes) {
-      std::set<CXFA_Node*> sNew;
-      std::set<CXFA_Node*> sBefore;
+      NodeSet sNew;
       CXFA_NodeIteratorTemplate<CXFA_Node,
                                 CXFA_TraverseStrategy_XFAContainerNode>
           sIteratorNew(pNewInstance);
       for (CXFA_Node* pNode = sIteratorNew.GetCurrent(); pNode;
            pNode = sIteratorNew.MoveToNext()) {
         CXFA_Node* pDataNode = pNode->GetBindData();
-        if (!pDataNode)
-          continue;
-
-        sNew.insert(pDataNode);
+        if (pDataNode)
+          sNew.insert(pDataNode);
       }
+      NodeSet sBefore;
       CXFA_NodeIteratorTemplate<CXFA_Node,
                                 CXFA_TraverseStrategy_XFAContainerNode>
           sIteratorBefore(pBeforeInstance);
       for (CXFA_Node* pNode = sIteratorBefore.GetCurrent(); pNode;
            pNode = sIteratorBefore.MoveToNext()) {
         CXFA_Node* pDataNode = pNode->GetBindData();
-        if (!pDataNode)
-          continue;
-
-        sBefore.insert(pDataNode);
+        if (pDataNode)
+          sBefore.insert(pDataNode);
       }
       ReorderDataNodes(sNew, sBefore, true);
     }