From 9792f16f3ef27a1e0c7f0526cc69637a158e3010 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 16 May 2017 14:11:30 -0700 Subject: Properly ref-count CPDFXFA_Page Change-Id: Ibd1ebe4191f61e20d815de7f1a1094d78b72e6a7 Reviewed-on: https://pdfium-review.googlesource.com/5391 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- fpdfsdk/fpdfeditpage.cpp | 10 ++++---- fpdfsdk/fpdfview.cpp | 7 +++--- fpdfsdk/fpdfxfa/cpdfxfa_context.cpp | 37 +++++++++------------------ fpdfsdk/fpdfxfa/cpdfxfa_context.h | 19 ++++++-------- fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp | 40 ++++++++++++++++-------------- fpdfsdk/fpdfxfa/cpdfxfa_page.cpp | 12 +++------ fpdfsdk/fpdfxfa/cpdfxfa_page.h | 15 +++++------ 7 files changed, 60 insertions(+), 80 deletions(-) diff --git a/fpdfsdk/fpdfeditpage.cpp b/fpdfsdk/fpdfeditpage.cpp index 739723f6ee..66ff028141 100644 --- a/fpdfsdk/fpdfeditpage.cpp +++ b/fpdfsdk/fpdfeditpage.cpp @@ -118,15 +118,15 @@ DLLEXPORT FPDF_PAGE STDCALL FPDFPage_New(FPDF_DOCUMENT document, pPageDict->SetNewFor("Resources"); #ifdef PDF_ENABLE_XFA - CPDFXFA_Page* pPage = - new CPDFXFA_Page(static_cast(document), page_index); - pPage->LoadPDFPage(pPageDict); + auto pXFAPage = pdfium::MakeRetain( + static_cast(document), page_index); + pXFAPage->LoadPDFPage(pPageDict); + return pXFAPage.Leak(); #else // PDF_ENABLE_XFA CPDF_Page* pPage = new CPDF_Page(pDoc, pPageDict, true); pPage->ParseContent(); -#endif // PDF_ENABLE_XFA - return pPage; +#endif // PDF_ENABLE_XFA } DLLEXPORT int STDCALL FPDFPage_GetRotation(FPDF_PAGE page) { diff --git a/fpdfsdk/fpdfview.cpp b/fpdfsdk/fpdfview.cpp index 5fb963abd3..f8078073b7 100644 --- a/fpdfsdk/fpdfview.cpp +++ b/fpdfsdk/fpdfview.cpp @@ -628,7 +628,7 @@ DLLEXPORT FPDF_PAGE STDCALL FPDF_LoadPage(FPDF_DOCUMENT document, return nullptr; #ifdef PDF_ENABLE_XFA - return pDoc->GetXFAPage(page_index); + return pDoc->GetXFAPage(page_index).Leak(); #else // PDF_ENABLE_XFA CPDF_Dictionary* pDict = pDoc->GetPage(page_index); if (!pDict) @@ -974,7 +974,8 @@ DLLEXPORT void STDCALL FPDF_ClosePage(FPDF_PAGE page) { if (!page) return; #ifdef PDF_ENABLE_XFA - pPage->Release(); + // Take it back across the API and throw it away. + CFX_RetainPtr().Unleak(pPage); #else // PDF_ENABLE_XFA CPDFSDK_PageView* pPageView = static_cast(pPage->GetView()); @@ -1176,7 +1177,7 @@ DLLEXPORT int STDCALL FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document, int count = pDoc->GetPageCount(); if (page_index < 0 || page_index >= count) return false; - CPDFXFA_Page* pPage = pDoc->GetXFAPage(page_index); + CFX_RetainPtr pPage = pDoc->GetXFAPage(page_index); if (!pPage) return false; *width = pPage->GetPageWidth(); diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp index 93ecab0d08..1b69ff45b9 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp @@ -152,34 +152,30 @@ int CPDFXFA_Context::GetPageCount() const { } } -CPDFXFA_Page* CPDFXFA_Context::GetXFAPage(int page_index) { +CFX_RetainPtr CPDFXFA_Context::GetXFAPage(int page_index) { if (page_index < 0) return nullptr; - CPDFXFA_Page* pPage = nullptr; - int nCount = pdfium::CollectionSize(m_XFAPageList); - if (nCount > 0 && page_index < nCount) { - pPage = m_XFAPageList[page_index]; - if (pPage) { - pPage->Retain(); - return pPage; - } + if (pdfium::IndexInBounds(m_XFAPageList, page_index)) { + if (m_XFAPageList[page_index]) + return m_XFAPageList[page_index]; } else { m_nPageCount = GetPageCount(); m_XFAPageList.resize(m_nPageCount); } - pPage = new CPDFXFA_Page(this, page_index); - if (!pPage->LoadPage()) { - pPage->Release(); + auto pPage = pdfium::MakeRetain(this, page_index); + if (!pPage->LoadPage()) return nullptr; - } + if (pdfium::IndexInBounds(m_XFAPageList, page_index)) m_XFAPageList[page_index] = pPage; + return pPage; } -CPDFXFA_Page* CPDFXFA_Context::GetXFAPage(CXFA_FFPageView* pPage) const { +CFX_RetainPtr CPDFXFA_Context::GetXFAPage( + CXFA_FFPageView* pPage) const { if (!pPage) return nullptr; @@ -189,7 +185,7 @@ CPDFXFA_Page* CPDFXFA_Context::GetXFAPage(CXFA_FFPageView* pPage) const { if (m_iDocType != XFA_DocType::Dynamic) return nullptr; - for (CPDFXFA_Page* pTempPage : m_XFAPageList) { + for (auto& pTempPage : m_XFAPageList) { if (pTempPage && pTempPage->GetXFAPageView() == pPage) return pTempPage; } @@ -203,17 +199,8 @@ void CPDFXFA_Context::DeletePage(int page_index) { if (m_pPDFDoc) m_pPDFDoc->DeletePage(page_index); - if (!pdfium::IndexInBounds(m_XFAPageList, page_index)) - return; - - if (CPDFXFA_Page* pPage = m_XFAPageList[page_index]) - pPage->Release(); -} - -void CPDFXFA_Context::RemovePage(CPDFXFA_Page* page) { - int page_index = page->GetPageIndex(); if (pdfium::IndexInBounds(m_XFAPageList, page_index)) - m_XFAPageList[page_index] = nullptr; + m_XFAPageList[page_index].Reset(); } void CPDFXFA_Context::ClearChangeMark() { diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.h b/fpdfsdk/fpdfxfa/cpdfxfa_context.h index 690e1855e3..6d7e33b689 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_context.h +++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.h @@ -13,11 +13,11 @@ #include "core/fxcrt/cfx_unowned_ptr.h" #include "core/fxcrt/fx_system.h" #include "fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.h" +#include "fpdfsdk/fpdfxfa/cpdfxfa_page.h" #include "xfa/fxfa/cxfa_ffdoc.h" class CJS_Runtime; class CPDFSDK_FormFillEnvironment; -class CPDFXFA_Page; class CXFA_FFDocHandler; class IJS_EventContext; class IJS_Runtime; @@ -48,14 +48,10 @@ class CPDFXFA_Context : public IXFA_AppProvider { } void SetFormFillEnv(CPDFSDK_FormFillEnvironment* pFormFillEnv); - void DeletePage(int page_index); int GetPageCount() const; - - CPDFXFA_Page* GetXFAPage(int page_index); - CPDFXFA_Page* GetXFAPage(CXFA_FFPageView* pPage) const; - - void RemovePage(CPDFXFA_Page* page); - + void DeletePage(int page_index); + CFX_RetainPtr GetXFAPage(int page_index); + CFX_RetainPtr GetXFAPage(CXFA_FFPageView* pPage) const; void ClearChangeMark(); // IFXA_AppProvider: @@ -97,20 +93,21 @@ class CPDFXFA_Context : public IXFA_AppProvider { } LoadStatus GetLoadStatus() const { return m_nLoadStatus; } - std::vector* GetXFAPageList() { return &m_XFAPageList; } + std::vector>* GetXFAPageList() { + return &m_XFAPageList; + } private: void CloseXFADoc(); XFA_DocType m_iDocType; - std::unique_ptr m_pPDFDoc; std::unique_ptr m_pXFADoc; CFX_UnownedPtr m_pFormFillEnv; CFX_UnownedPtr m_pXFADocView; std::unique_ptr m_pXFAApp; std::unique_ptr m_pRuntime; - std::vector m_XFAPageList; + std::vector> m_XFAPageList; LoadStatus m_nLoadStatus; int m_nPageCount; diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp index 1a27d9b015..11b001fc7f 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp @@ -63,7 +63,7 @@ void CPDFXFA_DocEnvironment::InvalidateRect(CXFA_FFPageView* pPageView, if (m_pContext->GetDocType() != XFA_DocType::Dynamic) return; - CPDFXFA_Page* pPage = m_pContext->GetXFAPage(pPageView); + CFX_RetainPtr pPage = m_pContext->GetXFAPage(pPageView); if (!pPage) return; @@ -71,7 +71,7 @@ void CPDFXFA_DocEnvironment::InvalidateRect(CXFA_FFPageView* pPageView, if (!pFormFillEnv) return; - pFormFillEnv->Invalidate(static_cast(pPage), + pFormFillEnv->Invalidate(static_cast(pPage.Get()), CFX_FloatRect::FromCFXRectF(rt).ToFxRect()); } @@ -94,7 +94,7 @@ void CPDFXFA_DocEnvironment::DisplayCaret(CXFA_FFWidget* hWidget, if (!pPageView) return; - CPDFXFA_Page* pPage = m_pContext->GetXFAPage(pPageView); + CFX_RetainPtr pPage = m_pContext->GetXFAPage(pPageView); if (!pPage) return; @@ -103,8 +103,9 @@ void CPDFXFA_DocEnvironment::DisplayCaret(CXFA_FFWidget* hWidget, return; CFX_FloatRect rcCaret = CFX_FloatRect::FromCFXRectF(*pRtAnchor); - pFormFillEnv->DisplayCaret((FPDF_PAGE)pPage, bVisible, rcCaret.left, - rcCaret.top, rcCaret.right, rcCaret.bottom); + pFormFillEnv->DisplayCaret(static_cast(pPage.Get()), bVisible, + rcCaret.left, rcCaret.top, rcCaret.right, + rcCaret.bottom); } bool CPDFXFA_DocEnvironment::GetPopupPos(CXFA_FFWidget* hWidget, @@ -119,22 +120,21 @@ bool CPDFXFA_DocEnvironment::GetPopupPos(CXFA_FFWidget* hWidget, if (!pXFAPageView) return false; - CPDFXFA_Page* pPage = m_pContext->GetXFAPage(pXFAPageView); + CFX_RetainPtr pPage = m_pContext->GetXFAPage(pXFAPageView); if (!pPage) return false; - CXFA_WidgetAcc* pWidgetAcc = hWidget->GetDataAcc(); - int nRotate = pWidgetAcc->GetRotate(); CPDFSDK_FormFillEnvironment* pFormFillEnv = m_pContext->GetFormFillEnv(); if (!pFormFillEnv) return false; FS_RECTF pageViewRect = {0.0f, 0.0f, 0.0f, 0.0f}; - pFormFillEnv->GetPageViewRect(pPage, pageViewRect); + pFormFillEnv->GetPageViewRect(pPage.Get(), pageViewRect); int t1; int t2; CFX_FloatRect rcAnchor = CFX_FloatRect::FromCFXRectF(rtAnchor); + int nRotate = hWidget->GetDataAcc()->GetRotate(); switch (nRotate) { case 90: { t1 = (int)(pageViewRect.right - rcAnchor.right); @@ -232,7 +232,7 @@ bool CPDFXFA_DocEnvironment::PopupMenu(CXFA_FFWidget* hWidget, if (!pXFAPageView) return false; - CPDFXFA_Page* pPage = m_pContext->GetXFAPage(pXFAPageView); + CFX_RetainPtr pPage = m_pContext->GetXFAPage(pXFAPageView); if (!pPage) return false; @@ -254,7 +254,7 @@ bool CPDFXFA_DocEnvironment::PopupMenu(CXFA_FFWidget* hWidget, if (hWidget->CanSelectAll()) menuFlag |= FXFA_MENU_SELECTALL; - return pFormFillEnv->PopupMenu(pPage, hWidget, menuFlag, ptPopup); + return pFormFillEnv->PopupMenu(pPage.Get(), hWidget, menuFlag, ptPopup); } void CPDFXFA_DocEnvironment::PageViewEvent(CXFA_FFPageView* pPageView, @@ -278,11 +278,12 @@ void CPDFXFA_DocEnvironment::PageViewEvent(CXFA_FFPageView* pPageView, for (int iPageIter = 0; iPageIter < m_pContext->GetOriginalPageCount(); iPageIter++) { - CPDFXFA_Page* pPage = (*m_pContext->GetXFAPageList())[iPageIter]; + CFX_RetainPtr pPage = + (*m_pContext->GetXFAPageList())[iPageIter]; if (!pPage) continue; - m_pContext->GetFormFillEnv()->RemovePageView(pPage); + m_pContext->GetFormFillEnv()->RemovePageView(pPage.Get()); pPage->SetXFAPageView(pXFADocView->GetPageView(iPageIter)); } @@ -303,11 +304,13 @@ void CPDFXFA_DocEnvironment::WidgetPostAdd(CXFA_FFWidget* hWidget, if (!pPageView) return; - CPDFXFA_Page* pXFAPage = m_pContext->GetXFAPage(pPageView); + CFX_RetainPtr pXFAPage = m_pContext->GetXFAPage(pPageView); if (!pXFAPage) return; - m_pContext->GetFormFillEnv()->GetPageView(pXFAPage, true)->AddAnnot(hWidget); + m_pContext->GetFormFillEnv() + ->GetPageView(pXFAPage.Get(), true) + ->AddAnnot(hWidget); } void CPDFXFA_DocEnvironment::WidgetPreRemove(CXFA_FFWidget* hWidget, @@ -319,13 +322,14 @@ void CPDFXFA_DocEnvironment::WidgetPreRemove(CXFA_FFWidget* hWidget, if (!pPageView) return; - CPDFXFA_Page* pXFAPage = m_pContext->GetXFAPage(pPageView); + CFX_RetainPtr pXFAPage = m_pContext->GetXFAPage(pPageView); if (!pXFAPage) return; CPDFSDK_PageView* pSdkPageView = - m_pContext->GetFormFillEnv()->GetPageView(pXFAPage, true); - if (CPDFSDK_Annot* pAnnot = pSdkPageView->GetAnnotByXFAWidget(hWidget)) + m_pContext->GetFormFillEnv()->GetPageView(pXFAPage.Get(), true); + CPDFSDK_Annot* pAnnot = pSdkPageView->GetAnnotByXFAWidget(hWidget); + if (pAnnot) pSdkPageView->DeleteAnnot(pAnnot); } diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp index e5ab9d357f..24858f2f1a 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp @@ -17,15 +17,9 @@ #include "xfa/fxfa/cxfa_ffpageview.h" CPDFXFA_Page::CPDFXFA_Page(CPDFXFA_Context* pContext, int page_index) - : m_pXFAPageView(nullptr), - m_pContext(pContext), - m_iPageIndex(page_index), - m_iRef(1) {} - -CPDFXFA_Page::~CPDFXFA_Page() { - if (m_pContext) - m_pContext->RemovePage(this); -} + : m_pXFAPageView(nullptr), m_pContext(pContext), m_iPageIndex(page_index) {} + +CPDFXFA_Page::~CPDFXFA_Page() {} bool CPDFXFA_Page::LoadPDFPage() { if (!m_pContext) diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.h b/fpdfsdk/fpdfxfa/cpdfxfa_page.h index 05b9238eb7..d990813282 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.h +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.h @@ -9,6 +9,7 @@ #include +#include "core/fxcrt/cfx_retain_ptr.h" #include "core/fxcrt/fx_system.h" class CFX_Matrix; @@ -17,15 +18,10 @@ class CPDF_Dictionary; class CPDF_Page; class CXFA_FFPageView; -class CPDFXFA_Page { +class CPDFXFA_Page : public CFX_Retainable { public: - CPDFXFA_Page(CPDFXFA_Context* pContext, int page_index); - - void Retain() { m_iRef++; } - void Release() { - if (--m_iRef <= 0) - delete this; - } + template + friend CFX_RetainPtr pdfium::MakeRetain(Args&&... args); bool LoadPage(); bool LoadPDFPage(CPDF_Dictionary* pageDict); @@ -68,7 +64,8 @@ class CPDFXFA_Page { protected: // Refcounted class. - ~CPDFXFA_Page(); + CPDFXFA_Page(CPDFXFA_Context* pContext, int page_index); + ~CPDFXFA_Page() override; bool LoadPDFPage(); bool LoadXFAPageView(); -- cgit v1.2.3