commit 706619bb28a48276e7ea870995df6a80acf3860d
author Tom Sepez <tsepez@chromium.org> Thu May 09 18:03:28 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu May 09 18:03:28 2019 +0000
tree 6c93c664e38e54dc4056fb80efc08d14f8740deb
parent 00714a05fb5d4766e9f460d9764191eda9706db9

Remove tree-corrupting methods from CXFA_LayoutItem, try #3.

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

xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp

diff --git a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
index bb14e68..a07e396 100644
--- a/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
+++ b/xfa/fxfa/layout/cxfa_itemlayoutprocessor.cpp
@@ -732,17 +732,16 @@
     bOrphanedItem = true;
   }
 
-  CXFA_ContentLayoutItem* pChildren =
-      ToContentLayoutItem(pLayoutItem->GetFirstChild());
-  pLayoutItem->SetFirstChild(nullptr);
+  std::vector<CXFA_ContentLayoutItem*> children;
+  while (auto* pFirst = ToContentLayoutItem(pLayoutItem->GetFirstChild())) {
+    children.push_back(pFirst);
+    pLayoutItem->RemoveChild(pFirst);
+  }
 
   float lHeightForKeep = 0;
   float fAddMarginHeight = 0;
   std::vector<CXFA_ContentLayoutItem*> keepLayoutItems;
-  for (CXFA_ContentLayoutItem *pChildItem = pChildren, *pChildNext = nullptr;
-       pChildItem; pChildItem = pChildNext) {
-    pChildNext = ToContentLayoutItem(pChildItem->GetNextSibling());
-    pChildItem->SetNextSibling(nullptr);
+  for (auto* pChildItem : children) {
     if (fSplitPos <= fCurTopMargin + pChildItem->m_sPos.y + fCurBottomMargin +
                          kXFALayoutPrecision) {
       if (!ExistContainerKeep(pChildItem->GetFormNode(), true)) {
@@ -805,9 +804,9 @@
   CXFA_ContentLayoutItem* pLayoutItem = m_pLayoutItem;
   if (pLayoutItem) {
     m_pLayoutItem = ToContentLayoutItem(pLayoutItem->GetNextSibling());
-    pLayoutItem->SetNextSibling(nullptr);
+    if (pLayoutItem->GetParent())
+      pLayoutItem->GetParent()->RemoveChild(pLayoutItem);
   }
-
   if (m_nCurChildNodeStage != Stage::kDone || !m_pOldLayoutItem)
     return pLayoutItem;
 
@@ -1595,25 +1594,12 @@
         }
       }
 
-      if (ToContentLayoutItem(m_pLayoutItem->GetFirstChild()) == pLastChild) {
-        m_pLayoutItem->SetFirstChild(nullptr);
-      } else {
-        for (CXFA_LayoutItem* pLayoutNext = m_pLayoutItem->GetFirstChild();
-             pLayoutNext; pLayoutNext = pLayoutNext->GetNextSibling()) {
-          if (ToContentLayoutItem(pLayoutNext->GetNextSibling()) ==
-              pLastChild) {
-            pLayoutNext->SetNextSibling(nullptr);
-            break;
-          }
-        }
-      }
-
-      CXFA_ContentLayoutItem* pLayoutNextTemp = ToContentLayoutItem(pLastChild);
+      CXFA_ContentLayoutItem* pLayoutNextTemp = pLastChild;
       while (pLayoutNextTemp) {
-        pLayoutNextTemp->SetParent(nullptr);
         CXFA_ContentLayoutItem* pSaveLayoutNext =
             ToContentLayoutItem(pLayoutNextTemp->GetNextSibling());
-        pLayoutNextTemp->SetNextSibling(nullptr);
+        if (pLayoutNextTemp->GetParent())
+          pLayoutNextTemp->GetParent()->RemoveChild(pLayoutNextTemp);
         pLayoutNextTemp = pSaveLayoutNext;
       }
       pLastChild = nullptr;

xfa/fxfa/layout/cxfa_layoutitem.cpp

diff --git a/xfa/fxfa/layout/cxfa_layoutitem.cpp b/xfa/fxfa/layout/cxfa_layoutitem.cpp
index 00debb1..af586b9 100644
--- a/xfa/fxfa/layout/cxfa_layoutitem.cpp
+++ b/xfa/fxfa/layout/cxfa_layoutitem.cpp
@@ -52,7 +52,9 @@
 CXFA_LayoutItem::CXFA_LayoutItem(CXFA_Node* pNode, ItemType type)
     : m_ItemType(type), m_pFormNode(pNode) {}
 
-CXFA_LayoutItem::~CXFA_LayoutItem() = default;
+CXFA_LayoutItem::~CXFA_LayoutItem() {
+  CHECK(!GetParent());
+}
 
 CXFA_ViewLayoutItem* CXFA_LayoutItem::AsViewLayoutItem() {
   return IsViewLayoutItem() ? static_cast<CXFA_ViewLayoutItem*>(this) : nullptr;

xfa/fxfa/layout/cxfa_layoutitem.h

diff --git a/xfa/fxfa/layout/cxfa_layoutitem.h b/xfa/fxfa/layout/cxfa_layoutitem.h
index b735b66..c5fd157 100644
--- a/xfa/fxfa/layout/cxfa_layoutitem.h
+++ b/xfa/fxfa/layout/cxfa_layoutitem.h
@@ -32,11 +32,6 @@
   CXFA_Node* GetFormNode() const { return m_pFormNode.Get(); }
   void SetFormNode(CXFA_Node* pNode) { m_pFormNode = pNode; }
 
-  // TODO(tsepez) replace these calls with AddChild() etc.
-  void SetParent(CXFA_LayoutItem* pParent) { m_pParent = pParent; }
-  void SetFirstChild(CXFA_LayoutItem* pChild) { m_pFirstChild = pChild; }
-  void SetNextSibling(CXFA_LayoutItem* pSibling) { m_pNextSibling = pSibling; }
-
   void AddChild(CXFA_LayoutItem* pChildItem);
   void AddHeadChild(CXFA_LayoutItem* pChildItem);
   void RemoveChild(CXFA_LayoutItem* pChildItem);

xfa/fxfa/layout/cxfa_layoutpagemgr.cpp

diff --git a/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp b/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
index 905c860..c4a3db9 100644
--- a/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
+++ b/xfa/fxfa/layout/cxfa_layoutpagemgr.cpp
@@ -645,7 +645,7 @@
     while (pPrePageSet->GetNextSibling()) {
       pPrePageSet = pPrePageSet->GetNextSibling()->AsViewLayoutItem();
     }
-    pPrePageSet->SetNextSibling(pPageSetLayoutItem);
+    pPrePageSet->GetParent()->InsertChild(pPrePageSet, pPageSetLayoutItem);
     m_pPageSetCurRoot = pPageSetLayoutItem;
   } else {
     pParentPageSetLayout->AddChild(pPageSetLayoutItem);