LibWeb: Make CSSStyleDeclaration an abstract class

This patch moves the CSS property+value storage down to a new subclass
of CSSStyleDeclaration called PropertyOwningCSSStyleDeclaration.

The JavaScript wrapper for CSSStyleDeclaration now calls virtual
functions on the C++ object.

This is preparation for supporting computed style CSSStyleDeclaration
objects which won't have internal property storage, but rather an
internal element pointer. :^)

Userland/Libraries/LibWeb/Bindings/CSSStyleDeclarationWrapperCustom.cpp

@@ -4,9 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/ScopeGuard.h>
 #include <LibWeb/Bindings/CSSStyleDeclarationWrapper.h>
-#include <LibWeb/CSS/Parser/Parser.h>
 #include <LibWeb/DOM/Element.h>
 
 namespace Web::Bindings {
@@ -28,10 +26,8 @@ JS::Value CSSStyleDeclarationWrapper::internal_get(JS::PropertyName const& name,
     auto property_id = CSS::property_id_from_string(name.to_string());
     if (property_id == CSS::PropertyID::Invalid)
         return Base::internal_get(name, receiver);
-    for (auto& property : impl().properties()) {
+    if (auto maybe_property = impl().property(property_id); maybe_property.has_value())
-        if (property.property_id == property_id)
+        return js_string(vm(), maybe_property->value->to_string());
-            return js_string(vm(), property.value->to_string());
-    }
     return js_string(vm(), String::empty());
 }
 
@@ -48,32 +44,7 @@ bool CSSStyleDeclarationWrapper::internal_set(JS::PropertyName const& name, JS::
     if (vm().exception())
         return false;
 
-    auto new_value = parse_css_value(CSS::ParsingContext {}, css_text, property_id);
+    return impl().set_property(property_id, css_text);
-    // FIXME: What are we supposed to do if we can't parse it?
-    if (!new_value)
-        return false;
-
-    ScopeGuard style_invalidation_guard = [&] {
-        auto& declaration = verify_cast<CSS::ElementInlineCSSStyleDeclaration>(impl());
-        if (auto* element = declaration.element())
-            element->invalidate_style();
-    };
-
-    // FIXME: I don't think '!important' is being handled correctly here..
-
-    for (auto& property : impl().m_properties) {
-        if (property.property_id == property_id) {
-            property.value = new_value.release_nonnull();
-            return true;
-        }
-    }
-
-    impl().m_properties.append(CSS::StyleProperty {
-        .property_id = property_id,
-        .value = new_value.release_nonnull(),
-        .important = false,
-    });
-    return true;
 }
 
 }

Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp

@@ -5,21 +5,26 @@
  */
 
 #include <LibWeb/CSS/CSSStyleDeclaration.h>
+#include <LibWeb/CSS/Parser/Parser.h>
 #include <LibWeb/DOM/Element.h>
 
 namespace Web::CSS {
 
-CSSStyleDeclaration::CSSStyleDeclaration(Vector<StyleProperty>&& properties, HashMap<String, StyleProperty>&& custom_properties)
+PropertyOwningCSSStyleDeclaration::PropertyOwningCSSStyleDeclaration(Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties)
     : m_properties(move(properties))
     , m_custom_properties(move(custom_properties))
 {
 }
 
+PropertyOwningCSSStyleDeclaration::~PropertyOwningCSSStyleDeclaration()
+{
+}
+
 CSSStyleDeclaration::~CSSStyleDeclaration()
 {
 }
 
-String CSSStyleDeclaration::item(size_t index) const
+String PropertyOwningCSSStyleDeclaration::item(size_t index) const
 {
     if (index >= m_properties.size())
         return {};
@@ -27,13 +32,13 @@ String CSSStyleDeclaration::item(size_t index) const
 }
 
 ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element)
-    : CSSStyleDeclaration({}, {})
+    : PropertyOwningCSSStyleDeclaration({}, {})
     , m_element(element.make_weak_ptr<DOM::Element>())
 {
 }
 
-ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, CSSStyleDeclaration& declaration)
+ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, PropertyOwningCSSStyleDeclaration& declaration)
-    : CSSStyleDeclaration(move(declaration.m_properties), move(declaration.m_custom_properties))
+    : PropertyOwningCSSStyleDeclaration(move(declaration.m_properties), move(declaration.m_custom_properties))
     , m_element(element.make_weak_ptr<DOM::Element>())
 {
 }
@@ -42,4 +47,48 @@ ElementInlineCSSStyleDeclaration::~ElementInlineCSSStyleDeclaration()
 {
 }
 
