commit 2c073dc44403700b92884d8ec98f2e0124567512
author Tom Sepez <tsepez@chromium.org> Thu Aug 15 23:53:14 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Aug 15 23:53:14 2019 +0000
tree e68cf5028b139dc73f14fb8bb1ab73933aaf1002
parent 664699467ba6429f51acad65cc2a0931c99344b6

Let CXFA_NodeIteratorTemplate use either unowned or retain pointers

Use it in CXFA_LayoutItemIterator to ensure items that are
being iterated over don't pop out of existence.

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

xfa/fxfa/layout/cxfa_traversestrategy_layoutitem.h

diff --git a/xfa/fxfa/layout/cxfa_traversestrategy_layoutitem.h b/xfa/fxfa/layout/cxfa_traversestrategy_layoutitem.h
index d35ce11..f71d708 100644
--- a/xfa/fxfa/layout/cxfa_traversestrategy_layoutitem.h
+++ b/xfa/fxfa/layout/cxfa_traversestrategy_layoutitem.h
@@ -7,6 +7,7 @@
 #ifndef XFA_FXFA_LAYOUT_CXFA_TRAVERSESTRATEGY_LAYOUTITEM_H_
 #define XFA_FXFA_LAYOUT_CXFA_TRAVERSESTRATEGY_LAYOUTITEM_H_
 
+#include "core/fxcrt/retain_ptr.h"
 #include "xfa/fxfa/layout/cxfa_layoutitem.h"
 #include "xfa/fxfa/parser/cxfa_nodeiteratortemplate.h"
 
@@ -25,6 +26,7 @@
 
 using CXFA_LayoutItemIterator =
     CXFA_NodeIteratorTemplate<CXFA_LayoutItem,
-                              CXFA_TraverseStrategy_LayoutItem>;
+                              CXFA_TraverseStrategy_LayoutItem,
+                              RetainPtr<CXFA_LayoutItem>>;
 
 #endif  // XFA_FXFA_LAYOUT_CXFA_TRAVERSESTRATEGY_LAYOUTITEM_H_

xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp

diff --git a/xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp b/xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp
index 6e6ba4a..0ff1777 100644
--- a/xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp
+++ b/xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp
@@ -1855,9 +1855,7 @@
       nPageIdx++;
       uint32_t dwRelevant =
           XFA_WidgetStatus_Viewable | XFA_WidgetStatus_Printable;
-      CXFA_NodeIteratorTemplate<CXFA_LayoutItem,
-                                CXFA_TraverseStrategy_LayoutItem>
-          iterator(pViewItem);
+      CXFA_LayoutItemIterator iterator(pViewItem);
       CXFA_LayoutItem* pChildLayoutItem = iterator.GetCurrent();
       while (pChildLayoutItem) {
         CXFA_ContentLayoutItem* pContentItem =

xfa/fxfa/parser/cxfa_nodeiteratortemplate.h

diff --git a/xfa/fxfa/parser/cxfa_nodeiteratortemplate.h b/xfa/fxfa/parser/cxfa_nodeiteratortemplate.h
index 10543fd..8803efa 100644
--- a/xfa/fxfa/parser/cxfa_nodeiteratortemplate.h
+++ b/xfa/fxfa/parser/cxfa_nodeiteratortemplate.h
@@ -9,7 +9,9 @@
 
 #include "core/fxcrt/unowned_ptr.h"
 
-template <class NodeType, class TraverseStrategy>
+template <class NodeType,
+          class TraverseStrategy,
+          typename HolderType = UnownedPtr<NodeType>>
 class CXFA_NodeIteratorTemplate {
  public:
   explicit CXFA_NodeIteratorTemplate(NodeType* pRoot)
@@ -24,7 +26,7 @@
       m_pCurrent = nullptr;
       return false;
     }
-    m_pCurrent = pNode;
+    m_pCurrent.Reset(pNode);
     return true;
   }
 
@@ -32,20 +34,18 @@
     if (!m_pRoot)
       return nullptr;
     if (!m_pCurrent) {
-      m_pCurrent = LastDescendant(m_pRoot.Get());
+      m_pCurrent.Reset(LastDescendant(m_pRoot.Get()));
       return m_pCurrent.Get();
     }
     NodeType* pSibling = PreviousSiblingWithinSubtree(m_pCurrent.Get());
     if (pSibling) {
-      m_pCurrent = LastDescendant(pSibling);
+      m_pCurrent.Reset(LastDescendant(pSibling));
       return m_pCurrent.Get();
     }
     NodeType* pParent = ParentWithinSubtree(m_pCurrent.Get());
-    if (pParent) {
-      m_pCurrent = pParent;
-      return pParent;
-    }
-    return nullptr;
+    if (pParent)
+      m_pCurrent.Reset(pParent);
+    return pParent;
   }
 
   NodeType* MoveToNext() {
@@ -53,7 +53,7 @@
       return nullptr;
     NodeType* pChild = TraverseStrategy::GetFirstChild(m_pCurrent.Get());
     if (pChild) {
-      m_pCurrent = pChild;
+      m_pCurrent.Reset(pChild);
       return pChild;
     }
     return SkipChildrenAndMoveToNext();
@@ -66,7 +66,7 @@
     while (pNode) {
       NodeType* pSibling = NextSiblingWithinSubtree(pNode);
       if (pSibling) {
-        m_pCurrent = pSibling;
+        m_pCurrent.Reset(pSibling);
         return pSibling;
       }
       pNode = ParentWithinSubtree(pNode);
@@ -77,23 +77,17 @@
 
  private:
   bool RootReachableFromNode(NodeType* pNode) {
-    if (!pNode)
-      return false;
-    if (pNode == m_pRoot)
-      return true;
-    return RootReachableFromNode(TraverseStrategy::GetParent(pNode));
+    return pNode && (pNode == m_pRoot ||
+                     RootReachableFromNode(TraverseStrategy::GetParent(pNode)));
   }
 
   NodeType* ParentWithinSubtree(NodeType* pNode) {
-    if (!pNode || pNode == m_pRoot)
-      return nullptr;
-    return TraverseStrategy::GetParent(pNode);
+    return pNode && pNode != m_pRoot ? TraverseStrategy::GetParent(pNode)
+                                     : nullptr;
   }
 
   NodeType* NextSiblingWithinSubtree(NodeType* pNode) {
-    if (pNode == m_pRoot)
-      return nullptr;
-    return TraverseStrategy::GetNextSibling(pNode);
+    return pNode != m_pRoot ? TraverseStrategy::GetNextSibling(pNode) : nullptr;
   }
 
   NodeType* PreviousSiblingWithinSubtree(NodeType* pNode) {
@@ -124,8 +118,8 @@
     return pChild ? LastDescendant(pChild) : pNode;
   }
 
-  UnownedPtr<NodeType> m_pRoot;
-  UnownedPtr<NodeType> m_pCurrent;
+  HolderType m_pRoot;
+  HolderType m_pCurrent;
 };
 
 #endif  // XFA_FXFA_PARSER_CXFA_NODEITERATORTEMPLATE_H_