Don't fetch resources for getComputedStyle for pseudo.

Calling getComputedStyle(div, "::before") would trigger a fetch for the
background-image url in:

  div::before { background-image: url(dont-fetch.png) }

which should be unnecessary.

Still, we couldn't simply check for PseudoElement being non-null since
we need to fetch background images for ::first-line for which we have no
PseudoElement.

Bug: 1086142
Change-Id: I93d2b59f703f339c104a3735794d0e42287475fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2218034
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772681}

third_party/blink/renderer/core/css/resolver/style_resolver.cc

@@ -1203,9 +1203,9 @@ scoped_refptr<ComputedStyle> StyleResolver::PseudoStyleForElement(
   if (!element)
     return nullptr;
 
-  StyleResolverState state(GetDocument(), *element,
-                           pseudo_style_request.pseudo_id, parent_style,
-                           parent_layout_object_style);
+  StyleResolverState state(
+      GetDocument(), *element, pseudo_style_request.pseudo_id,
+      pseudo_style_request.type, parent_style, parent_layout_object_style);
   if (!PseudoStyleForElementInternal(*element, pseudo_style_request, state)) {
     if (pseudo_style_request.type == PseudoElementStyleRequest::kForRenderer)
       return nullptr;

third_party/blink/renderer/core/css/resolver/style_resolver_state.cc

@@ -37,19 +37,15 @@ StyleResolverState::StyleResolverState(
     Document& document,
     Element& element,
     PseudoElement* pseudo_element,
+    PseudoElementStyleRequest::RequestType pseudo_request_type,
     AnimatingElementType animating_element_type,
     const ComputedStyle* parent_style,
     const ComputedStyle* layout_parent_style)
     : element_context_(element),
       document_(&document),
-      style_(nullptr),
       parent_style_(parent_style),
       layout_parent_style_(layout_parent_style),
-      is_animation_interpolation_map_ready_(false),
-      is_animating_custom_properties_(false),
-      has_dir_auto_attribute_(false),
-      cascaded_color_value_(nullptr),
-      cascaded_visited_color_value_(nullptr),
+      pseudo_request_type_(pseudo_request_type),
       font_builder_(&document),
       element_style_resources_(GetElement(),
                                document.DevicePixelRatio(),
@@ -78,18 +74,22 @@ StyleResolverState::StyleResolverState(Document& document,
     : StyleResolverState(document,
                          element,
                          nullptr /* pseudo_element */,
+                         PseudoElementStyleRequest::kForRenderer,
                          AnimatingElementType::kElement,
                          parent_style,
                          layout_parent_style) {}
 
-StyleResolverState::StyleResolverState(Document& document,
-                                       Element& element,
-                                       PseudoId pseudo_id,
-                                       const ComputedStyle* parent_style,
-                                       const ComputedStyle* layout_parent_style)
+StyleResolverState::StyleResolverState(
+    Document& document,
+    Element& element,
+    PseudoId pseudo_id,
+    PseudoElementStyleRequest::RequestType pseudo_request_type,
+    const ComputedStyle* parent_style,
+    const ComputedStyle* layout_parent_style)
     : StyleResolverState(document,
                          element,
                          element.GetPseudoElement(pseudo_id),
+                         pseudo_request_type,
                          AnimatingElementType::kPseudoElement,
                          parent_style,
                          layout_parent_style) {}
@@ -157,7 +157,8 @@ void StyleResolverState::CacheUserAgentBorderAndBackground() {
 }
 
 void StyleResolverState::LoadPendingResources() {
-  if ((ParentStyle() && ParentStyle()->IsEnsuredInDisplayNone()) ||
+  if (pseudo_request_type_ == PseudoElementStyleRequest::kForComputedStyle ||
+      (ParentStyle() && ParentStyle()->IsEnsuredInDisplayNone()) ||
       StyleRef().Display() == EDisplay::kNone ||
       StyleRef().Display() == EDisplay::kContents ||
       StyleRef().IsEnsuredOutsideFlatTree())

third_party/blink/renderer/core/css/resolver/style_resolver_state.h

@@ -32,6 +32,7 @@
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/css/css_to_length_conversion_data.h"
 #include "third_party/blink/renderer/core/css/parser/css_parser_mode.h"
+#include "third_party/blink/renderer/core/css/pseudo_style_request.h"
 #include "third_party/blink/renderer/core/css/resolver/css_to_style_map.h"
 #include "third_party/blink/renderer/core/css/resolver/element_resolve_context.h"
 #include "third_party/blink/renderer/core/css/resolver/element_style_resources.h"
@@ -60,6 +61,7 @@ class CORE_EXPORT StyleResolverState {
   StyleResolverState(Document&,
                      Element&,
                      PseudoId,
+                     PseudoElementStyleRequest::RequestType,
                      const ComputedStyle* parent_style,
                      const ComputedStyle* layout_parent_style);
   ~StyleResolverState();
@@ -263,6 +265,7 @@ class CORE_EXPORT StyleResolverState {
   StyleResolverState(Document&,
                      Element&,
                      PseudoElement*,
+                     PseudoElementStyleRequest::RequestType,
                      AnimatingElementType,
                      const ComputedStyle* parent_style,
                      const ComputedStyle* layout_parent_style);
@@ -287,19 +290,19 @@ class CORE_EXPORT StyleResolverState {
   scoped_refptr<const ComputedStyle> layout_parent_style_;
 
   CSSAnimationUpdate animation_update_;
-  bool is_animation_interpolation_map_ready_;
-  bool is_animating_custom_properties_;
+  bool is_animation_interpolation_map_ready_ = false;
+  bool is_animating_custom_properties_ = false;
   // We can't use the base computed style optimization when 'revert' appears
   // in a keyframe. (We need to build the cascade to know what to revert to).
   // TODO(crbug.com/1068515): Refactor caching to remove these flags.
   bool is_animating_revert_ = false;
   bool has_important_overrides_ = false;
   bool has_font_affecting_animation_ = false;
+  bool has_dir_auto_attribute_ = false;
+  PseudoElementStyleRequest::RequestType pseudo_request_type_;
 
-  bool has_dir_auto_attribute_;
-
-  const CSSValue* cascaded_color_value_;
-  const CSSValue* cascaded_visited_color_value_;
+  const CSSValue* cascaded_color_value_ = nullptr;
+  const CSSValue* cascaded_visited_color_value_ = nullptr;
 
   FontBuilder font_builder_;
 

third_party/blink/renderer/core/css/resolver/style_resolver_test.cc

@@ -311,17 +311,19 @@ INSTANTIATE_TEST_SUITE_P(All,
 
 namespace {
 
-const CSSImageValue& GetBackgroundImageValue(Element* element) {
-  DCHECK(element);
-  const auto* style = element->GetComputedStyle();
-  DCHECK(style);
+const CSSImageValue& GetBackgroundImageValue(const ComputedStyle& style) {
   const CSSValue* computed_value = ComputedStyleUtils::ComputedPropertyValue(
-      GetCSSPropertyBackgroundImage(), *style);
+      GetCSSPropertyBackgroundImage(), style);
 
   const CSSValueList* bg_img_list = To<CSSValueList>(computed_value);
   return To<CSSImageValue>(bg_img_list->Item(0));
 }
 
+const CSSImageValue& GetBackgroundImageValue(const Element* element) {
+  DCHECK(element);
+  return GetBackgroundImageValue(element->ComputedStyleRef());
+}
+
 }  // namespace
 
 TEST_F(StyleResolverTest, BackgroundImageFetch) {
@@ -348,6 +350,19 @@ TEST_F(StyleResolverTest, BackgroundImageFetch) {
       #non-slotted {
         background-image: url(img-non-slotted.png);
       }
+      #no-pseudo::before {
+        background-image: url(img-no-pseudo.png);
+      }
+      #first-line::first-line {
+        background-image: url(first-line.png);
+      }
+      #first-line-span::first-line {
+        background-image: url(first-line-span.png);
+      }
+      #first-line-none { display: none; }
+      #first-line-none::first-line {
+        background-image: url(first-line-none.png);
+      }
     </style>
     <div id="none">
       <div id="inside-none"></div>
@@ -359,6 +374,10 @@ TEST_F(StyleResolverTest, BackgroundImageFetch) {
     <div id="host">
       <div id="non-slotted"></div>
     </div>
+    <div id="no-pseudo"></div>
+    <div id="first-line">XXX</div>
+    <span id="first-line-span">XXX</span>
+    <div id="first-line-none">XXX</div>
   )HTML");
 
   GetDocument().getElementById("host")->AttachShadowRootInternal(
@@ -371,10 +390,32 @@ TEST_F(StyleResolverTest, BackgroundImageFetch) {
   auto* inside_hidden = GetDocument().getElementById("inside-hidden");
   auto* contents = GetDocument().getElementById("contents");
   auto* non_slotted = GetDocument().getElementById("non-slotted");
+  auto* no_pseudo = GetDocument().getElementById("no-pseudo");
+  auto* first_line = GetDocument().getElementById("first-line");
+  auto* first_line_span = GetDocument().getElementById("first-line-span");
+  auto* first_line_none = GetDocument().getElementById("first-line-none");
 
   inside_none->EnsureComputedStyle();
   non_slotted->EnsureComputedStyle();
-
+  auto* before_style = no_pseudo->EnsureComputedStyle(kPseudoIdBefore);
+  auto* first_line_style = first_line->EnsureComputedStyle(kPseudoIdFirstLine);
+  auto* first_line_span_style =
+      first_line_span->EnsureComputedStyle(kPseudoIdFirstLine);
+  auto* first_line_none_style =
+      first_line_none->EnsureComputedStyle(kPseudoIdFirstLine);
+
+  ASSERT_TRUE(before_style);
+  EXPECT_TRUE(GetBackgroundImageValue(*before_style).IsCachePending())
+      << "No fetch for non-generated ::before";
+  ASSERT_TRUE(first_line_style);
+  EXPECT_FALSE(GetBackgroundImageValue(*first_line_style).IsCachePending())
+      << "Fetched by layout of ::first-line";
+  ASSERT_TRUE(first_line_span_style);
+  EXPECT_TRUE(GetBackgroundImageValue(*first_line_span_style).IsCachePending())
+      << "No fetch for inline with ::first-line";
+  ASSERT_TRUE(first_line_none_style);
+  EXPECT_TRUE(GetBackgroundImageValue(*first_line_none_style).IsCachePending())
+      << "No fetch for display:none with ::first-line";
   EXPECT_TRUE(GetBackgroundImageValue(none).IsCachePending())
       << "No fetch for display:none";
   EXPECT_TRUE(GetBackgroundImageValue(inside_none).IsCachePending())