Properly ref-count CPDFXFA_Page

Change-Id: Ibd1ebe4191f61e20d815de7f1a1094d78b72e6a7
Reviewed-on: https://pdfium-review.googlesource.com/5391
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

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<CPDF_Dictionary>("Resources");
 
 #ifdef PDF_ENABLE_XFA
-  CPDFXFA_Page* pPage =
-      new CPDFXFA_Page(static_cast<CPDFXFA_Context*>(document), page_index);
-  pPage->LoadPDFPage(pPageDict);
+  auto pXFAPage = pdfium::MakeRetain<CPDFXFA_Page>(
+      static_cast<CPDFXFA_Context*>(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<CPDFXFA_Page>().Unleak(pPage);
 #else   // PDF_ENABLE_XFA
   CPDFSDK_PageView* pPageView =
       static_cast<CPDFSDK_PageView*>(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<CPDFXFA_Page> 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_Page> CPDFXFA_Context::GetXFAPage(int page_index) {
   if (page_index < 0)
     return nullptr;
 
-  CPDFXFA_Page* pPage = nullptr;
-  int nCount = pdfium::CollectionSize<int>(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<CPDFXFA_Page>(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_Page> 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<CPDFXFA_Page> GetXFAPage(int page_index);
+  CFX_RetainPtr<CPDFXFA_Page> 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<CPDFXFA_Page*>* GetXFAPageList() { return &m_XFAPageList; }
+  std::vector<CFX_RetainPtr<CPDFXFA_Page>>* GetXFAPageList() {
+    return &m_XFAPageList;
+  }
 
  private:
   void CloseXFADoc();
 
   XFA_DocType m_iDocType;
-
   std::unique_ptr<CPDF_Document> m_pPDFDoc;
   std::unique_ptr<CXFA_FFDoc> m_pXFADoc;
   CFX_UnownedPtr<CPDFSDK_FormFillEnvironment> m_pFormFillEnv;
   CFX_UnownedPtr<CXFA_FFDocView> m_pXFADocView;
   std::unique_ptr<CXFA_FFApp> m_pXFAApp;
   std::unique_ptr<CJS_Runtime> m_pRuntime;
-  std::vector<CPDFXFA_Page*> m_XFAPageList;
+  std::vector<CFX_RetainPtr<CPDFXFA_Page>> 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<CPDFXFA_Page> 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<FPDF_PAGE>(pPage),
+  pFormFillEnv->Invalidate(static_cast<FPDF_PAGE>(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<CPDFXFA_Page> 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<FPDF_PAGE>(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<CPDFXFA_Page> 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<CPDFXFA_Page> 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<CPDFXFA_Page> 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<CPDFXFA_Page> 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<CPDFXFA_Page> 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 <memory>
 
+#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 <typename T, typename... Args>
+  friend CFX_RetainPtr<T> 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();