LibWeb: Make StyleSheetList GC-allocated

Author: https://github.com/awesomekling
Commit: https://github.com/SerenityOS/serenity/commit/5366924f11
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

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

Userland/Libraries/LibWeb/Bindings/PlatformObject.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <LibJS/Heap/GCPtr.h>
 #include <LibJS/Runtime/Object.h>
 
 namespace Web::Bindings {

Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp

@@ -31,6 +31,12 @@ CSSStyleSheet::CSSStyleSheet(Bindings::WindowObject& window_object, NonnullRefPt
         rule.set_parent_style_sheet(this);
 }
 
+void CSSStyleSheet::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_style_sheet_list.ptr());
+}
+
 // https://www.w3.org/TR/cssom/#dom-cssstylesheet-insertrule
 DOM::ExceptionOr<unsigned> CSSStyleSheet::insert_rule(StringView rule, unsigned index)
 {

Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h

@@ -53,11 +53,13 @@ public:
     void set_style_sheet_list(Badge<StyleSheetList>, StyleSheetList*);
 
 private:
+    virtual void visit_edges(Cell::Visitor&) override;
+
     NonnullRefPtr<CSSRuleList> m_rules;
 
     WeakPtr<CSSRule> m_owner_css_rule;
 
-    WeakPtr<StyleSheetList> m_style_sheet_list;
+    JS::GCPtr<StyleSheetList> m_style_sheet_list;
 };
 
 }

Userland/Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -140,7 +140,7 @@ void StyleComputer::for_each_stylesheet(CascadeOrigin cascade_origin, Callback c
     }
     if (cascade_origin == CascadeOrigin::Author) {
         for (auto const& sheet : document().style_sheets().sheets()) {
-            callback(*sheet);
+            callback(sheet);
         }
     }
 }

Userland/Libraries/LibWeb/CSS/StyleSheet.h

@@ -49,10 +49,9 @@ public:
 
 protected:
     explicit StyleSheet(Bindings::WindowObject&);
-
-private:
     virtual void visit_edges(Cell::Visitor&) override;
 
+private:
     WeakPtr<DOM::Element> m_owner_node;
 
     CSSStyleSheet* m_parent_style_sheet { nullptr };

Userland/Libraries/LibWeb/CSS/StyleSheetList.cpp

