From 778e59ed40ed31f6176a68253b694acd31f640c9 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 5 Jun 2018 19:24:42 +0000 Subject: 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 Commit-Queue: Tom Sepez --- .../edit/cpdf_pagecontentgenerator_unittest.cpp | 6 ++++-- core/fpdfapi/page/cpdf_form.cpp | 22 +++++++++++----------- core/fpdfapi/page/cpdf_page.cpp | 14 ++++++-------- core/fpdfapi/page/cpdf_page.h | 3 --- core/fpdfapi/page/cpdf_pageobjectholder.cpp | 15 ++++++++++----- core/fpdfapi/page/cpdf_pageobjectholder.h | 21 +++++++++++---------- core/fpdfapi/render/cpdf_progressiverenderer.cpp | 10 ++++++---- fpdfsdk/formfiller/cffl_formfiller.cpp | 5 ++--- fpdfsdk/fpdf_view.cpp | 2 +- fxjs/cjs_field.cpp | 6 ++---- 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(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(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* parsedSet) { - if (m_ParseState == CONTENT_PARSED || m_ParseState == CONTENT_PARSING) + if (GetParseState() == ParseState::kParsed) return; - if (!parsedSet) { - if (!m_ParsedSet) - m_ParsedSet = pdfium::MakeUnique>(); - parsedSet = m_ParsedSet.get(); + if (GetParseState() == ParseState::kNotParsed) { + if (!parsedSet) { + if (!m_ParsedSet) + m_ParsedSet = pdfium::MakeUnique>(); + parsedSet = m_ParsedSet.get(); + } + StartParse(pdfium::MakeUnique( + this, pGraphicStates, pParentMatrix, pType3Char, parsedSet)); } - m_pParser = pdfium::MakeUnique( - 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(this); - m_ParseState = CONTENT_PARSING; -} + if (GetParseState() == ParseState::kNotParsed) + StartParse(pdfium::MakeUnique(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 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 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 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 m_FontsMap; protected: - enum ParseState { CONTENT_NOT_PARSED, CONTENT_PARSING, CONTENT_PARSED }; - void LoadTransInfo(); - const UnownedPtr m_pFormDict; - UnownedPtr m_pDocument; CFX_FloatRect m_BBox; CPDF_Transparency m_Transparency; + + private: bool m_bBackgroundAlphaNeeded = false; + ParseState m_ParseState = ParseState::kNotParsed; + const UnownedPtr m_pFormDict; + UnownedPtr m_pDocument; std::vector m_MaskBoundingBoxes; - ParseState m_ParseState = CONTENT_NOT_PARSED; std::unique_ptr 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 || -- cgit v1.2.3