Call Element::rebuildLayoutTree from Document::updateStyle directly

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 2fde2c526802721659102806ad693fc66f3f70b6..4a8342bb64b5a7ba2440fa844458c482e0820c2d 100644
--- a/third_party/WebKit/Source/core/dom/Element.cpp
+++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1903,14 +1903,14 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling) {
     }
     if (parentComputedStyle())
       change = recalcOwnStyle(change, nextTextSibling);
-    clearNeedsStyleRecalc();
-    clearNeedsReattachLayoutTree();
+    if (change != Reattach)

[nainar, 2017/02/16 08:05:57]

This was initially if (!needsReattachLayoutTree) - which is incorrect.

We need to retain this information in case we are about to reattach. Which is
change == Reattach 

[esprehn, 2017/02/16 21:24:33]

If we're going to reattach, then we did setNeedsReattachLayoutTree(), so change
== Reattach should imply needsReattachLayoutTree() ? Curious where we need this
bit still, needsAttach() already checks both.

[nainar, 2017/02/16 22:26:09]

change == Reattach does imply needsReattachLayoutTree() but not the other way
round. 
When we enter reattachLayoutTree() there is a check to prevent us doing useless
detach. If we clear the needsStyleRecalc bit that condition will always be true.
And we will be unable to skip detachLayoutTree() in cases like we have already
been through attachLayoutTree()

+      clearNeedsStyleRecalc();
   }
 
-  // If we reattached we don't need to recalc the style of our descendants
-  // anymore.
-  if ((change >= UpdatePseudoElements && change < Reattach) ||
-      childNeedsStyleRecalc()) {
+  // If we are going to reattach we don't need to recalc the style of
+  // our descendants anymore.
+  if (change < Reattach &&
+      (change >= UpdatePseudoElements || childNeedsStyleRecalc())) {
     SelectorFilterParentScope filterScope(*this);
     StyleSharingDepthScope sharingScope(*this);
 
@@ -1936,7 +1936,6 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling) {
                         childNeedsStyleRecalc() ? Force : change);
 
     clearChildNeedsStyleRecalc();
-    clearChildNeedsReattachLayoutTree();
   }
 
   if (hasCustomStyleCallbacks())
@@ -1996,7 +1995,7 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change,
     styleReattachData.nextTextSibling = nextTextSibling;
     document().addStyleReattachData(*this, styleReattachData);
     setNeedsReattachLayoutTree();
-    return rebuildLayoutTree();
+    return Reattach;
   }
 
   DCHECK(oldStyle);
@@ -2037,23 +2036,31 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change,
   return localChange;
 }
 
-StyleRecalcChange Element::rebuildLayoutTree() {
+void Element::rebuildLayoutTree() {
   DCHECK(inActiveDocument());
+  DCHECK(parentNode());
+
   StyleReattachData styleReattachData = document().getStyleReattachData(*this);
   AttachContext reattachContext;
   reattachContext.resolvedStyle = styleReattachData.computedStyle.get();
   bool layoutObjectWillChange = needsAttach() || layoutObject();
 
-  // We are calling Element::rebuildLayoutTree() from inside
-  // Element::recalcOwnStyle where we set the NeedsReattachLayoutTree
-  // flag - so needsReattachLayoutTree() should always be true.
-  DCHECK(parentNode());
-  DCHECK(parentNode()->childNeedsReattachLayoutTree());
-  DCHECK(needsReattachLayoutTree());
-  reattachLayoutTree(reattachContext);
-  // Since needsReattachLayoutTree() is always true we go into
-  // reattachLayoutTree() which reattaches all the descendant
-  // sub-trees. At this point no child should need reattaching.
+  if (needsReattachLayoutTree()) {
+    reattachLayoutTree(reattachContext);
+  } else if (childNeedsReattachLayoutTree()) {
+    DCHECK(!needsReattachLayoutTree());
+    SelectorFilterParentScope filterScope(*this);
+    StyleSharingDepthScope sharingScope(*this);
+    reattachPseudoElementLayoutTree(PseudoIdBefore);
+    rebuildShadowRootLayoutTree();
+    rebuildChildrenLayoutTrees();
+    reattachPseudoElementLayoutTree(PseudoIdAfter);
+    reattachPseudoElementLayoutTree(PseudoIdBackdrop);
+    reattachPseudoElementLayoutTree(PseudoIdFirstLetter);
+  }
+  DCHECK(!needsStyleRecalc());
+  DCHECK(!childNeedsStyleRecalc());
+  DCHECK(!needsReattachLayoutTree());
   DCHECK(!childNeedsReattachLayoutTree());
 
   if (layoutObjectWillChange || layoutObject()) {
@@ -2062,9 +2069,25 @@ StyleRecalcChange Element::rebuildLayoutTree() {
     // or store it.
     // The choice is between increased time and increased memory complexity.
     reattachWhitespaceSiblingsIfNeeded(styleReattachData.nextTextSibling);
-    return Reattach;
   }
-  return ReattachNoLayoutObject;
+}
+
+void Element::rebuildShadowRootLayoutTree() {
+  for (ShadowRoot* root = youngestShadowRoot(); root;
+       root = root->olderShadowRoot()) {
+    if (root->needsReattachLayoutTree() || root->childNeedsReattachLayoutTree())
+      root->rebuildLayoutTree();
+  }
+}
+
+void Element::reattachPseudoElementLayoutTree(PseudoId pseudoId) {
+  if (PseudoElement* element = pseudoElement(pseudoId)) {
+    if (element->needsReattachLayoutTree() ||
+        element->childNeedsReattachLayoutTree())
+      element->rebuildLayoutTree();
+  } else {
+    createPseudoElementIfNeeded(pseudoId);
+  }
 }
 
 void Element::updateCallbackSelectors(const ComputedStyle* oldStyle,