Remove bool arg from ResetPWLWindow(). Split into virtual RestorePWLWindow() and ResetPWLWindow() methods. -- add helper method to invoke one or the other based on age. -- re-org header to keep virtual methods together. -- avoid some assignments in conditionals. -- fix pre-existing IWYU. Change-Id: I683103f06369fc93087f0654b682ac3672e32afe Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/81231 Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/formfiller/cffl_formfield.cpp b/fpdfsdk/formfiller/cffl_formfield.cpp index 90ba187..c87e31c 100644 --- a/fpdfsdk/formfiller/cffl_formfield.cpp +++ b/fpdfsdk/formfiller/cffl_formfield.cpp
@@ -365,8 +365,8 @@ if (pPrivateData->AppearanceAgeEquals(m_pWidget->GetAppearanceAge())) return pWnd; - return ResetPWLWindow(pPageView, - pPrivateData->ValueAgeEquals(m_pWidget->GetValueAge())); + return ResetPWLWindowForValueAge(pPageView, m_pWidget, + pPrivateData->GetValueAge()); } void CFFL_FormField::DestroyPWLWindow(CPDFSDK_PageView* pPageView) { @@ -469,7 +469,7 @@ nFlag)) { if (!pObserved) return false; - ResetPWLWindow(pPageView, false); + ResetPWLWindow(pPageView); return true; } if (!pObserved) @@ -478,7 +478,7 @@ if (!pInteractiveFormFiller->OnValidate(&pObserved, pPageView, nFlag)) { if (!pObserved) return false; - ResetPWLWindow(pPageView, false); + ResetPWLWindow(pPageView); return true; } if (!pObserved) @@ -529,8 +529,18 @@ void CFFL_FormField::RestoreState(CPDFSDK_PageView* pPageView) {} -CPWL_Wnd* CFFL_FormField::ResetPWLWindow(CPDFSDK_PageView* pPageView, - bool bRestoreValue) { +CPWL_Wnd* CFFL_FormField::ResetPWLWindowForValueAge(CPDFSDK_PageView* pPageView, + CPDFSDK_Widget* pWidget, + uint32_t nValueAge) { + return nValueAge == pWidget->GetValueAge() ? RestorePWLWindow(pPageView) + : ResetPWLWindow(pPageView); +} + +CPWL_Wnd* CFFL_FormField::ResetPWLWindow(CPDFSDK_PageView* pPageView) { + return GetPWLWindow(pPageView); +} + +CPWL_Wnd* CFFL_FormField::RestorePWLWindow(CPDFSDK_PageView* pPageView) { return GetPWLWindow(pPageView); }
diff --git a/fpdfsdk/formfiller/cffl_formfield.h b/fpdfsdk/formfiller/cffl_formfield.h index 2359970..e7e6b88 100644 --- a/fpdfsdk/formfiller/cffl_formfield.h +++ b/fpdfsdk/formfiller/cffl_formfield.h
@@ -103,10 +103,13 @@ virtual std::unique_ptr<CPWL_Wnd> NewPWLWindow( const CPWL_Wnd::CreateParams& cp, std::unique_ptr<IPWL_SystemHandler::PerWindowData> pAttachedData) = 0; - virtual CPWL_Wnd* ResetPWLWindow(CPDFSDK_PageView* pPageView, - bool bRestoreValue); virtual void SaveState(CPDFSDK_PageView* pPageView); virtual void RestoreState(CPDFSDK_PageView* pPageView); + virtual bool IsDataChanged(CPDFSDK_PageView* pPageView); + virtual void SaveData(CPDFSDK_PageView* pPageView); +#ifdef PDF_ENABLE_XFA + virtual bool IsFieldFull(CPDFSDK_PageView* pPageView); +#endif // PDF_ENABLE_XFA CFX_Matrix GetCurMatrix(); CFX_FloatRect GetFocusBox(CPDFSDK_PageView* pPageView); @@ -114,15 +117,10 @@ CFX_FloatRect PWLtoFFL(const CFX_FloatRect& rect); CFX_PointF FFLtoPWL(const CFX_PointF& point); CFX_PointF PWLtoFFL(const CFX_PointF& point); - bool CommitData(CPDFSDK_PageView* pPageView, uint32_t nFlag); - virtual bool IsDataChanged(CPDFSDK_PageView* pPageView); - virtual void SaveData(CPDFSDK_PageView* pPageView); - -#ifdef PDF_ENABLE_XFA - virtual bool IsFieldFull(CPDFSDK_PageView* pPageView); -#endif // PDF_ENABLE_XFA - + CPWL_Wnd* ResetPWLWindowForValueAge(CPDFSDK_PageView* pPageView, + CPDFSDK_Widget* pWidget, + uint32_t nValueAge); CPWL_Wnd* GetPWLWindow(CPDFSDK_PageView* pPageView) const; CPWL_Wnd* CreateOrUpdatePWLWindow(CPDFSDK_PageView* pPageView); void DestroyPWLWindow(CPDFSDK_PageView* pPageView); @@ -137,6 +135,9 @@ CPDFSDK_Annot* GetSDKAnnot() const { return m_pWidget.Get(); } protected: + virtual CPWL_Wnd* ResetPWLWindow(CPDFSDK_PageView* pPageView); + virtual CPWL_Wnd* RestorePWLWindow(CPDFSDK_PageView* pPageView); + // If the inheriting widget has its own fontmap and a PWL_Edit widget that // access that fontmap then you have to call DestroyWindows before destroying // the font map in order to not get a use-after-free.
diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp index 38004b2..5728bdb 100644 --- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp +++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp
@@ -119,10 +119,9 @@ return; if (pWidget->IsAppModified()) { - if (CFFL_FormField* pFormField = GetFormField(pWidget)) { - pFormField->ResetPWLWindow(pPageView, - pWidget->GetValueAge() == nValueAge); - } + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); } } } @@ -153,10 +152,9 @@ return; if (pWidget->IsAppModified()) { - if (CFFL_FormField* pFormField = GetFormField(pWidget)) { - pFormField->ResetPWLWindow(pPageView, - nValueAge == pWidget->GetValueAge()); - } + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); } } } @@ -193,10 +191,9 @@ return true; if (pWidget->IsAppModified()) { - if (CFFL_FormField* pFormField = GetFormField(pWidget)) { - pFormField->ResetPWLWindow(pPageView, - nValueAge == pWidget->GetValueAge()); - } + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); } } } @@ -272,7 +269,7 @@ CFFL_FormField* pFormField = GetFormField(pWidget); if (pFormField) - pFormField->ResetPWLWindow(pPageView, nValueAge == pWidget->GetValueAge()); + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); return true; } @@ -404,10 +401,9 @@ return false; if (pWidget->IsAppModified()) { - if (CFFL_FormField* pFiller = GetFormField(pWidget)) { - pFiller->ResetPWLWindow(pPageView, - nValueAge == pWidget->GetValueAge()); - } + CFFL_FormField* pFiller = GetFormField(pWidget); + if (pFiller) + pFiller->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); } } } @@ -773,8 +769,9 @@ if (nAge == pWidget->GetAppearanceAge()) return false; - if (CFFL_FormField* pFormField = GetFormField(pWidget)) - pFormField->ResetPWLWindow(pPageView, nValueAge == pWidget->GetValueAge()); + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); return false; } @@ -803,9 +800,9 @@ if (nAge == pWidget->GetAppearanceAge()) return false; - if (CFFL_FormField* pFormField = GetFormField(pWidget)) - pFormField->ResetPWLWindow(pPageView, nValueAge == pWidget->GetValueAge()); - + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); return true; } @@ -834,9 +831,9 @@ if (nAge == pWidget->GetAppearanceAge()) return false; - if (CFFL_FormField* pFormField = GetFormField(pWidget)) - pFormField->ResetPWLWindow(pPageView, nValueAge == pWidget->GetValueAge()); - + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); return true; } @@ -862,12 +859,13 @@ m_bNotifying = false; if (!pAnnot->HasObservable() || !IsValidAnnot(pPageView, pWidget)) return true; + if (nAge == pWidget->GetAppearanceAge()) return false; - if (CFFL_FormField* pFormField = GetFormField(pWidget)) - pFormField->ResetPWLWindow(pPageView, nValueAge == pWidget->GetValueAge()); - + CFFL_FormField* pFormField = GetFormField(pWidget); + if (pFormField) + pFormField->ResetPWLWindowForValueAge(pPageView, pWidget, nValueAge); return true; } #endif // PDF_ENABLE_XFA @@ -936,8 +934,8 @@ bool bExit = false; if (nAge != pWidget->GetAppearanceAge()) { - CPWL_Wnd* pWnd = pFormField->ResetPWLWindow( - pPageView, nValueAge == pWidget->GetValueAge()); + CPWL_Wnd* pWnd = pFormField->ResetPWLWindowForValueAge( + pPageView, pWidget.Get(), nValueAge); if (!pWnd) return {true, true};
diff --git a/fpdfsdk/formfiller/cffl_privatedata.h b/fpdfsdk/formfiller/cffl_privatedata.h index f8c4e0c..e6e17f0 100644 --- a/fpdfsdk/formfiller/cffl_privatedata.h +++ b/fpdfsdk/formfiller/cffl_privatedata.h
@@ -7,6 +7,8 @@ #ifndef FPDFSDK_FORMFILLER_CFFL_PRIVATEDATA_H_ #define FPDFSDK_FORMFILLER_CFFL_PRIVATEDATA_H_ +#include <memory> + #include "core/fxcrt/observed_ptr.h" #include "core/fxcrt/unowned_ptr.h" #include "fpdfsdk/pwl/ipwl_systemhandler.h" @@ -31,7 +33,7 @@ bool AppearanceAgeEquals(uint32_t age) const { return age == m_nAppearanceAge; } - bool ValueAgeEquals(uint32_t age) const { return age == m_nValueAge; } + uint32_t GetValueAge() const { return m_nValueAge; } private: CFFL_PrivateData(const CFFL_PrivateData& that);
diff --git a/fpdfsdk/formfiller/cffl_textobject.cpp b/fpdfsdk/formfiller/cffl_textobject.cpp index d4f5430..cc58b8f 100644 --- a/fpdfsdk/formfiller/cffl_textobject.cpp +++ b/fpdfsdk/formfiller/cffl_textobject.cpp
@@ -9,22 +9,6 @@ #include "core/fpdfapi/page/cpdf_page.h" #include "core/fpdfdoc/cpdf_bafontmap.h" -CPWL_Wnd* CFFL_TextObject::ResetPWLWindow(CPDFSDK_PageView* pPageView, - bool bRestoreValue) { - if (bRestoreValue) - SaveState(pPageView); - - DestroyPWLWindow(pPageView); - if (bRestoreValue) - RestoreState(pPageView); - - ObservedPtr<CPWL_Wnd> pRet(bRestoreValue - ? GetPWLWindow(pPageView) - : CreateOrUpdatePWLWindow(pPageView)); - m_pWidget->UpdateField(); // May invoke JS, invalidating |pRet|. - return pRet.Get(); -} - CFFL_TextObject::CFFL_TextObject(CPDFSDK_FormFillEnvironment* pApp, CPDFSDK_Widget* pWidget) : CFFL_FormField(pApp, pWidget) {} @@ -35,6 +19,22 @@ DestroyWindows(); } +CPWL_Wnd* CFFL_TextObject::ResetPWLWindow(CPDFSDK_PageView* pPageView) { + DestroyPWLWindow(pPageView); + ObservedPtr<CPWL_Wnd> pRet(CreateOrUpdatePWLWindow(pPageView)); + m_pWidget->UpdateField(); // May invoke JS, invalidating |pRet|. + return pRet.Get(); +} + +CPWL_Wnd* CFFL_TextObject::RestorePWLWindow(CPDFSDK_PageView* pPageView) { + SaveState(pPageView); + DestroyPWLWindow(pPageView); + RestoreState(pPageView); + ObservedPtr<CPWL_Wnd> pRet(GetPWLWindow(pPageView)); + m_pWidget->UpdateField(); // May invoke JS, invalidating |pRet|. + return pRet.Get(); +} + CPDF_BAFontMap* CFFL_TextObject::MaybeCreateFontMap() { if (!m_pFontMap) { m_pFontMap = std::make_unique<CPDF_BAFontMap>(
diff --git a/fpdfsdk/formfiller/cffl_textobject.h b/fpdfsdk/formfiller/cffl_textobject.h index c7287a7..b5d3785 100644 --- a/fpdfsdk/formfiller/cffl_textobject.h +++ b/fpdfsdk/formfiller/cffl_textobject.h
@@ -18,8 +18,8 @@ class CFFL_TextObject : public CFFL_FormField { public: // CFFL_FormField: - CPWL_Wnd* ResetPWLWindow(CPDFSDK_PageView* pPageView, - bool bRestoreValue) override; + CPWL_Wnd* ResetPWLWindow(CPDFSDK_PageView* pPageView) override; + CPWL_Wnd* RestorePWLWindow(CPDFSDK_PageView* pPageView) override; protected: CFFL_TextObject(CPDFSDK_FormFillEnvironment* pApp, CPDFSDK_Widget* pWidget);