[Squad] Remove special code path for reattach recalc.

Do style recalc for re-attachment in the RecalcStyle machinery.

Change-Id: Ic16542e1914fbaf0fa4ed743e67762f577a3f320
Reviewed-on: https://chromium-review.googlesource.com/1098968
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567614}

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

@@ -1417,4 +1417,26 @@ TEST_F(StyleEngineTest, MediaQueriesChangeDefaultFontSize) {
                 GetCSSPropertyColor()));
 }
 
+TEST_F(StyleEngineTest, ShadowRootStyleRecalcCrash) {
+  GetDocument().body()->SetInnerHTMLFromString("<div id=host></div>");
+  HTMLElement* host = ToHTMLElement(GetDocument().getElementById("host"));
+  ASSERT_TRUE(host);
+
+  ShadowRoot& shadow_root =
+      host->AttachShadowRootInternal(ShadowRootType::kOpen);
+
+  shadow_root.SetInnerHTMLFromString(R"HTML(
+    <span id=span></span>
+    <style>
+      :nth-child(odd) { color: green }
+    </style>
+  )HTML");
+  GetDocument().View()->UpdateAllLifecyclePhases();
+
+  // This should not cause DCHECK errors on style recalc flags.
+  shadow_root.getElementById("span")->remove();
+  host->SetInlineStyleProperty(CSSPropertyDisplay, "inline");
+  GetDocument().View()->UpdateAllLifecyclePhases();
+}
+
 }  // namespace blink

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

@@ -1354,13 +1354,6 @@ void ContainerNode::RecalcDescendantStyles(StyleRecalcChange change) {
   }
 }
 
-void ContainerNode::RecalcDescendantStylesForReattach() {
-  for (Node* child = lastChild(); child; child = child->previousSibling()) {
-    if (child->IsElementNode() && !child->NeedsReattachLayoutTree())
-      ToElement(child)->RecalcStyleForReattach();
-  }
-}
-
 void ContainerNode::RebuildLayoutTreeForChild(
     Node* child,
     WhitespaceAttacher& whitespace_attacher) {

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

@@ -289,7 +289,6 @@ class CORE_EXPORT ContainerNode : public Node {
                                    Node* node_before_change,
                                    Node* node_after_change);
   void RecalcDescendantStyles(StyleRecalcChange);
-  void RecalcDescendantStylesForReattach();
   void RebuildChildrenLayoutTrees(WhitespaceAttacher&);
   void RebuildLayoutTreeForChild(Node* child, WhitespaceAttacher&);
   void RebuildNonDistributedChildren();

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

@@ -2120,6 +2120,18 @@ scoped_refptr<ComputedStyle> Element::OriginalStyleForLayoutObject() {
   return GetDocument().EnsureStyleResolver().StyleForElement(this);
 }
 
+bool Element::ShouldCallRecalcStyleForChildren(StyleRecalcChange change) {
+  if (change != kReattach)
+    return change >= kUpdatePseudoElements || ChildNeedsStyleRecalc();
+  if (!ChildrenCanHaveStyle())
+    return false;
+  if (const ComputedStyle* new_style = GetNonAttachedStyle()) {
+    return LayoutObjectIsNeeded(*new_style) ||
+           ShouldStoreNonLayoutObjectComputedStyle(*new_style);
+  }
+  return !CanParticipateInFlatTree();
+}
+
 void Element::RecalcStyle(StyleRecalcChange change) {
   DCHECK(GetDocument().InStyleRecalc());
   DCHECK(!GetDocument().Lifecycle().InDetach());
@@ -2161,29 +2173,27 @@ void Element::RecalcStyle(StyleRecalcChange change) {
           element_animations->SetAnimationStyleChange(false);
       }
     }
+
     if (ParentComputedStyle()) {
       change = RecalcOwnStyle(change);
-    } else if (NeedsAttach()) {
-      if (!CanParticipateInFlatTree()) {
-        // Recalculate style for reattachment of Shadow DOM v0 <content>
-        // fallback.
-        RecalcShadowIncludingDescendantStylesForReattach();
-      }
-      SetNeedsReattachLayoutTree();
-      change = kReattach;
+    } else if (!CanParticipateInFlatTree()) {
+      // Recalculate style for Shadow DOM v0 <content> insertion point.
+      // It does not take style since it's not part of the flat tree, but we
+      // need to traverse into fallback children for reattach.
+      if (NeedsAttach())
+        change = kReattach;
+      if (change == kReattach)
+        SetNeedsReattachLayoutTree();
     }
 
-    // Needed because the rebuildLayoutTree code needs to see what the
-    // styleChangeType() was on reattach roots. See Node::reattachLayoutTree()
+    // Needed because the RebuildLayoutTree code needs to see what the
+    // StyleChangeType() was on reattach roots. See Node::ReattachLayoutTree()
     // for an example.
     if (change != kReattach)
       ClearNeedsStyleRecalc();
   }
 
