Make CFXA_TextParser a GCed class. -- forward declare it in cxfa_texlayout.h and fix IWYU. -- initialize scalars in headers. -- cleanup in dtor not required. Bug: pdfium:1563 Change-Id: Id7a3e412720ae3076960449c65fde52d7542d217 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73435 Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp index 1a4e941..ea97aa8 100644 --- a/xfa/fxfa/cxfa_textlayout.cpp +++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -28,6 +28,7 @@ #include "xfa/fgas/layout/cfx_textuserdata.h" #include "xfa/fxfa/cxfa_ffdoc.h" #include "xfa/fxfa/cxfa_textparsecontext.h" +#include "xfa/fxfa/cxfa_textparser.h" #include "xfa/fxfa/cxfa_textprovider.h" #include "xfa/fxfa/cxfa_texttabstopscontext.h" #include "xfa/fxfa/parser/cxfa_font.h" @@ -82,19 +83,20 @@ CXFA_TextLayout::CXFA_TextLayout(CXFA_FFDoc* doc, CXFA_TextProvider* pTextProvider) - : m_pDoc(doc), m_pTextProvider(pTextProvider) { + : m_pDoc(doc), + m_pTextProvider(pTextProvider), + m_pTextParser(cppgc::MakeGarbageCollected<CXFA_TextParser>( + doc->GetHeap()->GetAllocationHandle())) { ASSERT(m_pTextProvider); } -CXFA_TextLayout::~CXFA_TextLayout() { - m_textParser.Reset(); - Unload(); -} +CXFA_TextLayout::~CXFA_TextLayout() = default; void CXFA_TextLayout::Trace(cppgc::Visitor* visitor) const { visitor->Trace(m_pDoc); visitor->Trace(m_pTextProvider); visitor->Trace(m_pTextDataNode); + visitor->Trace(m_pTextParser); if (m_pLoader) m_pLoader->Trace(visitor); } @@ -117,7 +119,7 @@ void CXFA_TextLayout::GetTextDataNode() { CXFA_Node* pNode = m_pTextProvider->GetTextNode(&m_bRichText); if (pNode && m_bRichText) - m_textParser.Reset(); + m_pTextParser->Reset(); m_pTextDataNode = pNode; } @@ -149,8 +151,9 @@ auto pBreak = std::make_unique<CFX_RTFBreak>(dwStyle); pBreak->SetLineBreakTolerance(1); - pBreak->SetFont(m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, nullptr)); - pBreak->SetFontSize(m_textParser.GetFontSize(m_pTextProvider, nullptr)); + pBreak->SetFont( + m_pTextParser->GetFont(m_pDoc.Get(), m_pTextProvider, nullptr)); + pBreak->SetFontSize(m_pTextParser->GetFontSize(m_pTextProvider, nullptr)); return pBreak; } @@ -209,10 +212,10 @@ m_pBreak->SetCharSpace(font->GetLetterSpacing()); } - float fFontSize = m_textParser.GetFontSize(m_pTextProvider, nullptr); + float fFontSize = m_pTextParser->GetFontSize(m_pTextProvider, nullptr); m_pBreak->SetFontSize(fFontSize); m_pBreak->SetFont( - m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, nullptr)); + m_pTextParser->GetFont(m_pDoc.Get(), m_pTextProvider, nullptr)); m_pBreak->SetLineBreakTolerance(fFontSize * 0.2f); } @@ -282,21 +285,22 @@ fStart += fIndent; m_pBreak->SetLineStartPos(fStart); - m_pBreak->SetTabWidth(m_textParser.GetTabInterval(pStyle)); + m_pBreak->SetTabWidth(m_pTextParser->GetTabInterval(pStyle)); if (!m_pTabstopContext) m_pTabstopContext = std::make_unique<CXFA_TextTabstopsContext>(); - m_textParser.GetTabstops(pStyle, m_pTabstopContext.get()); + m_pTextParser->GetTabstops(pStyle, m_pTabstopContext.get()); for (const auto& stop : m_pTabstopContext->m_tabstops) m_pBreak->AddPositionedTab(stop.fTabstops); } - float fFontSize = m_textParser.GetFontSize(m_pTextProvider, pStyle); + float fFontSize = m_pTextParser->GetFontSize(m_pTextProvider, pStyle); m_pBreak->SetFontSize(fFontSize); m_pBreak->SetLineBreakTolerance(fFontSize * 0.2f); m_pBreak->SetFont( - m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, pStyle)); + m_pTextParser->GetFont(m_pDoc.Get(), m_pTextProvider, pStyle)); m_pBreak->SetHorizontalScale( - m_textParser.GetHorScale(m_pTextProvider, pStyle, pXMLNode)); - m_pBreak->SetVerticalScale(m_textParser.GetVerScale(m_pTextProvider, pStyle)); + m_pTextParser->GetHorScale(m_pTextProvider, pStyle, pXMLNode)); + m_pBreak->SetVerticalScale( + m_pTextParser->GetVerScale(m_pTextProvider, pStyle)); m_pBreak->SetCharSpace(pStyle->GetLetterSpacing().GetValue()); } @@ -370,7 +374,7 @@ if (m_Blocks.empty() && m_pLoader->fHeight > 0) { float fHeight = fTextHeight - GetLayoutHeight(); if (fHeight > 0) { - XFA_AttributeValue iAlign = m_textParser.GetVAlign(m_pTextProvider); + XFA_AttributeValue iAlign = m_pTextParser->GetVAlign(m_pTextProvider); if (iAlign == XFA_AttributeValue::Middle) fHeight /= 2.0f; else if (iAlign != XFA_AttributeValue::Bottom) @@ -454,7 +458,7 @@ m_fMaxWidth = 0; Loader(width, &fLinePos, false); if (fLinePos < 0.1f) - fLinePos = m_textParser.GetFontSize(m_pTextProvider, nullptr); + fLinePos = m_pTextParser->GetFontSize(m_pTextProvider, nullptr); m_pTabstopContext.reset(); return CFX_SizeF(m_fMaxWidth, fLinePos); @@ -645,7 +649,7 @@ if (fHeight < 0.1f) return; - switch (m_textParser.GetVAlign(m_pTextProvider)) { + switch (m_pTextParser->GetVAlign(m_pTextProvider)) { case XFA_AttributeValue::Middle: fHeight /= 2.0f; break; @@ -677,10 +681,10 @@ if (!pXMLContainer) return; - if (!m_textParser.IsParsed()) - m_textParser.DoParse(pXMLContainer, m_pTextProvider); + if (!m_pTextParser->IsParsed()) + m_pTextParser->DoParse(pXMLContainer, m_pTextProvider); - auto pRootStyle = m_textParser.CreateRootStyle(m_pTextProvider); + auto pRootStyle = m_pTextParser->CreateRootStyle(m_pTextProvider); LoadRichText(pXMLContainer, textWidth, pLinePos, pRootStyle, bSavePieces, nullptr, true, false, 0); } @@ -734,7 +738,7 @@ return false; CXFA_TextParseContext* pContext = - m_textParser.GetParseContextFromMap(pXMLNode); + m_pTextParser->GetParseContextFromMap(pXMLNode); CFX_CSSDisplay eDisplay = CFX_CSSDisplay::None; bool bContentNode = false; float fSpaceBelow = 0; @@ -763,7 +767,7 @@ return true; } - pStyle = m_textParser.ComputeStyle(pXMLNode, pParentStyle.Get()); + pStyle = m_pTextParser->ComputeStyle(pXMLNode, pParentStyle.Get()); InitBreak(bContentNode ? pParentStyle.Get() : pStyle.Get(), eDisplay, textWidth, pXMLNode, pParentStyle.Get()); if ((eDisplay == CFX_CSSDisplay::Block || @@ -785,10 +789,10 @@ pLinkData = pdfium::MakeRetain<CFX_LinkUserData>(wsLinkContent); } - int32_t iTabCount = m_textParser.CountTabs( + int32_t iTabCount = m_pTextParser->CountTabs( bContentNode ? pParentStyle.Get() : pStyle.Get()); - bool bSpaceRun = m_textParser.IsSpaceRun(bContentNode ? pParentStyle.Get() - : pStyle.Get()); + bool bSpaceRun = m_pTextParser->IsSpaceRun( + bContentNode ? pParentStyle.Get() : pStyle.Get()); WideString wsText; if (bContentNode && iTabCount == 0) { wsText = ToXMLText(pXMLNode)->GetText(); @@ -806,7 +810,7 @@ wsText += L'\t'; } else { Optional<WideString> obj = - m_textParser.GetEmbeddedObj(m_pTextProvider, pXMLNode); + m_pTextParser->GetEmbeddedObj(m_pTextProvider, pXMLNode); if (obj) wsText = *obj; } @@ -960,7 +964,7 @@ TextPiece* pPiece = pPieceLine->m_textPieces[iPieces - 1].get(); int32_t& iTabstopsIndex = m_pTabstopContext->m_iTabIndex; - int32_t iCount = m_textParser.CountTabs(pStyle); + int32_t iCount = m_pTextParser->CountTabs(pStyle); if (!pdfium::IndexInBounds(m_pTabstopContext->m_tabstops, iTabstopsIndex)) return; @@ -1034,24 +1038,25 @@ pTP->iHorScale = pPiece->m_iHorizontalScale; pTP->iVerScale = pPiece->m_iVerticalScale; pTP->iUnderline = - m_textParser.GetUnderline(m_pTextProvider, pStyle.Get()); + m_pTextParser->GetUnderline(m_pTextProvider, pStyle.Get()); pTP->iPeriod = - m_textParser.GetUnderlinePeriod(m_pTextProvider, pStyle.Get()); + m_pTextParser->GetUnderlinePeriod(m_pTextProvider, pStyle.Get()); pTP->iLineThrough = - m_textParser.GetLinethrough(m_pTextProvider, pStyle.Get()); - pTP->dwColor = m_textParser.GetColor(m_pTextProvider, pStyle.Get()); + m_pTextParser->GetLinethrough(m_pTextProvider, pStyle.Get()); + pTP->dwColor = m_pTextParser->GetColor(m_pTextProvider, pStyle.Get()); pTP->pFont = - m_textParser.GetFont(m_pDoc.Get(), m_pTextProvider, pStyle.Get()); - pTP->fFontSize = m_textParser.GetFontSize(m_pTextProvider, pStyle.Get()); + m_pTextParser->GetFont(m_pDoc.Get(), m_pTextProvider, pStyle.Get()); + pTP->fFontSize = + m_pTextParser->GetFontSize(m_pTextProvider, pStyle.Get()); pTP->rtPiece.left = pPiece->m_iStartPos / 20000.0f; pTP->rtPiece.width = pPiece->m_iWidth / 20000.0f; pTP->rtPiece.height = static_cast<float>(pPiece->m_iFontSize) * fVerScale / 20.0f; float fBaseLineTemp = - m_textParser.GetBaseline(m_pTextProvider, pStyle.Get()); + m_pTextParser->GetBaseline(m_pTextProvider, pStyle.Get()); pTP->rtPiece.top = fBaseLineTemp; - float fLineHeight = m_textParser.GetLineHeight( + float fLineHeight = m_pTextParser->GetLineHeight( m_pTextProvider, pStyle.Get(), m_iLines == 0, fVerScale); if (fBaseLineTemp > 0) { float fLineHeightTmp = fBaseLineTemp + pTP->rtPiece.height; @@ -1079,8 +1084,9 @@ if (pUserData) pStyle = pUserData->m_pStyle; float fVerScale = pPiece->m_iVerticalScale / 100.0f; - float fBaseLine = m_textParser.GetBaseline(m_pTextProvider, pStyle.Get()); - float fLineHeight = m_textParser.GetLineHeight( + float fBaseLine = + m_pTextParser->GetBaseline(m_pTextProvider, pStyle.Get()); + float fLineHeight = m_pTextParser->GetLineHeight( m_pTextProvider, pStyle.Get(), m_iLines == 0, fVerScale); if (fBaseLine > 0) { float fLineHeightTmp =
diff --git a/xfa/fxfa/cxfa_textlayout.h b/xfa/fxfa/cxfa_textlayout.h index 7512cfb..4c29fd0 100644 --- a/xfa/fxfa/cxfa_textlayout.h +++ b/xfa/fxfa/cxfa_textlayout.h
@@ -15,22 +15,24 @@ #include "core/fxcrt/fx_string.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/unowned_ptr.h" +#include "core/fxge/fx_dib.h" #include "fxjs/gc/heap.h" #include "v8/include/cppgc/garbage-collected.h" #include "v8/include/cppgc/member.h" #include "v8/include/cppgc/visitor.h" #include "xfa/fgas/layout/cfx_char.h" #include "xfa/fgas/layout/cfx_textpiece.h" -#include "xfa/fxfa/cxfa_textparser.h" +#include "xfa/fxfa/fxfa_basic.h" class CFDE_RenderDevice; -class CFX_LinkUserData; class CFX_CSSComputedStyle; +class CFX_LinkUserData; class CFX_RTFBreak; class CFX_RenderDevice; class CFX_XMLNode; -class CFX_LinkUserData; +class CXFA_FFDoc; class CXFA_Node; +class CXFA_TextParser; class CXFA_TextProvider; class CXFA_TextTabstopsContext; class TextCharPos; @@ -180,9 +182,9 @@ cppgc::Member<CXFA_FFDoc> const m_pDoc; cppgc::Member<CXFA_TextProvider> const m_pTextProvider; cppgc::Member<CXFA_Node> m_pTextDataNode; + cppgc::Member<CXFA_TextParser> m_pTextParser; std::unique_ptr<CFX_RTFBreak> m_pBreak; std::unique_ptr<LoaderContext> m_pLoader; - CXFA_TextParser m_textParser; std::vector<std::unique_ptr<PieceLine>> m_pieceLines; std::unique_ptr<CXFA_TextTabstopsContext> m_pTabstopContext; };
diff --git a/xfa/fxfa/cxfa_textparser.cpp b/xfa/fxfa/cxfa_textparser.cpp index fa53a53..d9198f1 100644 --- a/xfa/fxfa/cxfa_textparser.cpp +++ b/xfa/fxfa/cxfa_textparser.cpp
@@ -55,8 +55,7 @@ } // namespace -CXFA_TextParser::CXFA_TextParser() - : m_bParsed(false), m_cssInitialized(false) {} +CXFA_TextParser::CXFA_TextParser() = default; CXFA_TextParser::~CXFA_TextParser() = default; @@ -623,7 +622,6 @@ return true; } -CXFA_TextParser::TagProvider::TagProvider() - : m_bTagAvailable(false), m_bContent(false) {} +CXFA_TextParser::TagProvider::TagProvider() = default; CXFA_TextParser::TagProvider::~TagProvider() = default;
diff --git a/xfa/fxfa/cxfa_textparser.h b/xfa/fxfa/cxfa_textparser.h index fbae3cf..c81bf6d 100644 --- a/xfa/fxfa/cxfa_textparser.h +++ b/xfa/fxfa/cxfa_textparser.h
@@ -14,7 +14,9 @@ #include "core/fxcrt/fx_system.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxge/fx_dib.h" +#include "fxjs/gc/heap.h" #include "third_party/base/optional.h" +#include "v8/include/cppgc/garbage-collected.h" #include "xfa/fxfa/fxfa_basic.h" class CFGAS_GEFont; @@ -27,11 +29,13 @@ class CXFA_TextProvider; class CXFA_TextTabstopsContext; -class CXFA_TextParser { +class CXFA_TextParser : public cppgc::GarbageCollected<CXFA_TextParser> { public: - CXFA_TextParser(); + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; virtual ~CXFA_TextParser(); + void Trace(cppgc::Visitor* visitor) const {} + void Reset(); void DoParse(const CFX_XMLNode* pXMLContainer, CXFA_TextProvider* pTextProvider); @@ -84,6 +88,8 @@ CXFA_TextParseContext* GetParseContextFromMap(const CFX_XMLNode* pXMLNode); protected: + CXFA_TextParser(); + bool TagValidate(const WideString& str) const; private: @@ -103,8 +109,8 @@ return m_Attributes[wsAttr]; } - bool m_bTagAvailable; - bool m_bContent; + bool m_bTagAvailable = false; + bool m_bContent = false; private: WideString m_wsTagName; @@ -121,8 +127,8 @@ RetainPtr<CFX_CSSComputedStyle> CreateStyle( const CFX_CSSComputedStyle* pParentStyle); - bool m_bParsed; - bool m_cssInitialized; + bool m_bParsed = false; + bool m_cssInitialized = false; std::unique_ptr<CFX_CSSStyleSelector> m_pSelector; std::map<const CFX_XMLNode*, std::unique_ptr<CXFA_TextParseContext>> m_mapXMLNodeToParseContext;
diff --git a/xfa/fxfa/cxfa_textparser_unittest.cpp b/xfa/fxfa/cxfa_textparser_unittest.cpp index 5198638..a61f441 100644 --- a/xfa/fxfa/cxfa_textparser_unittest.cpp +++ b/xfa/fxfa/cxfa_textparser_unittest.cpp
@@ -4,37 +4,44 @@ #include "xfa/fxfa/cxfa_textparser.h" +#include "fxjs/gc/heap.h" +#include "testing/fxgc_unittest.h" #include "testing/gtest/include/gtest/gtest.h" class CXFA_TestTextParser final : public CXFA_TextParser { public: - CXFA_TestTextParser() : CXFA_TextParser() {} + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; private: + CXFA_TestTextParser() = default; + // Add test cases as friends to access protected member functions. - FRIEND_TEST(CXFA_TextParser, TagValidate); + FRIEND_TEST(CXFATextParserTest, TagValidate); }; -TEST(CXFA_TextParser, TagValidate) { - CXFA_TestTextParser parser; - EXPECT_TRUE(parser.TagValidate(L"br")); - EXPECT_TRUE(parser.TagValidate(L"Br")); - EXPECT_TRUE(parser.TagValidate(L"BR")); - EXPECT_TRUE(parser.TagValidate(L"a")); - EXPECT_TRUE(parser.TagValidate(L"b")); - EXPECT_TRUE(parser.TagValidate(L"i")); - EXPECT_TRUE(parser.TagValidate(L"p")); - EXPECT_TRUE(parser.TagValidate(L"li")); - EXPECT_TRUE(parser.TagValidate(L"ol")); - EXPECT_TRUE(parser.TagValidate(L"ul")); - EXPECT_TRUE(parser.TagValidate(L"sub")); - EXPECT_TRUE(parser.TagValidate(L"sup")); - EXPECT_TRUE(parser.TagValidate(L"span")); - EXPECT_TRUE(parser.TagValidate(L"body")); - EXPECT_TRUE(parser.TagValidate(L"html")); +class CXFATextParserTest : public FXGCUnitTest {}; - EXPECT_FALSE(parser.TagValidate(L"")); - EXPECT_FALSE(parser.TagValidate(L"tml")); - EXPECT_FALSE(parser.TagValidate(L"xhtml")); - EXPECT_FALSE(parser.TagValidate(L"htmlx")); +TEST_F(CXFATextParserTest, TagValidate) { + auto* parser = cppgc::MakeGarbageCollected<CXFA_TestTextParser>( + heap()->GetAllocationHandle()); + EXPECT_TRUE(parser->TagValidate(L"br")); + EXPECT_TRUE(parser->TagValidate(L"Br")); + EXPECT_TRUE(parser->TagValidate(L"BR")); + EXPECT_TRUE(parser->TagValidate(L"a")); + EXPECT_TRUE(parser->TagValidate(L"b")); + EXPECT_TRUE(parser->TagValidate(L"i")); + EXPECT_TRUE(parser->TagValidate(L"p")); + EXPECT_TRUE(parser->TagValidate(L"li")); + EXPECT_TRUE(parser->TagValidate(L"ol")); + EXPECT_TRUE(parser->TagValidate(L"ul")); + EXPECT_TRUE(parser->TagValidate(L"sub")); + EXPECT_TRUE(parser->TagValidate(L"sup")); + EXPECT_TRUE(parser->TagValidate(L"span")); + EXPECT_TRUE(parser->TagValidate(L"body")); + EXPECT_TRUE(parser->TagValidate(L"html")); + + EXPECT_FALSE(parser->TagValidate(L"")); + EXPECT_FALSE(parser->TagValidate(L"tml")); + EXPECT_FALSE(parser->TagValidate(L"xhtml")); + EXPECT_FALSE(parser->TagValidate(L"htmlx")); }