Call Element::rebuildLayoutTree from Document::updateStyle directly

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

Index: third_party/WebKit/Source/core/dom/ContainerNode.cpp
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.cpp b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
index 207659d70f65cfdf3da66036ac30f8ba15cce0d2..703bce5f908f377749cb4e820d553e6908d10c00 100644
--- a/third_party/WebKit/Source/core/dom/ContainerNode.cpp
+++ b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
@@ -788,6 +788,7 @@ void ContainerNode::detachLayoutTree(const AttachContext& context) {
     child->detachLayoutTree(childrenContext);
 
   setChildNeedsStyleRecalc();
+  setChildNeedsReattachLayoutTree();
   Node::detachLayoutTree(context);
 }
 
@@ -798,6 +799,8 @@ void ContainerNode::childrenChanged(const ChildrenChange& change) {
   if (change.isChildInsertion() && !childNeedsStyleRecalc()) {
     setChildNeedsStyleRecalc();
     markAncestorsWithChildNeedsStyleRecalc();
+    setChildNeedsReattachLayoutTree();

[esprehn, 2017/02/16 05:49:25]

You need a separate check for !childNeedsReattach... like the
!childNeedsStyleRecalc(), otherwise if this element already had kids that needed
a style recalc you'd fail to do this work.

I think you want:

if (change.isChildInsertion()) {
  if (!childNeedsStyleRecalc()) {
    // existing two lines.
  }
  if (!childNeedsReattachLayoutTree()) {
    // new two lines.
  }
}

Should be easy to write a test for that too, ex.

element.firstElementChild.style.color = "red"; // mark a child for style recalc.

element.appendChild(anotherElement); // we wouldn't do any of the code you have
here now since it's marked.

[nainar, 2017/02/16 06:47:54]

Done. Add test in this patch or in another one?

+    markAncestorsWithChildNeedsReattachLayoutTree();
   }
 }
 
@@ -1280,7 +1283,6 @@ void ContainerNode::setRestyleFlag(DynamicRestyleFlags mask) {
 void ContainerNode::recalcDescendantStyles(StyleRecalcChange change) {
   DCHECK(document().inStyleRecalc());
   DCHECK(change >= UpdatePseudoElements || childNeedsStyleRecalc());
-  DCHECK(!needsStyleRecalc());

[esprehn, 2017/02/16 05:49:25]

add this back? Recomputing the descendant styles while yourself is still dirty
is a bug.

[nainar, 2017/02/16 06:47:54]

Done. Needed to move the 

if (!needsReattachLayoutTree())
      clearNeedsStyleRecalc();

up into the scope in which we are calling recalcOwnStyle

 
   // This loop is deliberately backwards because we use insertBefore in the
   // layout tree, and want to avoid a potentially n^2 loop to find the insertion
@@ -1305,6 +1307,25 @@ void ContainerNode::recalcDescendantStyles(StyleRecalcChange change) {
   }
 }
 
+void ContainerNode::rebuildChildrenLayoutTrees() {
+  DCHECK(!needsReattachLayoutTree());
+
+  for (Node* child = lastChild(); child; child = child->previousSibling()) {
+    if (child->needsReattachLayoutTree() ||
+        child->childNeedsReattachLayoutTree()) {
+      if (child->isTextNode())
+        toText(child)->rebuildTextLayoutTree();
+      else if (child->isElementNode())
+        toElement(child)->rebuildLayoutTree();
+    }
+  }
+  clearNeedsStyleRecalc();
+  // This is done in ContainerNode::attachLayoutTree but will never be cleared
+  // if we don't enter ContainerNode::attachLayoutTree so we do it here.
+  clearChildNeedsStyleRecalc();
+  clearChildNeedsReattachLayoutTree();
+}
+
 void ContainerNode::checkForSiblingStyleChanges(SiblingCheckType changeType,
                                                 Element* changedElement,
                                                 Node* nodeBeforeChange,