+size_t PropertyOwningCSSStyleDeclaration::length() const
+{
+    return m_properties.size();
+}
+
+Optional<StyleProperty> PropertyOwningCSSStyleDeclaration::property(PropertyID property_id) const
+{
+    for (auto& property : m_properties) {
+        if (property.property_id == property_id)
+            return property;
+    }
+    return {};
+}
+
+bool PropertyOwningCSSStyleDeclaration::set_property(PropertyID property_id, StringView css_text)
+{
+    auto new_value = parse_css_value(CSS::ParsingContext {}, css_text, property_id);
+    // FIXME: What are we supposed to do if we can't parse it?
+    if (!new_value)
+        return false;
+
+    ScopeGuard style_invalidation_guard = [&] {
+        auto& declaration = verify_cast<CSS::ElementInlineCSSStyleDeclaration>(*this);
+        if (auto* element = declaration.element())
+            element->invalidate_style();
+    };
+
+    // FIXME: I don't think '!important' is being handled correctly here..
+
+    for (auto& property : m_properties) {
+        if (property.property_id == property_id) {
+            property.value = new_value.release_nonnull();
+            return true;
+        }
+    }
+
+    m_properties.append(CSS::StyleProperty {
+        .property_id = property_id,
+        .value = new_value.release_nonnull(),
+        .important = false,
+    });
+    return true;
+}
+
 }

Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h

@@ -26,35 +26,51 @@ class CSSStyleDeclaration
 public:
     using WrapperType = Bindings::CSSStyleDeclarationWrapper;
 
-    static NonnullRefPtr<CSSStyleDeclaration> create(Vector<StyleProperty>&& properties, HashMap<String, StyleProperty>&& custom_properties)
-    {
-        return adopt_ref(*new CSSStyleDeclaration(move(properties), move(custom_properties)));
-    }
-
     virtual ~CSSStyleDeclaration();
 
-    const Vector<StyleProperty>& properties() const { return m_properties; }
+    virtual size_t length() const = 0;
-    const Optional<StyleProperty> custom_property(const String& custom_property_name) const { return m_custom_properties.get(custom_property_name); }
+    virtual String item(size_t index) const = 0;
-    size_t custom_property_count() const { return m_custom_properties.size(); };
 
-    size_t length() const { return m_properties.size(); }
+    virtual Optional<StyleProperty> property(PropertyID) const = 0;
-    String item(size_t index) const;
+    virtual bool set_property(PropertyID, StringView css_text) = 0;
 
 protected:
-    explicit CSSStyleDeclaration(Vector<StyleProperty>&&, HashMap<String, StyleProperty>&&);
+    CSSStyleDeclaration() { }
+};
+
+class PropertyOwningCSSStyleDeclaration : public CSSStyleDeclaration {
+    friend class ElementInlineCSSStyleDeclaration;
+
+public:
+    static NonnullRefPtr<PropertyOwningCSSStyleDeclaration> create(Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties)
+    {
+        return adopt_ref(*new PropertyOwningCSSStyleDeclaration(move(properties), move(custom_properties)));
+    }
+
+    virtual ~PropertyOwningCSSStyleDeclaration() override;
+
+    virtual size_t length() const override;
+    virtual String item(size_t index) const override;
+
+    virtual Optional<StyleProperty> property(PropertyID) const override;
+    virtual bool set_property(PropertyID, StringView css_text) override;
+
+    const Vector<StyleProperty>& properties() const { return m_properties; }
+    Optional<StyleProperty> custom_property(const String& custom_property_name) const { return m_custom_properties.get(custom_property_name); }
+    size_t custom_property_count() const { return m_custom_properties.size(); }
+
+protected:
+    explicit PropertyOwningCSSStyleDeclaration(Vector<StyleProperty>, HashMap<String, StyleProperty>);
 
 private:
-    friend class ElementInlineCSSStyleDeclaration;
-    friend class Bindings::CSSStyleDeclarationWrapper;
-
     Vector<StyleProperty> m_properties;
     HashMap<String, StyleProperty> m_custom_properties;
 };
 
-class ElementInlineCSSStyleDeclaration final : public CSSStyleDeclaration {
+class ElementInlineCSSStyleDeclaration final : public PropertyOwningCSSStyleDeclaration {
 public:
     static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create(DOM::Element& element) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element)); }
-    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create_and_take_properties_from(DOM::Element& element, CSSStyleDeclaration& declaration) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, declaration)); }
+    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create_and_take_properties_from(DOM::Element& element, PropertyOwningCSSStyleDeclaration& declaration) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, declaration)); }
     virtual ~ElementInlineCSSStyleDeclaration() override;
 
     DOM::Element* element() { return m_element.ptr(); }
