Make CPDF_Document cache CPDF_Pages

We cache pages not by page number, which can bounce around as pages
are inserted or removed, but by page dictionary's object number.

Since the page may be created under one function and used under
another, we can't take the shortcut of not instantiating a render
cache nor not parsing the page.

Change-Id: I9a325cda8b3141153544ac53e78a51a44e6b411a
Reviewed-on: https://pdfium-review.googlesource.com/32830
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

12 files changed, 43 insertions, 32 deletions

diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
index 1244b12331..859f5b3991 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
@@ -129,7 +129,7 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessGraphics) {
   auto pDoc = pdfium::MakeUnique<CPDF_Document>(nullptr);
   pDoc->CreateNewDoc();
   CPDF_Dictionary* pPageDict = pDoc->CreateNewPage(0);
-  auto pTestPage = pdfium::MakeRetain<CPDF_Page>(pDoc.get(), pPageDict, false);
+  RetainPtr<CPDF_Page> pTestPage = pDoc->GetOrCreatePDFPage(pPageDict);
   CPDF_PageContentGenerator generator(pTestPage.Get());
   std::ostringstream buf;
   TestProcessPath(&generator, &buf, pPathObj.get());
@@ -168,7 +168,7 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessStandardText) {
   auto pDoc = pdfium::MakeUnique<CPDF_Document>(nullptr);
   pDoc->CreateNewDoc();
   CPDF_Dictionary* pPageDict = pDoc->CreateNewPage(0);
-  auto pTestPage = pdfium::MakeRetain<CPDF_Page>(pDoc.get(), pPageDict, false);
+  RetainPtr<CPDF_Page> pTestPage = pDoc->GetOrCreatePDFPage(pPageDict);
   CPDF_PageContentGenerator generator(pTestPage.Get());
   auto pTextObj = pdfium::MakeUnique<CPDF_TextObject>();
   CPDF_Font* pFont = CPDF_Font::GetStockFont(pDoc.get(), "Times-Roman");
@@ -231,7 +231,7 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessText) {
   auto pDoc = pdfium::MakeUnique<CPDF_Document>(nullptr);
   pDoc->CreateNewDoc();
   CPDF_Dictionary* pPageDict = pDoc->CreateNewPage(0);
-  auto pTestPage = pdfium::MakeRetain<CPDF_Page>(pDoc.get(), pPageDict, false);
+  RetainPtr<CPDF_Page> pTestPage = pDoc->GetOrCreatePDFPage(pPageDict);
   CPDF_PageContentGenerator generator(pTestPage.Get());
 
   std::ostringstream buf;
diff --git a/core/fpdfapi/page/cpdf_page.cpp b/core/fpdfapi/page/cpdf_page.cpp
index 7c0a3234c9..c5c85635b2 100644
--- a/core/fpdfapi/page/cpdf_page.cpp
+++ b/core/fpdfapi/page/cpdf_page.cpp
@@ -21,11 +21,11 @@
 
 CPDF_Page::CPDF_Page(CPDF_Document* pDocument,
                      CPDF_Dictionary* pPageDict,
-                     bool bPageCache)
+                     bool bUseRenderCache)
     : CPDF_PageObjectHolder(pDocument, pPageDict),
       m_PageSize(100, 100),
       m_pPDFDocument(pDocument) {
-  if (bPageCache)
+  if (bUseRenderCache)
     m_pPageRender = pdfium::MakeUnique<CPDF_PageRenderCache>(this);
   if (!pPageDict)
     return;
diff --git a/core/fpdfapi/page/cpdf_page.h b/core/fpdfapi/page/cpdf_page.h
index 0bb99b73f1..b94326e47b 100644
--- a/core/fpdfapi/page/cpdf_page.h
+++ b/core/fpdfapi/page/cpdf_page.h
@@ -12,6 +12,7 @@
 #include "core/fpdfapi/page/cpdf_pageobjectholder.h"
 #include "core/fxcrt/fx_coordinates.h"
 #include "core/fxcrt/fx_system.h"
+#include "core/fxcrt/observable.h"
 #include "core/fxcrt/retain_ptr.h"
 #include "core/fxcrt/unowned_ptr.h"
 #include "third_party/base/optional.h"
@@ -22,7 +23,9 @@ class CPDF_Object;
 class CPDF_PageRenderCache;
 class CPDF_PageRenderContext;
 
-class CPDF_Page : public Retainable, public CPDF_PageObjectHolder {
+class CPDF_Page : public Retainable,
+                  public Observable<CPDF_Page>,
+                  public CPDF_PageObjectHolder {
  public:
   class View {};  // Caller implements as desired, empty here due to layering.
   class Extension : public Retainable {};  // XFA page parent class, layering.
@@ -66,7 +69,7 @@ class CPDF_Page : public Retainable, public CPDF_PageObjectHolder {
  private:
   CPDF_Page(CPDF_Document* pDocument,
             CPDF_Dictionary* pPageDict,
-            bool bPageCache);
+            bool bUseRenderCache);
   ~CPDF_Page() override;
 
   void StartParse();
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index 7968d1b019..2e4baabe91 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -602,6 +602,19 @@ bool CPDF_Document::InsertNewPage(int iPage, CPDF_Dictionary* pPageDict) {
   return true;
 }
 
+RetainPtr<CPDF_Page> CPDF_Document::GetOrCreatePDFPage(
+    CPDF_Dictionary* pPageDict) {
+  std::pair<uint32_t, uint32_t> key = {pPageDict->GetObjNum(),
+                                       pPageDict->GetGenNum()};
+  if (m_PageMap[key])
+    return RetainPtr<CPDF_Page>(m_PageMap[key].Get());
+
+  auto pPage = pdfium::MakeRetain<CPDF_Page>(this, pPageDict, true);
+  pPage->ParseContent();
+  m_PageMap[key].Reset(pPage.Get());
+  return pPage;
+}
+
 void CPDF_Document::DeletePage(int iPage) {
   CPDF_Dictionary* pPages = GetPagesDict();
   if (!pPages)
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index c2774b163a..c240e77567 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -8,6 +8,7 @@
 #define CORE_FPDFAPI_PARSER_CPDF_DOCUMENT_H_
 
 #include <functional>
+#include <map>
 #include <memory>
 #include <set>
 #include <utility>
@@ -64,6 +65,7 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
   const CPDF_Dictionary* GetInfo() const { return m_pInfoDict.Get(); }
   CPDF_Dictionary* GetInfo() { return m_pInfoDict.Get(); }
 
+  RetainPtr<CPDF_Page> GetOrCreatePDFPage(CPDF_Dictionary* pPageDict);
   void DeletePage(int iPage);
   int GetPageCount() const;
   bool IsPageLoaded(int iPage) const;
@@ -167,7 +169,13 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
   std::unique_ptr<CPDF_DocRenderData> m_pDocRender;
   std::unique_ptr<JBig2_DocumentContext> m_pCodecContext;
   std::unique_ptr<CPDF_LinkList> m_pLinksContext;
-  std::vector<uint32_t> m_PageList;  // Page number to page's dict objnum.
+
+  // Page number (index) to page's dict objnum.
+  std::vector<uint32_t> m_PageList;
+
+  // Dict {objnum, gennum} to page mapping.
+  std::map<std::pair<uint32_t, uint32_t>, CPDF_Page::ObservedPtr> m_PageMap;
+
   UnownedPtr<Extension> m_pExtension;
 };
 
diff --git a/fpdfsdk/fpdf_doc_embeddertest.cpp b/fpdfsdk/fpdf_doc_embeddertest.cpp
index c1f5e02ed6..a9e4701e1d 100644
--- a/fpdfsdk/fpdf_doc_embeddertest.cpp
+++ b/fpdfsdk/fpdf_doc_embeddertest.cpp
@@ -27,11 +27,7 @@ TEST_F(FPDFDocEmbeddertest, MultipleSamePage) {
     ref.reset(FPDF_LoadPage(document(), 0));
     unique_pages.insert(ref.get());
   }
-#ifdef PDF_ENABLE_XFA
   EXPECT_EQ(1u, unique_pages.size());
-#else   // PDF_ENABLE_XFA
-  EXPECT_EQ(4u, unique_pages.size());
-#endif  // PDF_ENABLE_XFA
 }
 
 TEST_F(FPDFDocEmbeddertest, DestGetPageIndex) {
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index 39cf85fb95..9fb6c1fedf 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -196,8 +196,7 @@ 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::MakeRetain<CPDF_Page>(pDoc, pPageDict, true);
-  pPage->ParseContent();
+  RetainPtr<CPDF_Page> pPage = pDoc->GetOrCreatePDFPage(pPageDict);
   return FPDFPageFromUnderlying(pPage.Leak());  // Caller takes ownership.
 #endif  // PDF_ENABLE_XFA
 }
diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp
index 720fa9ff55..2d81f1230e 100644
--- a/fpdfsdk/fpdf_flatten.cpp
+++ b/fpdfsdk/fpdf_flatten.cpp
@@ -47,9 +47,7 @@ bool IsValidRect(const CFX_FloatRect& rect, const CFX_FloatRect& rcPage) {
 void GetContentsRect(CPDF_Document* pDoc,
                      CPDF_Dictionary* pDict,
                      std::vector<CFX_FloatRect>* pRectArray) {
-  auto pPDFPage = pdfium::MakeRetain<CPDF_Page>(pDoc, pDict, false);
-  pPDFPage->ParseContent();
-
+  RetainPtr<CPDF_Page> pPDFPage = pDoc->GetOrCreatePDFPage(pDict);
   for (const auto& pPageObject : *pPDFPage->GetPageObjectList()) {
     CFX_FloatRect rc;
     rc.left = pPageObject->m_Left;
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index b73042466b..4ea3334b65 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -618,7 +618,7 @@ bool CPDF_NPageToOneExporter::ExportNPagesToOne(
       if (!pSrcPageDict)
         return false;
 
-      auto srcPage = pdfium::MakeRetain<CPDF_Page>(src(), pSrcPageDict, true);
+      RetainPtr<CPDF_Page> srcPage = src()->GetOrCreatePDFPage(pSrcPageDict);
       NupPageSettings settings =
           nupState.CalculateNewPagePosition(srcPage->GetPageSize());
       AddSubPage(pSrcPageDict, settings, &objectNumberMap, &pageXObjectMap,
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index af9117579d..9af0ca4755 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -354,8 +354,7 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document,
   if (!pDict)
     return nullptr;
 
-  auto pPage = pdfium::MakeRetain<CPDF_Page>(pDoc, pDict, true);
-  pPage->ParseContent();
+  RetainPtr<CPDF_Page> pPage = pDoc->GetOrCreatePDFPage(pDict);
   return FPDFPageFromUnderlying(pPage.Leak());
 #endif  // PDF_ENABLE_XFA
 }
@@ -962,7 +961,7 @@ FPDF_EXPORT int FPDF_CALLCONV FPDF_GetPageSizeByIndex(FPDF_DOCUMENT document,
   if (!pDict)
     return false;
 
-  auto page = pdfium::MakeRetain<CPDF_Page>(pDoc, pDict, true);
+  RetainPtr<CPDF_Page> page = pDoc->GetOrCreatePDFPage(pDict);
   *width = page->GetPageWidth();
   *height = page->GetPageHeight();
   return true;
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
index 4d7e3bc222..f4120896a8 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_page.cpp
@@ -34,10 +34,9 @@ bool CPDFXFA_Page::LoadPDFPage() {
   if (!pDict)
     return false;
 
-  if (!m_pPDFPage || m_pPDFPage->GetFormDict() != pDict) {
-    m_pPDFPage = pdfium::MakeRetain<CPDF_Page>(pPDFDoc, pDict, true);
-    m_pPDFPage->ParseContent();
-  }
+  if (!m_pPDFPage || m_pPDFPage->GetFormDict() != pDict)
+    m_pPDFPage = pPDFDoc->GetOrCreatePDFPage(pDict);
+
   return true;
 }
 
@@ -80,9 +79,7 @@ bool CPDFXFA_Page::LoadPDFPage(CPDF_Dictionary* pageDict) {
   if (!m_pContext || m_iPageIndex < 0 || !pageDict)
     return false;
 
-  m_pPDFPage =
-      pdfium::MakeRetain<CPDF_Page>(m_pContext->GetPDFDoc(), pageDict, true);
-  m_pPDFPage->ParseContent();
+  m_pPDFPage = m_pContext->GetPDFDoc()->GetOrCreatePDFPage(pageDict);
   return true;
 }
 
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index 66577062a6..3a4ca2763f 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -1256,8 +1256,7 @@ CJS_Return CJS_Document::getPageNthWord(
   if (!pPageDict)
     return CJS_Return(false);
 
-  auto page = pdfium::MakeRetain<CPDF_Page>(pDocument, pPageDict, true);
-  page->ParseContent();
+  RetainPtr<CPDF_Page> page = pDocument->GetOrCreatePDFPage(pPageDict);
 
   int nWords = 0;
   WideString swRet;
@@ -1305,8 +1304,7 @@ CJS_Return CJS_Document::getPageNumWords(
   if (!pPageDict)
     return CJS_Return(false);
 
-  auto page = pdfium::MakeRetain<CPDF_Page>(pDocument, pPageDict, true);
-  page->ParseContent();
+  RetainPtr<CPDF_Page> page = pDocument->GetOrCreatePDFPage(pPageDict);
 
   int nWords = 0;
   for (auto& pPageObj : *page->GetPageObjectList()) {