[css-typed-om] Refactor CSSStyleImageValue to prepare for gradients.

Currently CSSStyleImageValue implements the CSSImageValue class in
Typed OM. CSSImageValue can be anything that's an <image> i.e. a <url>,
<gradient>, or <cross-fade>.

However, the current implementation of CSSStyleImageValue only really
works for <url>s. This patch moves <url> specific logic into
CSSURLImageValue. This prepares for the implementation of <gradient> and
<cross-fade>, which will be subclasses of CSSStyleImageValue.

We also do some cleanup work (e.g. making things const, moving code in
header to cpp).

Bug: 803680
Change-Id: I848b3eab5dbbcb9bbe7f0aea0d377497f63bfda3
Reviewed-on: https://chromium-review.googlesource.com/882786
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: nainar <nainar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533949}

third_party/WebKit/Source/core/css/BUILD.gn

@@ -354,6 +354,7 @@ blink_core_sources("css") {
     "cssom/CSSTransformValue.h",
     "cssom/CSSTranslate.cpp",
     "cssom/CSSTranslate.h",
+    "cssom/CSSURLImageValue.cpp",
     "cssom/CSSURLImageValue.h",
     "cssom/CSSUnitValue.cpp",
     "cssom/CSSUnitValue.h",

third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.cpp

@@ -7,26 +7,30 @@
 namespace blink {
 
 double CSSStyleImageValue::intrinsicWidth(bool& is_null) const {
-  is_null = Status() != ResourceStatus::kCached;
-  if (is_null)
+  const WTF::Optional<IntSize> size = IntrinsicSize();
+  if (!size) {
+    is_null = true;
     return 0;
-  return ImageSize().Width();
+  }
+  return size.value().Width();
 }
 
 double CSSStyleImageValue::intrinsicHeight(bool& is_null) const {
-  is_null = Status() != ResourceStatus::kCached;
-  if (is_null)
+  const WTF::Optional<IntSize> size = IntrinsicSize();
+  if (!size) {
+    is_null = true;
     return 0;
-  return ImageSize().Height();
+  }
+  return size.value().Height();
 }
 
-double CSSStyleImageValue::intrinsicRatio(bool& is_null) {
-  is_null = Status() != ResourceStatus::kCached;
-  if (is_null || intrinsicHeight(is_null) == 0) {
+double CSSStyleImageValue::intrinsicRatio(bool& is_null) const {
+  const WTF::Optional<IntSize> size = IntrinsicSize();
+  if (!size || size.value().Height() == 0) {
     is_null = true;
     return 0;
   }
-  return intrinsicWidth(is_null) / intrinsicHeight(is_null);
+  return static_cast<double>(size.value().Width()) / size.value().Height();
 }
 
 FloatSize CSSStyleImageValue::ElementSize(
@@ -35,21 +39,4 @@ FloatSize CSSStyleImageValue::ElementSize(
   return FloatSize(intrinsicWidth(not_used), intrinsicHeight(not_used));
 }
 
-bool CSSStyleImageValue::IsAccelerated() const {
-  return GetImage() && GetImage()->IsTextureBacked();
-}
-
-scoped_refptr<Image> CSSStyleImageValue::GetImage() const {
-  if (IsCachePending())
-    return nullptr;
-  // cachedImage can be null if image is StyleInvalidImage
-  ImageResourceContent* cached_image =
-      image_value_->CachedImage()->CachedImage();
-  if (cached_image) {
-    // getImage() returns the nullImage() if the image is not available yet
-    return cached_image->GetImage()->ImageForDefaultFrame();
-  }
-  return nullptr;
-}
-
 }  // namespace blink

third_party/WebKit/Source/core/css/cssom/CSSStyleImageValue.h

@@ -7,13 +7,9 @@
 
 #include "base/macros.h"
 #include "core/CoreExport.h"
-#include "core/css/CSSImageValue.h"
 #include "core/css/cssom/CSSResourceValue.h"
-#include "core/css/cssom/CSSStyleValue.h"
 #include "core/html/canvas/CanvasImageSource.h"
-#include "core/imagebitmap/ImageBitmapSource.h"
-#include "core/loader/resource/ImageResourceContent.h"
-#include "core/style/StyleImage.h"
+#include "platform/wtf/Optional.h"
 
 namespace blink {
 
@@ -26,9 +22,10 @@ class CORE_EXPORT CSSStyleImageValue : public CSSResourceValue,
  public:
   virtual ~CSSStyleImageValue() = default;
 
+  // IDL
   double intrinsicWidth(bool& is_null) const;
   double intrinsicHeight(bool& is_null) const;
-  double intrinsicRatio(bool& is_null);
+  double intrinsicRatio(bool& is_null) const;
 
   // CanvasImageSource
   bool IsCSSImageValue() const final { return true; }
@@ -37,45 +34,13 @@ class CORE_EXPORT CSSStyleImageValue : public CSSResourceValue,
     return true;
   }
   FloatSize ElementSize(const FloatSize& default_object_size) const final;
-  scoped_refptr<Image> GetSourceImageForCanvas(SourceImageStatus*,
-                                               AccelerationHint,
-                                               const FloatSize&) final {
-    return GetImage();
-  }
-  bool IsAccelerated() const override;
-
-  virtual void Trace(blink::Visitor* visitor) {
-    visitor->Trace(image_value_);
-    CSSResourceValue::Trace(visitor);
-  }
 
  protected:
-  CSSStyleImageValue(const CSSImageValue* image_value)
-      : image_value_(image_value) {}
-
-  virtual IntSize ImageSize() const {
-    DCHECK(!IsCachePending());
-    ImageResourceContent* resource_content =
-        image_value_->CachedImage()->CachedImage();
-    return resource_content
-               ? resource_content->IntrinsicSize(kDoNotRespectImageOrientation)
-               : IntSize(0, 0);
-  }
+  CSSStyleImageValue() = default;
 
-  virtual bool IsCachePending() const { return image_value_->IsCachePending(); }
-
-  ResourceStatus Status() const override {
-    if (IsCachePending())
-      return ResourceStatus::kNotStarted;
-    return image_value_->CachedImage()->CachedImage()->GetContentStatus();
-  }
-
-  const CSSImageValue* CssImageValue() const { return image_value_.Get(); };
+  virtual WTF::Optional<IntSize> IntrinsicSize() const = 0;
 
  private:
-  scoped_refptr<Image> GetImage() const;
-
-  Member<const CSSImageValue> image_value_;
   DISALLOW_COPY_AND_ASSIGN(CSSStyleImageValue);
 };
 

third_party/WebKit/Source/core/css/cssom/CSSStyleImageValueTest.cpp

@@ -4,6 +4,7 @@
 
 #include "core/css/cssom/CSSStyleImageValue.h"
 
+#include "platform/graphics/Image.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace blink {
@@ -12,22 +13,30 @@ namespace {
 
 class FakeCSSStyleImageValue : public CSSStyleImageValue {
  public:
-  FakeCSSStyleImageValue(CSSImageValue* image_value,
-                         bool cache_pending,
-                         IntSize size)
-      : CSSStyleImageValue(image_value),
-        cache_pending_(cache_pending),
-        size_(size) {}
+  FakeCSSStyleImageValue(bool cache_pending, IntSize size)
+      : cache_pending_(cache_pending), size_(size) {}
 
-  bool IsCachePending() const final { return cache_pending_; }
-  IntSize ImageSize() const final { return size_; }
+  // CSSStyleImageValue
+  WTF::Optional<IntSize> IntrinsicSize() const final {
+    if (cache_pending_)
+      return WTF::nullopt;
+    return size_;
+  }
 
+  // CanvasImageSource
+  scoped_refptr<Image> GetSourceImageForCanvas(SourceImageStatus*,
+                                               AccelerationHint,
+                                               const FloatSize&) final {
+    return nullptr;
+  }
   ResourceStatus Status() const final {
-    if (IsCachePending())
+    if (cache_pending_)
       return ResourceStatus::kNotStarted;
     return ResourceStatus::kCached;
   }
+  bool IsAccelerated() const final { return false; }
 
+  // CSSStyleValue
   const CSSValue* ToCSSValue() const final { return nullptr; }
   StyleValueType GetType() const final { return kUnknownType; }
 
@@ -39,22 +48,20 @@ class FakeCSSStyleImageValue : public CSSStyleImageValue {
 }  // namespace
 
 TEST(CSSStyleImageValueTest, PendingCache) {
-  FakeCSSStyleImageValue* style_image_value = new FakeCSSStyleImageValue(
-      CSSImageValue::Create(""), true, IntSize(100, 100));
+  FakeCSSStyleImageValue style_image_value(true, IntSize(100, 100));
   bool is_null;
-  EXPECT_EQ(style_image_value->intrinsicWidth(is_null), 0);
-  EXPECT_EQ(style_image_value->intrinsicHeight(is_null), 0);
-  EXPECT_EQ(style_image_value->intrinsicRatio(is_null), 0);
+  EXPECT_EQ(style_image_value.intrinsicWidth(is_null), 0);
+  EXPECT_EQ(style_image_value.intrinsicHeight(is_null), 0);
+  EXPECT_EQ(style_image_value.intrinsicRatio(is_null), 0);
   EXPECT_TRUE(is_null);
 }
 
 TEST(CSSStyleImageValueTest, ValidLoadedImage) {
-  FakeCSSStyleImageValue* style_image_value = new FakeCSSStyleImageValue(
-      CSSImageValue::Create(""), false, IntSize(480, 120));
+  FakeCSSStyleImageValue style_image_value(false, IntSize(480, 120));
   bool is_null;
-  EXPECT_EQ(style_image_value->intrinsicWidth(is_null), 480);
-  EXPECT_EQ(style_image_value->intrinsicHeight(is_null), 120);
-  EXPECT_EQ(style_image_value->intrinsicRatio(is_null), 4);
+  EXPECT_EQ(style_image_value.intrinsicWidth(is_null), 480);
+  EXPECT_EQ(style_image_value.intrinsicHeight(is_null), 120);
+  EXPECT_EQ(style_image_value.intrinsicRatio(is_null), 4);
   EXPECT_FALSE(is_null);
 }
 

third_party/WebKit/Source/core/css/cssom/CSSURLImageValue.cpp

+// Copyright 2016 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 "core/css/cssom/CSSURLImageValue.h"
+
+#include "bindings/core/v8/ExceptionState.h"
+#include "core/css/CSSImageValue.h"
+#include "core/dom/ExecutionContext.h"
+#include "core/loader/resource/ImageResourceContent.h"
+#include "core/style/StyleImage.h"
+#include "platform/bindings/ScriptState.h"
+
+namespace blink {
+
+CSSURLImageValue* CSSURLImageValue::Create(ScriptState* script_state,
+                                           const AtomicString& url,
+                                           ExceptionState& exception_state) {
+  const auto* execution_context = ExecutionContext::From(script_state);
+  DCHECK(execution_context);
+  KURL parsed_url = execution_context->CompleteURL(url);
+  if (!parsed_url.IsValid()) {
+    exception_state.ThrowTypeError("Failed to parse URL from " + url);
+    return nullptr;
+  }
+  // Use absolute URL for CSSImageValue but keep relative URL for
+  // getter and serialization.
+  return new CSSURLImageValue(
+      *CSSImageValue::Create(url, parsed_url, Referrer()));
+}
+
+CSSURLImageValue* CSSURLImageValue::FromCSSValue(const CSSImageValue& value) {
+  return new CSSURLImageValue(value);
+}
+
+const String& CSSURLImageValue::url() const {
+  return value_->RelativeUrl();
+}
+
+WTF::Optional<IntSize> CSSURLImageValue::IntrinsicSize() const {
+  if (Status() != ResourceStatus::kCached)
+    return WTF::nullopt;
+
+  DCHECK(!value_->IsCachePending());
+  ImageResourceContent* resource_content = value_->CachedImage()->CachedImage();
+  return resource_content
+             ? resource_content->IntrinsicSize(kDoNotRespectImageOrientation)
+             : IntSize(0, 0);
+}
+
+ResourceStatus CSSURLImageValue::Status() const {
+  if (value_->IsCachePending())
+    return ResourceStatus::kNotStarted;
+  return value_->CachedImage()->CachedImage()->GetContentStatus();
+}
+
+scoped_refptr<Image> CSSURLImageValue::GetSourceImageForCanvas(
+    SourceImageStatus*,
+    AccelerationHint,
+    const FloatSize&) {
+  return GetImage();
+}
+
+scoped_refptr<Image> CSSURLImageValue::GetImage() const {
+  if (value_->IsCachePending())
+    return nullptr;
+  // cachedImage can be null if image is StyleInvalidImage
+  ImageResourceContent* cached_image = value_->CachedImage()->CachedImage();
+  if (cached_image) {
+    // getImage() returns the nullImage() if the image is not available yet
+    return cached_image->GetImage()->ImageForDefaultFrame();
+  }
+  return nullptr;
+}
+
+bool CSSURLImageValue::IsAccelerated() const {
+  return GetImage() && GetImage()->IsTextureBacked();
+}
+
+const CSSValue* CSSURLImageValue::ToCSSValue() const {
+  return value_;
+}
+
+void CSSURLImageValue::Trace(blink::Visitor* visitor) {
+  visitor->Trace(value_);
+  CSSStyleImageValue::Trace(visitor);
+}
+
+}  // namespace blink

third_party/WebKit/Source/core/css/cssom/CSSURLImageValue.h

@@ -7,42 +7,46 @@
 
 #include "base/macros.h"
 #include "core/css/cssom/CSSStyleImageValue.h"
-#include "platform/bindings/ScriptState.h"
 
 namespace blink {
 
+class ScriptState;
+class CSSImageValue;
+
 class CORE_EXPORT CSSURLImageValue final : public CSSStyleImageValue {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  static CSSURLImageValue* Create(ScriptState* script_state,
+  static CSSURLImageValue* Create(ScriptState*,
                                   const AtomicString& url,
-                                  ExceptionState& exception_state) {
-    const auto* execution_context = ExecutionContext::From(script_state);
-    DCHECK(execution_context);
-    KURL parsed_url = execution_context->CompleteURL(url);
-    if (!parsed_url.IsValid()) {
-      exception_state.ThrowTypeError("Failed to parse URL from " + url);
-      return nullptr;
-    }
-    // Use absolute URL for CSSImageValue but keep relative URL for
-    // getter and serialization.
-    return new CSSURLImageValue(
-        CSSImageValue::Create(url, parsed_url, Referrer()));
-  }
-  static CSSURLImageValue* Create(const CSSImageValue* image_value) {
-    return new CSSURLImageValue(image_value);
-  }
-
-  StyleValueType GetType() const override { return kURLImageType; }
-
-  const CSSValue* ToCSSValue() const override { return CssImageValue(); }
-
-  const String& url() const { return CssImageValue()->RelativeUrl(); }
+                                  ExceptionState&);
+
+  static CSSURLImageValue* FromCSSValue(const CSSImageValue&);
+
+  const String& url() const;
+
+  // CSSStyleImageValue
+  WTF::Optional<IntSize> IntrinsicSize() const final;
+
+  // CanvasImageSource
+  ResourceStatus Status() const final;
+  scoped_refptr<Image> GetSourceImageForCanvas(SourceImageStatus*,
+                                               AccelerationHint,
+                                               const FloatSize&) final;
+  bool IsAccelerated() const final;
+
+  // CSSStyleValue
+  StyleValueType GetType() const final { return kURLImageType; }
+  const CSSValue* ToCSSValue() const final;
+
+  virtual void Trace(blink::Visitor*);
 
  private:
-  explicit CSSURLImageValue(const CSSImageValue* image_value)
-      : CSSStyleImageValue(image_value) {}
+  explicit CSSURLImageValue(const CSSImageValue& value) : value_(value) {}
+
+  scoped_refptr<Image> GetImage() const;
+
+  Member<const CSSImageValue> value_;
   DISALLOW_COPY_AND_ASSIGN(CSSURLImageValue);
 };
 

third_party/WebKit/Source/core/css/cssom/StyleValueFactory.cpp

@@ -55,7 +55,7 @@ CSSStyleValue* CreateStyleValue(const CSSValue& value) {
     return CSSUnparsedValue::FromCSSValue(*variable_data);
   }
   if (value.IsImageValue()) {
-    return CSSURLImageValue::Create(ToCSSImageValue(value).Clone());
+    return CSSURLImageValue::FromCSSValue(*ToCSSImageValue(value).Clone());
   }
   return nullptr;
 }