Add correct equality testing for ComputedStyle's StyleVariableData

objects. This fixes the linked bug (large subtree style recalcs always when using variables) and prevents a crash in release-with-asserts builds that attempt to equality check the base style with a freshly computed style during animations. BUG=570176 Review URL: https://codereview.chromium.org/1537523003 Cr-Commit-Position: refs/heads/master@{#365814}
Denger-Network · Dec 17, 2015 · b6cb0b1 · b6cb0b1
1 parent 5c6bf86
commit b6cb0b1
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 3 deletions.
diff --git a/...party/WebKit/LayoutTests/fast/css/variables/no-assert-mixing-variables-and-animation.html b/...party/WebKit/LayoutTests/fast/css/variables/no-assert-mixing-variables-and-animation.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+
+<style>
+body {
+  --b: 420px;
+}
+#element {
+  --a: var(--b);
+  width: 100px;
+  height: 100px;
+  background-color: red;
+  transition: width 1s;
+}
+
+#element.triggered {
+  width: var(--a);
+}
+</style>
+
+<div id='element'></div>
+
+<script>
+var test = async_test("this test passes if it doesn't assert");
+
+requestAnimationFrame(x => {
+  element.classList.add('triggered');
+  requestAnimationFrame(x => {
+    requestAnimationFrame(x => {
+      requestAnimationFrame(x => {
+        requestAnimationFrame(x => {
+          test.done();
+        })
+      })
+    })
+  })
+});
+</script>
diff --git a/third_party/WebKit/Source/core/core.gypi b/third_party/WebKit/Source/core/core.gypi
@@ -797,6 +797,8 @@
             'style/StyleSelfAlignmentData.h',
             'style/StyleSurroundData.cpp',
             'style/StyleTransformData.cpp',
+            'style/StyleVariableData.cpp',
+            'style/StyleVariableData.h',
             'style/StyleVisualData.cpp',
             'style/StyleWillChangeData.cpp',
             'layout/svg/line/SVGInlineFlowBox.cpp',

diff --git a/third_party/WebKit/Source/core/css/CSSVariableData.cpp b/third_party/WebKit/Source/core/css/CSSVariableData.cpp
@@ -27,6 +27,11 @@ template<typename CharacterType> void CSSVariableData::updateTokens(const CSSPar
     ASSERT(currentOffset == m_backingString.getCharacters<CharacterType>() + m_backingString.length());
 }
 
+bool CSSVariableData::operator==(const CSSVariableData& other) const
+{
+    return tokens() == other.tokens();
+}
+
 void CSSVariableData::consumeAndUpdateTokens(const CSSParserTokenRange& range)
 {
     StringBuilder stringBuilder;

diff --git a/third_party/WebKit/Source/core/css/CSSVariableData.h b/third_party/WebKit/Source/core/css/CSSVariableData.h
@@ -30,7 +30,9 @@ class CSSVariableData : public RefCounted<CSSVariableData> {
 
     CSSParserTokenRange tokenRange() { return m_tokens; }
 
-    const Vector<CSSParserToken>& tokens() { return m_tokens; }
+    const Vector<CSSParserToken>& tokens() const { return m_tokens; }
+
+    bool operator==(const CSSVariableData& other) const;
 
     bool needsVariableResolution() const { return m_needsVariableResolution; }
 private:

diff --git a/third_party/WebKit/Source/core/css/parser/CSSParserToken.h b/third_party/WebKit/Source/core/css/parser/CSSParserToken.h
@@ -82,6 +82,39 @@ class CORE_EXPORT CSSParserToken {
 
     CSSParserToken(HashTokenType, CSSParserString);
 
+    bool operator==(const CSSParserToken& other) const
+    {
+        if (m_type != other.m_type)
+            return false;
+        switch (m_type) {
+        case DelimiterToken:
+            return delimiter() == other.delimiter();
+        case HashToken:
+            if (m_hashTokenType != other.m_hashTokenType)
+                return false;
+            // fallthrough
+        case IdentToken:
+        case FunctionToken:
+        case StringToken:
+        case UrlToken:
+            return m_valueDataCharRaw == other.m_valueDataCharRaw && m_valueLength == other.m_valueLength && m_valueIs8Bit == other.m_valueIs8Bit;
+        case NumberToken:
+            if (m_numericSign != other.m_numericSign)
+                return false;
+            // fallthrough
+        case DimensionToken:
+            if (m_valueDataCharRaw != other.m_valueDataCharRaw || m_valueLength != other.m_valueLength || m_valueIs8Bit != other.m_valueIs8Bit)
+                return false;
+            // fallthrough
+        case PercentageToken:
+            return m_numericValue == other.m_numericValue && m_numericValueType == other.m_numericValueType;
+        case UnicodeRangeToken:
+            return m_unicodeRange.start == other.m_unicodeRange.start && m_unicodeRange.end == other.m_unicodeRange.end;
+        default:
+            return true;
+        }
+    }
+
     // Converts NumberToken to DimensionToken.
     void convertToDimensionWithUnit(CSSParserString);
 

diff --git a/third_party/WebKit/Source/core/style/StyleRareInheritedData.cpp b/third_party/WebKit/Source/core/style/StyleRareInheritedData.cpp
@@ -223,7 +223,7 @@ bool StyleRareInheritedData::operator==(const StyleRareInheritedData& o) const
         && m_rubyPosition == o.m_rubyPosition
         && dataEquivalent(listStyleImage.get(), o.listStyleImage.get())
         && dataEquivalent(appliedTextDecorations, o.appliedTextDecorations)
-        && variables == o.variables;
+        && dataEquivalent(variables, o.variables);
 }
 
 bool StyleRareInheritedData::shadowDataEquivalent(const StyleRareInheritedData& o) const

diff --git a/third_party/WebKit/Source/core/style/StyleVariableData.cpp b/third_party/WebKit/Source/core/style/StyleVariableData.cpp
@@ -0,0 +1,27 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "core/style/StyleVariableData.h"
+
+#include "core/style/DataEquivalency.h"
+
+namespace blink {
+
+bool StyleVariableData::operator==(const StyleVariableData& other) const
+{
+    if (m_data.size() != other.m_data.size()) {
+        return false;
+    }
+
+    for (const auto& iter : m_data) {
+        RefPtr<CSSVariableData> otherData = other.m_data.get(iter.key);
+        if (!dataEquivalent(iter.value, otherData))
+            return false;
+    }
+
+    return true;
+}
+
+} // namespace blink
diff --git a/third_party/WebKit/Source/core/style/StyleVariableData.h b/third_party/WebKit/Source/core/style/StyleVariableData.h
@@ -1,3 +1,7 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
 #ifndef StyleVariableData_h
 #define StyleVariableData_h
 
@@ -14,7 +18,7 @@ class StyleVariableData : public RefCounted<StyleVariableData> {
     static PassRefPtr<StyleVariableData> create() { return adoptRef(new StyleVariableData()); }
     PassRefPtr<StyleVariableData> copy() const { return adoptRef(new StyleVariableData(*this)); }
 
-    bool operator==(const StyleVariableData& other) const { return other.m_data == m_data; }
+    bool operator==(const StyleVariableData& other) const;
     bool operator!=(const StyleVariableData& other) const { return !(*this == other); }
 
     void setVariable(const AtomicString& name, PassRefPtr<CSSVariableData> value) { m_data.set(name, value); }