From cc4d6d85b58a7a1d9d7b798c63d3343f3dac06a9 Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 16 May 2016 13:21:03 -0700 Subject: Remove { delete this; } anti-pattern from IXFA_WidgetIterator Review-Url: https://codereview.chromium.org/1976123003 --- fpdfsdk/fpdfformfill.cpp | 4 ++-- fpdfsdk/fsdk_annothandler.cpp | 12 +++++------- fpdfsdk/fsdk_mgr.cpp | 11 +++++------ xfa/fxfa/app/xfa_ffpageview.cpp | 6 +++--- xfa/fxfa/app/xfa_rendercontext.cpp | 6 ++---- xfa/fxfa/include/fxfa.h | 6 ++---- xfa/fxfa/include/xfa_ffdocview.h | 22 +++++++++++----------- xfa/fxfa/include/xfa_ffpageview.h | 9 +++------ 8 files changed, 33 insertions(+), 43 deletions(-) diff --git a/fpdfsdk/fpdfformfill.cpp b/fpdfsdk/fpdfformfill.cpp index 153c1057a5..94a8bf12a5 100644 --- a/fpdfsdk/fpdfformfill.cpp +++ b/fpdfsdk/fpdfformfill.cpp @@ -91,8 +91,8 @@ DLLEXPORT int STDCALL FPDFPage_HasFormFieldAtPoint(FPDF_FORMHANDLE hHandle, if (!pWidgetHandler) return -1; - std::unique_ptr> - pWidgetIterator(pPageView->CreateWidgetIterator( + std::unique_ptr pWidgetIterator( + pPageView->CreateWidgetIterator( XFA_TRAVERSEWAY_Form, XFA_WIDGETFILTER_Viewable | XFA_WIDGETFILTER_AllType)); if (!pWidgetIterator) diff --git a/fpdfsdk/fsdk_annothandler.cpp b/fpdfsdk/fsdk_annothandler.cpp index 73701cfb4c..db2d20b2bf 100644 --- a/fpdfsdk/fsdk_annothandler.cpp +++ b/fpdfsdk/fsdk_annothandler.cpp @@ -376,22 +376,20 @@ CPDFSDK_Annot* CPDFSDK_AnnotHandlerMgr::GetNextAnnot(CPDFSDK_Annot* pSDKAnnot, return pNext; } // for xfa annots - IXFA_WidgetIterator* pWidgetIterator = + std::unique_ptr pWidgetIterator( pPage->GetXFAPageView()->CreateWidgetIterator( XFA_TRAVERSEWAY_Tranvalse, XFA_WIDGETFILTER_Visible | XFA_WIDGETFILTER_Viewable | - XFA_WIDGETFILTER_Field); - if (pWidgetIterator == NULL) - return NULL; + XFA_WIDGETFILTER_Field)); + if (!pWidgetIterator) + return nullptr; if (pWidgetIterator->GetCurrentWidget() != pSDKAnnot->GetXFAWidget()) pWidgetIterator->SetCurrentWidget(pSDKAnnot->GetXFAWidget()); - CXFA_FFWidget* hNextFocus = NULL; - hNextFocus = + CXFA_FFWidget* hNextFocus = bNext ? pWidgetIterator->MoveToNext() : pWidgetIterator->MoveToPrevious(); if (!hNextFocus && pSDKAnnot) hNextFocus = pWidgetIterator->MoveToFirst(); - pWidgetIterator->Release(); return pPageView->GetAnnotByXFAWidget(hNextFocus); #else // PDF_ENABLE_XFA CBA_AnnotIterator ai(pSDKAnnot->GetPageView(), "Widget", ""); diff --git a/fpdfsdk/fsdk_mgr.cpp b/fpdfsdk/fsdk_mgr.cpp index 4ab4850c4d..2d641783f9 100644 --- a/fpdfsdk/fsdk_mgr.cpp +++ b/fpdfsdk/fsdk_mgr.cpp @@ -905,10 +905,11 @@ void CPDFSDK_PageView::LoadFXAnnots() { m_page->AddRef(); if (m_pSDKDoc->GetXFADocument()->GetDocType() == DOCTYPE_DYNAMIC_XFA) { CXFA_FFPageView* pageView = m_page->GetXFAPageView(); - IXFA_WidgetIterator* pWidgetHander = pageView->CreateWidgetIterator( - XFA_TRAVERSEWAY_Form, XFA_WIDGETFILTER_Visible | - XFA_WIDGETFILTER_Viewable | - XFA_WIDGETFILTER_AllType); + std::unique_ptr pWidgetHander( + pageView->CreateWidgetIterator(XFA_TRAVERSEWAY_Form, + XFA_WIDGETFILTER_Visible | + XFA_WIDGETFILTER_Viewable | + XFA_WIDGETFILTER_AllType)); if (!pWidgetHander) { m_page->Release(); SetLock(FALSE); @@ -919,11 +920,9 @@ void CPDFSDK_PageView::LoadFXAnnots() { CPDFSDK_Annot* pAnnot = pAnnotHandlerMgr->NewAnnot(pXFAAnnot, this); if (!pAnnot) continue; - m_fxAnnotArray.push_back(pAnnot); pAnnotHandlerMgr->Annot_OnLoad(pAnnot); } - pWidgetHander->Release(); } else { CPDF_Page* pPage = m_page->GetPDFPage(); ASSERT(pPage); diff --git a/xfa/fxfa/app/xfa_ffpageview.cpp b/xfa/fxfa/app/xfa_ffpageview.cpp index f1898c83f1..e5a05734dc 100644 --- a/xfa/fxfa/app/xfa_ffpageview.cpp +++ b/xfa/fxfa/app/xfa_ffpageview.cpp @@ -197,6 +197,7 @@ CXFA_FFWidget* CXFA_FFPageWidgetIterator::GetWidget( } return NULL; } + CXFA_FFTabOrderPageWidgetIterator::CXFA_FFTabOrderPageWidgetIterator( CXFA_FFPageView* pPageView, uint32_t dwFilter) @@ -206,10 +207,9 @@ CXFA_FFTabOrderPageWidgetIterator::CXFA_FFTabOrderPageWidgetIterator( ->GetCurVersionMode() < XFA_VERSION_205; Reset(); } + CXFA_FFTabOrderPageWidgetIterator::~CXFA_FFTabOrderPageWidgetIterator() {} -void CXFA_FFTabOrderPageWidgetIterator::Release() { - delete this; -} + void CXFA_FFTabOrderPageWidgetIterator::Reset() { CreateTabOrderWidgetArray(); m_iCurWidget = -1; diff --git a/xfa/fxfa/app/xfa_rendercontext.cpp b/xfa/fxfa/app/xfa_rendercontext.cpp index cc2ea4b9ac..e7bacec281 100644 --- a/xfa/fxfa/app/xfa_rendercontext.cpp +++ b/xfa/fxfa/app/xfa_rendercontext.cpp @@ -68,8 +68,6 @@ int32_t CXFA_RenderContext::DoRender(IFX_Pause* pPause) { return XFA_RENDERSTATUS_Done; } void CXFA_RenderContext::StopRender() { - if (m_pWidgetIterator) { - m_pWidgetIterator->Release(); - m_pWidgetIterator = NULL; - } + delete m_pWidgetIterator; + m_pWidgetIterator = nullptr; } diff --git a/xfa/fxfa/include/fxfa.h b/xfa/fxfa/include/fxfa.h index 871ab71164..a173a0e299 100644 --- a/xfa/fxfa/include/fxfa.h +++ b/xfa/fxfa/include/fxfa.h @@ -541,7 +541,8 @@ class CXFA_RenderOptions { class IXFA_WidgetIterator { public: - virtual void Release() = 0; + virtual ~IXFA_WidgetIterator() {} + virtual void Reset() = 0; virtual CXFA_FFWidget* MoveToFirst() = 0; virtual CXFA_FFWidget* MoveToLast() = 0; @@ -549,9 +550,6 @@ class IXFA_WidgetIterator { virtual CXFA_FFWidget* MoveToPrevious() = 0; virtual CXFA_FFWidget* GetCurrentWidget() = 0; virtual FX_BOOL SetCurrentWidget(CXFA_FFWidget* hWidget) = 0; - - protected: - virtual ~IXFA_WidgetIterator() {} }; #endif // XFA_FXFA_INCLUDE_FXFA_H_ diff --git a/xfa/fxfa/include/xfa_ffdocview.h b/xfa/fxfa/include/xfa_ffdocview.h index 7f1c631da3..473b820def 100644 --- a/xfa/fxfa/include/xfa_ffdocview.h +++ b/xfa/fxfa/include/xfa_ffdocview.h @@ -125,20 +125,20 @@ class CXFA_FFDocView { int32_t m_iLock; friend class CXFA_FFNotify; }; + class CXFA_FFDocWidgetIterator : public IXFA_WidgetIterator { public: CXFA_FFDocWidgetIterator(CXFA_FFDocView* pDocView, CXFA_Node* pTravelRoot); - virtual ~CXFA_FFDocWidgetIterator(); - - virtual void Release() { delete this; } - - virtual void Reset(); - virtual CXFA_FFWidget* MoveToFirst(); - virtual CXFA_FFWidget* MoveToLast(); - virtual CXFA_FFWidget* MoveToNext(); - virtual CXFA_FFWidget* MoveToPrevious(); - virtual CXFA_FFWidget* GetCurrentWidget(); - virtual FX_BOOL SetCurrentWidget(CXFA_FFWidget* hWidget); + ~CXFA_FFDocWidgetIterator() override; + + // IXFA_WidgetIterator: + void Reset() override; + CXFA_FFWidget* MoveToFirst() override; + CXFA_FFWidget* MoveToLast() override; + CXFA_FFWidget* MoveToNext() override; + CXFA_FFWidget* MoveToPrevious() override; + CXFA_FFWidget* GetCurrentWidget() override; + FX_BOOL SetCurrentWidget(CXFA_FFWidget* hWidget) override; protected: CXFA_ContainerIterator m_ContentIterator; diff --git a/xfa/fxfa/include/xfa_ffpageview.h b/xfa/fxfa/include/xfa_ffpageview.h index 75e418b25b..79b0afb602 100644 --- a/xfa/fxfa/include/xfa_ffpageview.h +++ b/xfa/fxfa/include/xfa_ffpageview.h @@ -36,12 +36,11 @@ class CXFA_FFPageView : public CXFA_ContainerLayoutItem { typedef CXFA_NodeIteratorTemplate CXFA_LayoutItemIterator; + class CXFA_FFPageWidgetIterator : public IXFA_WidgetIterator { public: CXFA_FFPageWidgetIterator(CXFA_FFPageView* pPageView, uint32_t dwFilter); - virtual ~CXFA_FFPageWidgetIterator(); - - void Release() override { delete this; } + ~CXFA_FFPageWidgetIterator() override; void Reset() override; CXFA_FFWidget* MoveToFirst() override; @@ -73,9 +72,7 @@ class CXFA_FFTabOrderPageWidgetIterator : public IXFA_WidgetIterator { public: CXFA_FFTabOrderPageWidgetIterator(CXFA_FFPageView* pPageView, uint32_t dwFilter); - virtual ~CXFA_FFTabOrderPageWidgetIterator(); - - void Release() override; + ~CXFA_FFTabOrderPageWidgetIterator() override; void Reset() override; CXFA_FFWidget* MoveToFirst() override; -- cgit v1.2.3