Skip marking style dirty for more cases.

SetStyleChangeOnInsertion() did not skip marking nodes style dirty
like we did for SetNeedsStyleRecalc(). Create a common method to check
that.

Also, skip marking text nodes for re-attachment. We already skipped
marking elements and in display:none in SetForceReattachLayoutTree(),
but not text nodes. When changing text data on text nodes we force a
re-attach from Text::UpdateTextLayoutObject() is case we change from
non-rendered white-space to something that is rendered.

The two tests below would fail with FlatTreeStyleRecalc enabled
without this change.

TEST=fast/dom/shadow/adopt-node-with-shadow-root.html
TEST=shadow-dom/slots-2.html

Bug: 972752
Change-Id: Ifbad22725cf4316068d0ddd20e561ecb56266607
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901032
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712994}

third_party/blink/renderer/core/dom/node.cc

@@ -1253,6 +1253,29 @@ class AllowDirtyShadowV0TraversalScope {
 #define ALLOW_DIRTY_SHADOW_V0_TRAVERSAL_SCOPE(document)
 #endif  // DCHECK_IS_ON()
 
+bool Node::ShouldSkipMarkingStyleDirty() const {
+  if (GetComputedStyle())
+    return false;
+
+  ALLOW_DIRTY_SHADOW_V0_TRAVERSAL_SCOPE(GetDocument());
+
+  // If we don't have a computed style, and our parent element does not have a
+  // computed style it's not necessary to mark this node for style recalc.
+  if (auto* parent = GetStyleRecalcParent()) {
+    while (parent && !parent->CanParticipateInFlatTree())
+      parent = parent->GetStyleRecalcParent();
+    return !parent || !parent->GetComputedStyle();
+  }
+  if (!RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled())
+    return false;
+  // If this is the root element, and it does not have a computed style, we
+  // still need to mark it for style recalc since it may change from
+  // display:none. Otherwise, the node is not in the flat tree, and we can
+  // skip marking it dirty.
+  auto* root_element = GetDocument().documentElement();
+  return root_element && root_element != this;
+}
+
 void Node::MarkAncestorsWithChildNeedsStyleRecalc() {
   ALLOW_DIRTY_SHADOW_V0_TRAVERSAL_SCOPE(GetDocument());
   ContainerNode* ancestor = GetStyleRecalcParent();
@@ -1308,6 +1331,7 @@ Element* Node::FlatTreeParentForChildDirty() const {
   if (IsChildOfV1ShadowHost()) {
     if (auto* data = GetFlatTreeNodeData())
       return data->AssignedSlot();
+    return nullptr;
   }
   if (IsInV0ShadowTree() || IsChildOfV0ShadowHost()) {
     if (ShadowRootWhereNodeCanBeDistributedForV0(*this)) {
@@ -1366,25 +1390,8 @@ void Node::SetNeedsStyleRecalc(StyleChangeType change_type,
 
   if (!InActiveDocument())
     return;
-  if (!GetComputedStyle()) {
-    ALLOW_DIRTY_SHADOW_V0_TRAVERSAL_SCOPE(GetDocument());
-    // If we don't have a computed style, and our parent element does not have a
-    // computed style it's not necessary to mark this node for style recalc.
-    if (auto* parent = GetStyleRecalcParent()) {
-      while (parent && !parent->CanParticipateInFlatTree())
-        parent = parent->GetStyleRecalcParent();
-      DCHECK(parent);
-      if (!parent->GetComputedStyle())
-        return;
-    } else if (RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled() &&
-               GetDocument().documentElement() != this) {
-      // If this is the root element, and it does not have a computed style, we
-      // still need to mark it for style recalc since it may change from
-      // display:none. Otherwise, the node is not in the flat tree, and we can
-      // return here.
-      return;
-    }
-  }
+  if (ShouldSkipMarkingStyleDirty())
+    return;
 
   TRACE_EVENT_INSTANT1(
       TRACE_DISABLED_BY_DEFAULT("devtools.timeline.invalidationTracking"),
@@ -1643,9 +1650,15 @@ void Node::SetForceReattachLayoutTree() {
     return;
   if (!InActiveDocument())
     return;
-  if (IsElementNode() && !GetComputedStyle()) {
-    DCHECK(!GetLayoutObject());
-    return;
+  if (IsElementNode()) {
+    if (!GetComputedStyle()) {
+      DCHECK(!GetLayoutObject());
+      return;
+    }
+  } else {
+    DCHECK(IsTextNode());
+    if (!GetLayoutObject() && ShouldSkipMarkingStyleDirty())
+      return;
   }
   SetFlag(kForceReattachLayoutTree);
   if (!NeedsStyleRecalc()) {

third_party/blink/renderer/core/dom/node.h

@@ -463,6 +463,8 @@ class CORE_EXPORT Node : public EventTarget {
   // a micro-benchmark regression (https://crbug.com/926343).
   void SetStyleChangeOnInsertion() {
     DCHECK(isConnected());
+    if (ShouldSkipMarkingStyleDirty())
+      return;
     if (!NeedsStyleRecalc())
       SetStyleChange(kLocalStyleChange);
     MarkAncestorsWithChildNeedsStyleRecalc();
@@ -716,6 +718,7 @@ class CORE_EXPORT Node : public EventTarget {
   const ComputedStyle* GetComputedStyle() const;
   const ComputedStyle* ParentComputedStyle() const;
   const ComputedStyle& ComputedStyleRef() const;
+  bool ShouldSkipMarkingStyleDirty() const;
 
   const ComputedStyle* EnsureComputedStyle(
       PseudoId pseudo_element_specifier = kPseudoIdNone) {

third_party/blink/renderer/core/dom/node_test.cc

@@ -341,7 +341,8 @@ TEST_F(NodeTest, MutationOutsideFlatTreeStyleDirty) {
   GetDocument()
       .getElementById("nonslotted")
       ->setAttribute("style", "color:green");
-  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
+  EXPECT_EQ(!RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled(),
+            GetDocument().NeedsLayoutTreeUpdate());
 }
 
 TEST_F(NodeTest, SkipStyleDirtyHostChild) {