Ignore NeedsAdjacentStyleRecalc() for sanity DCHECK.

NeedsLayoutTreeUpdateForNode() is true if an ancestor returns true for
NeedsAdjacentStyleRecalc(). For getComputedStyle we may start out with a
tree where we don't need a full style/layout-tree update, but matching
selectors as part of EnsureComputedStyle() up the ancestor chain may
cause those flags to be set if we match adjacent selectors which were not
previously matched. We need to ignore those flags for the sanity check
in EnsureComputedStyle.

This may be the situation in which the DCHECK triggers for 900138 as
well.

Bug: 906830, 900138
Change-Id: I51927fdb9f4bc44e3612628cf9b9710d26e2d8ea
Reviewed-on: https://chromium-review.googlesource.com/c/1344130Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609956}

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

@@ -63,4 +63,46 @@ TEST_F(CSSComputedStyleDeclarationTest, CleanShadowAncestorsNoRecalc) {
   EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
 }
 
+TEST_F(CSSComputedStyleDeclarationTest, NeedsAdjacentStyleRecalc) {
+  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
+    <style>
+      #a + #b { color: green }
+    </style>
+    <div id="container" style="display:none">
+      <span id="a"></span>
+      <span id="b">
+        <span id="c"></span>
+        <span id="d"></span>
+      </span>
+    </div>
+  )HTML");
+
+  UpdateAllLifecyclePhasesForTest();
+
+  EXPECT_FALSE(GetDocument().NeedsLayoutTreeUpdate());
+
+  Element* container = GetDocument().getElementById("container");
+  Element* c_span = GetDocument().getElementById("c");
+  Element* d_span = GetDocument().getElementById("d");
+  d_span->setAttribute("style", "color:pink");
+
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdateForNode(*d_span));
+  EXPECT_FALSE(GetDocument().NeedsLayoutTreeUpdateForNode(*c_span));
+  EXPECT_FALSE(GetDocument().NeedsLayoutTreeUpdateForNode(*c_span, true));
+  EXPECT_FALSE(container->NeedsAdjacentStyleRecalc());
+
+  CSSComputedStyleDeclaration* computed =
+      CSSComputedStyleDeclaration::Create(c_span);
+
+  EXPECT_STREQ("rgb(0, 128, 0)",
+               computed->GetPropertyValue(CSSPropertyColor).Utf8().data());
+
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdateForNode(*d_span));
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdateForNode(*c_span));
+  EXPECT_FALSE(GetDocument().NeedsLayoutTreeUpdateForNode(*c_span, true));
+  EXPECT_TRUE(container->NeedsAdjacentStyleRecalc());
+}
+
 }  // namespace blink

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

@@ -2413,7 +2413,8 @@ void Document::NotifyLayoutTreeOfSubtreeChanges() {
   lifecycle_.AdvanceTo(DocumentLifecycle::kLayoutSubtreeChangeClean);
 }
 
-bool Document::NeedsLayoutTreeUpdateForNode(const Node& node) const {
+bool Document::NeedsLayoutTreeUpdateForNode(const Node& node,
+                                            bool ignore_adjacent_style) const {
   if (!node.CanParticipateInFlatTree())
     return false;
   if (!NeedsLayoutTreeUpdate())
@@ -2433,7 +2434,7 @@ bool Document::NeedsLayoutTreeUpdateForNode(const Node& node) const {
       }
     }
     if (ancestor->NeedsStyleRecalc() || ancestor->NeedsStyleInvalidation() ||
-        ancestor->NeedsAdjacentStyleRecalc()) {
+        (ancestor->NeedsAdjacentStyleRecalc() && !ignore_adjacent_style)) {
       return true;
     }
   }

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

@@ -531,7 +531,9 @@ class CORE_EXPORT Document : public ContainerNode,
   void SetupFontBuilder(ComputedStyle& document_style);
 
   bool NeedsLayoutTreeUpdate() const;
-  bool NeedsLayoutTreeUpdateForNode(const Node&) const;
+  bool NeedsLayoutTreeUpdateForNode(const Node&,
+                                    bool ignore_adjacent_style = false) const;
+
   // Update ComputedStyles and attach LayoutObjects if necessary, but don't
   // lay out.
   void UpdateStyleAndLayoutTree();

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

@@ -3779,7 +3779,14 @@ const ComputedStyle* Element::EnsureComputedStyle(
   // EnsureComputedStyle. In some cases you might be fine using GetComputedStyle
   // without updating the style, but in most cases you want a clean tree for
   // that as well.
-  DCHECK(!GetDocument().NeedsLayoutTreeUpdateForNode(*this));
+  //
+  // Adjacent styling bits may be set and affect NeedsLayoutTreeUpdateForNode as
+  // part of EnsureComputedStyle in an ancestor chain.
+  // (see CSSComputedStyleDeclarationTest::NeedsAdjacentStyleRecalc). It is OK
+  // that it happens, but we need to ignore the effect on
+  // NeedsLayoutTreeUpdateForNode here.
+  DCHECK(!GetDocument().NeedsLayoutTreeUpdateForNode(
+      *this, true /* ignore_adjacent_style */));
 
   // FIXME: Find and use the layoutObject from the pseudo element instead of the
   // actual element so that the 'length' properties, which are only known by the