Refactor StyleVariableData to fallback on root map for performance reasons.

third_party/WebKit/Source/core/style/StyleVariableData.h

Index: third_party/WebKit/Source/core/style/StyleVariableData.h
diff --git a/third_party/WebKit/Source/core/style/StyleVariableData.h b/third_party/WebKit/Source/core/style/StyleVariableData.h
index 98d465517ebf3485d3c35a9dd209ea5687c0cbda..a86001729dd07dd942b1d8f825c91791c5254698 100644
--- a/third_party/WebKit/Source/core/style/StyleVariableData.h
+++ b/third_party/WebKit/Source/core/style/StyleVariableData.h
@@ -16,25 +16,40 @@ namespace blink {
 class StyleVariableData : public RefCounted<StyleVariableData> {
 public:
     static PassRefPtr<StyleVariableData> create() { return adoptRef(new StyleVariableData()); }
-    PassRefPtr<StyleVariableData> copy() const { return adoptRef(new StyleVariableData(*this)); }
+    PassRefPtr<StyleVariableData> copy();
 
     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); }
-    CSSVariableData* getVariable(const AtomicString& name) const { return m_data.get(name); }
-    void removeVariable(const AtomicString& name) { return m_data.remove(name); }
+    CSSVariableData* getVariable(const AtomicString& name) const;
+    void removeVariable(const AtomicString& name) { return setVariable(name, nullptr); }
 
     // This map will contain null pointers if variables are invalid due to
     // cycles or referencing invalid variables without using a fallback.
-    const HashMap<AtomicString, RefPtr<CSSVariableData>>* getVariables() const { return &m_data; }
+    // Note that this method is slow as a new map is sometimes constructed.

[Timothy Loh, 2016/04/05 03:13:37]

Isn't this always making a new map?

[shans, 2016/04/08 00:55:49]

Uh .. yeah.

+    PassOwnPtr<HashMap<AtomicString, RefPtr<CSSVariableData>>> getVariables() const

[Timothy Loh, 2016/04/05 03:13:37]

This function is too big to be in the header, move it to the .cpp file

[shans, 2016/04/08 00:55:49]

Done.

+    {
+        HashMap<AtomicString, RefPtr<CSSVariableData>>* result;
+        if (m_root) {
+            result = new HashMap<AtomicString, RefPtr<CSSVariableData>>(m_root->m_data);
+            for (auto it = m_data.begin(); it != m_data.end(); ++it)
+                result->set(it->key, it->value);
+        } else {
+            result = new HashMap<AtomicString, RefPtr<CSSVariableData>>(m_data);
+        }
+        return adoptPtr(result);
+    }
 private:
-    StyleVariableData() = default;
-    StyleVariableData(const StyleVariableData& other) : RefCounted<StyleVariableData>(), m_data(other.m_data) { }
+    StyleVariableData()

[Timothy Loh, 2016/04/05 03:13:37]

this probably didn't need to change, RefPtr's default ctor initializes to
nullptr anyway

[shans, 2016/04/08 00:55:49]

I changed this because m_root wasn't being initialized (and was causing a bunch
of the layout tests to crash).

[Timothy Loh, 2016/04/08 03:42:06]

:-|

+        : m_root(nullptr)
+        { }
+    StyleVariableData(StyleVariableData& other);
 
     friend class CSSVariableResolver;
 
     HashMap<AtomicString, RefPtr<CSSVariableData>> m_data;
+    RefPtr<StyleVariableData> m_root;
 };
 
 } // namespace blink