From a715f524e66a1fd8b974f6e2af0db3c23b8c5190 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Tue, 17 Jan 2017 16:03:48 -0500 Subject: More css parser tests; more memory fixes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I929b00204e05eea71c6fd4d52e480cc9c6d6018e Reviewed-on: https://pdfium-review.googlesource.com/2230 Reviewed-by: Nicolás Peña Commit-Queue: dsinclair --- xfa/fde/css/cfde_cssstylesheet_unittest.cpp | 130 ++++++++++++++++++++++++---- xfa/fde/css/fde_cssstylesheet.cpp | 29 +++---- xfa/fde/css/fde_cssstylesheet.h | 9 +- 3 files changed, 132 insertions(+), 36 deletions(-) diff --git a/xfa/fde/css/cfde_cssstylesheet_unittest.cpp b/xfa/fde/css/cfde_cssstylesheet_unittest.cpp index 1edc4b5f51..3715ef7005 100644 --- a/xfa/fde/css/cfde_cssstylesheet_unittest.cpp +++ b/xfa/fde/css/cfde_cssstylesheet_unittest.cpp @@ -7,9 +7,11 @@ #include "xfa/fde/css/fde_cssstylesheet.h" #include +#include #include "testing/gtest/include/gtest/gtest.h" #include "third_party/base/ptr_util.h" +#include "third_party/base/stl_util.h" class CFDE_CSSStyleSheetTest : public testing::Test { public: @@ -20,18 +22,27 @@ class CFDE_CSSStyleSheetTest : public testing::Test { void TearDown() override { decl_ = nullptr; } - void LoadAndVerifyDecl(const FX_WCHAR* buf, size_t decl_count) { + void LoadAndVerifyDecl(const FX_WCHAR* buf, + const std::vector& selectors, + size_t decl_count) { ASSERT(sheet_); EXPECT_TRUE(sheet_->LoadFromBuffer(buf, FXSYS_wcslen(buf))); - EXPECT_EQ(1, sheet_->CountRules()); + EXPECT_EQ(sheet_->CountRules(), 1); CFDE_CSSRule* rule = sheet_->GetRule(0); - EXPECT_EQ(FDE_CSSRuleType::Style, rule->GetType()); + EXPECT_EQ(rule->GetType(), FDE_CSSRuleType::Style); CFDE_CSSStyleRule* style = static_cast(rule); + EXPECT_EQ(selectors.size(), style->CountSelectorLists()); + + for (size_t i = 0; i < selectors.size(); i++) { + uint32_t hash = FX_HashCode_GetW(selectors[i].AsStringC(), true); + EXPECT_EQ(hash, style->GetSelectorList(i)->GetNameHash()); + } + decl_ = style->GetDeclaration(); - EXPECT_EQ(decl_count, decl_->PropertyCountForTesting()); + EXPECT_EQ(decl_->PropertyCountForTesting(), decl_count); } void VerifyFloat(FDE_CSSProperty prop, float val, FDE_CSSPrimitiveType type) { @@ -40,8 +51,8 @@ class CFDE_CSSStyleSheetTest : public testing::Test { bool important; CFDE_CSSValue* v = decl_->GetProperty(prop, important); CFDE_CSSPrimitiveValue* pval = static_cast(v); - EXPECT_EQ(type, pval->GetPrimitiveType()); - EXPECT_EQ(val, pval->GetFloat()); + EXPECT_EQ(pval->GetPrimitiveType(), type); + EXPECT_EQ(pval->GetFloat(), val); } void VerifyEnum(FDE_CSSProperty prop, FDE_CSSPropertyValue val) { @@ -50,16 +61,105 @@ class CFDE_CSSStyleSheetTest : public testing::Test { bool important; CFDE_CSSValue* v = decl_->GetProperty(prop, important); CFDE_CSSPrimitiveValue* pval = static_cast(v); - EXPECT_EQ(FDE_CSSPrimitiveType::Enum, pval->GetPrimitiveType()); - EXPECT_EQ(val, pval->GetEnum()); + EXPECT_EQ(pval->GetPrimitiveType(), FDE_CSSPrimitiveType::Enum); + EXPECT_EQ(pval->GetEnum(), val); + } + + void VerifyList(FDE_CSSProperty prop, + std::vector values) { + ASSERT(decl_); + + bool important; + CFDE_CSSValue* v = decl_->GetProperty(prop, important); + CFDE_CSSValueList* list = static_cast(v); + EXPECT_EQ(list->CountValues(), pdfium::CollectionSize(values)); + + for (size_t i = 0; i < values.size(); i++) { + CFDE_CSSValue* val = list->GetValue(i); + CFDE_CSSPrimitiveValue* pval = static_cast(val); + EXPECT_EQ(pval->GetPrimitiveType(), FDE_CSSPrimitiveType::Enum); + EXPECT_EQ(pval->GetEnum(), values[i]); + } } std::unique_ptr sheet_; CFDE_CSSDeclaration* decl_; }; +TEST_F(CFDE_CSSStyleSheetTest, ParseMultipleSelectors) { + const FX_WCHAR* buf = + L"a { border: 10px; }\nb { text-decoration: underline; }"; + EXPECT_TRUE(sheet_->LoadFromBuffer(buf, FXSYS_wcslen(buf))); + EXPECT_EQ(2, sheet_->CountRules()); + + CFDE_CSSRule* rule = sheet_->GetRule(0); + EXPECT_EQ(FDE_CSSRuleType::Style, rule->GetType()); + + CFDE_CSSStyleRule* style = static_cast(rule); + EXPECT_EQ(1UL, style->CountSelectorLists()); + + bool found_selector = false; + uint32_t hash = FX_HashCode_GetW(L"a", true); + for (size_t i = 0; i < style->CountSelectorLists(); i++) { + if (style->GetSelectorList(i)->GetNameHash() == hash) { + found_selector = true; + break; + } + } + EXPECT_TRUE(found_selector); + + decl_ = style->GetDeclaration(); + EXPECT_EQ(4UL, decl_->PropertyCountForTesting()); + + VerifyFloat(FDE_CSSProperty::BorderLeftWidth, 10.0, + FDE_CSSPrimitiveType::Pixels); + VerifyFloat(FDE_CSSProperty::BorderRightWidth, 10.0, + FDE_CSSPrimitiveType::Pixels); + VerifyFloat(FDE_CSSProperty::BorderTopWidth, 10.0, + FDE_CSSPrimitiveType::Pixels); + VerifyFloat(FDE_CSSProperty::BorderBottomWidth, 10.0, + FDE_CSSPrimitiveType::Pixels); + + rule = sheet_->GetRule(1); + EXPECT_EQ(FDE_CSSRuleType::Style, rule->GetType()); + + style = static_cast(rule); + EXPECT_EQ(1UL, style->CountSelectorLists()); + + found_selector = false; + hash = FX_HashCode_GetW(L"b", true); + for (size_t i = 0; i < style->CountSelectorLists(); i++) { + if (style->GetSelectorList(i)->GetNameHash() == hash) { + found_selector = true; + break; + } + } + EXPECT_TRUE(found_selector); + + decl_ = style->GetDeclaration(); + EXPECT_EQ(1UL, decl_->PropertyCountForTesting()); + VerifyList(FDE_CSSProperty::TextDecoration, + {FDE_CSSPropertyValue::Underline}); +} + +TEST_F(CFDE_CSSStyleSheetTest, ParseMultipleSelectorsCombined) { + LoadAndVerifyDecl(L"a, b, c { border: 5px; }", {L"a", L"b", L"c"}, 4); +} + +TEST_F(CFDE_CSSStyleSheetTest, ParseWithPseudo) { + // TODO(dsinclair): I think this is wrong, as the selector just becomes + // :before and we lose the a? + LoadAndVerifyDecl(L"a:before { border: 10px; }", {L":before"}, 4); +} + +TEST_F(CFDE_CSSStyleSheetTest, ParseWithSelectorsAndPseudo) { + // TODO(dsinclair): I think this is wrong as we lose the b on the b:before + LoadAndVerifyDecl(L"a, b:before, c { border: 1px; }", + {L"a", L":before", L"c"}, 4); +} + TEST_F(CFDE_CSSStyleSheetTest, ParseBorder) { - LoadAndVerifyDecl(L"a { border: 5px; }", 4); + LoadAndVerifyDecl(L"a { border: 5px; }", {L"a"}, 4); VerifyFloat(FDE_CSSProperty::BorderLeftWidth, 5.0, FDE_CSSPrimitiveType::Pixels); VerifyFloat(FDE_CSSProperty::BorderRightWidth, 5.0, @@ -71,7 +171,7 @@ TEST_F(CFDE_CSSStyleSheetTest, ParseBorder) { } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderFull) { - LoadAndVerifyDecl(L"a { border: 5px solid red; }", 4); + LoadAndVerifyDecl(L"a { border: 5px solid red; }", {L"a"}, 4); VerifyFloat(FDE_CSSProperty::BorderLeftWidth, 5.0, FDE_CSSPrimitiveType::Pixels); VerifyFloat(FDE_CSSProperty::BorderRightWidth, 5.0, @@ -83,30 +183,30 @@ TEST_F(CFDE_CSSStyleSheetTest, ParseBorderFull) { } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderLeft) { - LoadAndVerifyDecl(L"a { border-left: 2.5pc; }", 1); + LoadAndVerifyDecl(L"a { border-left: 2.5pc; }", {L"a"}, 1); VerifyFloat(FDE_CSSProperty::BorderLeftWidth, 2.5, FDE_CSSPrimitiveType::Picas); } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderLeftThick) { - LoadAndVerifyDecl(L"a { border-left: thick; }", 1); + LoadAndVerifyDecl(L"a { border-left: thick; }", {L"a"}, 1); VerifyEnum(FDE_CSSProperty::BorderLeftWidth, FDE_CSSPropertyValue::Thick); } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderRight) { - LoadAndVerifyDecl(L"a { border-right: 2.5pc; }", 1); + LoadAndVerifyDecl(L"a { border-right: 2.5pc; }", {L"a"}, 1); VerifyFloat(FDE_CSSProperty::BorderRightWidth, 2.5, FDE_CSSPrimitiveType::Picas); } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderTop) { - LoadAndVerifyDecl(L"a { border-top: 2.5pc; }", 1); + LoadAndVerifyDecl(L"a { border-top: 2.5pc; }", {L"a"}, 1); VerifyFloat(FDE_CSSProperty::BorderTopWidth, 2.5, FDE_CSSPrimitiveType::Picas); } TEST_F(CFDE_CSSStyleSheetTest, ParseBorderBottom) { - LoadAndVerifyDecl(L"a { border-bottom: 2.5pc; }", 1); + LoadAndVerifyDecl(L"a { border-bottom: 2.5pc; }", {L"a"}, 1); VerifyFloat(FDE_CSSProperty::BorderBottomWidth, 2.5, FDE_CSSPrimitiveType::Picas); } diff --git a/xfa/fde/css/fde_cssstylesheet.cpp b/xfa/fde/css/fde_cssstylesheet.cpp index a3667b202f..50f2bddfbf 100644 --- a/xfa/fde/css/fde_cssstylesheet.cpp +++ b/xfa/fde/css/fde_cssstylesheet.cpp @@ -62,7 +62,6 @@ CFDE_CSSStyleSheet::~CFDE_CSSStyleSheet() { void CFDE_CSSStyleSheet::Reset() { m_RuleArray.clear(); - m_Selectors.clear(); m_StringCache.clear(); } @@ -133,7 +132,7 @@ bool CFDE_CSSStyleSheet::LoadFromSyntax(CFDE_CSSSyntaxParser* pSyntax) { break; } } while (eStatus >= FDE_CSSSyntaxStatus::None); - m_Selectors.clear(); + m_StringCache.clear(); return eStatus != FDE_CSSSyntaxStatus::Error; } @@ -185,7 +184,7 @@ FDE_CSSSyntaxStatus CFDE_CSSStyleSheet::LoadMediaRule( FDE_CSSSyntaxStatus CFDE_CSSStyleSheet::LoadStyleRule( CFDE_CSSSyntaxParser* pSyntax, std::vector>* ruleArray) { - m_Selectors.clear(); + std::vector> selectors; CFDE_CSSStyleRule* pStyleRule = nullptr; const FX_WCHAR* pszValue = nullptr; @@ -200,8 +199,9 @@ FDE_CSSSyntaxStatus CFDE_CSSStyleSheet::LoadStyleRule( pszValue = pSyntax->GetCurrentString(iValueLen); auto pSelector = CFDE_CSSSelector::FromString(pszValue, iValueLen); if (pSelector) - m_Selectors.push_back(std::move(pSelector)); - } break; + selectors.push_back(std::move(pSelector)); + break; + } case FDE_CSSSyntaxStatus::PropertyName: pszValue = pSyntax->GetCurrentString(iValueLen); propertyArgs.pProperty = @@ -226,10 +226,10 @@ FDE_CSSSyntaxStatus CFDE_CSSStyleSheet::LoadStyleRule( } break; case FDE_CSSSyntaxStatus::DeclOpen: - if (!pStyleRule && !m_Selectors.empty()) { + if (!pStyleRule && !selectors.empty()) { auto rule = pdfium::MakeUnique(); pStyleRule = rule.get(); - pStyleRule->SetSelector(m_Selectors); + pStyleRule->SetSelector(&selectors); ruleArray->push_back(std::move(rule)); } else { SkipRuleSet(pSyntax); @@ -336,18 +336,16 @@ FDE_CSSSyntaxStatus CFDE_CSSStyleSheet::SkipRuleSet( } } -CFDE_CSSStyleRule::CFDE_CSSStyleRule() - : CFDE_CSSRule(FDE_CSSRuleType::Style), - m_iSelectors(0) {} +CFDE_CSSStyleRule::CFDE_CSSStyleRule() : CFDE_CSSRule(FDE_CSSRuleType::Style) {} CFDE_CSSStyleRule::~CFDE_CSSStyleRule() {} -int32_t CFDE_CSSStyleRule::CountSelectorLists() const { - return m_iSelectors; +size_t CFDE_CSSStyleRule::CountSelectorLists() const { + return m_ppSelector.size(); } CFDE_CSSSelector* CFDE_CSSStyleRule::GetSelectorList(int32_t index) const { - return m_ppSelector[index]; + return m_ppSelector[index].get(); } CFDE_CSSDeclaration* CFDE_CSSStyleRule::GetDeclaration() { @@ -355,11 +353,10 @@ CFDE_CSSDeclaration* CFDE_CSSStyleRule::GetDeclaration() { } void CFDE_CSSStyleRule::SetSelector( - const std::vector>& list) { + std::vector>* list) { ASSERT(m_ppSelector.empty()); - for (const auto& item : list) - m_ppSelector.push_back(item.get()); + m_ppSelector.swap(*list); } CFDE_CSSMediaRule::CFDE_CSSMediaRule(uint32_t dwMediaList) diff --git a/xfa/fde/css/fde_cssstylesheet.h b/xfa/fde/css/fde_cssstylesheet.h index 7acccd5776..46c0897694 100644 --- a/xfa/fde/css/fde_cssstylesheet.h +++ b/xfa/fde/css/fde_cssstylesheet.h @@ -51,16 +51,16 @@ class CFDE_CSSStyleRule : public CFDE_CSSRule { CFDE_CSSStyleRule(); ~CFDE_CSSStyleRule() override; - int32_t CountSelectorLists() const; + size_t CountSelectorLists() const; CFDE_CSSSelector* GetSelectorList(int32_t index) const; CFDE_CSSDeclaration* GetDeclaration(); CFDE_CSSDeclaration& GetDeclImp() { return m_Declaration; } - void SetSelector(const std::vector>& list); + + void SetSelector(std::vector>* list); private: CFDE_CSSDeclaration m_Declaration; - std::vector m_ppSelector; // Owned by the stylessheet. - int32_t m_iSelectors; + std::vector> m_ppSelector; }; class CFDE_CSSMediaRule : public CFDE_CSSRule { @@ -126,7 +126,6 @@ class CFDE_CSSStyleSheet : public IFX_Retainable { uint32_t m_dwMediaList; std::vector> m_RuleArray; CFX_WideString m_szUrl; - std::vector> m_Selectors; std::unordered_map m_StringCache; }; -- cgit v1.2.3