@@ -62,7 +78,7 @@ public:
 
 private:
     explicit ElementInlineCSSStyleDeclaration(DOM::Element&);
-    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, CSSStyleDeclaration&);
+    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, PropertyOwningCSSStyleDeclaration&);
 
     WeakPtr<DOM::Element> m_element;
 };

Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -1085,13 +1085,13 @@ Optional<StyleProperty> Parser::parse_a_declaration(TokenStream<T>& tokens)
     return {};
 }
 
-RefPtr<CSSStyleDeclaration> Parser::parse_as_list_of_declarations()
+RefPtr<PropertyOwningCSSStyleDeclaration> Parser::parse_as_list_of_declarations()
 {
     return parse_a_list_of_declarations(m_token_stream);
 }
 
 template<typename T>
-RefPtr<CSSStyleDeclaration> Parser::parse_a_list_of_declarations(TokenStream<T>& tokens)
+RefPtr<PropertyOwningCSSStyleDeclaration> Parser::parse_a_list_of_declarations(TokenStream<T>& tokens)
 {
     dbgln_if(CSS_PARSER_DEBUG, "Parser::parse_as_list_of_declarations");
 
@@ -1119,7 +1119,7 @@ RefPtr<CSSStyleDeclaration> Parser::parse_a_list_of_declarations(TokenStream<T>&
         }
     }
 
-    return CSSStyleDeclaration::create(move(properties), move(custom_properties));
+    return PropertyOwningCSSStyleDeclaration::create(move(properties), move(custom_properties));
 }
 
 Optional<StyleComponentValueRule> Parser::parse_as_component_value()
@@ -1295,7 +1295,7 @@ RefPtr<CSSRule> Parser::convert_to_rule(NonnullRefPtr<StyleRule> rule)
     return {};
 }
 
-RefPtr<CSSStyleDeclaration> Parser::convert_to_declaration(NonnullRefPtr<StyleBlockRule> block)
+RefPtr<PropertyOwningCSSStyleDeclaration> Parser::convert_to_declaration(NonnullRefPtr<StyleBlockRule> block)
 {
     dbgln_if(CSS_PARSER_DEBUG, "Parser::convert_to_declaration");
 
@@ -3487,10 +3487,10 @@ RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const& context, StringV
     return parser.parse_as_stylesheet();
 }
 
-RefPtr<CSS::CSSStyleDeclaration> parse_css_declaration(CSS::ParsingContext const& context, StringView const& css)
+RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_declaration(CSS::ParsingContext const& context, StringView const& css)
 {
     if (css.is_empty())
-        return CSS::CSSStyleDeclaration::create({}, {});
+        return CSS::PropertyOwningCSSStyleDeclaration::create({}, {});
     CSS::Parser parser(context, css);
     return parser.parse_as_list_of_declarations();
 }

Userland/Libraries/LibWeb/CSS/Parser/Parser.h

@@ -86,7 +86,7 @@ public:
     // Used in @supports conditions. [CSS3-CONDITIONAL]
     Optional<StyleProperty> parse_as_declaration();
     // For the contents of a style attribute, which parses text into the contents of a single style rule.
-    RefPtr<CSSStyleDeclaration> parse_as_list_of_declarations();
+    RefPtr<PropertyOwningCSSStyleDeclaration> parse_as_list_of_declarations();
     // For things that need to consume a single value, like the parsing rules for attr().
     Optional<StyleComponentValueRule> parse_as_component_value();
     // For the contents of presentational attributes, which parse text into a single declaration’s value, or for parsing a stand-alone selector [SELECT] or list of Media Queries [MEDIAQ], as in Selectors API or the media HTML attribute.
@@ -109,7 +109,7 @@ private:
     template<typename T>
     Optional<StyleProperty> parse_a_declaration(TokenStream<T>&);
     template<typename T>
-    RefPtr<CSSStyleDeclaration> parse_a_list_of_declarations(TokenStream<T>&);
+    RefPtr<PropertyOwningCSSStyleDeclaration> parse_a_list_of_declarations(TokenStream<T>&);
     template<typename T>
     Optional<StyleComponentValueRule> parse_a_component_value(TokenStream<T>&);
     template<typename T>
@@ -160,7 +160,7 @@ private:
     [[nodiscard]] NonnullRefPtr<StyleFunctionRule> consume_a_function(TokenStream<T>&);
 
     [[nodiscard]] RefPtr<CSSRule> convert_to_rule(NonnullRefPtr<StyleRule>);
