Clear style recalc root for flat tree.

For FlatTreeStyleRecalc enabled, we did not walk the slot and ancestors
clearing child-dirty bits when clearing a style recalc root as part of
removing shadow host children. That would leave stray child-dirty bits.

Here, we find the child-dirty slot and start clearing child-dirty bits
from the slot and up the flat tree ancestor chain. If the slot was
removed before the shadow host child, fall back to use the light tree
parent of the removed nodes as the new style recalc root instead.

Fall back to use the light tree parent for Shadow DOM V0 as trying to
find the flat tree parent to start from is too complicated.

TEST=shadow-dom/crashes/cyclic-detach-crash2.html

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

third_party/blink/renderer/core/css/layout_tree_rebuild_root.cc

@@ -48,8 +48,7 @@ bool LayoutTreeRebuildRoot::IsDirty(const Node& node) const {
   return node.NeedsReattachLayoutTree();
 }
 
-void LayoutTreeRebuildRoot::ClearChildDirtyForAncestors(
-    ContainerNode& parent) const {
+void LayoutTreeRebuildRoot::RootRemoved(ContainerNode& parent) {
   Element* ancestor = DynamicTo<Element>(parent);
   if (!ancestor)
     ancestor = parent.ParentOrShadowHostElement();
@@ -58,6 +57,7 @@ void LayoutTreeRebuildRoot::ClearChildDirtyForAncestors(
     DCHECK(!ancestor->NeedsReattachLayoutTree());
     ancestor->ClearChildNeedsReattachLayoutTree();
   }
+  Clear();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/layout_tree_rebuild_root.h

@@ -21,7 +21,7 @@ class CORE_EXPORT LayoutTreeRebuildRoot : public StyleTraversalRoot {
   bool IsChildDirty(const ContainerNode& node) const final;
 #endif  // DCHECK_IS_ON()
   bool IsDirty(const Node& node) const final;
-  void ClearChildDirtyForAncestors(ContainerNode& parent) const final;
+  void RootRemoved(ContainerNode& parent) final;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_invalidation_root.cc

@@ -33,14 +33,14 @@ bool StyleInvalidationRoot::IsDirty(const Node& node) const {
   return node.NeedsStyleInvalidation();
 }
 
-void StyleInvalidationRoot::ClearChildDirtyForAncestors(
-    ContainerNode& parent) const {
+void StyleInvalidationRoot::RootRemoved(ContainerNode& parent) {
   for (Node* ancestor = &parent; ancestor;
        ancestor = ancestor->ParentOrShadowHostNode()) {
     DCHECK(ancestor->ChildNeedsStyleInvalidation());
     DCHECK(!ancestor->NeedsStyleInvalidation());
     ancestor->ClearChildNeedsStyleInvalidation();
   }
+  Clear();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_invalidation_root.h

@@ -21,7 +21,7 @@ class CORE_EXPORT StyleInvalidationRoot : public StyleTraversalRoot {
   bool IsChildDirty(const ContainerNode& node) const final;
 #endif  // DCHECK_IS_ON()
   bool IsDirty(const Node& node) const final;
-  void ClearChildDirtyForAncestors(ContainerNode& parent) const final;
+  void RootRemoved(ContainerNode& parent) final;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_recalc_root.cc

@@ -6,6 +6,8 @@
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/shadow_root.h"
+#include "third_party/blink/renderer/core/dom/slot_assignment.h"
+#include "third_party/blink/renderer/core/html/html_slot_element.h"
 
 namespace blink {
 
@@ -48,17 +50,64 @@ bool StyleRecalcRoot::IsDirty(const Node& node) const {
   return node.IsDirtyForStyleRecalc();
 }
 
-void StyleRecalcRoot::ClearChildDirtyForAncestors(ContainerNode& parent) const {
+namespace {
+
+base::Optional<Member<Element>> FirstFlatTreeAncestorForChildDirty(
+    ContainerNode& parent) {
+  DCHECK(RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled());
+  if (!parent.IsElementNode()) {
+    // The flat tree does not contain shadow roots or the document node. The
+    // closest ancestor for dirty bits is the shadow host or nullptr.
+    return parent.ParentOrShadowHostElement();
+  }
+  ShadowRoot* root = parent.GetShadowRoot();
+  if (!root)
+    return To<Element>(&parent);
+  if (root->IsV0()) {
+    // Fall back to use the parent as the new style recalc root for Shadow DOM
+    // V0. It is too complicated to try to find the closest flat tree parent.
+    return base::nullopt;
+  }
+  // The child has already been removed, so we cannot look up its slot
+  // assignment directly. Find the slot which was part of the ancestor chain
+  // before the removal by checking the child-dirty bits. Since the recalc root
+  // was removed, there is at most one such child-dirty slot.
+  for (const auto slot : root->GetSlotAssignment().Slots()) {
+    if (slot->ChildNeedsStyleRecalc())
+      return slot;
+  }
+  // The slot has also been removed. Fall back to using the light tree parent as
+  // the new recalc root.
+  return base::nullopt;
+}
+
+}  // namespace
+
+void StyleRecalcRoot::RootRemoved(ContainerNode& parent) {
   ContainerNode* ancestor = &parent;
-  if (RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled() &&
-      !parent.IsElementNode()) {
-    ancestor = parent.ParentOrShadowHostElement();
+  if (RuntimeEnabledFeatures::FlatTreeStyleRecalcEnabled()) {
+    // We are notified with the light tree parent of the node(s) which were
+    // removed from the DOM. If 'parent' is a shadow host, there are elements in
+    // its shadow tree which are marked child-dirty which needs to be cleared in
+    // order to clear the recalc root below. If we are not able to find the
+    // closest flat tree ancestor for traversal, fall back to using the 'parent'
+    // as the new recalc root to allow the child-dirty bits to be cleared on the
+    // next style recalc.
+    auto opt_ancestor = FirstFlatTreeAncestorForChildDirty(parent);
+    if (!opt_ancestor) {
+      Update(&parent, &parent);
+      DCHECK(!IsSingleRoot());
+      DCHECK_EQ(GetRootNode(), ancestor);
+      return;
+    }
+    ancestor = opt_ancestor.value();
   }
   for (; ancestor; ancestor = ancestor->GetStyleRecalcParent()) {
     DCHECK(ancestor->ChildNeedsStyleRecalc());
     DCHECK(!ancestor->NeedsStyleRecalc());
     ancestor->ClearChildNeedsStyleRecalc();
   }
+  Clear();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_recalc_root.h

@@ -21,7 +21,7 @@ class CORE_EXPORT StyleRecalcRoot : public StyleTraversalRoot {
   bool IsChildDirty(const ContainerNode& node) const final;
 #endif  // DCHECK_IS_ON()
   bool IsDirty(const Node& node) const final;
-  void ClearChildDirtyForAncestors(ContainerNode& parent) const final;
+  void RootRemoved(ContainerNode& parent) final;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_traversal_root.cc

@@ -53,10 +53,8 @@ void StyleTraversalRoot::Update(ContainerNode* common_ancestor,
 }
 
 void StyleTraversalRoot::ChildrenRemoved(ContainerNode& parent) {
-  if (!root_node_ || root_node_->isConnected())
-    return;
-  ClearChildDirtyForAncestors(parent);
-  Clear();
+  if (root_node_ && !root_node_->isConnected())
+    RootRemoved(parent);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/style_traversal_root.h

@@ -63,8 +63,8 @@ class CORE_EXPORT StyleTraversalRoot {
   // Return true if the given node is dirty.
   virtual bool IsDirty(const Node&) const = 0;
 
-  // Clear the child-dirty flag on the ancestors of the given node.
-  virtual void ClearChildDirtyForAncestors(ContainerNode& parent) const = 0;
+  // Update the root node when removed.
+  virtual void RootRemoved(ContainerNode& parent) = 0;
 
   bool IsSingleRoot() const { return root_type_ == RootType::kSingleRoot; }
 

third_party/blink/renderer/core/css/style_traversal_root_test.cc

@@ -38,11 +38,12 @@ class StyleTraversalRootTestImpl : public StyleTraversalRoot {
   bool IsDirty(const Node& node) const final {
     return dirty_nodes_.Contains(&node);
   }
-  void ClearChildDirtyForAncestors(ContainerNode& parent) const override {
+  void RootRemoved(ContainerNode& parent) override {
     for (ContainerNode* ancestor = &parent; ancestor;
          ancestor = ParentInternal(*ancestor)) {
       child_dirty_nodes_.erase(ancestor);
     }
+    Clear();
   }
 
   HeapHashSet<Member<const Node>> dirty_nodes_;

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

@@ -431,4 +431,75 @@ TEST_F(NodeTest, UpdateChildDirtyAncestorsOnSlotAssignment) {
   EXPECT_TRUE(ancestor->ChildNeedsStyleRecalc());
 }
 
+TEST_F(NodeTest, UpdateChildDirtySlotAfterRemoval) {
+  ScopedFlatTreeStyleRecalcForTest scope(true);
+
+  SetBodyContent("<div id=host><span></span></div>");
+  Element* host = GetDocument().getElementById("host");
+  ShadowRoot& shadow_root =
+      host->AttachShadowRootInternal(ShadowRootType::kOpen);
+  shadow_root.SetInnerHTMLFromString("<slot></slot>");
+  UpdateAllLifecyclePhasesForTest();
+
+  auto* span = To<Element>(host->firstChild());
+  auto* slot = shadow_root.firstChild();
+
+  // Make sure the span is dirty, and the slot marked child-dirty before the
+  // removal.
+  span->setAttribute("style", "color:green");
+  EXPECT_TRUE(span->NeedsStyleRecalc());
+  EXPECT_TRUE(slot->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(host->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().body()->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().GetStyleEngine().NeedsStyleRecalc());
+
+  // The StyleRecalcRoot is now the span. Removing the span should clear the
+  // root and the child-dirty bits on the ancestors.
+  span->remove();
+
+  EXPECT_FALSE(slot->ChildNeedsStyleRecalc());
+  EXPECT_FALSE(host->ChildNeedsStyleRecalc());
+  EXPECT_FALSE(GetDocument().body()->ChildNeedsStyleRecalc());
+  EXPECT_FALSE(GetDocument().GetStyleEngine().NeedsStyleRecalc());
+}
+
+TEST_F(NodeTest, UpdateChildDirtyAfterSlotRemoval) {
+  ScopedFlatTreeStyleRecalcForTest scope(true);
+
+  SetBodyContent("<div id=host><span></span></div>");
+  Element* host = GetDocument().getElementById("host");
+  ShadowRoot& shadow_root =
+      host->AttachShadowRootInternal(ShadowRootType::kOpen);
+  shadow_root.SetInnerHTMLFromString("<div><slot></slot></div>");
+  UpdateAllLifecyclePhasesForTest();
+
+  auto* span = To<Element>(host->firstChild());
+  auto* div = shadow_root.firstChild();
+  auto* slot = div->firstChild();
+
+  // Make sure the span is dirty, and the slot marked child-dirty before the
+  // removal.
+  span->setAttribute("style", "color:green");
+  EXPECT_TRUE(span->NeedsStyleRecalc());
+  EXPECT_TRUE(slot->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(div->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(host->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().body()->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().GetStyleEngine().NeedsStyleRecalc());
+
+  // The StyleRecalcRoot is now the span. Removing the slot would break the flat
+  // tree ancestor chain so that when removing the span we would no longer be
+  // able to clear the dirty bits for all of the previous ancestor chain. Thus,
+  // we fall back to use the host as the style recalc root to be able to
+  // traverse and clear the dirty bit of the shadow tree div element on the next
+  // style recalc.
+  slot->remove();
+  span->remove();
+
+  EXPECT_TRUE(div->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(host->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().body()->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(GetDocument().GetStyleEngine().NeedsStyleRecalc());
+}
+
 }  // namespace blink