Consider GetForceReattachLayoutTree() as a dirty node.

When considering dirty nodes for style recalc roots, we did not consider
nodes which were marked GetForceReattachLayoutTree(), but not marked for
recalc, as dirty nodes. With the current code, it simply caused us to
have the recalc root higher up the tree than strictly necessary, but
with FlatTreeStyleRecalc enabled, it caused fullscreen test faillures.
The reason is that Node::SetIsInTopLayer only marks node for forced
reattach and since the Document node is no longer marked as
ChildNeedsStyleRecalc(), a subsequent SetNeedsStyleRecalc on a
descendant would walk all the way to the top without finding a dirty
node or a child-dirty node.

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

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

@@ -1581,8 +1581,8 @@ void StyleEngine::MarkForWhitespaceReattachment() {
           *element);
       continue;
     }
-      DCHECK(!element->NeedsStyleRecalc());
-      DCHECK(!element->ChildNeedsStyleRecalc());
+    DCHECK(!element->NeedsStyleRecalc());
+    DCHECK(!element->ChildNeedsStyleRecalc());
     if (Node* first_child = LayoutTreeBuilderTraversal::FirstChild(*element))
       first_child->MarkAncestorsWithChildNeedsReattachLayoutTree();
   }

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

@@ -69,6 +69,10 @@ class StyleEngineTest : public testing::Test {
         DocumentLifecycle::LifecycleUpdateReason::kTest);
   }
 
+  Node* GetStyleRecalcRoot() {
+    return GetStyleEngine().style_recalc_root_.GetRootNode();
+  }
+
  private:
   std::unique_ptr<DummyPageHolder> dummy_page_holder_;
 };
@@ -2256,4 +2260,21 @@ TEST_F(StyleEngineTest, NeedsLayoutTreeRebuild) {
   EXPECT_TRUE(GetStyleEngine().NeedsLayoutTreeRebuild());
 }
 
+TEST_F(StyleEngineTest, ForceReattachLayoutTreeStyleRecalcRoot) {
+  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
+    <div id="outer">
+      <div id="inner"></div>
+    </div>
+  )HTML");
+  UpdateAllLifecyclePhases();
+
+  Element* outer = GetDocument().getElementById("outer");
+  Element* inner = GetDocument().getElementById("inner");
+
+  outer->SetForceReattachLayoutTree();
+  inner->SetInlineStyleProperty(CSSPropertyID::kColor, "blue");
+
+  EXPECT_EQ(outer, GetStyleRecalcRoot());
+}
+
 }  // namespace blink

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

@@ -45,7 +45,7 @@ bool StyleRecalcRoot::IsChildDirty(const ContainerNode& node) const {
 #endif  // DCHECK_IS_ON()
 
 bool StyleRecalcRoot::IsDirty(const Node& node) const {
-  return node.NeedsStyleRecalc();
+  return node.IsDirtyForStyleRecalc();
 }
 
 void StyleRecalcRoot::ClearChildDirtyForAncestors(ContainerNode& parent) const {

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

@@ -1209,13 +1209,13 @@ void Node::MarkAncestorsWithChildNeedsDistributionRecalc() {
 
 void Node::MarkAncestorsWithChildNeedsStyleRecalc() {
   ContainerNode* ancestor = GetStyleRecalcParent();
-  bool parent_dirty = ancestor && ancestor->NeedsStyleRecalc();
+  bool parent_dirty = ancestor && ancestor->IsDirtyForStyleRecalc();
   for (; ancestor && !ancestor->ChildNeedsStyleRecalc();
        ancestor = ancestor->GetStyleRecalcParent()) {
     if (!ancestor->isConnected())
       return;
     ancestor->SetChildNeedsStyleRecalc();
-    if (ancestor->NeedsStyleRecalc())
+    if (ancestor->IsDirtyForStyleRecalc())
       break;
     // If we reach a locked ancestor, we should abort since the ancestor marking
     // will be done when the lock is committed.

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

@@ -497,6 +497,10 @@ class CORE_EXPORT Node : public EventTarget {
     return GetFlag(kForceReattachLayoutTree);
   }
 
+  bool IsDirtyForStyleRecalc() const {
+    return NeedsStyleRecalc() || GetForceReattachLayoutTree();
+  }
+
   bool NeedsDistributionRecalc() const;
 
   bool ChildNeedsDistributionRecalc() const {