Add a fast-path for independent inherited properties

third_party/WebKit/Source/core/dom/Element.cpp

Index: third_party/WebKit/Source/core/dom/Element.cpp
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp
index 1a0e0246e0fa4a97e6dcedc9ff84f8415984e545..c6297a0a6d29138cce8f0a7a944613a363660884 100644
--- a/third_party/WebKit/Source/core/dom/Element.cpp
+++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1708,12 +1708,13 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling)
     if (hasCustomStyleCallbacks())
         willRecalcStyle(change);
 
-    if (change >= Inherit || needsStyleRecalc()) {
+    if (change >= IndependentInherit || needsStyleRecalc()) {
         if (hasRareData()) {
             ElementRareData* data = elementRareData();
-            data->clearComputedStyle();
+            if (change != IndependentInherit)
+                data->clearComputedStyle();
 
-            if (change >= Inherit) {
+            if (change >= IndependentInherit) {
                 if (ElementAnimations* elementAnimations = data->elementAnimations())
                     elementAnimations->setAnimationStyleChange(false);
             }
@@ -1757,15 +1758,37 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling)
         reattachWhitespaceSiblingsIfNeeded(nextTextSibling);
 }
 
+bool Element::canPropagateInheritedProperties(StyleRecalcChange change)
+{
+    return change == IndependentInherit

[esprehn, 2016/07/19 03:46:26]

if (change != ...)
  return false;
if (!parentComputedStyle())
  return false;
if (hasAnimations())
  return false;
ComputedStyle* style = ...;
return !style->animations() && !style->transitions();

That style is easier to read and doesn't recompute the same stuff repeatedly. :)

[sashab, 2016/07/19 06:32:38]

Nice :) Thanks, done

+        && !needsStyleRecalc()
+        && computedStyle()

[esprehn, 2016/07/19 03:46:26]

computedStyle() contains a bunch of branches, you don't want to call it
repeatedly like this

[sashab, 2016/07/19 06:32:38]

Yup nice, thanks

+        && parentComputedStyle()
+        && !computedStyle()->animations()
+        && !computedStyle()->transitions()
+        && !hasAnimations();
+}
+
 StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change)
 {
     DCHECK(document().inStyleRecalc());
     DCHECK(!parentOrShadowHostNode()->needsStyleRecalc());
-    DCHECK(change >= Inherit || needsStyleRecalc());
+    DCHECK(change >= IndependentInherit || needsStyleRecalc());
     DCHECK(parentComputedStyle());
 
     RefPtr<ComputedStyle> oldStyle = mutableComputedStyle();
-    RefPtr<ComputedStyle> newStyle = styleForLayoutObject();
+    RefPtr<ComputedStyle> newStyle;
+
+    // When propagating inherited changes, we don't need to do a full style recalc
+    // if the only changed properties are independent. In this case, we can simply
+    // set these directly on the ComputedStyle object.
+    if (canPropagateInheritedProperties(change)) {
+        newStyle = ComputedStyle::clone(*computedStyle());

[Timothy Loh, 2016/07/19 01:29:32]

clone(oldStyle)?

[sashab, 2016/07/19 03:31:35]

Done

[esprehn, 2016/07/19 03:46:26]

yeah, don't call computedStyle() repeatedly it's got a bunch of branches in it
:)

+        ComputedStyle::propagateIndependentInheritedProperties(*parentComputedStyle(), *newStyle);

[esprehn, 2016/07/19 03:46:26]

parentComputedStyle() isn't free, I think you want to merge this and the
canPropagateInheritedProperties into a function like:

bool propagateInheritedProperties(change)

that has the early returns and avoids calling it multiple times.

then do:

if (!propagateInheritedProperties(change))
  ...

[sashab, 2016/07/19 06:32:38]

Nice!! Done.

Had to pass newStyle because its used later in the function. Could technically
get rid of the double-call from oldStyle = mutableComputedStyle() but probably
not worth it. :)

+        INCREMENT_STYLE_STATS_COUNTER(document().styleEngine(), independentInheritedStylesPropagated, 1);
+    } else {
+        newStyle = styleForLayoutObject();
+    }
     DCHECK(newStyle);
 
     StyleRecalcChange localChange = ComputedStyle::stylePropagationDiff(oldStyle.get(), newStyle.get());

[esprehn, 2016/07/19 03:46:26]

do we even need to run any of this now if we took the propagation path above?

[sashab, 2016/07/19 06:32:38]

That's a good question. I've left it this way now so that:
1) The stats counter gets incremented
2) Callback selectors get updated (what is that even?)

but that's it. The rest is skipped. If you think neither of those things are
worth it then we can early return.

@@ -1805,7 +1828,7 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change)
     if (change > Inherit || localChange > Inherit)
         return max(localChange, change);
 
-    if (localChange < Inherit) {
+    if (localChange < IndependentInherit) {
         if (oldStyle->hasChildDependentFlags()) {
             if (childNeedsStyleRecalc())
                 return Inherit;