From 43c720db81aa324f1e99b6bbe0086ca7def36326 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 16 Mar 2024 09:24:57 +0100 Subject: [PATCH] LibWeb: Remove a bunch of redundant Document::navigable() lookups Document::navigable() can be unpleasantly slow, since we don't have a direct link between documents and navigables at the moment. So let's not call it twice when once is enough. --- Userland/Libraries/LibWeb/DOM/Document.cpp | 19 +++++++++++-------- Userland/Libraries/LibWeb/DOM/Element.cpp | 5 +++-- .../LibWeb/HTML/EventLoop/EventLoop.cpp | 6 ++++-- .../LibWeb/HTML/HTMLTitleElement.cpp | 5 +++-- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 5d0c0be217f..9efb9b7fb17 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1042,8 +1042,9 @@ void Document::update_layout() // NOTE: If our parent document needs a relayout, we must do that *first*. // This is necessary as the parent layout may cause our viewport to change. - if (navigable() && navigable()->container()) - navigable()->container()->document().update_layout(); + auto navigable = this->navigable(); + if (navigable && navigable->container()) + navigable->container()->document().update_layout(); update_style(); @@ -1054,7 +1055,7 @@ void Document::update_layout() if (m_created_for_appropriate_template_contents) return; - if (!navigable()) + if (!navigable) return; auto viewport_rect = this->viewport_rect(); @@ -1097,10 +1098,10 @@ void Document::update_layout() // Broadcast the current viewport rect to any new paintables, so they know whether they're visible or not. inform_all_viewport_clients_about_the_current_viewport_rect(); - navigable()->set_needs_display(); + navigable->set_needs_display(); set_needs_to_resolve_paint_only_properties(); - if (navigable()->is_traversable()) { + if (navigable->is_traversable()) { // NOTE: The assignment of scroll frames only needs to occur for traversables because they take care of all // nested navigable documents. paintable()->assign_scroll_frames(); @@ -2120,9 +2121,10 @@ void Document::update_readiness(HTML::DocumentReadyState readiness_value) dispatch_event(Event::create(realm(), HTML::EventNames::readystatechange)); if (readiness_value == HTML::DocumentReadyState::Complete) { - if (navigable() && navigable()->is_traversable()) { + auto navigable = this->navigable(); + if (navigable && navigable->is_traversable()) { HTML::HTMLLinkElement::load_fallback_favicon_if_needed(*this).release_value_but_fixme_should_propagate_errors(); - navigable()->traversable_navigable()->page().client().page_did_finish_loading(url()); + navigable->traversable_navigable()->page().client().page_did_finish_loading(url()); } else { m_needs_to_call_page_did_load = true; } @@ -2275,7 +2277,8 @@ bool Document::is_fully_active() const bool Document::is_active() const { - return navigable() && navigable()->active_document() == this; + auto navigable = this->navigable(); + return navigable && navigable->active_document() == this; } // https://html.spec.whatwg.org/multipage/history.html#dom-document-location diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 525081f6e33..1fe2e8132da 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -922,8 +922,9 @@ JS::NonnullGCPtr Element::get_client_rects() const // FIXME: - Replace each anonymous block box with its child box(es) and repeat this until no anonymous block boxes // are left in the final list. const_cast(document()).update_layout(); - VERIFY(document().navigable()); - auto viewport_offset = document().navigable()->viewport_scroll_offset(); + auto navigable = document().navigable(); + VERIFY(navigable); + auto viewport_offset = navigable->viewport_scroll_offset(); if (document().paintable()) { // NOTE: Make sure CSS transforms are resolved before it is used to calculate the rect position. diff --git a/Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp b/Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp index 807d09dbf70..19318d59f35 100644 --- a/Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp +++ b/Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp @@ -158,7 +158,8 @@ void EventLoop::process() // 2. Rendering opportunities: Remove from docs all Document objects whose node navigables do not have a rendering opportunity. docs.remove_all_matching([&](auto& document) { - return document->navigable() && !document->navigable()->has_a_rendering_opportunity(); + auto navigable = document->navigable(); + return navigable && !navigable->has_a_rendering_opportunity(); }); // 3. If docs is not empty, then set hasARenderingOpportunity to true @@ -254,7 +255,8 @@ void EventLoop::process() // 16. For each fully active Document in docs, update the rendering or user interface of that Document and its browsing context to reflect the current state. for_each_fully_active_document_in_docs([&](DOM::Document& document) { - if (document.navigable() && document.navigable()->needs_repaint()) { + auto navigable = document.navigable(); + if (navigable && navigable->needs_repaint()) { auto* browsing_context = document.browsing_context(); auto& page = browsing_context->page(); page.client().schedule_repaint(); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTitleElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLTitleElement.cpp index 44639700ab3..a65e15158ec 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTitleElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLTitleElement.cpp @@ -29,8 +29,9 @@ void HTMLTitleElement::initialize(JS::Realm& realm) void HTMLTitleElement::children_changed() { HTMLElement::children_changed(); - if (navigable() && navigable()->is_traversable()) { - navigable()->traversable_navigable()->page().client().page_did_change_title(document().title().to_byte_string()); + auto navigable = this->navigable(); + if (navigable && navigable->is_traversable()) { + navigable->traversable_navigable()->page().client().page_did_change_title(document().title().to_byte_string()); } }