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

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 <memory>
+
 #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<CPDF_Page> 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<CXFA_FFWidget*>(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_LayoutItem,
                                   CXFA_TraverseStrategy_LayoutItem>
     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;
 }