LibWeb/CSS: Store StyleValue pointer instead of string on CSSStyleValue

When setting style to a CSSStyleValue we need to convert it to a
StyleValue. If we already have one, we might as well use it avoid the
work of serialization and re-parsing.

I realised I misunderstood what "constructed from a USVString" means, so
I've adjusted based on that. It does raise a question on what the source
USVString is if that string resulted in multiple CSSStyleValues being
created - see the linked issue.

Libraries/LibWeb/CSS/CSSImageValue.cpp

@@ -7,20 +7,20 @@
 #include "CSSImageValue.h"
 #include <LibWeb/Bindings/CSSImageValuePrototype.h>
 #include <LibWeb/Bindings/Intrinsics.h>
+#include <LibWeb/CSS/StyleValues/StyleValue.h>
 #include <LibWeb/WebIDL/ExceptionOr.h>
 
 namespace Web::CSS {
 
 GC_DEFINE_ALLOCATOR(CSSImageValue);
 
-GC::Ref<CSSImageValue> CSSImageValue::create(JS::Realm& realm, String constructed_from_string)
+GC::Ref<CSSImageValue> CSSImageValue::create(JS::Realm& realm, NonnullRefPtr<StyleValue const> source_value)
 {
-    return realm.create<CSSImageValue>(realm, move(constructed_from_string));
+    return realm.create<CSSImageValue>(realm, move(source_value));
 }
 
-CSSImageValue::CSSImageValue(JS::Realm& realm, String constructed_from_string)
+CSSImageValue::CSSImageValue(JS::Realm& realm, NonnullRefPtr<StyleValue const> source_value)
-    : CSSStyleValue(realm)
+    : CSSStyleValue(realm, move(source_value))
-    , m_constructed_from_string(move(constructed_from_string))
 {
 }
 
@@ -34,8 +34,8 @@ void CSSImageValue::initialize(JS::Realm& realm)
 WebIDL::ExceptionOr<String> CSSImageValue::to_string() const
 {
     // AD-HOC: The spec doesn't say how to serialize this, as it's intentionally a black box.
-    //         We just serialize the source string that was used to construct this.
+    //         We just rely on CSSStyleValue serializing its held StyleValue.
-    return m_constructed_from_string;
+    return Base::to_string();
 }
 
 }

Libraries/LibWeb/CSS/CSSImageValue.h

@@ -16,18 +16,16 @@ class CSSImageValue final : public CSSStyleValue {
     GC_DECLARE_ALLOCATOR(CSSImageValue);
 
 public:
-    [[nodiscard]] static GC::Ref<CSSImageValue> create(JS::Realm&, String constructed_from_string);
+    [[nodiscard]] static GC::Ref<CSSImageValue> create(JS::Realm&, NonnullRefPtr<StyleValue const> source_value);
 
     virtual ~CSSImageValue() override = default;
 
     virtual WebIDL::ExceptionOr<String> to_string() const override;
 
 private:
-    explicit CSSImageValue(JS::Realm&, String constructed_from_string);
+    explicit CSSImageValue(JS::Realm&, NonnullRefPtr<StyleValue const> source_value);
 
     virtual void initialize(JS::Realm&) override;
-
-    String m_constructed_from_string;
 };
 
 }

Libraries/LibWeb/CSS/CSSStyleValue.cpp

@@ -9,15 +9,16 @@
 #include <LibWeb/Bindings/Intrinsics.h>
 #include <LibWeb/CSS/Parser/Parser.h>
 #include <LibWeb/CSS/PropertyNameAndID.h>
+#include <LibWeb/CSS/StyleValues/StyleValue.h>
 #include <LibWeb/WebIDL/ExceptionOr.h>
 
 namespace Web::CSS {
 
 GC_DEFINE_ALLOCATOR(CSSStyleValue);
 
-GC::Ref<CSSStyleValue> CSSStyleValue::create(JS::Realm& realm, FlyString associated_property, String constructed_from_string)
+GC::Ref<CSSStyleValue> CSSStyleValue::create(JS::Realm& realm, FlyString associated_property, NonnullRefPtr<StyleValue const> source_value)
 {
-    return realm.create<CSSStyleValue>(realm, move(associated_property), move(constructed_from_string));
+    return realm.create<CSSStyleValue>(realm, move(associated_property), move(source_value));
 }
 
 CSSStyleValue::CSSStyleValue(JS::Realm& realm)
@@ -25,10 +26,16 @@ CSSStyleValue::CSSStyleValue(JS::Realm& realm)
 {
 }
 
-CSSStyleValue::CSSStyleValue(JS::Realm& realm, FlyString associated_property, String constructed_from_string)
+CSSStyleValue::CSSStyleValue(JS::Realm& realm, NonnullRefPtr<StyleValue const> source_value)
+    : PlatformObject(realm)
+    , m_source_value(move(source_value))
+{
+}
+
+CSSStyleValue::CSSStyleValue(JS::Realm& realm, FlyString associated_property, NonnullRefPtr<StyleValue const> source_value)
     : PlatformObject(realm)
     , m_associated_property(move(associated_property))
-    , m_constructed_from_string(move(constructed_from_string))
+    , m_source_value(move(source_value))
 {
 }
 
@@ -38,6 +45,8 @@ void CSSStyleValue::initialize(JS::Realm& realm)
     Base::initialize(realm);
 }
 
