Ver Fonte

LibWeb+WebContent: Store console clients on the DOM document

We explicitly stopped visting the map of documents to console clients in
commit 44659f2f2a44266984a46a297debce87ae538edd to avoid keeping the
document alive. However, if nothing else visits the console clients, we
may set the top-level console client to a client that has been garbage
collected.

So instead of storing this map, just store the console client on the
document itself. This will allow the document to visit its client.
Timothy Flynn há 11 meses atrás
pai
commit
0a819e628e

+ 1 - 6
Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -415,12 +415,6 @@ WebIDL::ExceptionOr<void> Document::populate_with_html_head_and_body()
     return {};
 }
 
-void Document::finalize()
-{
-    Base::finalize();
-    page().client().page_did_destroy_document(*this);
-}
-
 void Document::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
@@ -490,6 +484,7 @@ void Document::visit_edges(Cell::Visitor& visitor)
 
     visitor.visit(m_top_layer_elements);
     visitor.visit(m_top_layer_pending_removals);
+    visitor.visit(m_console_client);
 }
 
 // https://w3c.github.io/selection-api/#dom-document-getselection

+ 6 - 1
Userland/Libraries/LibWeb/DOM/Document.h

@@ -16,6 +16,7 @@
 #include <AK/WeakPtr.h>
 #include <LibCore/DateTime.h>
 #include <LibCore/Forward.h>
+#include <LibJS/Console.h>
 #include <LibJS/Forward.h>
 #include <LibURL/URL.h>
 #include <LibWeb/CSS/CSSStyleSheet.h>
@@ -684,10 +685,12 @@ public:
     void parse_html_from_a_string(StringView);
     static JS::NonnullGCPtr<Document> parse_html_unsafe(JS::VM&, StringView);
 
+    void set_console_client(JS::GCPtr<JS::ConsoleClient> console_client) { m_console_client = console_client; }
+    JS::GCPtr<JS::ConsoleClient> console_client() const { return m_console_client; }
+
 protected:
     virtual void initialize(JS::Realm&) override;
     virtual void visit_edges(Cell::Visitor&) override;
-    virtual void finalize() override;
 
     Document(JS::Realm&, URL::URL const&, TemporaryDocumentForFragmentParsing = TemporaryDocumentForFragmentParsing::No);
 
@@ -945,6 +948,8 @@ private:
 
     // https://dom.spec.whatwg.org/#document-allow-declarative-shadow-roots
     bool m_allow_declarative_shadow_roots { false };
+
+    JS::GCPtr<JS::ConsoleClient> m_console_client;
 };
 
 template<>

+ 0 - 1
Userland/Libraries/LibWeb/Page/Page.h

@@ -311,7 +311,6 @@ public:
     virtual void page_did_start_loading(URL::URL const&, bool is_redirect) { (void)is_redirect; }
     virtual void page_did_create_new_document(Web::DOM::Document&) { }
     virtual void page_did_change_active_document_in_top_level_browsing_context(Web::DOM::Document&) { }
-    virtual void page_did_destroy_document(Web::DOM::Document&) { }
     virtual void page_did_finish_loading(URL::URL const&) { }
     virtual void page_did_request_cursor_change(Gfx::StandardCursor) { }
     virtual void page_did_request_context_menu(CSSPixelPoint) { }

+ 5 - 14
Userland/Services/WebContent/PageClient.cpp

@@ -80,7 +80,6 @@ void PageClient::visit_edges(JS::Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_page);
-    visitor.ignore(m_console_clients);
 }
 
 ConnectionFromClient& PageClient::client() const
@@ -342,13 +341,10 @@ void PageClient::page_did_create_new_document(Web::DOM::Document& document)
 
 void PageClient::page_did_change_active_document_in_top_level_browsing_context(Web::DOM::Document& document)
 {
-    if (auto console_client = m_console_clients.get(document); console_client.has_value())
-        m_top_level_document_console_client = *console_client.value();
-}
-
-void PageClient::page_did_destroy_document(Web::DOM::Document& document)
-{
-    destroy_js_console(document);
+    if (auto console_client = document.console_client()) {
+        auto& web_content_console_client = verify_cast<WebContentConsoleClient>(*console_client);
+        m_top_level_document_console_client = web_content_console_client;
+    }
 }
 
 void PageClient::page_did_finish_loading(URL::URL const& url)
@@ -671,12 +667,7 @@ void PageClient::initialize_js_console(Web::DOM::Document& document)
     auto console_client = heap().allocate_without_realm<WebContentConsoleClient>(console_object->console(), document.realm(), *this);
     console_object->console().set_client(*console_client);
 
-    m_console_clients.set(document, console_client);
-}
-
-void PageClient::destroy_js_console(Web::DOM::Document& document)
-{
-    m_console_clients.remove(document);
+    document.set_console_client(console_client);
 }
 
 void PageClient::js_console_input(ByteString const& js_source)

+ 0 - 4
Userland/Services/WebContent/PageClient.h

@@ -76,7 +76,6 @@ public:
     void ready_to_paint();
 
     void initialize_js_console(Web::DOM::Document& document);
-    void destroy_js_console(Web::DOM::Document& document);
     void js_console_input(ByteString const& js_source);
     void run_javascript(ByteString const& js_source);
     void js_console_request_messages(i32 start_index);
@@ -131,7 +130,6 @@ private:
     virtual void page_did_start_loading(URL::URL const&, bool) override;
     virtual void page_did_create_new_document(Web::DOM::Document&) override;
     virtual void page_did_change_active_document_in_top_level_browsing_context(Web::DOM::Document&) override;
-    virtual void page_did_destroy_document(Web::DOM::Document&) override;
     virtual void page_did_finish_loading(URL::URL const&) override;
     virtual void page_did_request_alert(String const&) override;
     virtual void page_did_request_confirm(String const&) override;
@@ -204,8 +202,6 @@ private:
 
     BackingStoreManager m_backing_store_manager;
 
-    // NOTE: These documents are not visited, but manually removed from the map on document finalization.
-    HashMap<JS::RawGCPtr<Web::DOM::Document>, JS::NonnullGCPtr<WebContentConsoleClient>> m_console_clients;
     WeakPtr<WebContentConsoleClient> m_top_level_document_console_client;
 
     JS::Handle<JS::GlobalObject> m_console_global_object;