Rework loading and computed value handling for image-set()

To avoid having <image> loading code spread out in multiple places, move the <image> selection and resolving for image-set() from the CSSValue to the StyleImageLoader helper class in element_style_resources.cc. This means there's only a single point in the code that handles each type of <image>. Similarly for the (faux) computed value side of things, move CSSImageSetValue::ComputedCSSValue into the ComputedCSSValueBuilder used for StylePendingImage. Move that helper out of anonymity and rename it to StyleImageComputedCSSValueBuilder. This also means that the plumbing of state in the StyleImageLoader helper class can be simplified a little bit. The image-set() option parser is changed to not allow certain functions at parse-time instead of rejecting them at computed value-time. Bug: 1369996 Change-Id: I26dfefff0906f6903ba85b564dd0cd4f63789a38 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4942358 Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org> Commit-Queue: Fredrik Söderquist <fs@opera.com> Cr-Commit-Position: refs/heads/main@{#1211401}
chromium · Oct 18, 2023 · efaf606 · efaf606
1 parent c8023cf
commit efaf606
Show file tree

Hide file tree

Showing 12 changed files with 212 additions and 224 deletions.
diff --git a/third_party/blink/renderer/core/css/css_image_set_option_value.cc b/third_party/blink/renderer/core/css/css_image_set_option_value.cc
@@ -5,50 +5,13 @@
 #include "third_party/blink/renderer/core/css/css_image_set_option_value.h"
 
 #include "base/memory/values_equivalent.h"
-#include "third_party/blink/renderer/core/css/css_gradient_value.h"
-#include "third_party/blink/renderer/core/css/css_image_value.h"
+#include "third_party/blink/renderer/core/css/css_image_set_type_value.h"
 #include "third_party/blink/renderer/core/css/css_numeric_literal_value.h"