+CSSStyleValue::~CSSStyleValue() = default;
+
 // https://drafts.css-houdini.org/css-typed-om-1/#dom-cssstylevalue-parse
 WebIDL::ExceptionOr<GC::Ref<CSSStyleValue>> CSSStyleValue::parse(JS::VM& vm, FlyString const& property, String css_text)
 {
@@ -83,6 +92,8 @@ WebIDL::ExceptionOr<Variant<GC::Ref<CSSStyleValue>, GC::RootVector<GC::Ref<CSSSt
     reified_values.append(whole_value->reify(*vm.current_realm(), property->name()));
 
     // 6. If parseMultiple is false, return values[0]. Otherwise, return values.
+    // FIXME: We need to somehow store the source css_text on the returned CSSStyleValue.
+    //        https://github.com/w3c/css-houdini-drafts/issues/1156
     if (parse_multiple == ParseMultiple::No)
         return reified_values.take_first();
     return reified_values;
@@ -91,10 +102,10 @@ WebIDL::ExceptionOr<Variant<GC::Ref<CSSStyleValue>, GC::RootVector<GC::Ref<CSSSt
 // https://drafts.css-houdini.org/css-typed-om-1/#stylevalue-serialization
 WebIDL::ExceptionOr<String> CSSStyleValue::to_string() const
 {
-    // if the value was constructed from a USVString
+    // FIXME: if the value was constructed from a USVString
-    if (m_constructed_from_string.has_value()) {
+    // NB: Basically, if this was constructed with "parse a CSSStyleValue", regardless of what CSSStyleValue type it is now.
+    {
         // the serialization is the USVString from which the value was constructed.
-        return m_constructed_from_string.value();
     }
     // otherwise, if the value was constructed using an IDL constructor
     {
@@ -102,6 +113,9 @@ WebIDL::ExceptionOr<String> CSSStyleValue::to_string() const
         // NB: This is handled by subclasses overriding this to_string() method.
     }
     // FIXME: otherwise, if the value was extracted from the CSSOM
+    // NB: For CSSStyleValue itself, we use the source value we were created from.
+    if (m_source_value)
+        return m_source_value->to_string(SerializationMode::Normal);
     {
         // the serialization is specified in §6.7 Serialization from CSSOM Values below.
     }

Libraries/LibWeb/CSS/CSSStyleValue.h

@@ -17,12 +17,15 @@ class CSSStyleValue : public Bindings::PlatformObject {
     GC_DECLARE_ALLOCATOR(CSSStyleValue);
 
 public:
-    [[nodiscard]] static GC::Ref<CSSStyleValue> create(JS::Realm&, FlyString associated_property, String constructed_from_string);
+    [[nodiscard]] static GC::Ref<CSSStyleValue> create(JS::Realm&, FlyString associated_property, NonnullRefPtr<StyleValue const>);
 
-    virtual ~CSSStyleValue() override = default;
+    virtual ~CSSStyleValue() override;
 
     virtual void initialize(JS::Realm&) override;
 
+    Optional<FlyString> const& associated_property() const { return m_associated_property; }
+    RefPtr<StyleValue const> const& source_value() const { return m_source_value; }
+
     static WebIDL::ExceptionOr<GC::Ref<CSSStyleValue>> parse(JS::VM&, FlyString const& property, String css_text);
     static WebIDL::ExceptionOr<GC::RootVector<GC::Ref<CSSStyleValue>>> parse_all(JS::VM&, FlyString const& property, String css_text);
 
@@ -30,9 +33,10 @@ public:
 
 protected:
     explicit CSSStyleValue(JS::Realm&);
+    explicit CSSStyleValue(JS::Realm&, NonnullRefPtr<StyleValue const> source_value);
 
 private:
-    explicit CSSStyleValue(JS::Realm&, FlyString associated_property, String constructed_from_string);
+    explicit CSSStyleValue(JS::Realm&, FlyString associated_property, NonnullRefPtr<StyleValue const> source_value);
 
     enum class ParseMultiple : u8 {
         No,
@@ -43,7 +47,7 @@ private:
     // https://drafts.css-houdini.org/css-typed-om-1/#dom-cssstylevalue-associatedproperty-slot
     Optional<FlyString> m_associated_property;
 
-    Optional<String> m_constructed_from_string;
+    RefPtr<StyleValue const> m_source_value;
 };
 
 }

Libraries/LibWeb/CSS/StyleValues/AbstractImageStyleValue.cpp

@@ -13,7 +13,7 @@ namespace Web::CSS {
 GC::Ref<CSSStyleValue> AbstractImageStyleValue::reify(JS::Realm& realm, FlyString const&) const
 {
     // AD-HOC: There's no spec description of how to reify as a CSSImageValue.
-    return CSSImageValue::create(realm, to_string(SerializationMode::Normal));
+    return CSSImageValue::create(realm, *this);
 }
 
 }

Libraries/LibWeb/CSS/StyleValues/StyleValue.cpp

@@ -148,7 +148,7 @@ Vector<Parser::ComponentValue> StyleValue::tokenize() const
 GC::Ref<CSSStyleValue> StyleValue::reify(JS::Realm& realm, FlyString const& associated_property) const
 {
     // 1. Return a new CSSStyleValue object representing value whose [[associatedProperty]] internal slot is set to property.
-    return CSSStyleValue::create(realm, associated_property, to_string(SerializationMode::Normal));
+    return CSSStyleValue::create(realm, associated_property, *this);
 }
 
 }