More css parser tests; more memory fixes.

Change-Id: I929b00204e05eea71c6fd4d52e480cc9c6d6018e
Reviewed-on: https://pdfium-review.googlesource.com/2230
Reviewed-by: Nicolás Peña <npm@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

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 <memory>
+#include <vector>
 
 #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<CFX_WideString>& 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<CFDE_CSSStyleRule*>(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<CFDE_CSSPrimitiveValue*>(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<CFDE_CSSPrimitiveValue*>(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<FDE_CSSPropertyValue> values) {
+    ASSERT(decl_);
+
+    bool important;
+    CFDE_CSSValue* v = decl_->GetProperty(prop, important);
+    CFDE_CSSValueList* list = static_cast<CFDE_CSSValueList*>(v);
+    EXPECT_EQ(list->CountValues(), pdfium::CollectionSize<int32_t>(values));
+
+    for (size_t i = 0; i < values.size(); i++) {
+      CFDE_CSSValue* val = list->GetValue(i);
+      CFDE_CSSPrimitiveValue* pval = static_cast<CFDE_CSSPrimitiveValue*>(val);
+      EXPECT_EQ(pval->GetPrimitiveType(), FDE_CSSPrimitiveType::Enum);
+      EXPECT_EQ(pval->GetEnum(), values[i]);
+    }
   }
 
   std::unique_ptr<CFDE_CSSStyleSheet> 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<CFDE_CSSStyleRule*>(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<CFDE_CSSStyleRule*>(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<std::unique_ptr<CFDE_CSSRule>>* ruleArray) {
-  m_Selectors.clear();
+  std::vector<std::unique_ptr<CFDE_CSSSelector>> 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<CFDE_CSSStyleRule>();
           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<std::unique_ptr<CFDE_CSSSelector>>& list) {
+    std::vector<std::unique_ptr<CFDE_CSSSelector>>* 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<std::unique_ptr<CFDE_CSSSelector>>& list);
+
+  void SetSelector(std::vector<std::unique_ptr<CFDE_CSSSelector>>* list);
 
  private:
   CFDE_CSSDeclaration m_Declaration;
-  std::vector<CFDE_CSSSelector*> m_ppSelector;  // Owned by the stylessheet.
-  int32_t m_iSelectors;
+  std::vector<std::unique_ptr<CFDE_CSSSelector>> m_ppSelector;
 };
 
 class CFDE_CSSMediaRule : public CFDE_CSSRule {
@@ -126,7 +126,6 @@ class CFDE_CSSStyleSheet : public IFX_Retainable {
   uint32_t m_dwMediaList;
   std::vector<std::unique_ptr<CFDE_CSSRule>> m_RuleArray;
   CFX_WideString m_szUrl;
-  std::vector<std::unique_ptr<CFDE_CSSSelector>> m_Selectors;
   std::unordered_map<uint32_t, FX_WCHAR*> m_StringCache;
 };