Don't style-invalidate children of display:none elements.

If an element has a null ComputedStyle, its descendants can only have a
ComputedStyle on the next lifecycle update if the element's
ComputedStyle is updated. That would follow directly from inheritance in
Element::RecalcStyle() and we won't need to mark those descendant with
NeedsStyleRecalc(). Thus, we can skip style invalidation of descendants
of display:none elements.

This is a StyleInvalidator optimization, but also we are preparing for
not marking style dirty for elements outside the flat tree for making it
feasible to e.g. skip non-slotted subtrees of shadow hosts during style
recalc.

Bug: 972752
Change-Id: If586ac229c842189b6e3e664dfa756b262a9162c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1783140
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693108}

third_party/blink/renderer/core/css/BUILD.gn

@@ -615,6 +615,7 @@ blink_core_tests("unit_tests") {
     "font_face_cache_test.cc",
     "invalidation/invalidation_set_test.cc",
     "invalidation/pending_invalidations_test.cc",
+    "invalidation/style_invalidator_test.cc",
     "media_query_evaluator_test.cc",
     "media_query_list_test.cc",
     "media_query_matcher_test.cc",

third_party/blink/renderer/core/css/invalidation/style_invalidator.cc

@@ -9,6 +9,7 @@
 #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/element_traversal.h"
+#include "third_party/blink/renderer/core/dom/node_computed_style.h"
 #include "third_party/blink/renderer/core/dom/shadow_root.h"
 #include "third_party/blink/renderer/core/html/html_slot_element.h"
 #include "third_party/blink/renderer/core/inspector/inspector_trace_events.h"
@@ -317,9 +318,12 @@ void StyleInvalidator::Invalidate(Element& element, SiblingData& sibling_data) {
   //   could apply to the descendants.
   // * there are invalidation sets attached to descendants then we need to
   //   clear the flags on the nodes, whether we use the sets or not.
-  if ((!WholeSubtreeInvalid() && HasInvalidationSets()) ||
+  if ((!WholeSubtreeInvalid() && HasInvalidationSets() &&
+       element.GetComputedStyle()) ||
       element.ChildNeedsStyleInvalidation()) {
     InvalidateChildren(element);
+  } else {
+    ClearPendingNthSiblingInvalidationSets();
   }
 
   element.ClearChildNeedsStyleInvalidation();

third_party/blink/renderer/core/css/invalidation/style_invalidator.h

@@ -81,8 +81,9 @@ class CORE_EXPORT StyleInvalidator {
   void PushNthSiblingInvalidationSets(SiblingData& sibling_data) {
     for (const auto* invalidation_set : pending_nth_sets_)
       sibling_data.PushInvalidationSet(*invalidation_set);
-    pending_nth_sets_.resize(0);
+    ClearPendingNthSiblingInvalidationSets();
   }
+  void ClearPendingNthSiblingInvalidationSets() { pending_nth_sets_.resize(0); }
 
   PendingInvalidationMap& pending_invalidation_map_;
   using DescendantInvalidationSets = Vector<const InvalidationSet*, 16>;

third_party/blink/renderer/core/css/invalidation/style_invalidator_test.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/css/invalidation/style_invalidator.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/html/html_element.h"
+#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
+
+namespace blink {
+
+class StyleInvalidatorTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    dummy_page_holder_ = std::make_unique<DummyPageHolder>(IntSize(800, 600));
+  }
+
+  Document& GetDocument() { return dummy_page_holder_->GetDocument(); }
+
+ private:
+  std::unique_ptr<DummyPageHolder> dummy_page_holder_;
+};
+
+TEST_F(StyleInvalidatorTest, SkipDisplayNone) {
+  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
+    <div id="root">
+      <div style="display:none">
+        <div class="a"></div>
+        <div class="a"></div>
+      </div>
+    </div>
+  )HTML");
+
+  GetDocument().View()->UpdateAllLifecyclePhases(
+      DocumentLifecycle::LifecycleUpdateReason::kTest);
+
+  PendingInvalidations pending;
+  {
+    InvalidationLists lists;
+    scoped_refptr<InvalidationSet> set = DescendantInvalidationSet::Create();
+    set->AddClass("a");
+    lists.descendants.push_back(set);
+    pending.ScheduleInvalidationSetsForNode(
+        lists, *GetDocument().getElementById("root"));
+  }
+
+  StyleInvalidator invalidator(pending.GetPendingInvalidationMap());
+  invalidator.Invalidate(GetDocument(), GetDocument().body());
+
+  EXPECT_FALSE(GetDocument().NeedsLayoutTreeUpdate());
+}
+
+TEST_F(StyleInvalidatorTest, SkipDisplayNoneClearPendingNth) {
+  GetDocument().body()->SetInnerHTMLFromString(R"HTML(
+    <div id="none" style="display:none">
+      <div class="a"></div>
+      <div class="a"></div>
+    </div>
+    <div id="descendant">
+      <div class="a"></div>
+    </div>
+  )HTML");
+
+  GetDocument().View()->UpdateAllLifecyclePhases(
+      DocumentLifecycle::LifecycleUpdateReason::kTest);
+
+  PendingInvalidations pending;
+  {
+    InvalidationLists lists;
+    scoped_refptr<InvalidationSet> set = NthSiblingInvalidationSet::Create();
+    set->AddClass("a");
+    lists.siblings.push_back(set);
+    pending.ScheduleInvalidationSetsForNode(
+        lists, *GetDocument().getElementById("none"));
+  }
+  {
+    InvalidationLists lists;
+    scoped_refptr<InvalidationSet> set = DescendantInvalidationSet::Create();
+    set->AddClass("a");
+    lists.descendants.push_back(set);
+    pending.ScheduleInvalidationSetsForNode(
+        lists, *GetDocument().getElementById("descendant"));
+  }
+
+  StyleInvalidator invalidator(pending.GetPendingInvalidationMap());
+  invalidator.Invalidate(GetDocument(), GetDocument().body());
+
+  EXPECT_TRUE(GetDocument().NeedsLayoutTreeUpdate());
+  EXPECT_FALSE(GetDocument().getElementById("none")->ChildNeedsStyleRecalc());
+  EXPECT_TRUE(
+      GetDocument().getElementById("descendant")->ChildNeedsStyleRecalc());
+}
+
+}  // namespace blink