Use UnownedPtr to CXFA_Node from outside the treechromium/3477

Comment raw pointers subject to nondeterministic tree destruction
order as such to avoid re-attempting to convert to the unowned
mechanism.

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

6 files changed, 27 insertions, 22 deletions

diff --git a/fxjs/xfa/cjx_eventpseudomodel.cpp b/fxjs/xfa/cjx_eventpseudomodel.cpp
index 2211905223..a1457432f9 100644
--- a/fxjs/xfa/cjx_eventpseudomodel.cpp
+++ b/fxjs/xfa/cjx_eventpseudomodel.cpp
@@ -188,7 +188,7 @@ CJS_Return CJX_EventPseudoModel::emit(
   if (!pWidgetHandler)
     return CJS_Return();
 
-  pWidgetHandler->ProcessEvent(pEventParam->m_pTarget, pEventParam);
+  pWidgetHandler->ProcessEvent(pEventParam->m_pTarget.Get(), pEventParam);
   return CJS_Return();
 }
 
diff --git a/xfa/fxfa/cxfa_eventparam.h b/xfa/fxfa/cxfa_eventparam.h
index 3fc5780008..b882d4fa75 100644
--- a/xfa/fxfa/cxfa_eventparam.h
+++ b/xfa/fxfa/cxfa_eventparam.h
@@ -7,6 +7,7 @@
 #ifndef XFA_FXFA_CXFA_EVENTPARAM_H_
 #define XFA_FXFA_CXFA_EVENTPARAM_H_
 
+#include "core/fxcrt/unowned_ptr.h"
 #include "xfa/fxfa/fxfa_basic.h"
 
 class CXFA_Node;
@@ -52,8 +53,9 @@ class CXFA_EventParam {
   CXFA_EventParam(const CXFA_EventParam& other);
 
   void Reset();
+  WideString GetNewText() const;
 
-  CXFA_Node* m_pTarget;
+  UnownedPtr<CXFA_Node> m_pTarget;
   XFA_EVENTTYPE m_eType;
   WideString m_wsResult;
   bool m_bCancelAction;
@@ -72,8 +74,6 @@ class CXFA_EventParam {
   WideString m_wsSoapFaultCode;
   WideString m_wsSoapFaultString;
   bool m_bIsFormReady;
-
-  WideString GetNewText() const;
 };
 
 #endif  // XFA_FXFA_CXFA_EVENTPARAM_H_
diff --git a/xfa/fxfa/cxfa_loadercontext.h b/xfa/fxfa/cxfa_loadercontext.h
index 06e3f117d8..36ec95b7f3 100644
--- a/xfa/fxfa/cxfa_loadercontext.h
+++ b/xfa/fxfa/cxfa_loadercontext.h
@@ -11,6 +11,7 @@
 
 #include "core/fxcrt/css/cfx_csscomputedstyle.h"
 #include "core/fxcrt/fx_system.h"
+#include "core/fxcrt/unowned_ptr.h"
 
 class CFX_XMLNode;
 class CXFA_Node;
@@ -30,7 +31,7 @@ class CXFA_LoaderContext {
   int32_t m_iTotalLines;
   uint32_t m_dwFlags;
   CFX_XMLNode* m_pXMLNode;
-  CXFA_Node* m_pNode;
+  UnownedPtr<CXFA_Node> m_pNode;
   RetainPtr<CFX_CSSComputedStyle> m_pParentStyle;
   std::vector<float> m_lineHeights;
   std::vector<float> m_BlocksHeight;
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp
index c0f786bbcf..eb7fab383a 100644
--- a/xfa/fxfa/cxfa_textlayout.cpp
+++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -99,7 +99,7 @@ std::unique_ptr<CFX_RTFBreak> CXFA_TextLayout::CreateBreak(bool bDefault) {
 
   auto pBreak = pdfium::MakeUnique<CFX_RTFBreak>(dwStyle);
   pBreak->SetLineBreakTolerance(1);
-  pBreak->SetFont(m_textParser.GetFont(m_pDoc, m_pTextProvider, nullptr));
+  pBreak->SetFont(m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, nullptr));
   pBreak->SetFontSize(m_textParser.GetFontSize(m_pTextProvider, nullptr));
   return pBreak;
 }
@@ -161,7 +161,8 @@ void CXFA_TextLayout::InitBreak(float fLineWidth) {
 
   float fFontSize = m_textParser.GetFontSize(m_pTextProvider, nullptr);
   m_pBreak->SetFontSize(fFontSize);
-  m_pBreak->SetFont(m_textParser.GetFont(m_pDoc, m_pTextProvider, nullptr));
+  m_pBreak->SetFont(
+      m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, nullptr));
   m_pBreak->SetLineBreakTolerance(fFontSize * 0.2f);
 }
 
