From 5d6cb9cbdbb1ac309d68c41929ab84f73f68753c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 7 Aug 2022 13:51:40 +0200 Subject: [PATCH] LibWeb: Make CSSRuleList GC-allocated --- .../LibWeb/WrapperGenerator/IDLGenerators.cpp | 3 ++ .../Libraries/LibWeb/CSS/CSSGroupingRule.cpp | 4 ++- .../Libraries/LibWeb/CSS/CSSGroupingRule.h | 8 ++--- Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp | 17 +++++++++-- Userland/Libraries/LibWeb/CSS/CSSRuleList.h | 29 ++++++++++--------- Userland/Libraries/LibWeb/CSS/CSSRuleList.idl | 2 +- .../Libraries/LibWeb/CSS/CSSStyleSheet.cpp | 2 +- Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h | 8 ++--- Userland/Libraries/LibWeb/DOM/Document.cpp | 3 +- Userland/Libraries/LibWeb/Forward.h | 1 - Userland/Libraries/LibWeb/idl_files.cmake | 2 +- 11 files changed, 50 insertions(+), 29 deletions(-) diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp index 16d2eb457a..8b22a1b2e4 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp +++ b/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; } diff --git a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.cpp index 725a2f09d6..bb214da973 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.cpp @@ -4,13 +4,15 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include namespace Web::CSS { CSSGroupingRule::CSSGroupingRule(NonnullRefPtrVector&& 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); diff --git a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h index 0f533b6c50..19dae632d3 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h +++ b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h @@ -23,9 +23,9 @@ public: virtual ~CSSGroupingRule() = default; - CSSRuleList const& css_rules() const { return m_rules; } - CSSRuleList& css_rules() { return m_rules; } - NonnullRefPtr css_rules_for_bindings() { return m_rules; } + CSSRuleList const& css_rules() const { return *m_rules; } + CSSRuleList& css_rules() { return *m_rules; } + CSSRuleList* css_rules_for_bindings() { return m_rules.cell(); } DOM::ExceptionOr insert_rule(StringView rule, u32 index = 0); DOM::ExceptionOr delete_rule(u32 index); @@ -37,7 +37,7 @@ protected: explicit CSSGroupingRule(NonnullRefPtrVector&&); private: - NonnullRefPtr m_rules; + JS::Handle m_rules; }; } diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp index 1ea00262e7..b076c58feb 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.cpp @@ -5,6 +5,8 @@ */ #include +#include +#include #include #include #include @@ -13,8 +15,14 @@ namespace Web::CSS { -CSSRuleList::CSSRuleList(NonnullRefPtrVector&& rules) - : m_rules(move(rules)) +CSSRuleList* CSSRuleList::create(Bindings::WindowObject& window_object, NonnullRefPtrVector&& rules) +{ + return window_object.heap().allocate(window_object.realm(), window_object, move(rules)); +} + +CSSRuleList::CSSRuleList(Bindings::WindowObject& window_object, NonnullRefPtrVector&& rules) + : Bindings::LegacyPlatformObject(window_object.ensure_web_prototype("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); +} + } diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.h b/Userland/Libraries/LibWeb/CSS/CSSRuleList.h index 9dd88099e5..af4157f61b 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.h +++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2021-2022, Sam Atkins + * Copyright (c) 2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -9,8 +10,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -18,16 +19,14 @@ namespace Web::CSS { // https://www.w3.org/TR/cssom/#the-cssrulelist-interface -class CSSRuleList - : public RefCounted - , public Bindings::Wrappable { -public: - using WrapperType = Bindings::CSSRuleListWrapper; +class CSSRuleList : public Bindings::LegacyPlatformObject { + JS_OBJECT(CSSRuleList, Bindings::LegacyPlatformObject); - static NonnullRefPtr create(NonnullRefPtrVector&& rules) - { - return adopt_ref(*new CSSRuleList(move(rules))); - } +public: + CSSRuleList& impl() { return *this; } + + static CSSRuleList* create(Bindings::WindowObject&, NonnullRefPtrVector&& rules); + CSSRuleList(Bindings::WindowObject&, NonnullRefPtrVector&&); ~CSSRuleList() = default; RefPtr 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 remove_a_css_rule(u32 index); DOM::ExceptionOr insert_a_css_rule(Variant>, u32 index); @@ -57,9 +57,12 @@ public: bool evaluate_media_queries(HTML::Window const&); private: - explicit CSSRuleList(NonnullRefPtrVector&&); - NonnullRefPtrVector m_rules; }; } + +namespace Web::Bindings { +inline JS::Object* wrap(JS::GlobalObject&, Web::CSS::CSSRuleList& object) { return &object; } +using CSSRuleListWrapper = Web::CSS::CSSRuleList; +} diff --git a/Userland/Libraries/LibWeb/CSS/CSSRuleList.idl b/Userland/Libraries/LibWeb/CSS/CSSRuleList.idl index 67b4a66cb7..cd995bd138 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRuleList.idl +++ b/Userland/Libraries/LibWeb/CSS/CSSRuleList.idl @@ -1,6 +1,6 @@ #import -[Exposed=Window] +[Exposed=Window, NoInstanceWrapper] interface CSSRuleList { getter CSSRule? item(unsigned long index); diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp index 3d12f3feba..dd0e33c731 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.cpp +++ b/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 rules, Optional 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("CSSStyleSheet")); diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h b/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h index 63501ac608..3dd08e1733 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSStyleSheet.h +++ b/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& rules() { return m_rules; } - void set_rules(NonnullRefPtr rules) { m_rules = move(rules); } + CSSRuleList const& rules() const { return *m_rules; } + CSSRuleList& rules() { return *m_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 m_rules; + CSSRuleList* m_rules { nullptr }; WeakPtr m_owner_css_rule; diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index d10634a1d3..0f816d1246 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -279,12 +279,13 @@ NonnullRefPtr Document::create(AK::URL const& url) Document::Document(const AK::URL& url) : ParentNode(*this, NodeType::DOCUMENT_NODE) , m_style_computer(make(*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] { diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index d47a18c08d..8e437f2736 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/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; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index d27be3a83a..bb4d5c4444 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/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)