From 0894dc84013cd6a814136ccd40f585fc2eb895f3 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 29 Jun 2018 21:54:09 +0000 Subject: Use UnownedPtr to CXFA_Node from outside the tree 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 Reviewed-by: Lei Zhang --- fxjs/xfa/cjx_eventpseudomodel.cpp | 2 +- xfa/fxfa/cxfa_eventparam.h | 6 +++--- xfa/fxfa/cxfa_loadercontext.h | 3 ++- xfa/fxfa/cxfa_textlayout.cpp | 13 ++++++++----- xfa/fxfa/cxfa_textlayout.h | 6 +++--- xfa/fxfa/parser/cxfa_node.h | 19 ++++++++++--------- 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 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 m_pNode; RetainPtr m_pParentStyle; std::vector m_lineHeights; std::vector 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 CXFA_TextLayout::CreateBreak(bool bDefault) { auto pBreak = pdfium::MakeUnique(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 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 m_pBreak; std::unique_ptr 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 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> binding_nodes_; bool m_bIsNull = true; bool m_bPreNull = true; -- cgit v1.2.3