@@ -1,9 +1,10 @@
 /*
- * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2020-2022, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <LibWeb/Bindings/StyleSheetListPrototype.h>
 #include <LibWeb/CSS/StyleSheetList.h>
 #include <LibWeb/DOM/Document.h>
 
@@ -12,7 +13,7 @@ namespace Web::CSS {
 void StyleSheetList::add_sheet(CSSStyleSheet& sheet)
 {
     sheet.set_style_sheet_list({}, this);
-    m_sheets.append(JS::make_handle(sheet));
+    m_sheets.append(sheet);
 
     m_document.style_computer().invalidate_rule_cache();
     m_document.style_computer().load_fonts_from_sheet(sheet);
@@ -22,15 +23,30 @@ void StyleSheetList::add_sheet(CSSStyleSheet& sheet)
 void StyleSheetList::remove_sheet(CSSStyleSheet& sheet)
 {
     sheet.set_style_sheet_list({}, nullptr);
-    m_sheets.remove_first_matching([&](auto& entry) { return &*entry == &sheet; });
+    m_sheets.remove_first_matching([&](auto& entry) { return &entry == &sheet; });
 
     m_document.style_computer().invalidate_rule_cache();
     m_document.invalidate_style();
 }
 
-StyleSheetList::StyleSheetList(DOM::Document& document)
-    : m_document(document)
+StyleSheetList* StyleSheetList::create(DOM::Document& document)
 {
+    auto& realm = document.preferred_window_object().realm();
+    return realm.heap().allocate<StyleSheetList>(realm, document);
+}
+
+StyleSheetList::StyleSheetList(DOM::Document& document)
+    : Bindings::LegacyPlatformObject(document.preferred_window_object().ensure_web_prototype<Bindings::StyleSheetListPrototype>("StyleSheetList"))
+    , m_document(document)
+{
+}
+
+void StyleSheetList::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_document);
+    for (auto& sheet : m_sheets)
+        visitor.visit(&sheet);
 }
 
 // https://www.w3.org/TR/cssom/#ref-for-dfn-supported-property-indices%E2%91%A1
@@ -44,4 +60,12 @@ bool StyleSheetList::is_supported_property_index(u32 index) const
     return index < m_sheets.size();
 }
 
+JS::Value StyleSheetList::item_value(size_t index) const
+{
+    if (index >= m_sheets.size())
+        return JS::js_undefined();
+
+    return &m_sheets[index];
+}
+
 }

Userland/Libraries/LibWeb/CSS/StyleSheetList.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020-2021, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2020-2022, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -8,55 +8,52 @@
 
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/RefCounted.h>
+#include <LibWeb/Bindings/LegacyPlatformObject.h>
 #include <LibWeb/Bindings/Wrappable.h>
 #include <LibWeb/CSS/CSSStyleSheet.h>
 #include <LibWeb/Forward.h>
 
 namespace Web::CSS {
 
-class StyleSheetList
-    : public RefCounted<StyleSheetList>
-    , public Weakable<StyleSheetList>
-    , public Bindings::Wrappable {
-public:
-    using WrapperType = Bindings::StyleSheetListWrapper;
+class StyleSheetList : public Bindings::LegacyPlatformObject {
+    JS_OBJECT(StyleSheetList, Bindings::LegacyPlatformObject);
 
-    static NonnullRefPtr<StyleSheetList> create(DOM::Document& document)
-    {
-        return adopt_ref(*new StyleSheetList(document));
-    }
+public:
+    StyleSheetList& impl() { return *this; }
+    static StyleSheetList* create(DOM::Document& document);
+    explicit StyleSheetList(DOM::Document&);
 
     void add_sheet(CSSStyleSheet&);
     void remove_sheet(CSSStyleSheet&);
 
-    Vector<JS::Handle<CSSStyleSheet>> const& sheets() const { return m_sheets; }
-    Vector<JS::Handle<CSSStyleSheet>>& sheets() { return m_sheets; }
+    Vector<CSSStyleSheet&> const& sheets() const { return m_sheets; }
+    Vector<CSSStyleSheet&>& sheets() { return m_sheets; }
 
     CSSStyleSheet* item(size_t index) const
     {
         if (index >= m_sheets.size())
             return {};
-        return const_cast<CSSStyleSheet*>(m_sheets[index].cell());
+        return &const_cast<CSSStyleSheet&>(m_sheets[index]);
     }
 
     size_t length() const { return m_sheets.size(); }
 
-    bool is_supported_property_index(u32) const;
+    virtual bool is_supported_property_index(u32 index) const override;
+    virtual JS::Value item_value(size_t index) const override;
 
     DOM::Document& document() { return m_document; }
     DOM::Document const& document() const { return m_document; }
 
 private:
-    explicit StyleSheetList(DOM::Document&);
+    virtual void visit_edges(Cell::Visitor&) override;
 
     DOM::Document& m_document;
-    Vector<JS::Handle<CSSStyleSheet>> m_sheets;
+    Vector<CSSStyleSheet&> m_sheets;
 };
 
 }
 
 namespace Web::Bindings {
-
-StyleSheetListWrapper* wrap(JS::Realm&, CSS::StyleSheetList&);
-
+inline JS::Object* wrap(JS::Realm&, Web::CSS::StyleSheetList& object) { return &object; }
+using StyleSheetListWrapper = Web::CSS::StyleSheetList;
 }

Userland/Libraries/LibWeb/CSS/StyleSheetList.idl

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

Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -279,7 +279,7 @@ 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(CSS::StyleSheetList::create(*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))
@@ -1610,7 +1610,7 @@ void Document::evaluate_media_rules()
 {
     bool any_media_queries_changed_match_state = false;
     for (auto& style_sheet : style_sheets().sheets()) {
-        if (style_sheet->evaluate_media_queries(window()))
+        if (style_sheet.evaluate_media_queries(window()))
             any_media_queries_changed_match_state = true;
     }
 

Userland/Libraries/LibWeb/DOM/Document.h

@@ -99,7 +99,7 @@ public:
     CSS::StyleSheetList& style_sheets() { return *m_style_sheets; }
     const CSS::StyleSheetList& style_sheets() const { return *m_style_sheets; }
 
-    NonnullRefPtr<CSS::StyleSheetList> style_sheets_for_bindings() { return *m_style_sheets; }
+    CSS::StyleSheetList* style_sheets_for_bindings() { return m_style_sheets.cell(); }
 
     virtual FlyString node_name() const override { return "#document"; }
 
@@ -409,7 +409,7 @@ private:
     size_t m_next_layout_node_serial_id { 0 };
 
     OwnPtr<CSS::StyleComputer> m_style_computer;
-    RefPtr<CSS::StyleSheetList> m_style_sheets;
+    JS::Handle<CSS::StyleSheetList> m_style_sheets;
     RefPtr<Node> m_hovered_node;
     RefPtr<Node> m_inspected_node;
     RefPtr<Node> m_active_favicon;

Userland/Libraries/LibWeb/Forward.h

@@ -601,7 +601,6 @@ class ScreenWrapper;
 class SelectionWrapper;
 class StaticRangeWrapper;
 class StorageWrapper;
-class StyleSheetListWrapper;
 class SubmitEventWrapper;
 class SubtleCryptoWrapper;
 class SVGAnimatedLengthWrapper;

Userland/Libraries/LibWeb/idl_files.cmake

@@ -19,7 +19,7 @@ libweb_js_wrapper(CSS/MediaQueryList)
 libweb_js_wrapper(CSS/MediaQueryListEvent)
 libweb_js_wrapper(CSS/Screen)
 libweb_js_wrapper(CSS/StyleSheet NO_INSTANCE)
-libweb_js_wrapper(CSS/StyleSheetList)
+libweb_js_wrapper(CSS/StyleSheetList NO_INSTANCE)
 libweb_js_wrapper(DOM/AbstractRange)
 libweb_js_wrapper(DOM/Attribute)
 libweb_js_wrapper(DOM/AbortController)

Userland/Services/WebContent/ConnectionFromClient.cpp

@@ -206,7 +206,7 @@ void ConnectionFromClient::debug_request(String const& request, String const& ar
     if (request == "dump-style-sheets") {
         if (auto* doc = page().top_level_browsing_context().active_document()) {
             for (auto& sheet : doc->style_sheets().sheets()) {
-                Web::dump_sheet(*sheet);
+                Web::dump_sheet(sheet);
             }
         }
     }