Tidy parser lifecycle state machine in CPDF_PageObjectHolder.

Only update state in CPDF_PageObjectHolder itself.
Make more data private.
Remove CPDF_Page::GetPageBBox() as exact duplicate of
CPDF_PageObjectHolder::GetBBox().

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

10 files changed, 53 insertions, 51 deletions

diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
index 1244b12331..5d9e029267 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
@@ -299,7 +299,8 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessEmptyForm) {
   auto pTestForm =
       pdfium::MakeUnique<CPDF_Form>(pDoc.get(), nullptr, pStream.get());
   pTestForm->ParseContent(nullptr, nullptr, nullptr, nullptr);
-  ASSERT_TRUE(pTestForm->IsParsed());
+  ASSERT_EQ(CPDF_PageObjectHolder::ParseState::kParsed,
+            pTestForm->GetParseState());
 
   // The generated stream for the empty form should be an empty string.
   CPDF_PageContentGenerator generator(pTestForm.get());
@@ -325,7 +326,8 @@ TEST_F(CPDF_PageContentGeneratorTest, ProcessFormWithPath) {
   auto pTestForm =
       pdfium::MakeUnique<CPDF_Form>(pDoc.get(), nullptr, pStream.get());
   pTestForm->ParseContent(nullptr, nullptr, nullptr, nullptr);
-  ASSERT_TRUE(pTestForm->IsParsed());
+  ASSERT_EQ(CPDF_PageObjectHolder::ParseState::kParsed,
+            pTestForm->GetParseState());
 
   CPDF_PageContentGenerator generator(pTestForm.get());
   std::ostringstream process_buf;
diff --git a/core/fpdfapi/page/cpdf_form.cpp b/core/fpdfapi/page/cpdf_form.cpp
index 5132029072..e73f1dfa43 100644
--- a/core/fpdfapi/page/cpdf_form.cpp
+++ b/core/fpdfapi/page/cpdf_form.cpp
@@ -19,13 +19,12 @@ CPDF_Form::CPDF_Form(CPDF_Document* pDoc,
                      CPDF_Dictionary* pParentResources)
     : CPDF_PageObjectHolder(pDoc, pFormStream->GetDict()) {
   m_pFormStream = pFormStream;
-  m_pResources = m_pFormDict->GetDictFor("Resources");
+  m_pResources = GetFormDict()->GetDictFor("Resources");
   m_pPageResources = pPageResources;
   if (!m_pResources)
     m_pResources = pParentResources;
   if (!m_pResources)
     m_pResources = pPageResources;
-  m_Transparency = CPDF_Transparency();
   LoadTransInfo();
 }
 
@@ -35,19 +34,20 @@ void CPDF_Form::ParseContent(CPDF_AllStates* pGraphicStates,
                              const CFX_Matrix* pParentMatrix,
                              CPDF_Type3Char* pType3Char,
                              std::set<const uint8_t*>* parsedSet) {
-  if (m_ParseState == CONTENT_PARSED || m_ParseState == CONTENT_PARSING)
+  if (GetParseState() == ParseState::kParsed)
     return;
 
-  if (!parsedSet) {
-    if (!m_ParsedSet)
-      m_ParsedSet = pdfium::MakeUnique<std::set<const uint8_t*>>();
-    parsedSet = m_ParsedSet.get();
+  if (GetParseState() == ParseState::kNotParsed) {
+    if (!parsedSet) {
+      if (!m_ParsedSet)
+        m_ParsedSet = pdfium::MakeUnique<std::set<const uint8_t*>>();
+      parsedSet = m_ParsedSet.get();
+    }
+    StartParse(pdfium::MakeUnique<CPDF_ContentParser>(
+        this, pGraphicStates, pParentMatrix, pType3Char, parsedSet));
   }
 
-  m_pParser = pdfium::MakeUnique<CPDF_ContentParser>(
-      this, pGraphicStates, pParentMatrix, pType3Char, parsedSet);
-  m_ParseState = CONTENT_PARSING;
-
+  ASSERT(GetParseState() == ParseState::kParsing);
   ContinueParse(nullptr);
 }
 
diff --git a/core/fpdfapi/page/cpdf_page.cpp b/core/fpdfapi/page/cpdf_page.cpp
index 7c0a3234c9..019c2b6418 100644
--- a/core/fpdfapi/page/cpdf_page.cpp
+++ b/core/fpdfapi/page/cpdf_page.cpp
@@ -78,16 +78,14 @@ bool CPDF_Page::IsPage() const {
   return true;
 }
 
-void CPDF_Page::StartParse() {
-  if (m_ParseState == CONTENT_PARSED || m_ParseState == CONTENT_PARSING)
+void CPDF_Page::ParseContent() {
+  if (GetParseState() == ParseState::kParsed)
     return;
 
-  m_pParser = pdfium::MakeUnique<CPDF_ContentParser>(this);
-  m_ParseState = CONTENT_PARSING;
-}
+  if (GetParseState() == ParseState::kNotParsed)
+    StartParse(pdfium::MakeUnique<CPDF_ContentParser>(this));
 
-void CPDF_Page::ParseContent() {
-  StartParse();
+  ASSERT(GetParseState() == ParseState::kParsing);
   ContinueParse(nullptr);
 }
 
@@ -97,7 +95,7 @@ void CPDF_Page::SetRenderContext(
 }
 
 CPDF_Object* CPDF_Page::GetPageAttr(const ByteString& name) const {
-  CPDF_Dictionary* pPageDict = m_pFormDict.Get();
+  CPDF_Dictionary* pPageDict = GetFormDict();
   std::set<CPDF_Dictionary*> visited;
   while (1) {
     visited.insert(pPageDict);
diff --git a/core/fpdfapi/page/cpdf_page.h b/core/fpdfapi/page/cpdf_page.h
index 0bb99b73f1..1d0186181a 100644
--- a/core/fpdfapi/page/cpdf_page.h
+++ b/core/fpdfapi/page/cpdf_page.h
@@ -48,7 +48,6 @@ class CPDF_Page : public Retainable, public CPDF_PageObjectHolder {
   float GetPageHeight() const { return m_PageSize.height; }
   const CFX_SizeF& GetPageSize() const { return m_PageSize; }
 
-  const CFX_FloatRect& GetPageBBox() const { return m_BBox; }
   int GetPageRotation() const;
   CPDF_PageRenderCache* GetRenderCache() const { return m_pPageRender.get(); }
 
@@ -69,8 +68,6 @@ class CPDF_Page : public Retainable, public CPDF_PageObjectHolder {
             bool bPageCache);
   ~CPDF_Page() override;
 
-  void StartParse();
-
   CPDF_Object* GetPageAttr(const ByteString& name) const;
   CFX_FloatRect GetBox(const ByteString& name) const;
 
diff --git a/core/fpdfapi/page/cpdf_pageobjectholder.cpp b/core/fpdfapi/page/cpdf_pageobjectholder.cpp
index 4d4fc56ac2..81ee6e2e11 100644
--- a/core/fpdfapi/page/cpdf_pageobjectholder.cpp
+++ b/core/fpdfapi/page/cpdf_pageobjectholder.cpp
@@ -29,18 +29,23 @@ bool CPDF_PageObjectHolder::IsPage() const {
   return false;
 }
 
+void CPDF_PageObjectHolder::StartParse(
+    std::unique_ptr<CPDF_ContentParser> pParser) {
+  ASSERT(m_ParseState == ParseState::kNotParsed);
+  m_pParser = std::move(pParser);
+  m_ParseState = ParseState::kParsing;
+}
+
 void CPDF_PageObjectHolder::ContinueParse(PauseIndicatorIface* pPause) {
-  if (!m_pParser) {
-    m_ParseState = CONTENT_PARSED;
+  if (m_ParseState == ParseState::kParsed)
     return;
-  }
 
+  ASSERT(m_ParseState == ParseState::kParsing);
   if (m_pParser->Continue(pPause))
     return;
 
-  m_ParseState = CONTENT_PARSED;
+  m_ParseState = ParseState::kParsed;
   m_pDocument->IncrementParsedPageCount();
-
   if (m_pParser->GetCurStates())
     m_LastCTM = m_pParser->GetCurStates()->m_CTM;
 
diff --git a/core/fpdfapi/page/cpdf_pageobjectholder.h b/core/fpdfapi/page/cpdf_pageobjectholder.h
index 816a294149..79b2166d52 100644
--- a/core/fpdfapi/page/cpdf_pageobjectholder.h
+++ b/core/fpdfapi/page/cpdf_pageobjectholder.h
@@ -41,21 +41,22 @@ struct FontData {
 
 class CPDF_PageObjectHolder {
  public:
+  enum class ParseState : uint8_t { kNotParsed, kParsing, kParsed };
+
   CPDF_PageObjectHolder(CPDF_Document* pDoc, CPDF_Dictionary* pFormDict);
   virtual ~CPDF_PageObjectHolder();
 
   virtual bool IsPage() const;
 
+  void StartParse(std::unique_ptr<CPDF_ContentParser> pParser);
   void ContinueParse(PauseIndicatorIface* pPause);
-  bool IsParsed() const { return m_ParseState == CONTENT_PARSED; }
+  ParseState GetParseState() const { return m_ParseState; }
 
-  const CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
-  CPDF_Document* GetDocument() { return m_pDocument.Get(); }
+  CPDF_Document* GetDocument() const { return m_pDocument.Get(); }
 
   // TODO(thestig): Can this return nullptr? If not, audit callers and simplify
   // the ones that assume it can.
-  const CPDF_Dictionary* GetFormDict() const { return m_pFormDict.Get(); }
-  CPDF_Dictionary* GetFormDict() { return m_pFormDict.Get(); }
+  CPDF_Dictionary* GetFormDict() const { return m_pFormDict.Get(); }
 
   const CPDF_PageObjectList* GetPageObjectList() const {
     return &m_PageObjectList;
@@ -90,17 +91,17 @@ class CPDF_PageObjectHolder {
   std::map<FontData, ByteString> m_FontsMap;
 
  protected:
-  enum ParseState { CONTENT_NOT_PARSED, CONTENT_PARSING, CONTENT_PARSED };
-
   void LoadTransInfo();
 
-  const UnownedPtr<CPDF_Dictionary> m_pFormDict;
-  UnownedPtr<CPDF_Document> m_pDocument;
   CFX_FloatRect m_BBox;
   CPDF_Transparency m_Transparency;
+
+ private:
   bool m_bBackgroundAlphaNeeded = false;
+  ParseState m_ParseState = ParseState::kNotParsed;
+  const UnownedPtr<CPDF_Dictionary> m_pFormDict;
+  UnownedPtr<CPDF_Document> m_pDocument;
   std::vector<CFX_FloatRect> m_MaskBoundingBoxes;
-  ParseState m_ParseState = CONTENT_NOT_PARSED;
   std::unique_ptr<CPDF_ContentParser> m_pParser;
   CPDF_PageObjectList m_PageObjectList;
   CFX_Matrix m_LastCTM;
diff --git a/core/fpdfapi/render/cpdf_progressiverenderer.cpp b/core/fpdfapi/render/cpdf_progressiverenderer.cpp
index e388f7fa10..de140d8a22 100644
--- a/core/fpdfapi/render/cpdf_progressiverenderer.cpp
+++ b/core/fpdfapi/render/cpdf_progressiverenderer.cpp
@@ -114,20 +114,22 @@ void CPDF_ProgressiveRenderer::Continue(PauseIndicatorIface* pPause) {
       if (is_mask && iter != iterEnd)
         return;
     }
-    if (m_pCurrentLayer->m_pObjectHolder->IsParsed()) {
+    if (m_pCurrentLayer->m_pObjectHolder->GetParseState() ==
+        CPDF_PageObjectHolder::ParseState::kParsed) {
       m_pRenderStatus.reset();
       m_pDevice->RestoreState(false);
       m_pCurrentLayer = nullptr;
       m_LayerIndex++;
-      if (is_mask || (pPause && pPause->NeedToPauseNow())) {
+      if (is_mask || (pPause && pPause->NeedToPauseNow()))
         return;
-      }
     } else if (is_mask) {
       return;
     } else {
       m_pCurrentLayer->m_pObjectHolder->ContinueParse(pPause);
-      if (!m_pCurrentLayer->m_pObjectHolder->IsParsed())
+      if (m_pCurrentLayer->m_pObjectHolder->GetParseState() !=
+          CPDF_PageObjectHolder::ParseState::kParsed) {
         return;
+      }
     }
   }
 }
diff --git a/fpdfsdk/formfiller/cffl_formfiller.cpp b/fpdfsdk/formfiller/cffl_formfiller.cpp
index 261cb653e5..4438df0b25 100644
--- a/fpdfsdk/formfiller/cffl_formfiller.cpp
+++ b/fpdfsdk/formfiller/cffl_formfiller.cpp
@@ -489,9 +489,8 @@ CFX_FloatRect CFFL_FormFiller::GetFocusBox(CPDFSDK_PageView* pPageView) {
     return CFX_FloatRect();
 
   CFX_FloatRect rcFocus = FFLtoWnd(pPageView, PWLtoFFL(pWnd->GetFocusRect()));
-  return pPageView->GetPDFPage()->GetPageBBox().Contains(rcFocus)
-             ? rcFocus
-             : CFX_FloatRect();
+  return pPageView->GetPDFPage()->GetBBox().Contains(rcFocus) ? rcFocus
+                                                              : CFX_FloatRect();
 }
 
 CFX_FloatRect CFFL_FormFiller::FFLtoPWL(const CFX_FloatRect& rect) {
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 1b33952204..96de5d56e1 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -385,7 +385,7 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDF_GetPageBoundingBox(FPDF_PAGE page,
   if (!pPage)
     return false;
 
-  FSRECTFFromCFXFloatRect(pPage->GetPageBBox(), rect);
+  FSRECTFFromCFXFloatRect(pPage->GetBBox(), rect);
   return true;
 }
 
diff --git a/fxjs/cjs_field.cpp b/fxjs/cjs_field.cpp
index b9bc8cbdc9..0a322477ec 100644
--- a/fxjs/cjs_field.cpp
+++ b/fxjs/cjs_field.cpp
@@ -1647,7 +1647,7 @@ void CJS_Field::SetRect(CPDFSDK_FormFillEnvironment* pFormFillEnv,
           CFX_FloatRect crRect = rect;
 
           CPDF_Page* pPDFPage = pWidget->GetPDFPage();
-          crRect.Intersect(pPDFPage->GetPageBBox());
+          crRect.Intersect(pPDFPage->GetBBox());
 
           if (!crRect.IsEmpty()) {
             CFX_FloatRect rcOld = pWidget->GetRect();
@@ -1672,10 +1672,8 @@ void CJS_Field::SetRect(CPDFSDK_FormFillEnvironment* pFormFillEnv,
             pFormField->GetControl(nControlIndex)) {
       if (CPDFSDK_Widget* pWidget = pInterForm->GetWidget(pFormControl)) {
         CFX_FloatRect crRect = rect;
-
         CPDF_Page* pPDFPage = pWidget->GetPDFPage();
-        crRect.Intersect(pPDFPage->GetPageBBox());
-
+        crRect.Intersect(pPDFPage->GetBBox());
         if (!crRect.IsEmpty()) {
           CFX_FloatRect rcOld = pWidget->GetRect();
           if (crRect.left != rcOld.left || crRect.right != rcOld.right ||