Changeset 132516 in webkit

Timestamp:

Oct 25, 2012, 12:34:56 PM (12 years ago)

Author:

Antti Koivisto

Message:

Avoid unnecessary style recalcs on id attribute mutation.
​https://bugs.webkit.org/show_bug.cgi?id=100395

Reviewed by Andreas Kling.

There is no need to invalidate element style on id attribute change if neither the old nor the new id were
mentioned in any stylesheet. This is similar to the optimization we already have for class attributes.

Recalculating element style is expensive. It seems id attribute mutation is often used in scripts for purposes other than styling.

• css/StyleResolver.cpp:

(WebCore::StyleResolver::hasSelectorForId):
(WebCore):

• css/StyleResolver.h:
• dom/Element.cpp:

(WebCore::makeIdForStyleResolution):
(WebCore):
(WebCore::Element::attributeChanged):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/StyleResolver.cpp (modified) (1 diff)
• css/StyleResolver.h (modified) (1 diff)
• dom/Element.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-25  Antti Koivisto  <antti@apple.com>
+
+        Avoid unnecessary style recalcs on id attribute mutation.
+        https://bugs.webkit.org/show_bug.cgi?id=100395
+
+        Reviewed by Andreas Kling.
+
+        There is no need to invalidate element style on id attribute change if neither the old nor the new id were 
+        mentioned in any stylesheet. This is similar to the optimization we already have for class attributes.
+        
+        Recalculating element style is expensive. It seems id attribute mutation is often used in scripts for purposes other than styling. 
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::hasSelectorForId):
+        (WebCore):
+        * css/StyleResolver.h:
+        * dom/Element.cpp:
+        (WebCore::makeIdForStyleResolution):
+        (WebCore):
+        (WebCore::Element::attributeChanged):
+
 2012-10-25  Dominic Mazzoni  <dmazzoni@google.com>
 

trunk/Source/WebCore/css/StyleResolver.cpp

@@ -4241 +4241 @@
 }
 
+bool StyleResolver::hasSelectorForId(const AtomicString& idValue) const
+{
+    if (idValue.isEmpty())
+        return false;
+    return m_features.idsInRules.contains(idValue.impl());
+}
+
 void StyleResolver::addViewportDependentMediaQueryResult(const MediaQueryExp* expr, bool result)
 {

trunk/Source/WebCore/css/StyleResolver.h

@@ -219 +219 @@
     Color colorFromPrimitiveValue(CSSPrimitiveValue*, bool forVisitedLink = false) const;
 
+    bool hasSelectorForId(const AtomicString&) const;
     bool hasSelectorForAttribute(const AtomicString&) const;
 

trunk/Source/WebCore/dom/Element.cpp

@@ -696 +696 @@
 }
 
+static inline AtomicString makeIdForStyleResolution(const AtomicString& value, bool inQuirksMode)
+{
+    if (inQuirksMode)
+        return value.lower();
+    return value;
+}
+
 void Element::attributeChanged(const QualifiedName& name, const AtomicString& newValue)
 {
@@ -703 +710 @@
 
     if (isIdAttributeName(name)) {
-        if (newValue != attributeData()->idForStyleResolution()) {
-            if (newValue.isNull())
-                attributeData()->setIdForStyleResolution(nullAtom);
-            else if (document()->inQuirksMode())
-                attributeData()->setIdForStyleResolution(newValue.lower());
-            else
-                attributeData()->setIdForStyleResolution(newValue);
-            setNeedsStyleRecalc();
+        AtomicString oldId = attributeData()->idForStyleResolution();
+        AtomicString newId = makeIdForStyleResolution(newValue, document()->inQuirksMode());
+        if (newId != oldId) {
+            attributeData()->setIdForStyleResolution(newId);
+            StyleResolver* styleResolver = document()->styleResolverIfExists();
+            if (attached() && (!styleResolver || (styleResolver->hasSelectorForId(newId) || styleResolver->hasSelectorForId(oldId))))
+                setNeedsStyleRecalc();
         }
     } else if (name == HTMLNames::nameAttr)