From a76ef04ae655eb2e89ae48fadc0d380171c97e41 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sun, 17 Sep 2023 16:11:16 +0200 Subject: [PATCH] LibWeb+WebContent: Create `WebContentConsoleClient` for every document Fixes regression introduced in b4fe118dff6c1bd839f0443f9d822ae078895814 The `WebContentConsoleClient` needs to be created not just once, but for every new document. Although the JS Console window allows communication only with the active document associated with the top-level browsing context, we still need a console client for each iframe's document to ensure their console logs are printed. In the future, we might consider adding the ability to switch which document the JS Console window communicates with. Fixes https://github.com/SerenityOS/serenity/issues/21117 --- Userland/Libraries/LibWeb/DOM/Document.cpp | 2 ++ Userland/Libraries/LibWeb/Page/Page.h | 1 + .../WebContent/ConnectionFromClient.cpp | 31 ++++++++++++------- .../WebContent/ConnectionFromClient.h | 6 ++-- Userland/Services/WebContent/PageHost.cpp | 5 +++ Userland/Services/WebContent/PageHost.h | 1 + .../WebContent/WebContentConsoleClient.h | 3 +- 7 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 36c5641816..78e2f982d8 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -2652,6 +2652,8 @@ Vector> Document::list_of_descendant_browsing_ // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document void Document::destroy() { + page()->client().page_did_destroy_document(*this); + // NOTE: Abort needs to happen before destory. There is currently bug in the spec: https://github.com/whatwg/html/issues/9148 // 4. Abort document. abort(); diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index c13f2c7b91..002a078747 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -203,6 +203,7 @@ public: virtual Gfx::IntRect page_did_request_fullscreen_window() { return {}; } virtual void page_did_start_loading(const AK::URL&, bool is_redirect) { (void)is_redirect; } virtual void page_did_create_new_document(Web::DOM::Document&) { } + virtual void page_did_destroy_document(Web::DOM::Document&) { } virtual void page_did_finish_loading(const AK::URL&) { } virtual void page_did_change_selection() { } virtual void page_did_request_cursor_change(Gfx::StandardCursor) { } diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 0b7e72c07d..a58c352bcb 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -649,21 +649,28 @@ Messages::WebContentServer::GetHoveredNodeIdResponse ConnectionFromClient::get_h void ConnectionFromClient::initialize_js_console(Badge, Web::DOM::Document& document) { - auto realm = document.realm().make_weak_ptr(); - if (m_realm.ptr() == realm.ptr()) - return; + auto& realm = document.realm(); + auto console_object = realm.intrinsics().console_object(); + auto console_client = make(console_object->console(), document.realm(), *this); + console_object->console().set_client(*console_client); - auto console_object = realm->intrinsics().console_object(); - m_realm = realm; - if (!m_console_client) - m_console_client = make(console_object->console(), *m_realm, *this); - console_object->console().set_client(*m_console_client.ptr()); + VERIFY(document.browsing_context()); + if (document.browsing_context()->is_top_level()) { + m_top_level_document_console_client = console_client->make_weak_ptr(); + } + + m_console_clients.set(&document, move(console_client)); +} + +void ConnectionFromClient::destroy_js_console(Badge, Web::DOM::Document& document) +{ + m_console_clients.remove(&document); } void ConnectionFromClient::js_console_input(DeprecatedString const& js_source) { - if (m_console_client) - m_console_client->handle_input(js_source); + if (m_top_level_document_console_client) + m_top_level_document_console_client->handle_input(js_source); } void ConnectionFromClient::run_javascript(DeprecatedString const& js_source) @@ -695,8 +702,8 @@ void ConnectionFromClient::run_javascript(DeprecatedString const& js_source) void ConnectionFromClient::js_console_request_messages(i32 start_index) { - if (m_console_client) - m_console_client->send_messages(start_index); + if (m_top_level_document_console_client) + m_top_level_document_console_client->send_messages(start_index); } Messages::WebContentServer::TakeDocumentScreenshotResponse ConnectionFromClient::take_document_screenshot() diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h index 6de1542d13..18d0d4af71 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.h +++ b/Userland/Services/WebContent/ConnectionFromClient.h @@ -34,6 +34,7 @@ public: virtual void die() override; void initialize_js_console(Badge, Web::DOM::Document& document); + void destroy_js_console(Badge, Web::DOM::Document& document); void request_file(Web::FileRequest); @@ -127,8 +128,9 @@ private: HashMap> m_backing_stores; - WeakPtr m_realm; - OwnPtr m_console_client; + HashMap> m_console_clients; + WeakPtr m_top_level_document_console_client; + JS::Handle m_console_global_object; HashMap m_requested_files {}; diff --git a/Userland/Services/WebContent/PageHost.cpp b/Userland/Services/WebContent/PageHost.cpp index 79f331b314..7492b33d70 100644 --- a/Userland/Services/WebContent/PageHost.cpp +++ b/Userland/Services/WebContent/PageHost.cpp @@ -287,6 +287,11 @@ void PageHost::page_did_create_new_document(Web::DOM::Document& document) m_client.initialize_js_console({}, document); } +void PageHost::page_did_destroy_document(Web::DOM::Document& document) +{ + m_client.destroy_js_console({}, document); +} + void PageHost::page_did_finish_loading(const URL& url) { m_client.async_did_finish_loading(url); diff --git a/Userland/Services/WebContent/PageHost.h b/Userland/Services/WebContent/PageHost.h index 25b20b08ac..c481c9315c 100644 --- a/Userland/Services/WebContent/PageHost.h +++ b/Userland/Services/WebContent/PageHost.h @@ -94,6 +94,7 @@ private: virtual void page_did_request_media_context_menu(Web::CSSPixelPoint, DeprecatedString const& target, unsigned modifiers, Web::Page::MediaContextMenu) override; virtual void page_did_start_loading(const URL&, bool) override; virtual void page_did_create_new_document(Web::DOM::Document&) override; + virtual void page_did_destroy_document(Web::DOM::Document&) override; virtual void page_did_finish_loading(const URL&) override; virtual void page_did_request_alert(String const&) override; virtual void page_did_request_confirm(String const&) override; diff --git a/Userland/Services/WebContent/WebContentConsoleClient.h b/Userland/Services/WebContent/WebContentConsoleClient.h index 208e8a5399..237ab1c07d 100644 --- a/Userland/Services/WebContent/WebContentConsoleClient.h +++ b/Userland/Services/WebContent/WebContentConsoleClient.h @@ -16,7 +16,8 @@ namespace WebContent { -class WebContentConsoleClient final : public JS::ConsoleClient { +class WebContentConsoleClient final : public JS::ConsoleClient + , public Weakable { public: WebContentConsoleClient(JS::Console&, JS::Realm&, ConnectionFromClient&);