LibWeb: Make CSSRuleList GC-allocated

Author: https://github.com/awesomekling
Commit: 5d6cb9cbdb
Pull-request: https://github.com/SerenityOS/serenity/pull/14816
Reviewed-by: https://github.com/ADKaster
Reviewed-by: https://github.com/linusg ✅

Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp

@@ -84,6 +84,9 @@ static bool impl_is_wrapper(Type const& type)
     if (type.name == "StyleSheetList"sv)
         return true;
 
+    if (type.name == "CSSRuleList"sv)
+        return true;
+
     return false;
 }
 

Userland/Libraries/LibWeb/CSS/CSSGroupingRule.cpp

@@ -4,13 +4,15 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <LibWeb/Bindings/MainThreadVM.h>
 #include <LibWeb/CSS/CSSGroupingRule.h>
 #include <LibWeb/CSS/CSSRuleList.h>
 
 namespace Web::CSS {
 
 CSSGroupingRule::CSSGroupingRule(NonnullRefPtrVector<CSSRule>&& rules)
-    : m_rules(CSSRuleList::create(move(rules)))
+    // FIXME: Use the same window object for the rule list.
+    : m_rules(JS::make_handle(CSSRuleList::create(Bindings::main_thread_internal_window_object(), move(rules))))
 {
     for (auto& rule : *m_rules)
         rule.set_parent_rule(this);

Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h

@@ -23,9 +23,9 @@ public:
 
     virtual ~CSSGroupingRule() = default;
 
-    CSSRuleList const& css_rules() const { return m_rules; }
+    CSSRuleList const& css_rules() const { return *m_rules; }
-    CSSRuleList& css_rules() { return m_rules; }
+    CSSRuleList& css_rules() { return *m_rules; }
-    NonnullRefPtr<CSSRuleList> css_rules_for_bindings() { return m_rules; }
+    CSSRuleList* css_rules_for_bindings() { return m_rules.cell(); }
     DOM::ExceptionOr<u32> insert_rule(StringView rule, u32 index = 0);
     DOM::ExceptionOr<void> delete_rule(u32 index);
 
@@ -37,7 +37,7 @@ protected:
     explicit CSSGroupingRule(NonnullRefPtrVector<CSSRule>&&);
 
 private:
-    NonnullRefPtr<CSSRuleList> m_rules;
+    JS::Handle<CSSRuleList> m_rules;
 };
 
 }

Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp

@@ -5,6 +5,8 @@
  */
 
 #include <AK/TypeCasts.h>
+#include <LibWeb/Bindings/CSSRuleListPrototype.h>
+#include <LibWeb/Bindings/WindowObject.h>
 #include <LibWeb/CSS/CSSImportRule.h>
 #include <LibWeb/CSS/CSSMediaRule.h>
 #include <LibWeb/CSS/CSSRuleList.h>
@@ -13,8 +15,14 @@
 
 namespace Web::CSS {
 
-CSSRuleList::CSSRuleList(NonnullRefPtrVector<CSSRule>&& rules)
+CSSRuleList* CSSRuleList::create(Bindings::WindowObject& window_object, NonnullRefPtrVector<Web::CSS::CSSRule>&& rules)
-    : m_rules(move(rules))
+{
+    return window_object.heap().allocate<CSSRuleList>(window_object.realm(), window_object, move(rules));
+}
+
+CSSRuleList::CSSRuleList(Bindings::WindowObject& window_object, NonnullRefPtrVector<CSSRule>&& rules)
+    : Bindings::LegacyPlatformObject(window_object.ensure_web_prototype<Bindings::CSSRuleListPrototype>("CSSRuleList"))
+    , m_rules(move(rules))
 {
 }
 
@@ -149,4 +157,9 @@ bool CSSRuleList::evaluate_media_queries(HTML::Window const& window)
     return any_media_queries_changed_match_state;
 }
 
+JS::Value CSSRuleList::item_value(size_t index) const
+{
+    return item(index);
+}
+
 }

Userland/Libraries/LibWeb/CSS/CSSRuleList.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021-2022, Sam Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2022, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -9,8 +10,8 @@
 #include <AK/Function.h>
 #include <AK/Iterator.h>
 #include <AK/NonnullRefPtrVector.h>
-#include <AK/RefCounted.h>
 #include <AK/RefPtr.h>
+#include <LibWeb/Bindings/LegacyPlatformObject.h>
 #include <LibWeb/CSS/CSSRule.h>
 #include <LibWeb/DOM/ExceptionOr.h>
 #include <LibWeb/Forward.h>
