commit cd89193d92c11c907f316d7f6e1c4af87d3f9aaf
author Lei Zhang <thestig@chromium.org> Thu Jun 18 22:33:31 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Jun 18 22:33:31 2020 +0000
tree 658f51ca5a0fbfc27096985724d7f8258181382b
parent 3001272cded61391907caf82e696c76abb14266c

Clean up CFX_CSSSelector:

- Remove parameters whose values are always the same.
- Inline all the getters since they are all trivial.
- Make GetNextSelector() return a const pointer and adjust unit tests
  accordingly.
- Mark |m_dwHash| as const.
- Give some variables clearer names.

Change-Id: I233d9a8666e9be4dd4ab9c2a1da686f686bdd76e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/70711
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxcrt/css/cfx_cssselector.cpp

diff --git a/core/fxcrt/css/cfx_cssselector.cpp b/core/fxcrt/css/cfx_cssselector.cpp
index 61c4df8..99a8a40 100644
--- a/core/fxcrt/css/cfx_cssselector.cpp
+++ b/core/fxcrt/css/cfx_cssselector.cpp
@@ -25,27 +25,12 @@
 
 }  // namespace
 
-CFX_CSSSelector::CFX_CSSSelector(CFX_CSSSelectorType eType,
-                                 const wchar_t* psz,
-                                 int32_t iLen,
-                                 bool bIgnoreCase)
-    : m_eType(eType),
-      m_dwHash(FX_HashCode_GetW(WideStringView(psz, iLen), bIgnoreCase)) {}
+CFX_CSSSelector::CFX_CSSSelector(const wchar_t* psz, int32_t iLen)
+    : m_dwHash(
+          FX_HashCode_GetW(WideStringView(psz, iLen), /*bIgnoreCase=*/true)) {}
 
 CFX_CSSSelector::~CFX_CSSSelector() = default;
 
-CFX_CSSSelectorType CFX_CSSSelector::GetType() const {
-  return m_eType;
-}
-
-uint32_t CFX_CSSSelector::GetNameHash() const {
-  return m_dwHash;
-}
-
-CFX_CSSSelector* CFX_CSSSelector::GetNextSelector() const {
-  return m_pNext.get();
-}
-
 // static.
 std::unique_ptr<CFX_CSSSelector> CFX_CSSSelector::FromString(
     WideStringView str) {
@@ -63,18 +48,17 @@
     }
   }
 
-  std::unique_ptr<CFX_CSSSelector> pFirst = nullptr;
+  std::unique_ptr<CFX_CSSSelector> head;
   for (psz = pStart; psz < pEnd;) {
     wchar_t wch = *psz;
     if ((isascii(wch) && isalpha(wch)) || wch == '*') {
       int32_t iNameLen = wch == '*' ? 1 : GetCSSNameLen(psz, pEnd);
-      auto p = std::make_unique<CFX_CSSSelector>(CFX_CSSSelectorType::Element,
-                                                 psz, iNameLen, true);
-      if (pFirst) {
-        pFirst->SetType(CFX_CSSSelectorType::Descendant);
-        p->SetNext(std::move(pFirst));
+      auto new_head = std::make_unique<CFX_CSSSelector>(psz, iNameLen);
+      if (head) {
+        head->SetDescendentType();
+        new_head->SetNext(std::move(head));
       }
-      pFirst = std::move(p);
+      head = std::move(new_head);
       psz += iNameLen;
     } else if (wch == ' ') {
       psz++;
@@ -82,5 +66,5 @@
       return nullptr;
     }
   }
-  return pFirst;
+  return head;
 }

core/fxcrt/css/cfx_cssselector.h

diff --git a/core/fxcrt/css/cfx_cssselector.h b/core/fxcrt/css/cfx_cssselector.h
index 14b61ee..56414b7 100644
--- a/core/fxcrt/css/cfx_cssselector.h
+++ b/core/fxcrt/css/cfx_cssselector.h
@@ -17,25 +17,22 @@
  public:
   static std::unique_ptr<CFX_CSSSelector> FromString(WideStringView str);
 
-  CFX_CSSSelector(CFX_CSSSelectorType eType,
-                  const wchar_t* psz,
-                  int32_t iLen,
-                  bool bIgnoreCase);
+  CFX_CSSSelector(const wchar_t* psz, int32_t iLen);
   ~CFX_CSSSelector();
 
-  CFX_CSSSelectorType GetType() const;
-  uint32_t GetNameHash() const;
-  CFX_CSSSelector* GetNextSelector() const;
+  CFX_CSSSelectorType GetType() const { return m_eType; }
+  uint32_t GetNameHash() const { return m_dwHash; }
+  const CFX_CSSSelector* GetNextSelector() const { return m_pNext.get(); }
 
   void SetNext(std::unique_ptr<CFX_CSSSelector> pNext) {
     m_pNext = std::move(pNext);
   }
 
  private:
-  void SetType(CFX_CSSSelectorType eType) { m_eType = eType; }
+  void SetDescendentType() { m_eType = CFX_CSSSelectorType::Descendant; }
 
-  CFX_CSSSelectorType m_eType;
-  uint32_t m_dwHash;
+  CFX_CSSSelectorType m_eType = CFX_CSSSelectorType::Element;
+  const uint32_t m_dwHash;
   std::unique_ptr<CFX_CSSSelector> m_pNext;
 };
 

core/fxcrt/css/cfx_cssstylesheet_unittest.cpp

diff --git a/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp b/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
index 082d2ac..90efc7d 100644
--- a/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
+++ b/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
@@ -177,7 +177,7 @@
   CFX_CSSStyleRule* style = sheet_->GetRule(0);
   EXPECT_EQ(1u, style->CountSelectorLists());
 
-  auto* sel = style->GetSelectorList(0);
+  const auto* sel = style->GetSelectorList(0);
   ASSERT_TRUE(sel);
   EXPECT_EQ(FX_HashCode_GetW(L"c", true), sel->GetNameHash());