From 2aa4b2f3928ccd8393f60db8f7b740c75f4e8a8d Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 25 May 2018 22:38:49 +0000 Subject: Make CPDF_Page retainable. Small step to reducing the differences between XFA and non-XFA. We still use the RetainPtr pretty much as if it were an unique_ptr, in that we're not yet caching pages and handing out multiple pointers to the same page in the non-XFA case. The one change is in page view cleanup, where we no longer need a boolean and can take (sufficient) page ownership with a RetainPtr. Tidy up some document.h -> page.h -> document.h circular inclusion while we're at it. NOTE: Wait for imminent branch to pass before landing. We'll want this to bake a while. Change-Id: I64a2f12ac3424ece1063d40583995b834117cf34 Reviewed-on: https://pdfium-review.googlesource.com/32790 Reviewed-by: dsinclair Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- fpdfsdk/cpdfsdk_pageview.cpp | 15 +----------- fpdfsdk/cpdfsdk_pageview.h | 13 +++++------ fpdfsdk/fpdf_editpage.cpp | 4 ++-- fpdfsdk/fpdf_flatten.cpp | 2 +- fpdfsdk/fpdf_ppo.cpp | 4 ++-- fpdfsdk/fpdf_view.cpp | 49 +++++++++++++++++----------------------- fpdfsdk/fpdfxfa/cpdfxfa_page.cpp | 4 ++-- fpdfsdk/fpdfxfa/cpdfxfa_page.h | 7 +++--- 8 files changed, 38 insertions(+), 60 deletions(-) (limited to 'fpdfsdk') diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp index 2eab6ca32b..c86adf323d 100644 --- a/fpdfsdk/cpdfsdk_pageview.cpp +++ b/fpdfsdk/cpdfsdk_pageview.cpp @@ -32,15 +32,7 @@ CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_FormFillEnvironment* pFormFillEnv, UnderlyingPageType* page) - : m_page(page), - m_pFormFillEnv(pFormFillEnv), -#ifndef PDF_ENABLE_XFA - m_bOwnsPage(false), -#endif // PDF_ENABLE_XFA - m_bOnWidget(false), - m_bValid(false), - m_bLocked(false), - m_bBeingDestroyed(false) { + : m_page(page), m_pFormFillEnv(pFormFillEnv) { CPDFSDK_InterForm* pInterForm = pFormFillEnv->GetInterForm(); CPDF_InterForm* pPDFInterForm = pInterForm->GetInterForm(); #ifdef PDF_ENABLE_XFA @@ -67,11 +59,6 @@ CPDFSDK_PageView::~CPDFSDK_PageView() { m_SDKAnnotArray.clear(); m_pAnnotList.reset(); - -#ifndef PDF_ENABLE_XFA - if (m_bOwnsPage) - delete m_page; -#endif // PDF_ENABLE_XFA } void CPDFSDK_PageView::PageView_OnDraw(CFX_RenderDevice* pDevice, diff --git a/fpdfsdk/cpdfsdk_pageview.h b/fpdfsdk/cpdfsdk_pageview.h index e85b32aa78..afb200e43b 100644 --- a/fpdfsdk/cpdfsdk_pageview.h +++ b/fpdfsdk/cpdfsdk_pageview.h @@ -102,8 +102,7 @@ class CPDFSDK_PageView final : public CPDF_Page::View { bool IsBeingDestroyed() const { return m_bBeingDestroyed; } #ifndef PDF_ENABLE_XFA - bool OwnsPage() const { return m_bOwnsPage; } - void TakePageOwnership() { m_bOwnsPage = true; } + void TakePageOwnership() { m_pOwnsPage.Reset(m_page); } #endif // PDF_ENABLE_XFA private: @@ -126,12 +125,12 @@ class CPDFSDK_PageView final : public CPDF_Page::View { UnownedPtr const m_pFormFillEnv; CPDFSDK_Annot::ObservedPtr m_pCaptureWidget; #ifndef PDF_ENABLE_XFA - bool m_bOwnsPage; + RetainPtr m_pOwnsPage; #endif // PDF_ENABLE_XFA - bool m_bOnWidget; - bool m_bValid; - bool m_bLocked; - bool m_bBeingDestroyed; + bool m_bOnWidget = false; + bool m_bValid = false; + bool m_bLocked = false; + bool m_bBeingDestroyed = false; }; #endif // FPDFSDK_CPDFSDK_PAGEVIEW_H_ diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index ea4659f54a..39cf85fb95 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp @@ -196,9 +196,9 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDFPage_New(FPDF_DOCUMENT document, // Eventually, fallthru into non-XFA case once page type is consistent. return nullptr; #else // PDF_ENABLE_XFA - auto pPage = pdfium::MakeUnique(pDoc, pPageDict, true); + auto pPage = pdfium::MakeRetain(pDoc, pPageDict, true); pPage->ParseContent(); - return FPDFPageFromUnderlying(pPage.release()); // Caller takes ownership. + return FPDFPageFromUnderlying(pPage.Leak()); // Caller takes ownership. #endif // PDF_ENABLE_XFA } diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp index 03a4f60b5c..720fa9ff55 100644 --- a/fpdfsdk/fpdf_flatten.cpp +++ b/fpdfsdk/fpdf_flatten.cpp @@ -47,7 +47,7 @@ bool IsValidRect(const CFX_FloatRect& rect, const CFX_FloatRect& rcPage) { void GetContentsRect(CPDF_Document* pDoc, CPDF_Dictionary* pDict, std::vector* pRectArray) { - auto pPDFPage = pdfium::MakeUnique(pDoc, pDict, false); + auto pPDFPage = pdfium::MakeRetain(pDoc, pDict, false); pPDFPage->ParseContent(); for (const auto& pPageObject : *pPDFPage->GetPageObjectList()) { diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp index 1790e17fb3..b73042466b 100644 --- a/fpdfsdk/fpdf_ppo.cpp +++ b/fpdfsdk/fpdf_ppo.cpp @@ -618,9 +618,9 @@ bool CPDF_NPageToOneExporter::ExportNPagesToOne( if (!pSrcPageDict) return false; - CPDF_Page srcPage(src(), pSrcPageDict, true); + auto srcPage = pdfium::MakeRetain(src(), pSrcPageDict, true); NupPageSettings settings = - nupState.CalculateNewPagePosition(srcPage.GetPageSize()); + nupState.CalculateNewPagePosition(srcPage->GetPageSize()); AddSubPage(pSrcPageDict, settings, &objectNumberMap, &pageXObjectMap, &xObjNameNumberMap, &bsContent); } diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index 6c4ad6020e..593a092fd3 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp @@ -355,9 +355,9 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document, if (!pDict) return nullptr; - CPDF_Page* pPage = new CPDF_Page(pDoc, pDict, true); + auto pPage = pdfium::MakeRetain(pDoc, pDict, true); pPage->ParseContent(); - return FPDFPageFromUnderlying(pPage); + return FPDFPageFromUnderlying(pPage.Leak()); #endif // PDF_ENABLE_XFA } @@ -723,35 +723,28 @@ FPDF_EXPORT FPDF_RECORDER FPDF_CALLCONV FPDF_RenderPageSkp(FPDF_PAGE page, #endif FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) { - UnderlyingPageType* pPage = UnderlyingFromFPDFPage(page); if (!page) return; -#ifdef PDF_ENABLE_XFA - // Take it back across the API and throw it away. - RetainPtr().Unleak(pPage); -#else // PDF_ENABLE_XFA + + // Take it back across the API and hold for duration of this function. + RetainPtr pPage; + pPage.Unleak(UnderlyingFromFPDFPage(page)); + +#ifndef PDF_ENABLE_XFA CPDFSDK_PageView* pPageView = static_cast(pPage->GetView()); - if (pPageView) { - // We're already destroying the pageview, so bail early. - if (pPageView->IsBeingDestroyed()) - return; - - if (pPageView->IsLocked()) { - pPageView->TakePageOwnership(); - return; - } + if (!pPageView || pPageView->IsBeingDestroyed()) + return; - bool owned = pPageView->OwnsPage(); - // This will delete the |pPageView| object. We must cleanup the PageView - // first because it will attempt to reset the View on the |pPage| during - // destruction. - pPageView->GetFormFillEnv()->RemovePageView(pPage); - // If the page was owned then the pageview will have deleted the page. - if (owned) - return; + if (pPageView->IsLocked()) { + pPageView->TakePageOwnership(); + return; } - delete pPage; + + // This will delete the |pPageView| object. We must cleanup the PageView + // first because it will attempt to reset the View on the |pPage| during + // destruction. + pPageView->GetFormFillEnv()->RemovePageView(pPage.Get()); #endif // PDF_ENABLE_XFA } @@ -970,9 +963,9 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document, if (!pDict) return false; - CPDF_Page page(pDoc, pDict, true); - *width = page.GetPageWidth(); - *height = page.GetPageHeight(); + auto page = pdfium::MakeRetain(pDoc, pDict, true); + *width = page->GetPageWidth(); + *height = page->GetPageHeight(); return true; } diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp index 170f17ea7a..4d7e3bc222 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp @@ -35,7 +35,7 @@ bool CPDFXFA_Page::LoadPDFPage() { return false; if (!m_pPDFPage || m_pPDFPage->GetFormDict() != pDict) { - m_pPDFPage = pdfium::MakeUnique(pPDFDoc, pDict, true); + m_pPDFPage = pdfium::MakeRetain(pPDFDoc, pDict, true); m_pPDFPage->ParseContent(); } return true; @@ -81,7 +81,7 @@ bool CPDFXFA_Page::LoadPDFPage(CPDF_Dictionary* pageDict) { return false; m_pPDFPage = - pdfium::MakeUnique(m_pContext->GetPDFDoc(), pageDict, true); + pdfium::MakeRetain(m_pContext->GetPDFDoc(), pageDict, true); m_pPDFPage->ParseContent(); return true; } diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.h b/fpdfsdk/fpdfxfa/cpdfxfa_page.h index 4f4d6b0a22..2573b9cebd 100644 --- a/fpdfsdk/fpdfxfa/cpdfxfa_page.h +++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.h @@ -29,11 +29,10 @@ class CPDFXFA_Page : public CPDF_Page::Extension { bool LoadPage(); bool LoadPDFPage(CPDF_Dictionary* pageDict); - // CPDF_Page::Extension: - CPDF_Document::Extension* GetDocumentExtension() const override; + CPDF_Document::Extension* GetDocumentExtension() const; int GetPageIndex() const { return m_iPageIndex; } - CPDF_Page* GetPDFPage() const { return m_pPDFPage.get(); } + CPDF_Page* GetPDFPage() const { return m_pPDFPage.Get(); } CXFA_FFPageView* GetXFAPageView() const { return m_pXFAPageView; } void SetXFAPageView(CXFA_FFPageView* pPageView) { @@ -61,7 +60,7 @@ class CPDFXFA_Page : public CPDF_Page::Extension { bool LoadXFAPageView(); private: - std::unique_ptr m_pPDFPage; + RetainPtr m_pPDFPage; CXFA_FFPageView* m_pXFAPageView; UnownedPtr const m_pContext; const int m_iPageIndex; -- cgit v1.2.3