-#include "third_party/blink/renderer/core/style/style_generated_image.h"
+#include "third_party/blink/renderer/core/css/css_primitive_value.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
 namespace blink {
 
-namespace {
-
-const CSSValue* ComputeImage(const CSSValue* value,
-                             const ComputedStyle& style,
-                             const bool allow_visited_style) {
-  if (auto* image = DynamicTo<CSSImageValue>(value)) {
-    return image->ComputedCSSValue();
-  }
-
-  if (!RuntimeEnabledFeatures::CSSImageSetEnabled()) {
-    return value;
-  }
-
-  if (auto* gradient = DynamicTo<cssvalue::CSSGradientValue>(value)) {
-    return gradient->ComputedCSSValue(style, allow_visited_style);
-  }
-
-  NOTREACHED();
-
-  return value;
-}
-
-const CSSPrimitiveValue* ComputeResolution(
-    const CSSPrimitiveValue* resolution) {
-  if (RuntimeEnabledFeatures::CSSImageSetEnabled() && resolution &&
-      resolution->IsResolution()) {
-    return CSSNumericLiteralValue::Create(
-        resolution->ComputeDotsPerPixel(),
-        CSSPrimitiveValue::UnitType::kDotsPerPixel);
-  }
-
-  return resolution;
-}
-
-}  // namespace
-
 CSSImageSetOptionValue::CSSImageSetOptionValue(
     const CSSValue* image,
     const CSSPrimitiveValue* resolution,
@@ -67,31 +30,6 @@ CSSImageSetOptionValue::CSSImageSetOptionValue(
 
 CSSImageSetOptionValue::~CSSImageSetOptionValue() = default;
 
-StyleImage* CSSImageSetOptionValue::CacheImage(
-    const Document& document,
-    const FetchParameters::ImageRequestBehavior image_request_behavior,
-    const CrossOriginAttributeValue cross_origin,
-    const CSSToLengthConversionData::ContainerSizes& container_sizes) const {
-  if (auto* image =
-          const_cast<CSSImageValue*>(DynamicTo<CSSImageValue>(image_.Get()))) {
-    return image->CacheImage(document, image_request_behavior, cross_origin,
-                             ComputedResolution());
-  }
-
-  if (!RuntimeEnabledFeatures::CSSImageSetEnabled()) {
-    return nullptr;
-  }
-
-  if (auto* gradient = DynamicTo<cssvalue::CSSGradientValue>(image_.Get())) {
-    return MakeGarbageCollected<StyleGeneratedImage>(*gradient,
-                                                     container_sizes);
-  }
-
-  NOTREACHED();
-
-  return nullptr;
-}
-
 double CSSImageSetOptionValue::ComputedResolution() const {
   return resolution_->ComputeDotsPerPixel();
 }
@@ -101,6 +39,18 @@ bool CSSImageSetOptionValue::IsSupported() const {
          (resolution_->ComputeDotsPerPixel() > 0.0);
 }
 
+CSSValue& CSSImageSetOptionValue::GetImage() const {
+  return const_cast<CSSValue&>(*image_);
+}
+
+const CSSPrimitiveValue& CSSImageSetOptionValue::GetResolution() const {
+  return *resolution_;
+}
+
+const CSSImageSetTypeValue* CSSImageSetOptionValue::GetType() const {
+  return type_;
+}
+
 String CSSImageSetOptionValue::CustomCSSText() const {
   StringBuilder result;
 
@@ -121,14 +71,6 @@ bool CSSImageSetOptionValue::Equals(const CSSImageSetOptionValue& other) const {
          base::ValuesEquivalent(type_, other.type_);
 }
 
-CSSImageSetOptionValue* CSSImageSetOptionValue::ComputedCSSValue(
-    const ComputedStyle& style,
-    const bool allow_visited_style) const {
-  return MakeGarbageCollected<CSSImageSetOptionValue>(
-      ComputeImage(image_, style, allow_visited_style),
-      ComputeResolution(resolution_), type_);
-}
-
 void CSSImageSetOptionValue::TraceAfterDispatch(blink::Visitor* visitor) const {
   visitor->Trace(image_);
   visitor->Trace(resolution_);

diff --git a/third_party/blink/renderer/core/css/css_image_set_option_value.h b/third_party/blink/renderer/core/css/css_image_set_option_value.h
@@ -5,19 +5,15 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_IMAGE_SET_OPTION_VALUE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_IMAGE_SET_OPTION_VALUE_H_
 
-#include "third_party/blink/renderer/core/css/css_image_set_type_value.h"
-#include "third_party/blink/renderer/core/css/css_primitive_value.h"
-#include "third_party/blink/renderer/core/css/css_to_length_conversion_data.h"
 #include "third_party/blink/renderer/core/css/css_value.h"
-#include "third_party/blink/renderer/core/dom/document.h"
-#include "third_party/blink/renderer/core/style/computed_style.h"
-#include "third_party/blink/renderer/core/style/style_image.h"
-#include "third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h"
 #include "third_party/blink/renderer/platform/wtf/casting.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
+class CSSImageSetTypeValue;
+class CSSPrimitiveValue;
+
 // This class represents an image-set-option as specified in:
 // https://w3c.github.io/csswg-drafts/css-images-4/#typedef-image-set-option
 // <image-set-option> = [ <image> | <string> ] [<resolution> || type(<string>)]
@@ -33,27 +29,21 @@ class CSSImageSetOptionValue : public CSSValue {
 
   ~CSSImageSetOptionValue();
 
-  StyleImage* CacheImage(
-      const Document& document,
-      const FetchParameters::ImageRequestBehavior image_request_behavior,
-      const CrossOriginAttributeValue cross_origin,
-      const CSSToLengthConversionData::ContainerSizes& container_sizes) const;
-
   // Gets the resolution value in Dots Per Pixel
   double ComputedResolution() const;
 
   // Returns true if the image-set-option uses an image format that the
   // browser can render.
   bool IsSupported() const;
 
+  CSSValue& GetImage() const;
+  const CSSPrimitiveValue& GetResolution() const;
+  const CSSImageSetTypeValue* GetType() const;
+
   String CustomCSSText() const;
 
   bool Equals(const CSSImageSetOptionValue& other) const;
 
-  CSSImageSetOptionValue* ComputedCSSValue(
-      const ComputedStyle& style,
-      const bool allow_visited_style) const;
-
   void TraceAfterDispatch(blink::Visitor* visitor) const;
 
  private:

diff --git a/third_party/blink/renderer/core/css/css_image_set_value.cc b/third_party/blink/renderer/core/css/css_image_set_value.cc
@@ -27,8 +27,10 @@
 
 #include <algorithm>
 
+#include "third_party/blink/renderer/core/css/css_image_set_option_value.h"
 #include "third_party/blink/renderer/core/loader/resource/image_resource_content.h"
 #include "third_party/blink/renderer/core/style/style_image_set.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
 namespace blink {
@@ -111,26 +113,10 @@ StyleImage* CSSImageSetValue::CachedImage(
   return cached_image_.Get();
 }
 
-StyleImage* CSSImageSetValue::CacheImage(
-    const Document& document,
-    const float device_scale_factor,
-    const FetchParameters::ImageRequestBehavior image_request_behavior,
-    const CrossOriginAttributeValue cross_origin,
-    const CSSToLengthConversionData::ContainerSizes& container_sizes) {
-  if (IsCachePending(device_scale_factor)) {
-    const CSSImageSetOptionValue* best_option =
-        GetBestOption(device_scale_factor);
-
-    StyleImage* style_image =
-        best_option ? best_option->CacheImage(document, image_request_behavior,
-                                              cross_origin, container_sizes)
-                    : nullptr;
-
-    cached_image_ = MakeGarbageCollected<StyleImageSet>(style_image, this);
-
-    cached_device_scale_factor_ = device_scale_factor;
-  }
-
+StyleImage* CSSImageSetValue::CacheImage(StyleImage* style_image,
+                                         const float device_scale_factor) {
+  cached_image_ = MakeGarbageCollected<StyleImageSet>(style_image, this);
+  cached_device_scale_factor_ = device_scale_factor;
   return cached_image_.Get();
 }
 
@@ -174,17 +160,4 @@ void CSSImageSetValue::TraceAfterDispatch(blink::Visitor* visitor) const {
   CSSValueList::TraceAfterDispatch(visitor);
 }
 
-CSSImageSetValue* CSSImageSetValue::ComputedCSSValue(
-    const ComputedStyle& style,
-    const bool allow_visited_style) const {
-  auto* value = MakeGarbageCollected<CSSImageSetValue>();
-
-  for (const auto& i : *this) {
-    value->Append(*To<CSSImageSetOptionValue>(i.Get())->ComputedCSSValue(
-        style, allow_visited_style));
-  }
-
-  return value;
-}
-
 }  // namespace blink
diff --git a/third_party/blink/renderer/core/css/css_image_set_value.h b/third_party/blink/renderer/core/css/css_image_set_value.h
@@ -26,19 +26,14 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_IMAGE_SET_VALUE_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_IMAGE_SET_VALUE_H_
 
-#include "third_party/blink/renderer/core/css/css_image_set_option_value.h"
-#include "third_party/blink/renderer/core/css/css_to_length_conversion_data.h"
 #include "third_party/blink/renderer/core/css/css_value_list.h"
-#include "third_party/blink/renderer/core/style/style_image.h"
 #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h"
-#include "third_party/blink/renderer/platform/loader/fetch/cross_origin_attribute_value.h"
-#include "third_party/blink/renderer/platform/loader/fetch/fetch_parameters.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 #include "third_party/blink/renderer/platform/wtf/casting.h"
 
 namespace blink {
 
-class Document;
+class CSSImageSetOptionValue;
 class StyleImage;
 
 class CORE_EXPORT CSSImageSetValue : public CSSValueList {
@@ -48,26 +43,17 @@ class CORE_EXPORT CSSImageSetValue : public CSSValueList {
 
   bool IsCachePending(const float device_scale_factor) const;
   StyleImage* CachedImage(const float device_scale_factor) const;
-  StyleImage* CacheImage(
-      const Document& document,
-      const float device_scale_factor,
-      const FetchParameters::ImageRequestBehavior image_request_behavior,
-      const CrossOriginAttributeValue cross_origin,
-      const CSSToLengthConversionData::ContainerSizes& container_sizes);
+  StyleImage* CacheImage(StyleImage*, const float device_scale_factor);
 
-  String CustomCSSText() const;
+  const CSSImageSetOptionValue* GetBestOption(const float device_scale_factor);
 
-  // Gets the computed CSS value of the image-set.
-  CSSImageSetValue* ComputedCSSValue(const ComputedStyle& style,
-                                     const bool allow_visited_style) const;
+  String CustomCSSText() const;
 
   bool HasFailedOrCanceledSubresources() const;
 
   void TraceAfterDispatch(blink::Visitor*) const;
 
  private:
-  const CSSImageSetOptionValue* GetBestOption(const float device_scale_factor);
-
   Member<StyleImage> cached_image_;
   float cached_device_scale_factor_{1.0f};
 

diff --git a/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc b/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
@@ -3038,10 +3038,14 @@ static CSSImageSetOptionValue* ConsumeImageSetOption(
       RuntimeEnabledFeatures::CSSImageSetEnabled()
           ? ConsumeStringUrlImagePolicy::kAllow
           : ConsumeStringUrlImagePolicy::kForbid;
+  const ConsumeGeneratedImagePolicy image_set_generated_image_policy =
+      RuntimeEnabledFeatures::CSSImageSetEnabled()
+          ? generated_image_policy
+          : ConsumeGeneratedImagePolicy::kForbid;
 
-  const CSSValue* image = ConsumeImage(range, context, generated_image_policy,
-                                       string_url_image_policy,
-                                       ConsumeImageSetImagePolicy::kForbid);
+  const CSSValue* image = ConsumeImage(
+      range, context, image_set_generated_image_policy, string_url_image_policy,
+      ConsumeImageSetImagePolicy::kForbid);
   if (!image) {
     return nullptr;
   }