Remove IPDFSDK_AnnotHandler::ReleaseAnnot(). For the CPDFSDK_Annot subclasses that need to do additional work before being deleted, do that work in their destructors. Also change CPDFSDK_PageView::DeleteAnnot() to do all the work, instead of letting its caller do some of the work. Rename it to DeleteAnnotForWidget() to better reflect what it does now, and do not return a bool, as the caller does not check the value. Change-Id: I09eebd597d0d47adbba7945e919630ce747718c1 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/92131 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.cpp b/fpdfsdk/cpdfsdk_annothandlermgr.cpp index 8d374d1..a1fc06b 100644 --- a/fpdfsdk/cpdfsdk_annothandlermgr.cpp +++ b/fpdfsdk/cpdfsdk_annothandlermgr.cpp
@@ -56,12 +56,6 @@ } #endif // PDF_ENABLE_XFA -void CPDFSDK_AnnotHandlerMgr::ReleaseAnnot( - std::unique_ptr<CPDFSDK_Annot> pAnnot) { - IPDFSDK_AnnotHandler* pAnnotHandler = GetAnnotHandler(pAnnot.get()); - pAnnotHandler->ReleaseAnnot(std::move(pAnnot)); -} - void CPDFSDK_AnnotHandlerMgr::Annot_OnLoad(CPDFSDK_Annot* pAnnot) { DCHECK(pAnnot); GetAnnotHandler(pAnnot)->OnLoad(pAnnot);
diff --git a/fpdfsdk/cpdfsdk_annothandlermgr.h b/fpdfsdk/cpdfsdk_annothandlermgr.h index 9b7744f..dd3143c 100644 --- a/fpdfsdk/cpdfsdk_annothandlermgr.h +++ b/fpdfsdk/cpdfsdk_annothandlermgr.h
@@ -43,7 +43,6 @@ std::unique_ptr<CPDFSDK_Annot> NewAnnotForXFA(CXFA_FFWidget* pFFWidget, CPDFSDK_PageView* pPageView); #endif // PDF_ENABLE_XFA - void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot); void Annot_OnLoad(CPDFSDK_Annot* pAnnot);
diff --git a/fpdfsdk/cpdfsdk_baannothandler.cpp b/fpdfsdk/cpdfsdk_baannothandler.cpp index 1689019..0ae7a26 100644 --- a/fpdfsdk/cpdfsdk_baannothandler.cpp +++ b/fpdfsdk/cpdfsdk_baannothandler.cpp
@@ -56,11 +56,6 @@ return std::make_unique<CPDFSDK_BAAnnot>(pAnnot, pPageView); } -void CPDFSDK_BAAnnotHandler::ReleaseAnnot( - std::unique_ptr<CPDFSDK_Annot> pAnnot) { - // pAnnot deleted by unique_ptr going out of scope. -} - void CPDFSDK_BAAnnotHandler::OnDraw(CPDFSDK_Annot* pAnnot, CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device,
diff --git a/fpdfsdk/cpdfsdk_baannothandler.h b/fpdfsdk/cpdfsdk_baannothandler.h index 2da9db2..50f9e45 100644 --- a/fpdfsdk/cpdfsdk_baannothandler.h +++ b/fpdfsdk/cpdfsdk_baannothandler.h
@@ -28,7 +28,6 @@ std::unique_ptr<CPDFSDK_Annot> NewAnnot(CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView) override; - void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot) override; CFX_FloatRect GetViewBBox(CPDFSDK_Annot* pAnnot) override; WideString GetText(CPDFSDK_Annot* pAnnot) override; WideString GetSelectedText(CPDFSDK_Annot* pAnnot) override;
diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp index f22cd39..00f73f0 100644 --- a/fpdfsdk/cpdfsdk_pageview.cpp +++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -23,7 +23,6 @@ #include "fpdfsdk/cpdfsdk_interactiveform.h" #include "third_party/base/check.h" #include "third_party/base/containers/contains.h" -#include "third_party/base/ptr_util.h" #ifdef PDF_ENABLE_XFA #include "fpdfsdk/fpdfxfa/cpdfxfa_page.h" @@ -47,17 +46,15 @@ CPDFSDK_PageView::~CPDFSDK_PageView() { if (!m_page->AsXFAPage()) { - // The call to |ReleaseAnnot| can cause the page pointed to by |m_page| to - // be freed, which will cause issues if we try to cleanup the pageview - // pointer in |m_page|. So, reset the pageview pointer before doing anything - // else. + // Deleting from `m_SDKAnnotArray` below can cause the page pointed to by + // `m_page` to be freed, which will cause issues if we try to cleanup the + // pageview pointer in `m_page`. So, reset the pageview pointer before doing + // anything else. m_page->AsPDFPage()->SetView(nullptr); } - CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr = - m_pFormFillEnv->GetAnnotHandlerMgr(); for (CPDFSDK_Annot* pAnnot : m_SDKAnnotArray) - pAnnotHandlerMgr->ReleaseAnnot(pdfium::WrapUnique(pAnnot)); + delete pAnnot; m_SDKAnnotArray.clear(); m_pAnnotList.reset(); @@ -136,33 +133,32 @@ return pSDKAnnot; } -bool CPDFSDK_PageView::DeleteAnnot(CPDFSDK_Annot* pAnnot) { +void CPDFSDK_PageView::DeleteAnnotForWidget(CXFA_FFWidget* pWidget) { + CPDFSDK_Annot* pAnnot = GetAnnotByXFAWidget(pWidget); + if (!pAnnot) + return; + IPDF_Page* pPage = pAnnot->GetXFAPage(); if (!pPage) - return false; + return; CPDF_Document::Extension* pContext = pPage->GetDocument()->GetExtension(); if (pContext && !pContext->ContainsExtensionForm()) - return false; + return; ObservedPtr<CPDFSDK_Annot> pObserved(pAnnot); if (GetFocusAnnot() == pAnnot) m_pFormFillEnv->KillFocusAnnot({}); // May invoke JS, invalidating pAnnot. if (pObserved) { - CPDFSDK_AnnotHandlerMgr* pAnnotHandler = - m_pFormFillEnv->GetAnnotHandlerMgr(); - if (pAnnotHandler) - pAnnotHandler->ReleaseAnnot(pdfium::WrapUnique(pObserved.Get())); + std::unique_ptr<CPDFSDK_Annot> to_be_deleted(pObserved.Get()); } - auto it = std::find(m_SDKAnnotArray.begin(), m_SDKAnnotArray.end(), pAnnot); if (it != m_SDKAnnotArray.end()) m_SDKAnnotArray.erase(it); + if (m_pCaptureWidget.Get() == pAnnot) m_pCaptureWidget.Reset(); - - return true; } CPDFXFA_Page* CPDFSDK_PageView::XFAPageIfNotBackedByPDFPage() {
diff --git a/fpdfsdk/cpdfsdk_pageview.h b/fpdfsdk/cpdfsdk_pageview.h index c58357e..c10ee66 100644 --- a/fpdfsdk/cpdfsdk_pageview.h +++ b/fpdfsdk/cpdfsdk_pageview.h
@@ -55,7 +55,7 @@ CPDFSDK_Annot* GetAnnotByDict(CPDF_Dictionary* pDict); #ifdef PDF_ENABLE_XFA - bool DeleteAnnot(CPDFSDK_Annot* pAnnot); + void DeleteAnnotForWidget(CXFA_FFWidget* pWidget); CPDFSDK_Annot* AddAnnot(CXFA_FFWidget* pPDFAnnot); CPDFSDK_Annot* GetAnnotByXFAWidget(CXFA_FFWidget* pWidget); IPDF_Page* GetXFAPage();
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp index 52b3ff6..4517c62 100644 --- a/fpdfsdk/cpdfsdk_widget.cpp +++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -49,7 +49,10 @@ : CPDFSDK_BAAnnot(pAnnot, pPageView), m_pInteractiveForm(pInteractiveForm) {} -CPDFSDK_Widget::~CPDFSDK_Widget() = default; +CPDFSDK_Widget::~CPDFSDK_Widget() { + m_pPageView->GetFormFillEnv()->GetInteractiveFormFiller()->OnDelete(this); + m_pInteractiveForm->RemoveMap(GetFormControl()); +} #ifdef PDF_ENABLE_XFA CXFA_FFWidget* CPDFSDK_Widget::GetMixXFAWidget() const {
diff --git a/fpdfsdk/cpdfsdk_widgethandler.cpp b/fpdfsdk/cpdfsdk_widgethandler.cpp index 52c8b4a..05284cd 100644 --- a/fpdfsdk/cpdfsdk_widgethandler.cpp +++ b/fpdfsdk/cpdfsdk_widgethandler.cpp
@@ -68,17 +68,6 @@ return pWidget; } -void CPDFSDK_WidgetHandler::ReleaseAnnot( - std::unique_ptr<CPDFSDK_Annot> pAnnot) { - DCHECK(pAnnot); - std::unique_ptr<CPDFSDK_Widget> pWidget(ToCPDFSDKWidget(pAnnot.release())); - GetFormFillEnvironment()->GetInteractiveFormFiller()->OnDelete(pWidget.get()); - - CPDFSDK_InteractiveForm* pForm = pWidget->GetInteractiveForm(); - CPDF_FormControl* pControl = pWidget->GetFormControl(); - pForm->RemoveMap(pControl); -} - void CPDFSDK_WidgetHandler::OnDraw(CPDFSDK_Annot* pAnnot, CFX_RenderDevice* pDevice, const CFX_Matrix& mtUser2Device,
diff --git a/fpdfsdk/cpdfsdk_widgethandler.h b/fpdfsdk/cpdfsdk_widgethandler.h index e5b2c60..055aba1 100644 --- a/fpdfsdk/cpdfsdk_widgethandler.h +++ b/fpdfsdk/cpdfsdk_widgethandler.h
@@ -28,7 +28,6 @@ std::unique_ptr<CPDFSDK_Annot> NewAnnot(CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView) override; - void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot) override; CFX_FloatRect GetViewBBox(CPDFSDK_Annot* pAnnot) override; WideString GetText(CPDFSDK_Annot* pAnnot) override; WideString GetSelectedText(CPDFSDK_Annot* pAnnot) override;
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp index 3160407..8189130 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp
@@ -345,9 +345,7 @@ CPDFSDK_PageView* pSdkPageView = m_pContext->GetFormFillEnv()->GetOrCreatePageView(pXFAPage.Get()); - CPDFSDK_Annot* pAnnot = pSdkPageView->GetAnnotByXFAWidget(hWidget); - if (pAnnot) - pSdkPageView->DeleteAnnot(pAnnot); + pSdkPageView->DeleteAnnotForWidget(hWidget); } int32_t CPDFXFA_DocEnvironment::CountPages(const CXFA_FFDoc* hDoc) const {
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp index bdfc8f1..2c60aed 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.cpp
@@ -240,9 +240,6 @@ void CPDFXFA_WidgetHandler::OnLoad(CPDFSDK_Annot* pAnnot) {} -void CPDFXFA_WidgetHandler::ReleaseAnnot( - std::unique_ptr<CPDFSDK_Annot> pAnnot) {} - CFX_FloatRect CPDFXFA_WidgetHandler::GetViewBBox(CPDFSDK_Annot* pAnnot) { CPDFXFA_Widget* pXFAWidget = ToXFAWidget(pAnnot); CXFA_Node* node = pXFAWidget->GetXFAFFWidget()->GetNode();
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h index f3aef2c..a87c92c 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h +++ b/fpdfsdk/fpdfxfa/cpdfxfa_widgethandler.h
@@ -32,7 +32,6 @@ std::unique_ptr<CPDFSDK_Annot> NewAnnot(CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView) override; - void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot) override; CFX_FloatRect GetViewBBox(CPDFSDK_Annot* pAnnot) override; WideString GetText(CPDFSDK_Annot* pAnnot) override; WideString GetSelectedText(CPDFSDK_Annot* pAnnot) override;
diff --git a/fpdfsdk/ipdfsdk_annothandler.h b/fpdfsdk/ipdfsdk_annothandler.h index 63daebb..2912063 100644 --- a/fpdfsdk/ipdfsdk_annothandler.h +++ b/fpdfsdk/ipdfsdk_annothandler.h
@@ -44,7 +44,6 @@ CPDF_Annot* pAnnot, CPDFSDK_PageView* pPageView) = 0; - virtual void ReleaseAnnot(std::unique_ptr<CPDFSDK_Annot> pAnnot) = 0; virtual CFX_FloatRect GetViewBBox(CPDFSDK_Annot* pAnnot) = 0; virtual WideString GetText(CPDFSDK_Annot* pAnnot) = 0; virtual WideString GetSelectedText(CPDFSDK_Annot* pAnnot) = 0;