From 0ae5c070c72a52c930c6419d77b9ea7b2403eb71 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Mon, 4 Dec 2023 21:57:13 +1300 Subject: [PATCH] LibWeb: Make Web::Page GC-allocated This is a first step towards removing the various Page& and Page* we have littering the engine with "trust me bro" safety guarantees. Co-Authored-By: Andreas Kling --- Userland/Libraries/LibWeb/Page/Page.cpp | 40 ++++++++++++------- Userland/Libraries/LibWeb/Page/Page.h | 20 ++++++---- .../LibWeb/SVG/SVGDecodedImageData.cpp | 6 +-- .../LibWeb/SVG/SVGDecodedImageData.h | 4 +- Userland/Services/WebContent/PageClient.cpp | 9 ++++- Userland/Services/WebContent/PageClient.h | 4 +- Userland/Services/WebWorker/PageHost.cpp | 9 ++++- Userland/Services/WebWorker/PageHost.h | 3 +- 8 files changed, 65 insertions(+), 30 deletions(-) diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index a7ebcb332f..866934880e 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -23,13 +23,25 @@ namespace Web { -Page::Page(PageClient& client) +JS::NonnullGCPtr Page::create(JS::VM& vm, JS::NonnullGCPtr page_client) +{ + return vm.heap().allocate_without_realm(page_client); +} + +Page::Page(JS::NonnullGCPtr client) : m_client(client) { } Page::~Page() = default; +void Page::visit_edges(JS::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_top_level_traversable); + visitor.visit(m_client); +} + HTML::BrowsingContext& Page::focused_context() { if (m_focused_context) @@ -56,13 +68,13 @@ void Page::load_html(StringView html) Gfx::Palette Page::palette() const { - return m_client.palette(); + return m_client->palette(); } // https://w3c.github.io/csswg-drafts/cssom-view-1/#web-exposed-screen-area CSSPixelRect Page::web_exposed_screen_area() const { - auto device_pixel_rect = m_client.screen_rect(); + auto device_pixel_rect = m_client->screen_rect(); auto scale = client().device_pixels_per_css_pixel(); return { device_pixel_rect.x().value() / scale, @@ -74,7 +86,7 @@ CSSPixelRect Page::web_exposed_screen_area() const CSS::PreferredColorScheme Page::preferred_color_scheme() const { - return m_client.preferred_color_scheme(); + return m_client->preferred_color_scheme(); } CSSPixelPoint Page::device_to_css_point(DevicePixelPoint point) const @@ -210,12 +222,12 @@ static ResponseType spin_event_loop_until_dialog_closed(PageClient& client, Opti void Page::did_request_alert(String const& message) { m_pending_dialog = PendingDialog::Alert; - m_client.page_did_request_alert(message); + m_client->page_did_request_alert(message); if (!message.is_empty()) m_pending_dialog_text = message; - spin_event_loop_until_dialog_closed(m_client, m_pending_alert_response); + spin_event_loop_until_dialog_closed(*m_client, m_pending_alert_response); } void Page::alert_closed() @@ -230,12 +242,12 @@ void Page::alert_closed() bool Page::did_request_confirm(String const& message) { m_pending_dialog = PendingDialog::Confirm; - m_client.page_did_request_confirm(message); + m_client->page_did_request_confirm(message); if (!message.is_empty()) m_pending_dialog_text = message; - return spin_event_loop_until_dialog_closed(m_client, m_pending_confirm_response); + return spin_event_loop_until_dialog_closed(*m_client, m_pending_confirm_response); } void Page::confirm_closed(bool accepted) @@ -250,12 +262,12 @@ void Page::confirm_closed(bool accepted) Optional Page::did_request_prompt(String const& message, String const& default_) { m_pending_dialog = PendingDialog::Prompt; - m_client.page_did_request_prompt(message, default_); + m_client->page_did_request_prompt(message, default_); if (!message.is_empty()) m_pending_dialog_text = message; - return spin_event_loop_until_dialog_closed(m_client, m_pending_prompt_response); + return spin_event_loop_until_dialog_closed(*m_client, m_pending_prompt_response); } void Page::prompt_closed(Optional response) @@ -273,11 +285,11 @@ void Page::dismiss_dialog() case PendingDialog::None: break; case PendingDialog::Alert: - m_client.page_did_request_accept_dialog(); + m_client->page_did_request_accept_dialog(); break; case PendingDialog::Confirm: case PendingDialog::Prompt: - m_client.page_did_request_dismiss_dialog(); + m_client->page_did_request_dismiss_dialog(); break; } } @@ -290,7 +302,7 @@ void Page::accept_dialog() case PendingDialog::Alert: case PendingDialog::Confirm: case PendingDialog::Prompt: - m_client.page_did_request_accept_dialog(); + m_client->page_did_request_accept_dialog(); break; } } @@ -301,7 +313,7 @@ void Page::did_request_color_picker(WeakPtr target, Colo m_pending_non_blocking_dialog = PendingNonBlockingDialog::ColorPicker; m_pending_non_blocking_dialog_target = move(target); - m_client.page_did_request_color_picker(current_color); + m_client->page_did_request_color_picker(current_color); } } diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index dce615f7b4..4ab3fd1a8f 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -1,7 +1,8 @@ /* - * Copyright (c) 2020-2021, Andreas Kling + * Copyright (c) 2020-2023, Andreas Kling * Copyright (c) 2021-2022, Linus Groh * Copyright (c) 2022, Sam Atkins + * Copyright (c) 2023, Shannon Booth * * SPDX-License-Identifier: BSD-2-Clause */ @@ -36,12 +37,14 @@ namespace Web { class PageClient; -class Page : public Weakable { - AK_MAKE_NONCOPYABLE(Page); - AK_MAKE_NONMOVABLE(Page); +class Page final + : public JS::Cell + , public Weakable { + JS_CELL(Page, JS::Cell); public: - explicit Page(PageClient&); + static JS::NonnullGCPtr create(JS::VM&, JS::NonnullGCPtr); + ~Page(); PageClient& client() { return m_client; } @@ -152,13 +155,16 @@ public: bool pdf_viewer_supported() const { return m_pdf_viewer_supported; } private: + explicit Page(JS::NonnullGCPtr); + virtual void visit_edges(Visitor&) override; + JS::GCPtr media_context_menu_element(); - PageClient& m_client; + JS::NonnullGCPtr m_client; WeakPtr m_focused_context; - JS::Handle m_top_level_traversable; + JS::GCPtr m_top_level_traversable; // FIXME: Enable this by default once CORS preflight checks are supported. bool m_same_origin_policy_enabled { false }; diff --git a/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.cpp b/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.cpp index e9de384064..d8c4d7d288 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.cpp @@ -57,7 +57,7 @@ private: ErrorOr> SVGDecodedImageData::create(Page& host_page, AK::URL const& url, ByteBuffer data) { auto page_client = SVGPageClient::create(Bindings::main_thread_vm(), host_page); - auto page = make(*page_client); + auto page = Page::create(Bindings::main_thread_vm(), *page_client); page_client->m_svg_page = page.ptr(); page->set_top_level_traversable(MUST(Web::HTML::TraversableNavigable::create_a_fresh_top_level_traversable(*page, AK::URL("about:blank")))); JS::NonnullGCPtr navigable = page->top_level_traversable(); @@ -98,10 +98,10 @@ ErrorOr> SVGDecodedImageData::create(Page& ho MUST(document->append_child(*svg_root)); - return adopt_nonnull_ref_or_enomem(new (nothrow) SVGDecodedImageData(move(page), move(page_client), move(document), move(svg_root))); + return adopt_nonnull_ref_or_enomem(new (nothrow) SVGDecodedImageData(page, move(page_client), move(document), move(svg_root))); } -SVGDecodedImageData::SVGDecodedImageData(NonnullOwnPtr page, JS::Handle page_client, JS::Handle document, JS::Handle root_element) +SVGDecodedImageData::SVGDecodedImageData(JS::NonnullGCPtr page, JS::Handle page_client, JS::Handle document, JS::Handle root_element) : m_page(move(page)) , m_page_client(move(page_client)) , m_document(move(document)) diff --git a/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.h b/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.h index c0c2f4b5bd..88475a3c50 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.h +++ b/Userland/Libraries/LibWeb/SVG/SVGDecodedImageData.h @@ -32,12 +32,12 @@ public: private: class SVGPageClient; - SVGDecodedImageData(NonnullOwnPtr, JS::Handle, JS::Handle, JS::Handle); + SVGDecodedImageData(JS::NonnullGCPtr, JS::Handle, JS::Handle, JS::Handle); RefPtr render(Gfx::IntSize) const; mutable RefPtr m_immutable_bitmap; - NonnullOwnPtr m_page; + JS::Handle m_page; JS::Handle m_page_client; JS::Handle m_document; diff --git a/Userland/Services/WebContent/PageClient.cpp b/Userland/Services/WebContent/PageClient.cpp index d55199fdba..e2bdee3e7e 100644 --- a/Userland/Services/WebContent/PageClient.cpp +++ b/Userland/Services/WebContent/PageClient.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -46,7 +47,7 @@ JS::NonnullGCPtr PageClient::create(JS::VM& vm, PageHost& page_host, PageClient::PageClient(PageHost& owner, u64 id) : m_owner(owner) - , m_page(make(*this)) + , m_page(Web::Page::create(Web::Bindings::main_thread_vm(), *this)) , m_id(id) { setup_palette(); @@ -56,6 +57,12 @@ PageClient::PageClient(PageHost& owner, u64 id) }); } +void PageClient::visit_edges(JS::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_page); +} + ConnectionFromClient& PageClient::client() const { return m_owner.client(); diff --git a/Userland/Services/WebContent/PageClient.h b/Userland/Services/WebContent/PageClient.h index 416a21e941..6121164ef5 100644 --- a/Userland/Services/WebContent/PageClient.h +++ b/Userland/Services/WebContent/PageClient.h @@ -61,6 +61,8 @@ public: private: PageClient(PageHost&, u64 id); + virtual void visit_edges(JS::Cell::Visitor&) override; + // ^PageClient virtual bool is_connection_open() const override; virtual Gfx::Palette palette() const override; @@ -131,7 +133,7 @@ private: ConnectionFromClient& client() const; PageHost& m_owner; - NonnullOwnPtr m_page; + JS::NonnullGCPtr m_page; RefPtr m_palette_impl; Web::DevicePixelRect m_screen_rect; Web::DevicePixelSize m_content_size; diff --git a/Userland/Services/WebWorker/PageHost.cpp b/Userland/Services/WebWorker/PageHost.cpp index ecb356728d..1de082b4c6 100644 --- a/Userland/Services/WebWorker/PageHost.cpp +++ b/Userland/Services/WebWorker/PageHost.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -75,9 +76,15 @@ void PageHost::request_file(Web::FileRequest request) PageHost::PageHost(ConnectionFromClient& client) : m_client(client) - , m_page(make(*this)) + , m_page(Web::Page::create(Web::Bindings::main_thread_vm(), *this)) { setup_palette(); } +void PageHost::visit_edges(JS::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_page); +} + } diff --git a/Userland/Services/WebWorker/PageHost.h b/Userland/Services/WebWorker/PageHost.h index eb979d88fa..44f26e5110 100644 --- a/Userland/Services/WebWorker/PageHost.h +++ b/Userland/Services/WebWorker/PageHost.h @@ -33,11 +33,12 @@ public: private: explicit PageHost(ConnectionFromClient&); + virtual void visit_edges(JS::Cell::Visitor&) override; void setup_palette(); ConnectionFromClient& m_client; - NonnullOwnPtr m_page; + JS::NonnullGCPtr m_page; RefPtr m_palette_impl; };