From 9f3dbbc54c6981f069b57e6714393ac6c0c64231 Mon Sep 17 00:00:00 2001 From: thestig Date: Wed, 13 Apr 2016 13:18:21 -0700 Subject: Cleanup CPDFXFA_Page. - Use std::unique_ptr. - Make dtor protected. - Simplify logic. - Remove unused bits from CXFA_FFPageView as well. Review URL: https://codereview.chromium.org/1878963004 --- fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp | 7 ++-- fpdfsdk/fpdfxfa/fpdfxfa_page.cpp | 53 ++++++++---------------- fpdfsdk/fpdfxfa/include/fpdfxfa_page.h | 28 ++++++++----- xfa/fxfa/app/xfa_ffdocview.cpp | 3 +- xfa/fxfa/app/xfa_ffpageview.cpp | 62 ++++++----------------------- xfa/fxfa/include/xfa_ffpageview.h | 18 ++++----- xfa/fxfa/parser/xfa_doclayout.h | 2 +- xfa/fxfa/parser/xfa_document_layout_imp.cpp | 4 +- 8 files changed, 62 insertions(+), 115 deletions(-) diff --git a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp index aa1a1a82c7..7fa258c506 100644 --- a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp +++ b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp @@ -4,12 +4,13 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com +#include "fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h" + #include "core/fpdfapi/fpdf_parser/include/cpdf_array.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_string.h" #include "fpdfsdk/fpdfxfa/include/fpdfxfa_app.h" -#include "fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h" #include "fpdfsdk/fpdfxfa/include/fpdfxfa_page.h" #include "fpdfsdk/fpdfxfa/include/fpdfxfa_util.h" #include "fpdfsdk/include/fsdk_define.h" @@ -168,7 +169,7 @@ CPDFXFA_Page* CPDFXFA_Document::GetPage(int page_index) { return pPage; pPage = new CPDFXFA_Page(this, page_index); if (!pPage->LoadPage()) { - delete pPage; + pPage->Release(); return nullptr; } m_XFAPageList.SetAt(page_index, pPage); @@ -508,8 +509,6 @@ void CPDFXFA_Document::PageViewEvent(CXFA_FFPageView* pPageView, m_pSDKDoc->RemovePageView(pPage); CXFA_FFPageView* pXFAPageView = pXFADocView->GetPageView(iPageIter); pPage->SetXFAPageView(pXFAPageView); - if (pXFAPageView) - pXFAPageView->LoadPageView(nullptr); } int flag = (nNewCount < m_nPageCount) ? FXFA_PAGEVIEWEVENT_POSTREMOVED diff --git a/fpdfsdk/fpdfxfa/fpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/fpdfxfa_page.cpp index 8179db5c11..8c0d6bbf98 100644 --- a/fpdfsdk/fpdfxfa/fpdfxfa_page.cpp +++ b/fpdfsdk/fpdfxfa/fpdfxfa_page.cpp @@ -16,22 +16,15 @@ #include "xfa/fxfa/include/xfa_ffpageview.h" CPDFXFA_Page::CPDFXFA_Page(CPDFXFA_Document* pDoc, int page_index) - : m_pPDFPage(NULL), - m_pXFAPageView(NULL), - m_iPageIndex(page_index), + : m_pXFAPageView(nullptr), m_pDocument(pDoc), + m_iPageIndex(page_index), m_iRef(1) {} -CPDFXFA_Page::~CPDFXFA_Page() { - if (m_pPDFPage) - delete m_pPDFPage; - m_pPDFPage = NULL; - m_pXFAPageView = NULL; -} +CPDFXFA_Page::~CPDFXFA_Page() {} void CPDFXFA_Page::Release() { - m_iRef--; - if (m_iRef > 0) + if (--m_iRef > 0) return; if (m_pDocument) @@ -43,31 +36,27 @@ void CPDFXFA_Page::Release() { FX_BOOL CPDFXFA_Page::LoadPDFPage() { if (!m_pDocument) return FALSE; + CPDF_Document* pPDFDoc = m_pDocument->GetPDFDoc(); - if (pPDFDoc) { - CPDF_Dictionary* pDict = pPDFDoc->GetPage(m_iPageIndex); - if (pDict == NULL) - return FALSE; - if (m_pPDFPage) { - if (m_pPDFPage->m_pFormDict == pDict) - return TRUE; + if (!pPDFDoc) + return FALSE; - delete m_pPDFPage; - m_pPDFPage = NULL; - } + CPDF_Dictionary* pDict = pPDFDoc->GetPage(m_iPageIndex); + if (!pDict) + return FALSE; - m_pPDFPage = new CPDF_Page; + if (!m_pPDFPage || m_pPDFPage->m_pFormDict != pDict) { + m_pPDFPage.reset(new CPDF_Page); m_pPDFPage->Load(pPDFDoc, pDict); m_pPDFPage->ParseContent(nullptr); - return TRUE; } - - return FALSE; + return TRUE; } FX_BOOL CPDFXFA_Page::LoadXFAPageView() { if (!m_pDocument) return FALSE; + CXFA_FFDoc* pXFADoc = m_pDocument->GetXFADoc(); if (!pXFADoc) return FALSE; @@ -80,11 +69,7 @@ FX_BOOL CPDFXFA_Page::LoadXFAPageView() { if (!pPageView) return FALSE; - if (m_pXFAPageView == pPageView) - return TRUE; - m_pXFAPageView = pPageView; - (void)m_pXFAPageView->LoadPageView(nullptr); return TRUE; } @@ -110,17 +95,13 @@ FX_BOOL CPDFXFA_Page::LoadPDFPage(CPDF_Dictionary* pageDict) { if (!m_pDocument || m_iPageIndex < 0 || !pageDict) return FALSE; - if (m_pPDFPage) - delete m_pPDFPage; - - m_pPDFPage = new CPDF_Page(); + m_pPDFPage.reset(new CPDF_Page()); m_pPDFPage->Load(m_pDocument->GetPDFDoc(), pageDict); m_pPDFPage->ParseContent(nullptr); - return TRUE; } -FX_FLOAT CPDFXFA_Page::GetPageWidth() { +FX_FLOAT CPDFXFA_Page::GetPageWidth() const { if (!m_pPDFPage && !m_pXFAPageView) return 0.0f; @@ -145,7 +126,7 @@ FX_FLOAT CPDFXFA_Page::GetPageWidth() { return 0.0f; } -FX_FLOAT CPDFXFA_Page::GetPageHeight() { +FX_FLOAT CPDFXFA_Page::GetPageHeight() const { if (!m_pPDFPage && !m_pXFAPageView) return 0.0f; diff --git a/fpdfsdk/fpdfxfa/include/fpdfxfa_page.h b/fpdfsdk/fpdfxfa/include/fpdfxfa_page.h index 2c860ffbdc..0f278912d6 100644 --- a/fpdfsdk/fpdfxfa/include/fpdfxfa_page.h +++ b/fpdfsdk/fpdfxfa/include/fpdfxfa_page.h @@ -7,6 +7,8 @@ #ifndef FPDFSDK_FPDFXFA_INCLUDE_FPDFXFA_PAGE_H_ #define FPDFSDK_FPDFXFA_INCLUDE_FPDFXFA_PAGE_H_ +#include + #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h" #include "core/fpdfapi/include/cpdf_modulemgr.h" #include "core/fxcrt/include/fx_coordinates.h" @@ -19,22 +21,23 @@ class CXFA_FFPageView; class CPDFXFA_Page { public: CPDFXFA_Page(CPDFXFA_Document* pDoc, int page_index); - ~CPDFXFA_Page(); - void Release(); void AddRef() { m_iRef++; } + void Release(); + FX_BOOL LoadPage(); FX_BOOL LoadPDFPage(CPDF_Dictionary* pageDict); - CPDFXFA_Document* GetDocument() { return m_pDocument; } - int GetPageIndex() { return m_iPageIndex; } - CPDF_Page* GetPDFPage() { return m_pPDFPage; } - CXFA_FFPageView* GetXFAPageView() { return m_pXFAPageView; } + CPDFXFA_Document* GetDocument() const { return m_pDocument; } + int GetPageIndex() const { return m_iPageIndex; } + CPDF_Page* GetPDFPage() const { return m_pPDFPage.get(); } + CXFA_FFPageView* GetXFAPageView() const { return m_pXFAPageView; } + void SetXFAPageView(CXFA_FFPageView* pPageView) { m_pXFAPageView = pPageView; } - FX_FLOAT GetPageWidth(); - FX_FLOAT GetPageHeight(); + FX_FLOAT GetPageWidth() const; + FX_FLOAT GetPageHeight() const; void DeviceToPage(int start_x, int start_y, @@ -63,14 +66,17 @@ class CPDFXFA_Page { int iRotate) const; protected: + // Refcounted class. + ~CPDFXFA_Page(); + FX_BOOL LoadPDFPage(); FX_BOOL LoadXFAPageView(); private: - CPDF_Page* m_pPDFPage; + std::unique_ptr m_pPDFPage; CXFA_FFPageView* m_pXFAPageView; - int m_iPageIndex; - CPDFXFA_Document* m_pDocument; + CPDFXFA_Document* const m_pDocument; + const int m_iPageIndex; int m_iRef; }; diff --git a/xfa/fxfa/app/xfa_ffdocview.cpp b/xfa/fxfa/app/xfa_ffdocview.cpp index 79bba442ad..9c627ea3c1 100644 --- a/xfa/fxfa/app/xfa_ffdocview.cpp +++ b/xfa/fxfa/app/xfa_ffdocview.cpp @@ -530,6 +530,7 @@ CXFA_WidgetAcc* CXFA_FFDocView::GetWidgetAccByName( } return NULL; } + void CXFA_FFDocView::OnPageEvent(CXFA_ContainerLayoutItem* pSender, XFA_PAGEEVENT eEvent, int32_t iPageIndex) { @@ -541,8 +542,8 @@ void CXFA_FFDocView::OnPageEvent(CXFA_ContainerLayoutItem* pSender, } m_pDoc->GetDocProvider()->PageViewEvent(pFFPageView, XFA_PAGEVIEWEVENT_PostAdded); - pFFPageView->LoadPageView(); } + void CXFA_FFDocView::LockUpdate() { m_iLock++; } diff --git a/xfa/fxfa/app/xfa_ffpageview.cpp b/xfa/fxfa/app/xfa_ffpageview.cpp index 62474b3e38..a480737281 100644 --- a/xfa/fxfa/app/xfa_ffpageview.cpp +++ b/xfa/fxfa/app/xfa_ffpageview.cpp @@ -19,72 +19,33 @@ #include "xfa/fxfa/include/xfa_ffwidget.h" CXFA_FFPageView::CXFA_FFPageView(CXFA_FFDocView* pDocView, CXFA_Node* pPageArea) - : CXFA_ContainerLayoutItem(pPageArea), - m_pDocView(pDocView), - m_bLoaded(FALSE) {} + : CXFA_ContainerLayoutItem(pPageArea), m_pDocView(pDocView) {} + CXFA_FFPageView::~CXFA_FFPageView() {} -CXFA_FFDocView* CXFA_FFPageView::GetDocView() { + +CXFA_FFDocView* CXFA_FFPageView::GetDocView() const { return m_pDocView; } -int32_t CXFA_FFPageView::GetPageViewIndex() { + +int32_t CXFA_FFPageView::GetPageViewIndex() const { return GetPageIndex(); } -void CXFA_FFPageView::GetPageViewRect(CFX_RectF& rtPage) { + +void CXFA_FFPageView::GetPageViewRect(CFX_RectF& rtPage) const { CFX_SizeF sz; GetPageSize(sz); rtPage.Set(0, 0, sz); } void CXFA_FFPageView::GetDisplayMatrix(CFX_Matrix& mt, const CFX_Rect& rtDisp, - int32_t iRotate) { + int32_t iRotate) const { CFX_SizeF sz; GetPageSize(sz); CFX_RectF fdePage; fdePage.Set(0, 0, sz.x, sz.y); FDE_GetPageMatrix(mt, fdePage, rtDisp, iRotate, 0); } -int32_t CXFA_FFPageView::LoadPageView(IFX_Pause* pPause) { - if (m_bLoaded) { - return 100; - } - m_bLoaded = TRUE; - return 100; -} -void CXFA_FFPageView::UnloadPageView() { - if (!m_bLoaded) { - return; - } -} -FX_BOOL CXFA_FFPageView::IsPageViewLoaded() { - return m_bLoaded; -} -CXFA_FFWidget* CXFA_FFPageView::GetWidgetByPos(FX_FLOAT fx, FX_FLOAT fy) { - if (!m_bLoaded) { - return nullptr; - } - IXFA_WidgetIterator* pIterator = CreateWidgetIterator(); - CXFA_FFWidget* pWidget = nullptr; - while ((pWidget = static_cast(pIterator->MoveToNext()))) { - if (!(pWidget->GetStatus() & XFA_WIDGETSTATUS_Visible)) { - continue; - } - CXFA_WidgetAcc* pAcc = pWidget->GetDataAcc(); - int32_t type = pAcc->GetClassID(); - if (type != XFA_ELEMENT_Field && type != XFA_ELEMENT_Draw) { - continue; - } - FX_FLOAT fWidgetx = fx; - FX_FLOAT fWidgety = fy; - pWidget->Rotate2Normal(fWidgetx, fWidgety); - uint32_t dwFlag = pWidget->OnHitTest(fWidgetx, fWidgety); - if ((FWL_WGTHITTEST_Client == dwFlag || - FWL_WGTHITTEST_HyperLink == dwFlag)) { - break; - } - } - pIterator->Release(); - return pWidget; -} + IXFA_WidgetIterator* CXFA_FFPageView::CreateWidgetIterator( uint32_t dwTraverseWay, uint32_t dwWidgetFilter) { @@ -94,8 +55,9 @@ IXFA_WidgetIterator* CXFA_FFPageView::CreateWidgetIterator( case XFA_TRAVERSEWAY_Form: return new CXFA_FFPageWidgetIterator(this, dwWidgetFilter); } - return NULL; + return nullptr; } + static FX_BOOL XFA_PageWidgetFilter(CXFA_FFWidget* pWidget, uint32_t dwFilter, FX_BOOL bTraversal, diff --git a/xfa/fxfa/include/xfa_ffpageview.h b/xfa/fxfa/include/xfa_ffpageview.h index 56d1a7312c..dcac3f1106 100644 --- a/xfa/fxfa/include/xfa_ffpageview.h +++ b/xfa/fxfa/include/xfa_ffpageview.h @@ -11,32 +11,28 @@ class CXFA_FFWidget; class CXFA_FFDocView; + class CXFA_FFPageView : public CXFA_ContainerLayoutItem { public: CXFA_FFPageView(CXFA_FFDocView* pDocView, CXFA_Node* pPageArea); ~CXFA_FFPageView() override; - CXFA_FFDocView* GetDocView(); - int32_t GetPageViewIndex(); - void GetPageViewRect(CFX_RectF& rtPage); + CXFA_FFDocView* GetDocView() const; + int32_t GetPageViewIndex() const; + void GetPageViewRect(CFX_RectF& rtPage) const; void GetDisplayMatrix(CFX_Matrix& mt, const CFX_Rect& rtDisp, - int32_t iRotate); - int32_t LoadPageView(IFX_Pause* pPause = NULL); - void UnloadPageView(); - CXFA_FFWidget* GetWidgetByPos(FX_FLOAT fx, FX_FLOAT fy); + int32_t iRotate) const; IXFA_WidgetIterator* CreateWidgetIterator( uint32_t dwTraverseWay = XFA_TRAVERSEWAY_Form, uint32_t dwWidgetFilter = XFA_WIDGETFILTER_Visible | XFA_WIDGETFILTER_Viewable | XFA_WIDGETFILTER_AllType); - FX_BOOL IsPageViewLoaded(); - protected: - CXFA_FFDocView* m_pDocView; - FX_BOOL m_bLoaded; + CXFA_FFDocView* const m_pDocView; }; + typedef CXFA_NodeIteratorTemplate CXFA_LayoutItemIterator; diff --git a/xfa/fxfa/parser/xfa_doclayout.h b/xfa/fxfa/parser/xfa_doclayout.h index 0bae0184ec..267f79980b 100644 --- a/xfa/fxfa/parser/xfa_doclayout.h +++ b/xfa/fxfa/parser/xfa_doclayout.h @@ -57,7 +57,7 @@ class CXFA_ContainerLayoutItem : public CXFA_LayoutItem { CXFA_LayoutProcessor* GetLayout() const; int32_t GetPageIndex() const; - void GetPageSize(CFX_SizeF& size); + void GetPageSize(CFX_SizeF& size) const; CXFA_Node* GetMasterPage() const; CXFA_Node* m_pOldSubform; diff --git a/xfa/fxfa/parser/xfa_document_layout_imp.cpp b/xfa/fxfa/parser/xfa_document_layout_imp.cpp index 79037d7ec7..eea99ece88 100644 --- a/xfa/fxfa/parser/xfa_document_layout_imp.cpp +++ b/xfa/fxfa/parser/xfa_document_layout_imp.cpp @@ -180,7 +180,8 @@ int32_t CXFA_ContainerLayoutItem::GetPageIndex() const { ->GetLayoutPageMgr() ->GetPageIndex(this); } -void CXFA_ContainerLayoutItem::GetPageSize(CFX_SizeF& size) { + +void CXFA_ContainerLayoutItem::GetPageSize(CFX_SizeF& size) const { size.clear(); CXFA_Node* pMedium = m_pFormNode->GetFirstChildByClass(XFA_ELEMENT_Medium); if (!pMedium) @@ -193,6 +194,7 @@ void CXFA_ContainerLayoutItem::GetPageSize(CFX_SizeF& size) { size = CFX_SizeF(size.y, size.x); } } + CXFA_Node* CXFA_ContainerLayoutItem::GetMasterPage() const { return m_pFormNode; } -- cgit v1.2.3