@@ -241,7 +242,8 @@ void CXFA_TextLayout::InitBreak(CFX_CSSComputedStyle* pStyle,
   float fFontSize = m_textParser.GetFontSize(m_pTextProvider, pStyle);
   m_pBreak->SetFontSize(fFontSize);
   m_pBreak->SetLineBreakTolerance(fFontSize * 0.2f);
-  m_pBreak->SetFont(m_textParser.GetFont(m_pDoc, m_pTextProvider, pStyle));
+  m_pBreak->SetFont(
+      m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, pStyle));
   m_pBreak->SetHorizontalScale(
       m_textParser.GetHorScale(m_pTextProvider, pStyle, pXMLNode));
   m_pBreak->SetVerticalScale(m_textParser.GetVerScale(m_pTextProvider, pStyle));
@@ -488,7 +490,7 @@ bool CXFA_TextLayout::Layout(int32_t iBlock) {
         }
       }
     } else {
-      pNode = m_pLoader->m_pNode;
+      pNode = m_pLoader->m_pNode.Get();
       if (!pNode)
         return true;
       LoadText(pNode, szText.width, &fLinePos, true);
@@ -1029,7 +1031,8 @@ void CXFA_TextLayout::AppendTextLine(CFX_BreakType dwStatus,
       m_textParser.GetLinethrough(m_pTextProvider, pStyle.Get(),
                                   pTP->iLineThrough);
       pTP->dwColor = m_textParser.GetColor(m_pTextProvider, pStyle.Get());
-      pTP->pFont = m_textParser.GetFont(m_pDoc, m_pTextProvider, pStyle.Get());
+      pTP->pFont =
+          m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, pStyle.Get());
       pTP->fFontSize = m_textParser.GetFontSize(m_pTextProvider, pStyle.Get());
       pTP->rtPiece.left = pPiece->m_iStartPos / 20000.0f;
       pTP->rtPiece.width = pPiece->m_iWidth / 20000.0f;
diff --git a/xfa/fxfa/cxfa_textlayout.h b/xfa/fxfa/cxfa_textlayout.h
index 8329e06352..25c73874a8 100644
--- a/xfa/fxfa/cxfa_textlayout.h
+++ b/xfa/fxfa/cxfa_textlayout.h
@@ -111,9 +111,9 @@ class CXFA_TextLayout {
   bool Layout(int32_t iBlock);
   int32_t CountBlocks() const;
 
-  CXFA_FFDoc* m_pDoc;
-  CXFA_TextProvider* m_pTextProvider;
-  CXFA_Node* m_pTextDataNode;
+  UnownedPtr<CXFA_FFDoc> m_pDoc;
+  CXFA_TextProvider* m_pTextProvider;  // Raw, TextProvider owned by tree node.
+  CXFA_Node* m_pTextDataNode;          // Raw, this class owned by tree node.
   bool m_bRichText;
   std::unique_ptr<CFX_RTFBreak> m_pBreak;
   std::unique_ptr<CXFA_LoaderContext> m_pLoader;
diff --git a/xfa/fxfa/parser/cxfa_node.h b/xfa/fxfa/parser/cxfa_node.h
index db01983a1a..99930c6686 100644
--- a/xfa/fxfa/parser/cxfa_node.h
+++ b/xfa/fxfa/parser/cxfa_node.h
@@ -494,21 +494,22 @@ class CXFA_Node : public CXFA_Object {
   const AttributeData* const m_Attributes;
   const uint32_t m_ValidPackets;
 
-  // These nodes are responsible for building the CXFA_Node tree. We don't use
-  // unowned ptrs here because the cleanup process will remove the nodes in an
-  // order that doesn't necessarily match up to the tree structure.
-  CXFA_Node* parent_;
-  CXFA_Node* next_sibling_;
-  CXFA_Node* prev_sibling_;
-  CXFA_Node* first_child_;
-  CXFA_Node* last_child_;
+  // These members are responsible for building the CXFA_Node tree. Node
+  // pointers within the tree (or in objects owned by nodes in the tree)
+  // can't be UnownedPtr<> because the cleanup process will remove the nodes
+  // in an order that doesn't necessarily match up to the tree structure.
+  CXFA_Node* parent_;        // Raw, intra-tree node pointer.
+  CXFA_Node* next_sibling_;  // Raw, intra-tree node pointer.
+  CXFA_Node* prev_sibling_;  // Raw, intra-tree node pointer.
+  CXFA_Node* first_child_;   // Raw, intra-tree node pointer.
+  CXFA_Node* last_child_;    // Raw, intra-tree node pointer.
 
   UnownedPtr<CFX_XMLNode> xml_node_;
   const XFA_PacketType m_ePacket;
   uint8_t m_ExecuteRecursionDepth = 0;
   uint16_t m_uNodeFlags;
   uint32_t m_dwNameHash;
-  CXFA_Node* m_pAuxNode;
+  CXFA_Node* m_pAuxNode;  // Raw, node tree cleanup order.
   std::vector<UnownedPtr<CXFA_Node>> binding_nodes_;
   bool m_bIsNull = true;
   bool m_bPreNull = true;