From 6fe32f898af3eea875fd01a6d18f719d17dd72f3 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 25 Oct 2018 23:25:58 +0000 Subject: Make CPWL_Wnd own its pAttachedData. This requires moving it out of CreateParams, since that must be a copyable struct, and implies that currently there is some questionable sharing going on. To resolve this, introduce a Clone() method so that each window gets its own copy. Make GetAttachedData() return a const pointer, so that callers can't free it behind our back. Tidy initializations along the way. Change-Id: Iadc97688b4692bf4fafefe8cff88af88672f7110 Reviewed-on: https://pdfium-review.googlesource.com/c/44590 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- fpdfsdk/formfiller/cffl_checkbox.cpp | 5 +-- fpdfsdk/formfiller/cffl_checkbox.h | 3 +- fpdfsdk/formfiller/cffl_combobox.cpp | 5 +-- fpdfsdk/formfiller/cffl_combobox.h | 3 +- fpdfsdk/formfiller/cffl_formfiller.cpp | 35 +++++++++------------ fpdfsdk/formfiller/cffl_formfiller.h | 5 +-- fpdfsdk/formfiller/cffl_interactiveformfiller.cpp | 32 +++++++++++++------- fpdfsdk/formfiller/cffl_interactiveformfiller.h | 37 ++++++++++++++--------- fpdfsdk/formfiller/cffl_listbox.cpp | 5 +-- fpdfsdk/formfiller/cffl_listbox.h | 3 +- fpdfsdk/formfiller/cffl_pushbutton.cpp | 7 +++-- fpdfsdk/formfiller/cffl_pushbutton.h | 3 +- fpdfsdk/formfiller/cffl_radiobutton.cpp | 5 +-- fpdfsdk/formfiller/cffl_radiobutton.h | 3 +- fpdfsdk/formfiller/cffl_textfield.cpp | 5 +-- fpdfsdk/formfiller/cffl_textfield.h | 3 +- 16 files changed, 92 insertions(+), 67 deletions(-) (limited to 'fpdfsdk/formfiller') diff --git a/fpdfsdk/formfiller/cffl_checkbox.cpp b/fpdfsdk/formfiller/cffl_checkbox.cpp index ac55125db2..1127d8f258 100644 --- a/fpdfsdk/formfiller/cffl_checkbox.cpp +++ b/fpdfsdk/formfiller/cffl_checkbox.cpp @@ -22,8 +22,9 @@ CFFL_CheckBox::CFFL_CheckBox(CPDFSDK_FormFillEnvironment* pApp, CFFL_CheckBox::~CFFL_CheckBox() {} std::unique_ptr CFFL_CheckBox::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->Create(cp); pWnd->SetCheck(m_pWidget->IsChecked()); return std::move(pWnd); diff --git a/fpdfsdk/formfiller/cffl_checkbox.h b/fpdfsdk/formfiller/cffl_checkbox.h index 2eae22273a..a8fe316356 100644 --- a/fpdfsdk/formfiller/cffl_checkbox.h +++ b/fpdfsdk/formfiller/cffl_checkbox.h @@ -20,7 +20,8 @@ class CFFL_CheckBox final : public CFFL_Button { // CFFL_Button: std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; bool OnKeyDown(CPDFSDK_Annot* pAnnot, uint32_t nKeyCode, uint32_t nFlags) override; diff --git a/fpdfsdk/formfiller/cffl_combobox.cpp b/fpdfsdk/formfiller/cffl_combobox.cpp index 69cc66bbc8..460fdce7ec 100644 --- a/fpdfsdk/formfiller/cffl_combobox.cpp +++ b/fpdfsdk/formfiller/cffl_combobox.cpp @@ -45,8 +45,9 @@ CPWL_Wnd::CreateParams CFFL_ComboBox::GetCreateParam() { } std::unique_ptr CFFL_ComboBox::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->AttachFFLData(this); pWnd->Create(cp); diff --git a/fpdfsdk/formfiller/cffl_combobox.h b/fpdfsdk/formfiller/cffl_combobox.h index 4ec5a9ec0c..66cd55c3e6 100644 --- a/fpdfsdk/formfiller/cffl_combobox.h +++ b/fpdfsdk/formfiller/cffl_combobox.h @@ -30,7 +30,8 @@ class CFFL_ComboBox final : public CFFL_TextObject, // CFFL_TextObject: CPWL_Wnd::CreateParams GetCreateParam() override; std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; bool OnChar(CPDFSDK_Annot* pAnnot, uint32_t nChar, uint32_t nFlags) override; bool IsDataChanged(CPDFSDK_PageView* pPageView) override; void SaveData(CPDFSDK_PageView* pPageView) override; diff --git a/fpdfsdk/formfiller/cffl_formfiller.cpp b/fpdfsdk/formfiller/cffl_formfiller.cpp index 8e9d0c7d65..dace98a8c1 100644 --- a/fpdfsdk/formfiller/cffl_formfiller.cpp +++ b/fpdfsdk/formfiller/cffl_formfiller.cpp @@ -28,15 +28,11 @@ CFFL_FormFiller::~CFFL_FormFiller() { void CFFL_FormFiller::DestroyWindows() { while (!m_Maps.empty()) { - std::unique_ptr pData; - { - auto it = m_Maps.begin(); - std::unique_ptr pWnd = std::move(it->second); - pData.reset(static_cast(pWnd->GetAttachedData())); - m_Maps.erase(it); - pWnd->InvalidateProvider(this); - pWnd->Destroy(); - } + auto it = m_Maps.begin(); + std::unique_ptr pWnd = std::move(it->second); + m_Maps.erase(it); + pWnd->InvalidateProvider(this); + pWnd->Destroy(); } } @@ -395,8 +391,7 @@ CPWL_Wnd* CFFL_FormFiller::GetPDFWindow(CPDFSDK_PageView* pPageView, pPrivateData->pPageView = pPageView; pPrivateData->nWidgetAppearanceAge = m_pWidget->GetAppearanceAge(); pPrivateData->nWidgetValueAge = 0; - cp.pAttachedData = pPrivateData.release(); - m_Maps[pPageView] = NewPDFWindow(cp); + m_Maps[pPageView] = NewPDFWindow(cp, std::move(pPrivateData)); return m_Maps[pPageView].get(); } @@ -404,7 +399,8 @@ CPWL_Wnd* CFFL_FormFiller::GetPDFWindow(CPDFSDK_PageView* pPageView, if (!bNew) return pWnd; - auto* pPrivateData = static_cast(pWnd->GetAttachedData()); + const auto* pPrivateData = + static_cast(pWnd->GetAttachedData()); if (pPrivateData->nWidgetAppearanceAge == m_pWidget->GetAppearanceAge()) return pWnd; @@ -417,18 +413,15 @@ void CFFL_FormFiller::DestroyPDFWindow(CPDFSDK_PageView* pPageView) { if (it == m_Maps.end()) return; - std::unique_ptr pData; - { - std::unique_ptr pWnd = std::move(it->second); - m_Maps.erase(it); - pData.reset(static_cast(pWnd->GetAttachedData())); - pWnd->Destroy(); - } + std::unique_ptr pWnd = std::move(it->second); + m_Maps.erase(it); + pWnd->Destroy(); } -CFX_Matrix CFFL_FormFiller::GetWindowMatrix(CPWL_Wnd::PrivateData* pAttached) { +CFX_Matrix CFFL_FormFiller::GetWindowMatrix( + const CPWL_Wnd::PrivateData* pAttached) { CFX_Matrix mt; - auto* pPrivateData = static_cast(pAttached); + const auto* pPrivateData = static_cast(pAttached); if (!pPrivateData || !pPrivateData->pPageView) return mt; diff --git a/fpdfsdk/formfiller/cffl_formfiller.h b/fpdfsdk/formfiller/cffl_formfiller.h index a6f7cdc78f..6b04e0c936 100644 --- a/fpdfsdk/formfiller/cffl_formfiller.h +++ b/fpdfsdk/formfiller/cffl_formfiller.h @@ -93,7 +93,7 @@ class CFFL_FormFiller : public CPWL_Wnd::ProviderIface, CFX_SystemHandler* GetSystemHandler() const override; // CPWL_Wnd::ProviderIface: - CFX_Matrix GetWindowMatrix(CPWL_Wnd::PrivateData* pAttached) override; + CFX_Matrix GetWindowMatrix(const CPWL_Wnd::PrivateData* pAttached) override; virtual void GetActionData(CPDFSDK_PageView* pPageView, CPDF_AAction::AActionType type, @@ -107,7 +107,8 @@ class CFFL_FormFiller : public CPWL_Wnd::ProviderIface, virtual CPWL_Wnd::CreateParams GetCreateParam(); virtual std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) = 0; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) = 0; virtual CPWL_Wnd* ResetPDFWindow(CPDFSDK_PageView* pPageView, bool bRestoreValue); virtual void SaveState(CPDFSDK_PageView* pPageView); diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp index c712610dcc..b578a440b7 100644 --- a/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp +++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.cpp @@ -570,12 +570,12 @@ void CFFL_InteractiveFormFiller::UnRegisterFormFiller(CPDFSDK_Annot* pAnnot) { } void CFFL_InteractiveFormFiller::QueryWherePopup( - CPWL_Wnd::PrivateData* pAttached, + const CPWL_Wnd::PrivateData* pAttached, float fPopupMin, float fPopupMax, bool* bBottom, float* fPopupRet) { - auto* pData = static_cast(pAttached); + auto* pData = static_cast(pAttached); CPDFSDK_Widget* pWidget = pData->pWidget; CPDF_Page* pPage = pWidget->GetPDFPage(); @@ -795,10 +795,9 @@ bool CFFL_InteractiveFormFiller::OnFull(CPDFSDK_Annot::ObservedPtr* pAnnot, } bool CFFL_InteractiveFormFiller::OnPopupPreOpen( - CPWL_Wnd::PrivateData* pAttached, + const CPWL_Wnd::PrivateData* pAttached, uint32_t nFlag) { - auto* pData = static_cast(pAttached); - ASSERT(pData); + auto* pData = static_cast(pAttached); ASSERT(pData->pWidget); CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget); @@ -806,10 +805,9 @@ bool CFFL_InteractiveFormFiller::OnPopupPreOpen( } bool CFFL_InteractiveFormFiller::OnPopupPostOpen( - CPWL_Wnd::PrivateData* pAttached, + const CPWL_Wnd::PrivateData* pAttached, uint32_t nFlag) { - auto* pData = static_cast(pAttached); - ASSERT(pData); + auto* pData = static_cast(pAttached); ASSERT(pData->pWidget); CPDFSDK_Annot::ObservedPtr pObserved(pData->pWidget); @@ -885,7 +883,7 @@ bool CFFL_InteractiveFormFiller::IsValidAnnot(CPDFSDK_PageView* pPageView, } std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( - CPWL_Wnd::PrivateData* pAttached, + const CPWL_Wnd::PrivateData* pAttached, WideString& strChange, const WideString& strChangeEx, int nSelStart, @@ -893,7 +891,8 @@ std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( bool bKeyDown, uint32_t nFlag) { // Copy the private data since the window owning it may not survive. - CFFL_PrivateData privateData = *static_cast(pAttached); + CFFL_PrivateData privateData = + *static_cast(pAttached); ASSERT(privateData.pWidget); CFFL_FormFiller* pFormFiller = GetFormFiller(privateData.pWidget, false); @@ -949,7 +948,8 @@ std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( privateData.pPageView, nValueAge == privateData.pWidget->GetValueAge()); if (!pWnd) return {true, true}; - privateData = *static_cast(pWnd->GetAttachedData()); + privateData = + *static_cast(pWnd->GetAttachedData()); bExit = true; } if (fa.bRC) { @@ -964,3 +964,13 @@ std::pair CFFL_InteractiveFormFiller::OnBeforeKeyStroke( pFormFiller->CommitData(privateData.pPageView, nFlag); return {false, true}; } + +CFFL_PrivateData::CFFL_PrivateData() = default; + +CFFL_PrivateData::CFFL_PrivateData(const CFFL_PrivateData& that) = default; + +CFFL_PrivateData::~CFFL_PrivateData() = default; + +std::unique_ptr CFFL_PrivateData::Clone() const { + return pdfium::MakeUnique(*this); +} diff --git a/fpdfsdk/formfiller/cffl_interactiveformfiller.h b/fpdfsdk/formfiller/cffl_interactiveformfiller.h index bf9fa3c09c..f4c24e3f56 100644 --- a/fpdfsdk/formfiller/cffl_interactiveformfiller.h +++ b/fpdfsdk/formfiller/cffl_interactiveformfiller.h @@ -131,26 +131,28 @@ class CFFL_InteractiveFormFiller final : public IPWL_Filler_Notify { std::map>; // IPWL_Filler_Notify: - void QueryWherePopup(CPWL_Wnd::PrivateData* pAttached, + void QueryWherePopup(const CPWL_Wnd::PrivateData* pAttached, float fPopupMin, float fPopupMax, bool* bBottom, float* fPopupRet) override; // Returns {bRC, bExit}. - std::pair OnBeforeKeyStroke(CPWL_Wnd::PrivateData* pAttached, - WideString& strChange, - const WideString& strChangeEx, - int nSelStart, - int nSelEnd, - bool bKeyDown, - uint32_t nFlag) override; + std::pair OnBeforeKeyStroke( + const CPWL_Wnd::PrivateData* pAttached, + WideString& strChange, + const WideString& strChangeEx, + int nSelStart, + int nSelEnd, + bool bKeyDown, + uint32_t nFlag) override; #ifdef PDF_ENABLE_XFA - bool OnPopupPreOpen(CPWL_Wnd::PrivateData* pAttached, + bool OnPopupPreOpen(const CPWL_Wnd::PrivateData* pAttached, uint32_t nFlag) override; - bool OnPopupPostOpen(CPWL_Wnd::PrivateData* pAttached, + bool OnPopupPostOpen(const CPWL_Wnd::PrivateData* pAttached, uint32_t nFlag) override; void SetFocusAnnotTab(CPDFSDK_Annot* pWidget, bool bSameField, bool bNext); #endif // PDF_ENABLE_XFA + void UnRegisterFormFiller(CPDFSDK_Annot* pAnnot); UnownedPtr const m_pFormFillEnv; @@ -160,10 +162,17 @@ class CFFL_InteractiveFormFiller final : public IPWL_Filler_Notify { class CFFL_PrivateData final : public CPWL_Wnd::PrivateData { public: - CPDFSDK_Widget* pWidget; - CPDFSDK_PageView* pPageView; - uint32_t nWidgetAppearanceAge; - uint32_t nWidgetValueAge; + CFFL_PrivateData(); + CFFL_PrivateData(const CFFL_PrivateData& that); + ~CFFL_PrivateData() override; + + // CPWL_Wnd::PrivateData: + std::unique_ptr Clone() const override; + + CPDFSDK_Widget* pWidget = nullptr; + CPDFSDK_PageView* pPageView = nullptr; + uint32_t nWidgetAppearanceAge = 0; + uint32_t nWidgetValueAge = 0; }; #endif // FPDFSDK_FORMFILLER_CFFL_INTERACTIVEFORMFILLER_H_ diff --git a/fpdfsdk/formfiller/cffl_listbox.cpp b/fpdfsdk/formfiller/cffl_listbox.cpp index 3c74875d43..88414cc9fc 100644 --- a/fpdfsdk/formfiller/cffl_listbox.cpp +++ b/fpdfsdk/formfiller/cffl_listbox.cpp @@ -40,8 +40,9 @@ CPWL_Wnd::CreateParams CFFL_ListBox::GetCreateParam() { } std::unique_ptr CFFL_ListBox::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->AttachFFLData(this); pWnd->Create(cp); pWnd->SetFillerNotify(m_pFormFillEnv->GetInteractiveFormFiller()); diff --git a/fpdfsdk/formfiller/cffl_listbox.h b/fpdfsdk/formfiller/cffl_listbox.h index eed542fde5..ab4005f7ec 100644 --- a/fpdfsdk/formfiller/cffl_listbox.h +++ b/fpdfsdk/formfiller/cffl_listbox.h @@ -23,7 +23,8 @@ class CFFL_ListBox final : public CFFL_TextObject { // CFFL_TextObject: CPWL_Wnd::CreateParams GetCreateParam() override; std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; bool OnChar(CPDFSDK_Annot* pAnnot, uint32_t nChar, uint32_t nFlags) override; bool IsDataChanged(CPDFSDK_PageView* pPageView) override; void SaveData(CPDFSDK_PageView* pPageView) override; diff --git a/fpdfsdk/formfiller/cffl_pushbutton.cpp b/fpdfsdk/formfiller/cffl_pushbutton.cpp index b57c911486..76719d95c8 100644 --- a/fpdfsdk/formfiller/cffl_pushbutton.cpp +++ b/fpdfsdk/formfiller/cffl_pushbutton.cpp @@ -15,11 +15,12 @@ CFFL_PushButton::CFFL_PushButton(CPDFSDK_FormFillEnvironment* pApp, CPDFSDK_Widget* pWidget) : CFFL_Button(pApp, pWidget) {} -CFFL_PushButton::~CFFL_PushButton() {} +CFFL_PushButton::~CFFL_PushButton() = default; std::unique_ptr CFFL_PushButton::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->Create(cp); return std::move(pWnd); } diff --git a/fpdfsdk/formfiller/cffl_pushbutton.h b/fpdfsdk/formfiller/cffl_pushbutton.h index cc2ff89184..f10dd67f29 100644 --- a/fpdfsdk/formfiller/cffl_pushbutton.h +++ b/fpdfsdk/formfiller/cffl_pushbutton.h @@ -18,7 +18,8 @@ class CFFL_PushButton final : public CFFL_Button { // CFFL_Button: std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; }; #endif // FPDFSDK_FORMFILLER_CFFL_PUSHBUTTON_H_ diff --git a/fpdfsdk/formfiller/cffl_radiobutton.cpp b/fpdfsdk/formfiller/cffl_radiobutton.cpp index 6ce4a0aedc..4ae80d6a77 100644 --- a/fpdfsdk/formfiller/cffl_radiobutton.cpp +++ b/fpdfsdk/formfiller/cffl_radiobutton.cpp @@ -21,8 +21,9 @@ CFFL_RadioButton::CFFL_RadioButton(CPDFSDK_FormFillEnvironment* pApp, CFFL_RadioButton::~CFFL_RadioButton() {} std::unique_ptr CFFL_RadioButton::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->Create(cp); pWnd->SetCheck(m_pWidget->IsChecked()); return std::move(pWnd); diff --git a/fpdfsdk/formfiller/cffl_radiobutton.h b/fpdfsdk/formfiller/cffl_radiobutton.h index 4702a27aec..0076919144 100644 --- a/fpdfsdk/formfiller/cffl_radiobutton.h +++ b/fpdfsdk/formfiller/cffl_radiobutton.h @@ -20,7 +20,8 @@ class CFFL_RadioButton final : public CFFL_Button { // CFFL_Button: std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; bool OnKeyDown(CPDFSDK_Annot* pAnnot, uint32_t nKeyCode, uint32_t nFlags) override; diff --git a/fpdfsdk/formfiller/cffl_textfield.cpp b/fpdfsdk/formfiller/cffl_textfield.cpp index ae3cb06e99..e3cde98c82 100644 --- a/fpdfsdk/formfiller/cffl_textfield.cpp +++ b/fpdfsdk/formfiller/cffl_textfield.cpp @@ -70,8 +70,9 @@ CPWL_Wnd::CreateParams CFFL_TextField::GetCreateParam() { } std::unique_ptr CFFL_TextField::NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) { - auto pWnd = pdfium::MakeUnique(); + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) { + auto pWnd = pdfium::MakeUnique(std::move(pAttachedData)); pWnd->AttachFFLData(this); pWnd->Create(cp); pWnd->SetFillerNotify(m_pFormFillEnv->GetInteractiveFormFiller()); diff --git a/fpdfsdk/formfiller/cffl_textfield.h b/fpdfsdk/formfiller/cffl_textfield.h index 3be00959e4..1ba0e6399f 100644 --- a/fpdfsdk/formfiller/cffl_textfield.h +++ b/fpdfsdk/formfiller/cffl_textfield.h @@ -35,7 +35,8 @@ class CFFL_TextField final : public CFFL_TextObject, // CFFL_TextObject: CPWL_Wnd::CreateParams GetCreateParam() override; std::unique_ptr NewPDFWindow( - const CPWL_Wnd::CreateParams& cp) override; + const CPWL_Wnd::CreateParams& cp, + std::unique_ptr pAttachedData) override; bool OnChar(CPDFSDK_Annot* pAnnot, uint32_t nChar, uint32_t nFlags) override; bool IsDataChanged(CPDFSDK_PageView* pPageView) override; void SaveData(CPDFSDK_PageView* pPageView) override; -- cgit v1.2.3