-    [[nodiscard]] RefPtr<CSSStyleDeclaration> convert_to_declaration(NonnullRefPtr<StyleBlockRule>);
+    [[nodiscard]] RefPtr<PropertyOwningCSSStyleDeclaration> convert_to_declaration(NonnullRefPtr<StyleBlockRule>);
     [[nodiscard]] Optional<StyleProperty> convert_to_style_property(StyleDeclarationRule&);
 
     static Optional<float> try_parse_float(StringView string);
@@ -228,7 +228,7 @@ private:
 namespace Web {
 
 RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const&, StringView const&);
-RefPtr<CSS::CSSStyleDeclaration> parse_css_declaration(CSS::ParsingContext const&, StringView const&);
+RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_declaration(CSS::ParsingContext const&, StringView const&);
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const&, StringView const&, CSS::PropertyID property_id = CSS::PropertyID::Invalid);
 Optional<CSS::SelectorList> parse_selector(CSS::ParsingContext const&, StringView const&);
 

Userland/Libraries/LibWeb/CSS/StyleResolver.cpp

@@ -514,7 +514,7 @@ StyleResolver::CustomPropertyResolutionTuple StyleResolver::resolve_custom_prope
         if (match.specificity < parent_resolved.specificity)
             continue;
 
-        auto custom_property_style = match.rule->declaration().custom_property(custom_property_name);
+        auto custom_property_style = verify_cast<PropertyOwningCSSStyleDeclaration>(match.rule->declaration()).custom_property(custom_property_name);
         if (custom_property_style.has_value()) {
             element.add_custom_property(custom_property_name, { custom_property_style.value(), match.specificity });
             return { custom_property_style.value(), match.specificity };
@@ -548,7 +548,7 @@ NonnullRefPtr<StyleProperties> StyleResolver::resolve_style(DOM::Element& elemen
     sort_matching_rules(matching_rules);
 
     for (auto& match : matching_rules) {
-        for (auto& property : match.rule->declaration().properties()) {
+        for (auto& property : verify_cast<PropertyOwningCSSStyleDeclaration>(match.rule->declaration()).properties()) {
             auto property_value = property.value;
             if (property.value->is_custom_property()) {
                 auto prop = reinterpret_cast<CSS::CustomStyleValue const*>(property.value.ptr());
@@ -562,7 +562,7 @@ NonnullRefPtr<StyleProperties> StyleResolver::resolve_style(DOM::Element& elemen
         }
     }
 
-    if (auto* inline_style = element.inline_style()) {
+    if (auto* inline_style = verify_cast<ElementInlineCSSStyleDeclaration>(element.inline_style())) {
         for (auto& property : inline_style->properties()) {
             set_property_expanding_shorthands(style, property.property_id, property.value, m_document);
         }

Userland/Libraries/LibWeb/DOM/Window.cpp

@@ -192,7 +192,7 @@ NonnullRefPtr<CSS::CSSStyleDeclaration> Window::get_computed_style(DOM::Element&
     dbgln("Generating CSS computed style for {} @ {:p}", element.node_name(), &element);
     Vector<CSS::StyleProperty> properties;
     HashMap<String, CSS::StyleProperty> custom_properties;
-    return CSS::CSSStyleDeclaration::create(move(properties), move(custom_properties));
+    return CSS::PropertyOwningCSSStyleDeclaration::create(move(properties), move(custom_properties));
 }
 
 NonnullRefPtr<CSS::MediaQueryList> Window::match_media(String media)

Userland/Libraries/LibWeb/Dump.cpp

@@ -499,7 +499,7 @@ void dump_style_rule(StringBuilder& builder, CSS::CSSStyleRule const& rule)
         dump_selector(builder, selector);
     }
     builder.append("  Declarations:\n");
-    for (auto& property : rule.declaration().properties()) {
+    for (auto& property : verify_cast<CSS::PropertyOwningCSSStyleDeclaration>(rule.declaration()).properties()) {
         builder.appendff("    {}: '{}'\n", CSS::string_from_property_id(property.property_id), property.value->to_string());
     }
 }

Userland/Libraries/LibWeb/Forward.h

@@ -22,6 +22,7 @@ class CSSStyleRule;
 class CSSStyleSheet;
 class ElementInlineCSSStyleDeclaration;
 class Length;
+class PropertyOwningCSSStyleDeclaration;
 class Screen;
 class Selector;
 class StyleProperties;