-  // If we are going to reattach we don't need to recalc the style of
-  // our descendants anymore.
-  if (change < kReattach &&
-      (change >= kUpdatePseudoElements || ChildNeedsStyleRecalc())) {
+  if (ShouldCallRecalcStyleForChildren(change)) {
     SelectorFilterParentScope filter_scope(*this);
 
     UpdatePseudoElement(kPseudoIdBefore, change);
@@ -2277,13 +2287,9 @@ StyleRecalcChange Element::RecalcOwnStyle(StyleRecalcChange change) {
     }
   }
 
-  if (local_change == kReattach) {
+  if (change == kReattach || local_change == kReattach) {
     SetNonAttachedStyle(new_style);
     SetNeedsReattachLayoutTree();
-    if (LayoutObjectIsNeeded(*new_style) ||
-        ShouldStoreNonLayoutObjectComputedStyle(*new_style)) {
-      RecalcShadowIncludingDescendantStylesForReattach();
-    }
     return kReattach;
   }
 
@@ -2329,48 +2335,6 @@ StyleRecalcChange Element::RecalcOwnStyle(StyleRecalcChange change) {
   return local_change;
 }
 
-void Element::RecalcStyleForReattach() {
-  DCHECK(!GetNonAttachedStyle());
-  if (HasCustomStyleCallbacks())
-    WillRecalcStyle(kReattach);
-
-  bool recalc_descendants = false;
-  if (ParentComputedStyle()) {
-    scoped_refptr<ComputedStyle> non_attached_style = StyleForLayoutObject();
-    SetNeedsReattachLayoutTree();
-    SetNonAttachedStyle(non_attached_style);
-    recalc_descendants =
-        LayoutObjectIsNeeded(*non_attached_style) ||
-        ShouldStoreNonLayoutObjectComputedStyle(*non_attached_style);
-  } else {
-    // Elements which cannot participate in the flat tree are <content> and
-    // <slot> if SlotInFlatTree is not enabled. Even though we should not
-    // compute their styles for re-attachment, we may need to compute their
-    // children's style if fallback is rendered.
-    recalc_descendants = !CanParticipateInFlatTree();
-  }
-  if (recalc_descendants)
-    RecalcShadowIncludingDescendantStylesForReattach();
-
-  ClearChildNeedsStyleRecalc();
-
-  if (HasCustomStyleCallbacks())
-    DidRecalcStyle(kReattach);
-}
-
-void Element::RecalcShadowIncludingDescendantStylesForReattach() {
-  if (!ChildrenCanHaveStyle())
-    return;
-  SelectorFilterParentScope filterScope(*this);
-  RecalcShadowRootStylesForReattach();
-  RecalcDescendantStylesForReattach();
-}
-
-void Element::RecalcShadowRootStylesForReattach() {
-  if (ShadowRoot* root = GetShadowRoot())
-    root->RecalcStylesForReattach();
-}
-
 void Element::RebuildLayoutTree(WhitespaceAttacher& whitespace_attacher) {
   DCHECK(InActiveDocument());
   DCHECK(parentNode());
@@ -3840,6 +3804,11 @@ void Element::CancelFocusAppearanceUpdate() {
 
 void Element::UpdatePseudoElement(PseudoId pseudo_id,
                                   StyleRecalcChange change) {
+  // TODO(futhark@chromium.org): Update pseudo elements and style as part of
+  // style recalc also when re-attaching.
+  if (change == kReattach)
+    return;
+
   DCHECK(!NeedsStyleRecalc());
   PseudoElement* element = GetPseudoElement(pseudo_id);
 

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

@@ -464,7 +464,6 @@ class CORE_EXPORT Element : public ContainerNode {
   virtual LayoutObject* CreateLayoutObject(const ComputedStyle&);
   virtual bool LayoutObjectIsNeeded(const ComputedStyle&) const;
   void RecalcStyle(StyleRecalcChange);
-  void RecalcStyleForReattach();
   bool NeedsRebuildLayoutTree(
       const WhitespaceAttacher& whitespace_attacher) const {
     // TODO(futhark@chromium.org): !CanParticipateInFlatTree() can be replaced
@@ -957,8 +956,10 @@ class CORE_EXPORT Element : public ContainerNode {
   scoped_refptr<ComputedStyle> PropagateInheritedProperties(StyleRecalcChange);
 
   StyleRecalcChange RecalcOwnStyle(StyleRecalcChange);
-  void RecalcShadowIncludingDescendantStylesForReattach();
-  void RecalcShadowRootStylesForReattach();
+
+  // Returns true if we should traverse shadow including children and pseudo
+  // elements for RecalcStyle.
+  bool ShouldCallRecalcStyleForChildren(StyleRecalcChange);
 
   void RebuildPseudoElementLayoutTree(PseudoId, WhitespaceAttacher&);
   void RebuildShadowRootLayoutTree(WhitespaceAttacher&);

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

@@ -150,25 +150,20 @@ void ShadowRoot::RecalcStyle(StyleRecalcChange change) {
   DCHECK(!HasCustomStyleCallbacks());
 
   if (GetStyleChangeType() >= kSubtreeStyleChange) {
-    change = kForce;
-    if (NeedsAttach())
+    if (change < kForce)
+      change = kForce;
+    if (NeedsAttach() || change == kReattach)
       SetNeedsReattachLayoutTree();
   }
-
-  // There's no style to update so just calling recalcStyle means we're updated.
-  ClearNeedsStyleRecalc();
+  // There's no style to update so just calling RecalcStyle means we're updated.
+  if (change != kReattach)
+    ClearNeedsStyleRecalc();
 
   if (change >= kUpdatePseudoElements || ChildNeedsStyleRecalc())
     RecalcDescendantStyles(change);
   ClearChildNeedsStyleRecalc();
 }
 
-void ShadowRoot::RecalcStylesForReattach() {
-  // ShadowRoot doesn't support custom callbacks.
-  DCHECK(!HasCustomStyleCallbacks());
-  RecalcDescendantStylesForReattach();
-}
-
 void ShadowRoot::RebuildLayoutTree(WhitespaceAttacher& whitespace_attacher) {
   ClearNeedsReattachLayoutTree();
   RebuildChildrenLayoutTrees(whitespace_attacher);

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

@@ -118,7 +118,6 @@ class CORE_EXPORT ShadowRoot final : public DocumentFragment, public TreeScope {
   unsigned ChildShadowRootCount() const { return child_shadow_root_count_; }
 
   void RecalcStyle(StyleRecalcChange);
-  void RecalcStylesForReattach();
   void RebuildLayoutTree(WhitespaceAttacher&);
 
   void RegisterScopedHTMLStyleChild();

third_party/blink/renderer/core/html/html_slot_element.cc

@@ -502,7 +502,7 @@ void HTMLSlotElement::DidRecalcStyle(StyleRecalcChange change) {
     return;
   for (auto& node : assigned_nodes_) {
     if (change == kReattach && node->IsElementNode()) {
-      ToElement(node)->RecalcStyleForReattach();
+      ToElement(node)->RecalcStyle(kReattach);
       continue;
     }
     // We only need to pick up changes for inherited style, we do not actually