From cfdb8f2a8e8a3a5351d44be8430dcaabb3ed61ef Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 8 Aug 2022 15:32:27 +0200 Subject: [PATCH] LibWeb: Make MediaList GC-allocated --- .../Libraries/LibWeb/CSS/CSSGroupingRule.h | 3 +- .../Libraries/LibWeb/CSS/CSSMediaRule.cpp | 18 +++++++---- Userland/Libraries/LibWeb/CSS/CSSMediaRule.h | 13 ++++---- Userland/Libraries/LibWeb/CSS/MediaList.cpp | 20 ++++++++++-- Userland/Libraries/LibWeb/CSS/MediaList.h | 32 ++++++++++--------- Userland/Libraries/LibWeb/CSS/MediaList.idl | 2 +- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 2 +- Userland/Libraries/LibWeb/Forward.h | 1 - Userland/Libraries/LibWeb/idl_files.cmake | 2 +- 9 files changed, 58 insertions(+), 35 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h index 271572d226c..a72b786b345 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h +++ b/Userland/Libraries/LibWeb/CSS/CSSGroupingRule.h @@ -36,10 +36,9 @@ public: protected: explicit CSSGroupingRule(Bindings::WindowObject&, CSSRuleList&); - -private: virtual void visit_edges(Cell::Visitor&) override; +private: CSSRuleList& m_rules; }; diff --git a/Userland/Libraries/LibWeb/CSS/CSSMediaRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSMediaRule.cpp index 698e563fa69..245a80bb84a 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSMediaRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSMediaRule.cpp @@ -11,26 +11,32 @@ namespace Web::CSS { -CSSMediaRule* CSSMediaRule::create(Bindings::WindowObject& window_object, NonnullRefPtr&& media_queries, CSSRuleList& rules) +CSSMediaRule* CSSMediaRule::create(Bindings::WindowObject& window_object, MediaList& media_queries, CSSRuleList& rules) { - return window_object.heap().allocate(window_object.realm(), window_object, move(media_queries), rules); + return window_object.heap().allocate(window_object.realm(), window_object, media_queries, rules); } -CSSMediaRule::CSSMediaRule(Bindings::WindowObject& window_object, NonnullRefPtr&& media, CSSRuleList& rules) +CSSMediaRule::CSSMediaRule(Bindings::WindowObject& window_object, MediaList& media, CSSRuleList& rules) : CSSConditionRule(window_object, rules) - , m_media(move(media)) + , m_media(media) { set_prototype(&window_object.ensure_web_prototype("CSSMediaRule")); } +void CSSMediaRule::visit_edges(Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(&m_media); +} + String CSSMediaRule::condition_text() const { - return m_media->media_text(); + return m_media.media_text(); } void CSSMediaRule::set_condition_text(String text) { - m_media->set_media_text(text); + m_media.set_media_text(text); } // https://www.w3.org/TR/cssom-1/#serialize-a-css-rule diff --git a/Userland/Libraries/LibWeb/CSS/CSSMediaRule.h b/Userland/Libraries/LibWeb/CSS/CSSMediaRule.h index f3563660d20..2c502a7e52f 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSMediaRule.h +++ b/Userland/Libraries/LibWeb/CSS/CSSMediaRule.h @@ -22,8 +22,8 @@ class CSSMediaRule final : public CSSConditionRule { public: CSSMediaRule& impl() { return *this; } - static CSSMediaRule* create(Bindings::WindowObject&, NonnullRefPtr&& media_queries, CSSRuleList&); - explicit CSSMediaRule(Bindings::WindowObject&, NonnullRefPtr&&, CSSRuleList&); + static CSSMediaRule* create(Bindings::WindowObject&, MediaList& media_queries, CSSRuleList&); + explicit CSSMediaRule(Bindings::WindowObject&, MediaList&, CSSRuleList&); virtual ~CSSMediaRule() = default; @@ -31,16 +31,17 @@ public: virtual String condition_text() const override; virtual void set_condition_text(String) override; - virtual bool condition_matches() const override { return m_media->matches(); } + virtual bool condition_matches() const override { return m_media.matches(); } - NonnullRefPtr const& media() const { return m_media; } + MediaList* media() const { return &m_media; } - bool evaluate(HTML::Window const& window) { return m_media->evaluate(window); } + bool evaluate(HTML::Window const& window) { return m_media.evaluate(window); } private: + virtual void visit_edges(Cell::Visitor&) override; virtual String serialized() const override; - NonnullRefPtr m_media; + MediaList& m_media; }; template<> diff --git a/Userland/Libraries/LibWeb/CSS/MediaList.cpp b/Userland/Libraries/LibWeb/CSS/MediaList.cpp index 13e6fc99232..dbcea1fa353 100644 --- a/Userland/Libraries/LibWeb/CSS/MediaList.cpp +++ b/Userland/Libraries/LibWeb/CSS/MediaList.cpp @@ -1,16 +1,25 @@ /* * Copyright (c) 2021, Sam Atkins + * Copyright (c) 2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ +#include +#include #include #include namespace Web::CSS { -MediaList::MediaList(NonnullRefPtrVector&& media) - : m_media(move(media)) +MediaList* MediaList::create(Bindings::WindowObject& window_object, NonnullRefPtrVector&& media) +{ + return window_object.heap().allocate(window_object.realm(), window_object, move(media)); +} + +MediaList::MediaList(Bindings::WindowObject& window_object, NonnullRefPtrVector&& media) + : Bindings::LegacyPlatformObject(window_object.ensure_web_prototype("MediaList")) + , m_media(move(media)) { } @@ -83,4 +92,11 @@ bool MediaList::matches() const return false; } +JS::Value MediaList::item_value(size_t index) const +{ + if (index >= m_media.size()) + return JS::js_undefined(); + return JS::js_string(vm(), m_media[index].to_string()); +} + } diff --git a/Userland/Libraries/LibWeb/CSS/MediaList.h b/Userland/Libraries/LibWeb/CSS/MediaList.h index 925707edb19..4ffe4b91067 100644 --- a/Userland/Libraries/LibWeb/CSS/MediaList.h +++ b/Userland/Libraries/LibWeb/CSS/MediaList.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2021-2022, Sam Atkins + * Copyright (c) 2022, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,44 +9,45 @@ #include #include -#include -#include +#include +#include #include namespace Web::CSS { // https://www.w3.org/TR/cssom-1/#the-medialist-interface -class MediaList final - : public RefCounted - , public Bindings::Wrappable - , public Weakable { +class MediaList final : public Bindings::LegacyPlatformObject { AK_MAKE_NONCOPYABLE(MediaList); AK_MAKE_NONMOVABLE(MediaList); + JS_OBJECT(MediaList, Bindings::LegacyPlatformObject); public: - using WrapperType = Bindings::MediaListWrapper; - - static NonnullRefPtr create(NonnullRefPtrVector&& media) - { - return adopt_ref(*new MediaList(move(media))); - } + static MediaList* create(Bindings::WindowObject&, NonnullRefPtrVector&& media); + explicit MediaList(Bindings::WindowObject&, NonnullRefPtrVector&&); ~MediaList() = default; + MediaList& impl() { return *this; } + String media_text() const; void set_media_text(String const&); size_t length() const { return m_media.size(); } - bool is_supported_property_index(u32 index) const; String item(u32 index) const; void append_medium(String); void delete_medium(String); + virtual bool is_supported_property_index(u32 index) const override; + virtual JS::Value item_value(size_t index) const override; + bool evaluate(HTML::Window const&); bool matches() const; private: - explicit MediaList(NonnullRefPtrVector&&); - NonnullRefPtrVector m_media; }; } + +namespace Web::Bindings { +inline JS::Object* wrap(JS::Realm&, Web::CSS::MediaList& object) { return &object; } +using MediaListWrapper = Web::CSS::MediaList; +} diff --git a/Userland/Libraries/LibWeb/CSS/MediaList.idl b/Userland/Libraries/LibWeb/CSS/MediaList.idl index 63eb6bfc5ab..d6a8531c8fa 100644 --- a/Userland/Libraries/LibWeb/CSS/MediaList.idl +++ b/Userland/Libraries/LibWeb/CSS/MediaList.idl @@ -1,4 +1,4 @@ -[Exposed=Window] +[Exposed=Window, NoInstanceWrapper] interface MediaList { [LegacyNullToEmptyString] stringifier attribute CSSOMString mediaText; readonly attribute unsigned long length; diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 5c28c2be494..dd3cf53e556 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -2648,7 +2648,7 @@ CSSRule* Parser::convert_to_rule(NonnullRefPtr rule) child_rules.append(child_rule); } auto* rule_list = CSSRuleList::create(m_context.window_object(), move(child_rules)); - return CSSMediaRule::create(m_context.window_object(), MediaList::create(move(media_query_list)), *rule_list); + return CSSMediaRule::create(m_context.window_object(), *MediaList::create(m_context.window_object(), move(media_query_list)), *rule_list); } else if (rule->at_rule_name().equals_ignoring_case("supports"sv)) { diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 282c31f1cf6..e24163f47a2 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -559,7 +559,6 @@ class ImageDataWrapper; class IntersectionObserverWrapper; class KeyboardEventWrapper; class LocationObject; -class MediaListWrapper; class MediaQueryListEventWrapper; class MediaQueryListWrapper; class MessageChannelWrapper; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index 9d8d2cf9846..faa10bd5121 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -14,7 +14,7 @@ libweb_js_wrapper(CSS/CSSStyleDeclaration NO_INSTANCE) libweb_js_wrapper(CSS/CSSStyleRule NO_INSTANCE) libweb_js_wrapper(CSS/CSSStyleSheet NO_INSTANCE) libweb_js_wrapper(CSS/CSSSupportsRule NO_INSTANCE) -libweb_js_wrapper(CSS/MediaList) +libweb_js_wrapper(CSS/MediaList NO_INSTANCE) libweb_js_wrapper(CSS/MediaQueryList) libweb_js_wrapper(CSS/MediaQueryListEvent) libweb_js_wrapper(CSS/Screen)