LibWeb: Streamline how inline CSS style declarations are constructed

When parsing the "style" attribute on elements, we'd previously ask the
CSS parser for a PropertyOwningCSSStyleDeclaration. Then we'd create a
new ElementCSSInlineStyleDeclaration and transfer the properties from
the first object to the second object.

This patch teaches the parser to make ElementCSSInlineStyleDeclaration
objects directly.

5 files changed, 34 insertions, 36 deletions

diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp
index f41c4c0541..436e8c7c1c 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp
+++ b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp
@@ -23,14 +23,8 @@ String PropertyOwningCSSStyleDeclaration::item(size_t index) const
     return CSS::string_from_property_id(m_properties[index].property_id);
 }
 
-ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element)
-    : PropertyOwningCSSStyleDeclaration({}, {})
-    , m_element(element.make_weak_ptr<DOM::Element>())
-{
-}
-
-ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, PropertyOwningCSSStyleDeclaration& declaration)
-    : PropertyOwningCSSStyleDeclaration(move(declaration.m_properties), move(declaration.m_custom_properties))
+ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties)
+    : PropertyOwningCSSStyleDeclaration(move(properties), move(custom_properties))
     , m_element(element.make_weak_ptr<DOM::Element>())
 {
 }
diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h
index 2383fca7cd..fcf5ff116a 100644
--- a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h
+++ b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h
@@ -86,16 +86,14 @@ private:
 
 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, PropertyOwningCSSStyleDeclaration& declaration) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, declaration)); }
+    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create(DOM::Element& element, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, move(properties), move(custom_properties))); }
     virtual ~ElementInlineCSSStyleDeclaration() override = default;
 
     DOM::Element* element() { return m_element.ptr(); }
     const DOM::Element* element() const { return m_element.ptr(); }
 
 private:
-    explicit ElementInlineCSSStyleDeclaration(DOM::Element&);
-    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, PropertyOwningCSSStyleDeclaration&);
+    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties);
 
     WeakPtr<DOM::Element> m_element;
 };
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
index e85c4c2991..fa52c3ad0b 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -1997,10 +1997,11 @@ Vector<Vector<StyleComponentValueRule>> Parser::parse_a_comma_separated_list_of_
     return lists;
 }
 
-RefPtr<PropertyOwningCSSStyleDeclaration> Parser::parse_as_style_attribute()
+RefPtr<ElementInlineCSSStyleDeclaration> Parser::parse_as_style_attribute(DOM::Element& element)
 {
     auto declarations_and_at_rules = parse_a_list_of_declarations(m_token_stream);
-    return convert_to_style_declaration(declarations_and_at_rules);
+    auto [properties, custom_properties] = extract_properties(declarations_and_at_rules);
+    return ElementInlineCSSStyleDeclaration::create(element, move(properties), move(custom_properties));
 }
 
 Optional<AK::URL> Parser::parse_url_function(StyleComponentValueRule const& component_value, AllowedDataUrlType allowed_data_url_type)
@@ -2169,30 +2170,32 @@ RefPtr<CSSRule> Parser::convert_to_rule(NonnullRefPtr<StyleRule> rule)
     return {};
 }
 
-RefPtr<PropertyOwningCSSStyleDeclaration> Parser::convert_to_style_declaration(Vector<DeclarationOrAtRule> declarations_and_at_rules)
+auto Parser::extract_properties(Vector<DeclarationOrAtRule> const& declarations_and_at_rules) -> PropertiesAndCustomProperties
 {
-    Vector<StyleProperty> properties;
-    HashMap<String, StyleProperty> custom_properties;
-
+    PropertiesAndCustomProperties result;
     for (auto& declaration_or_at_rule : declarations_and_at_rules) {
         if (declaration_or_at_rule.is_at_rule()) {
             dbgln_if(CSS_PARSER_DEBUG, "!!! CSS at-rule is not allowed here!");
             continue;
         }
 
-        auto& declaration = declaration_or_at_rule.m_declaration;
+        auto const& declaration = declaration_or_at_rule.declaration();
 
-        auto maybe_property = convert_to_style_property(declaration);
-        if (maybe_property.has_value()) {
-            auto property = maybe_property.value();
+        if (auto maybe_property = convert_to_style_property(declaration); maybe_property.has_value()) {
+            auto property = maybe_property.release_value();
             if (property.property_id == PropertyID::Custom) {
-                custom_properties.set(property.custom_name, property);
+                result.custom_properties.set(property.custom_name, property);
             } else {
-                properties.append(property);
+                result.properties.append(move(property));
             }
         }
     }
+    return result;
+}
 
+RefPtr<PropertyOwningCSSStyleDeclaration> Parser::convert_to_style_declaration(Vector<DeclarationOrAtRule> declarations_and_at_rules)
+{
+    auto [properties, custom_properties] = extract_properties(declarations_and_at_rules);
     return PropertyOwningCSSStyleDeclaration::create(move(properties), move(custom_properties));
 }
 
@@ -5206,13 +5209,12 @@ RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const& context, StringV
     return parser.parse_as_stylesheet();
 }
 
-RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const& context, StringView css)
+RefPtr<CSS::ElementInlineCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const& context, StringView css, DOM::Element& element)
 {
-    // FIXME: We could save some effort by creating an ElementInlineCSSStyleDeclaration right here.
     if (css.is_empty())
-        return CSS::PropertyOwningCSSStyleDeclaration::create({}, {});
+        return CSS::ElementInlineCSSStyleDeclaration::create(element, {}, {});
     CSS::Parser parser(context, css);
-    return parser.parse_as_style_attribute();
+    return parser.parse_as_style_attribute(element);
 }
 
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const& context, StringView string, CSS::PropertyID property_id)
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
index 786619b029..4e7f51348f 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
@@ -105,7 +105,7 @@ public:
     Vector<StyleComponentValueRule> parse_as_list_of_component_values();
     Vector<Vector<StyleComponentValueRule>> parse_as_comma_separated_list_of_component_values();
 
-    RefPtr<PropertyOwningCSSStyleDeclaration> parse_as_style_attribute();
+    RefPtr<ElementInlineCSSStyleDeclaration> parse_as_style_attribute(DOM::Element&);
 
     enum class SelectorParsingMode {
         Standard,
@@ -343,6 +343,13 @@ private:
     static bool has_ignored_vendor_prefix(StringView);
     static bool is_builtin(StringView);
 
+    struct PropertiesAndCustomProperties {
+        Vector<StyleProperty> properties;
+        HashMap<String, StyleProperty> custom_properties;
+    };
+
+    PropertiesAndCustomProperties extract_properties(Vector<DeclarationOrAtRule> const&);
+
     ParsingContext m_context;
 
     Tokenizer m_tokenizer;
@@ -355,7 +362,7 @@ private:
 namespace Web {
 
 RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const&, StringView);
-RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const&, StringView);
+RefPtr<CSS::ElementInlineCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const&, StringView, DOM::Element&);
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const&, StringView, CSS::PropertyID property_id = CSS::PropertyID::Invalid);
 Optional<CSS::SelectorList> parse_selector(CSS::ParsingContext const&, StringView);
 RefPtr<CSS::CSSRule> parse_css_rule(CSS::ParsingContext const&, StringView);
diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp
index 3a36bdb615..4cb6e7e3af 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Element.cpp
@@ -255,11 +255,8 @@ void Element::parse_attribute(const FlyString& name, const String& value)
         if (m_class_list)
             m_class_list->associated_attribute_changed(value);
     } else if (name == HTML::AttributeNames::style) {
-        auto parsed_style = parse_css_style_attribute(CSS::ParsingContext(document()), value);
-        if (!parsed_style.is_null()) {
-            m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create_and_take_properties_from(*this, parsed_style.release_nonnull());
-            set_needs_style_update(true);
-        }
+        m_inline_style = parse_css_style_attribute(CSS::ParsingContext(document()), value, *this);
+        set_needs_style_update(true);
     }
 }
 
@@ -452,7 +449,7 @@ void Element::set_shadow_root(RefPtr<ShadowRoot> shadow_root)
 NonnullRefPtr<CSS::CSSStyleDeclaration> Element::style_for_bindings()
 {
     if (!m_inline_style)
-        m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this);
+        m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this, {}, {});
     return *m_inline_style;
 }