Don't force re-attach for nodes without a ComputedStyle.

If the node needs a LayoutObject on the next lifecycle update, we will
trigger the re-attach going from null to non-null ComputedStyle in
RecalcStyle.

Had to remove the DisallowTransitionScope because it relied on the
lifecycle state being kVisualUpdatePending, so that the dirtying below
didn't change that. However that might not be true anymore since the
SetForceReattachLayoutTree() no longer does that for null
ComputedStyle.

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

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

@@ -1636,12 +1636,15 @@ const ComputedStyle* Node::VirtualEnsureComputedStyle(
 
 void Node::SetForceReattachLayoutTree() {
   DCHECK(!GetDocument().GetStyleEngine().InRebuildLayoutTree());
+  DCHECK(IsElementNode() || IsTextNode());
   if (GetForceReattachLayoutTree())
     return;
   if (!InActiveDocument())
     return;
-  if (!IsContainerNode() && !IsTextNode())
+  if (IsElementNode() && !GetComputedStyle()) {
+    DCHECK(!GetLayoutObject());
     return;
+  }
   SetFlag(kForceReattachLayoutTree);
   if (!NeedsStyleRecalc()) {
     // Make sure we traverse down to this node during style recalc.

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

@@ -15,6 +15,7 @@
 #include "third_party/blink/renderer/core/dom/pseudo_element.h"
 #include "third_party/blink/renderer/core/dom/shadow_root.h"
 #include "third_party/blink/renderer/core/dom/shadow_root_init.h"
+#include "third_party/blink/renderer/core/dom/slot_assignment_engine.h"
 #include "third_party/blink/renderer/core/editing/testing/editing_test_base.h"
 #include "third_party/blink/renderer/core/html/html_div_element.h"
 #include "third_party/blink/renderer/platform/heap/heap.h"
@@ -385,4 +386,23 @@ TEST_F(NodeTest, ContainsPseudo) {
   EXPECT_TRUE(a->contains(pseudo));
 }
 
+TEST_F(NodeTest, SkipForceReattachDisplayNone) {
+  SetBodyContent("<div id=host><span style='display:none'></span></div>");
+  Element* host = GetDocument().getElementById("host");
+  ShadowRoot& shadow_root =
+      host->AttachShadowRootInternal(ShadowRootType::kOpen);
+  shadow_root.SetInnerHTMLFromString("<slot name='target'></slot>");
+  UpdateAllLifecyclePhasesForTest();
+
+  Element* span = To<Element>(host->firstChild());
+  span->setAttribute(html_names::kSlotAttr, "target");
+  GetDocument().GetSlotAssignmentEngine().RecalcSlotAssignments();
+
+  // Node::FlatTreeParentChanged for a display:none could trigger style recalc,
+  // but we should skip a forced re-attach for nodes with a null ComputedStyle.
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
+  EXPECT_TRUE(span->NeedsStyleRecalc());
+  EXPECT_FALSE(span->GetForceReattachLayoutTree());
+}
+
 }  // namespace blink

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

@@ -5,13 +5,14 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_DOM_SLOT_ASSIGNMENT_ENGINE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_DOM_SLOT_ASSIGNMENT_ENGINE_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
 
 namespace blink {
 
 class ShadowRoot;
 
-class SlotAssignmentEngine final
+class CORE_EXPORT SlotAssignmentEngine final
     : public GarbageCollected<SlotAssignmentEngine> {
  public:
   explicit SlotAssignmentEngine();

third_party/blink/renderer/core/html/forms/html_input_element.cc

@@ -406,10 +406,6 @@ void HTMLInputElement::UpdateType() {
   DropInnerEditorElement();
   SetForceReattachLayoutTree();
 
-  // In this function, we should not do force to do style recalc and layout.
-  DocumentLifecycle::DisallowTransitionScope disallow_transition(
-      GetDocument().Lifecycle());
-
   if (input_type_->SupportsRequired() != new_type->SupportsRequired() &&
       IsRequired()) {
     PseudoStateChanged(CSSSelector::kPseudoRequired);