Stop caching CXFA_LayoutProcessor in CXFA_FFDocView.
Removing the UnownedPtr reduces the constraints upon the order
in which this might be freed down the road. It is easy enough
to get the layout processor from its rightful owner as needed.
-- rename method to GetLayoutProcessor() while were at it.
Change-Id: I8fe6731dad514636a3c2407f54277afb5bc2fd16
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/72131
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
index 61bea40..1839fe7 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
@@ -101,16 +101,8 @@
// The layout data can have pointers back into the script context. That
// context will be different if the form fill environment closes, so, force
// the layout data to clear.
- if (m_pXFADoc && m_pXFADoc->GetXFADoc()) {
- // The CPDF_XFADocView has a pointer to the CXFA_LayoutProcessor which is
- // owned by the CXFA_Document. The Layout Processor will be freed with the
- // ClearLayoutData() call. Make sure the doc view has already released the
- // pointer.
- if (m_pXFADocView)
- m_pXFADocView->ResetLayoutProcessor();
-
+ if (m_pXFADoc && m_pXFADoc->GetXFADoc())
m_pXFADoc->GetXFADoc()->ClearLayoutData();
- }
m_pFormFillEnv.Reset(pFormFillEnv);
}
diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp
index 3eb0a75..1a4c43b 100644
--- a/xfa/fxfa/cxfa_ffdoc.cpp
+++ b/xfa/fxfa/cxfa_ffdoc.cpp
@@ -118,8 +118,9 @@
}
CXFA_FFDocView* CXFA_FFDoc::GetDocView(CXFA_LayoutProcessor* pLayout) {
- return m_DocView && m_DocView->GetXFALayout() == pLayout ? m_DocView.get()
- : nullptr;
+ return m_DocView && m_DocView->GetLayoutProcessor() == pLayout
+ ? m_DocView.get()
+ : nullptr;
}
CXFA_FFDocView* CXFA_FFDoc::GetDocView() {
diff --git a/xfa/fxfa/cxfa_ffdocview.cpp b/xfa/fxfa/cxfa_ffdocview.cpp
index 49dbabf..43bf0f9 100644
--- a/xfa/fxfa/cxfa_ffdocview.cpp
+++ b/xfa/fxfa/cxfa_ffdocview.cpp
@@ -72,9 +72,8 @@
m_iStatus = XFA_DOCVIEW_LAYOUTSTATUS_Start;
m_pDoc->GetXFADoc()->DoProtoMerge();
m_pDoc->GetXFADoc()->DoDataMerge();
- m_pXFADocLayout = GetXFALayout();
- int32_t iStatus = m_pXFADocLayout->StartLayout(false);
+ int32_t iStatus = GetLayoutProcessor()->StartLayout(false);
if (iStatus < 0)
return iStatus;
@@ -93,7 +92,7 @@
}
int32_t CXFA_FFDocView::DoLayout() {
- int32_t iStatus = m_pXFADocLayout->DoLayout();
+ int32_t iStatus = GetLayoutProcessor()->DoLayout();
if (iStatus != 100)
return iStatus;
@@ -213,17 +212,20 @@
}
int32_t CXFA_FFDocView::CountPageViews() const {
- return m_pXFADocLayout ? m_pXFADocLayout->CountPages() : 0;
+ CXFA_LayoutProcessor* pProcessor = GetLayoutProcessor();
+ return pProcessor ? pProcessor->CountPages() : 0;
}
CXFA_FFPageView* CXFA_FFDocView::GetPageView(int32_t nIndex) const {
- if (!m_pXFADocLayout)
+ CXFA_LayoutProcessor* pProcessor = GetLayoutProcessor();
+ if (!pProcessor)
return nullptr;
- auto* pPage = m_pXFADocLayout->GetPage(nIndex);
+
+ auto* pPage = pProcessor->GetPage(nIndex);
return pPage ? pPage->GetPageView() : nullptr;
}
-CXFA_LayoutProcessor* CXFA_FFDocView::GetXFALayout() const {
+CXFA_LayoutProcessor* CXFA_FFDocView::GetLayoutProcessor() const {
return CXFA_LayoutProcessor::FromDocument(m_pDoc->GetXFADoc());
}
@@ -270,7 +272,8 @@
}
CXFA_FFWidget* CXFA_FFDocView::GetWidgetForNode(CXFA_Node* node) {
- return GetFFWidget(ToContentLayoutItem(GetXFALayout()->GetLayoutItem(node)));
+ return GetFFWidget(
+ ToContentLayoutItem(GetLayoutProcessor()->GetLayoutItem(node)));
}
CXFA_FFWidgetHandler* CXFA_FFDocView::GetWidgetHandler() {
@@ -471,9 +474,10 @@
bool CXFA_FFDocView::RunLayout() {
LockUpdate();
m_bInLayoutStatus = true;
- if (!m_pXFADocLayout->IncrementLayout() &&
- m_pXFADocLayout->StartLayout(false) < 100) {
- m_pXFADocLayout->DoLayout();
+
+ CXFA_LayoutProcessor* pProcessor = GetLayoutProcessor();
+ if (!pProcessor->IncrementLayout() && pProcessor->StartLayout(false) < 100) {
+ pProcessor->DoLayout();
UnlockUpdate();
m_bInLayoutStatus = false;
m_pDoc->GetDocEnvironment()->PageViewEvent(nullptr,
diff --git a/xfa/fxfa/cxfa_ffdocview.h b/xfa/fxfa/cxfa_ffdocview.h
index 33029cc..1df61ab 100644
--- a/xfa/fxfa/cxfa_ffdocview.h
+++ b/xfa/fxfa/cxfa_ffdocview.h
@@ -69,7 +69,7 @@
CXFA_FFWidget* GetWidgetForNode(CXFA_Node* node);
CXFA_FFWidget* GetWidgetByName(const WideString& wsName,
CXFA_FFWidget* pRefWidget);
- CXFA_LayoutProcessor* GetXFALayout() const;
+ CXFA_LayoutProcessor* GetLayoutProcessor() const;
void OnPageEvent(CXFA_ViewLayoutItem* pSender, uint32_t dwEvent);
void LockUpdate() { m_iLock++; }
void UnlockUpdate() { m_iLock--; }
@@ -101,8 +101,6 @@
bool m_bInLayoutStatus = false;
std::vector<WideString> m_NullTestMsgArray;
- void ResetLayoutProcessor() { m_pXFADocLayout.Release(); }
-
private:
bool RunEventLayoutReady();
void RunBindItems();
@@ -121,7 +119,6 @@
UnownedPtr<CXFA_FFDoc> const m_pDoc;
std::unique_ptr<CXFA_FFWidgetHandler> m_pWidgetHandler;
- UnownedPtr<CXFA_LayoutProcessor> m_pXFADocLayout;
UnownedPtr<CXFA_Node> m_pFocusNode;
ObservedPtr<CXFA_FFWidget> m_pFocusWidget;
std::deque<CXFA_Node*> m_ValidateNodes;
diff --git a/xfa/fxfa/cxfa_ffwidget.cpp b/xfa/fxfa/cxfa_ffwidget.cpp
index 7f7367f..ff7f899 100644
--- a/xfa/fxfa/cxfa_ffwidget.cpp
+++ b/xfa/fxfa/cxfa_ffwidget.cpp
@@ -621,7 +621,7 @@
if (!pParentNode)
return nullptr;
- CXFA_LayoutProcessor* layout = GetDocView()->GetXFALayout();
+ CXFA_LayoutProcessor* layout = GetDocView()->GetLayoutProcessor();
return layout->GetLayoutItem(pParentNode);
}