@@ -18,16 +19,14 @@
 namespace Web::CSS {
 
 // https://www.w3.org/TR/cssom/#the-cssrulelist-interface
-class CSSRuleList
+class CSSRuleList : public Bindings::LegacyPlatformObject {
-    : public RefCounted<CSSRuleList>
+    JS_OBJECT(CSSRuleList, Bindings::LegacyPlatformObject);
-    , public Bindings::Wrappable {
-public:
-    using WrapperType = Bindings::CSSRuleListWrapper;
 
-    static NonnullRefPtr<CSSRuleList> create(NonnullRefPtrVector<CSSRule>&& rules)
+public:
-    {
+    CSSRuleList& impl() { return *this; }
-        return adopt_ref(*new CSSRuleList(move(rules)));
+
-    }
+    static CSSRuleList* create(Bindings::WindowObject&, NonnullRefPtrVector<CSSRule>&& rules);
+    CSSRuleList(Bindings::WindowObject&, NonnullRefPtrVector<CSSRule>&&);
     ~CSSRuleList() = default;
 
     RefPtr<CSSRule> item(size_t index) const
@@ -47,7 +46,8 @@ public:
     ConstIterator const end() const { return m_rules.end(); }
     Iterator end() { return m_rules.end(); }
 
-    bool is_supported_property_index(u32 index) const;
+    virtual bool is_supported_property_index(u32 index) const override;
+    virtual JS::Value item_value(size_t index) const override;
 
     DOM::ExceptionOr<void> remove_a_css_rule(u32 index);
     DOM::ExceptionOr<unsigned> insert_a_css_rule(Variant<StringView, NonnullRefPtr<CSSRule>>, u32 index);
@@ -57,9 +57,12 @@ public:
     bool evaluate_media_queries(HTML::Window const&);
 
 private:
-    explicit CSSRuleList(NonnullRefPtrVector<CSSRule>&&);
-
     NonnullRefPtrVector<CSSRule> m_rules;
 };
 
 }
+
+namespace Web::Bindings {
+inline JS::Object* wrap(JS::GlobalObject&, Web::CSS::CSSRuleList& object) { return &object; }
+using CSSRuleListWrapper = Web::CSS::CSSRuleList;
+}

Userland/Libraries/LibWeb/CSS/CSSRuleList.idl

@@ -1,6 +1,6 @@
 #import <CSS/CSSRule.idl>
 
-[Exposed=Window]
+[Exposed=Window, NoInstanceWrapper]
 interface CSSRuleList {
 
     getter CSSRule? item(unsigned long index);

Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp

@@ -20,7 +20,7 @@ CSSStyleSheet* CSSStyleSheet::create(Bindings::WindowObject& window_object, Nonn
 
 CSSStyleSheet::CSSStyleSheet(Bindings::WindowObject& window_object, NonnullRefPtrVector<CSSRule> rules, Optional<AK::URL> location)
     : StyleSheet(window_object)
-    , m_rules(CSSRuleList::create(move(rules)))
+    , m_rules(CSSRuleList::create(window_object, move(rules)))
 {
     set_prototype(&window_object.ensure_web_prototype<Bindings::CSSStyleSheetPrototype>("CSSStyleSheet"));
 

Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h

@@ -35,9 +35,9 @@ public:
 
     virtual String type() const override { return "text/css"; }
 
-    CSSRuleList const& rules() const { return m_rules; }
+    CSSRuleList const& rules() const { return *m_rules; }
-    CSSRuleList& rules() { return m_rules; }
+    CSSRuleList& rules() { return *m_rules; }
-    void set_rules(NonnullRefPtr<CSSRuleList> rules) { m_rules = move(rules); }
+    void set_rules(CSSRuleList& rules) { m_rules = &rules; }
 
     CSSRuleList* css_rules() { return m_rules; }
     CSSRuleList const* css_rules() const { return m_rules; }
@@ -55,7 +55,7 @@ public:
 private:
     virtual void visit_edges(Cell::Visitor&) override;
 
-    NonnullRefPtr<CSSRuleList> m_rules;
+    CSSRuleList* m_rules { nullptr };
 
     WeakPtr<CSSRule> m_owner_css_rule;
 

Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -279,12 +279,13 @@ NonnullRefPtr<Document> Document::create(AK::URL const& url)
 Document::Document(const AK::URL& url)
     : ParentNode(*this, NodeType::DOCUMENT_NODE)
     , m_style_computer(make<CSS::StyleComputer>(*this))
-    , m_style_sheets(JS::make_handle(CSS::StyleSheetList::create(*this)))
     , m_url(url)
     , m_window(HTML::Window::create_with_document(*this))
     , m_implementation(DOMImplementation::create({}, *this))
     , m_history(HTML::History::create(*this))
 {
+    m_style_sheets = JS::make_handle(CSS::StyleSheetList::create(*this));
+
     HTML::main_thread_event_loop().register_document({}, *this);
 
     m_style_update_timer = Core::Timer::create_single_shot(0, [this] {

Userland/Libraries/LibWeb/Forward.h

@@ -464,7 +464,6 @@ class CSSFontFaceRuleWrapper;
 class CSSGroupingRuleWrapper;
 class CSSImportRuleWrapper;
 class CSSMediaRuleWrapper;
-class CSSRuleListWrapper;
 class CSSRuleWrapper;
 class CSSStyleDeclarationWrapper;
 class CSSStyleRuleWrapper;

Userland/Libraries/LibWeb/idl_files.cmake

@@ -9,7 +9,7 @@ libweb_js_wrapper(CSS/CSSGroupingRule)
 libweb_js_wrapper(CSS/CSSImportRule)
 libweb_js_wrapper(CSS/CSSMediaRule)
 libweb_js_wrapper(CSS/CSSRule)
-libweb_js_wrapper(CSS/CSSRuleList)
+libweb_js_wrapper(CSS/CSSRuleList NO_INSTANCE)
 libweb_js_wrapper(CSS/CSSStyleDeclaration)
 libweb_js_wrapper(CSS/CSSStyleRule)
 libweb_js_wrapper(CSS/CSSStyleSheet NO_INSTANCE)