From 7c95ebc302e5d98da703bae4c6ce7d9c87dbb767 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 15 Dec 2023 15:41:28 +0100 Subject: [PATCH] LibWeb: Make Document::page() return a Page& Now that Document always has a Page, and always keeps it alive, we can make this return a Page&, exposing various unnecessary null checks. --- .../Libraries/LibWeb/CSS/CSSImportRule.cpp | 2 +- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 7 ++--- .../CSS/StyleValues/IdentifierStyleValue.cpp | 5 +--- .../CSS/StyleValues/ImageStyleValue.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Document.cpp | 28 +++++++------------ Userland/Libraries/LibWeb/DOM/Document.h | 4 +-- Userland/Libraries/LibWeb/DOM/Element.cpp | 28 ++++++------------- .../LibWeb/Fetch/Fetching/Fetching.cpp | 5 +--- .../LibWeb/HTML/HTMLImageElement.cpp | 8 +++--- .../Libraries/LibWeb/HTML/HTMLLinkElement.cpp | 2 +- .../Libraries/LibWeb/HTML/HTMLMetaElement.cpp | 5 +--- .../LibWeb/HTML/HTMLObjectElement.cpp | 4 +-- .../LibWeb/HTML/NavigableContainer.cpp | 5 ++-- .../LibWeb/HTML/Scripting/Environments.cpp | 2 +- Userland/Libraries/LibWeb/HTML/Window.cpp | 4 +-- Userland/Libraries/LibWeb/Layout/Node.cpp | 2 +- .../Libraries/LibWeb/SVG/SVGTitleElement.cpp | 8 ++---- 17 files changed, 43 insertions(+), 78 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/CSSImportRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSImportRule.cpp index 0fdde7a71e..20f872eb24 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSImportRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSImportRule.cpp @@ -33,7 +33,7 @@ CSSImportRule::CSSImportRule(AK::URL url, DOM::Document& document) , m_document(document) { dbgln_if(CSS_LOADER_DEBUG, "CSSImportRule: Loading import URL: {}", m_url); - auto request = LoadRequest::create_for_url_on_page(m_url, document.page()); + auto request = LoadRequest::create_for_url_on_page(m_url, &document.page()); // NOTE: Mark this rule as delaying the document load event *before* calling set_resource() // as it may trigger a synchronous resource_did_load() callback. diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index a31651bdde..79a295befb 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -2334,11 +2334,8 @@ NonnullOwnPtr StyleComputer::make_rule_cache_for_casca void StyleComputer::build_rule_cache() { - // FIXME: How are we sometimes calculating style before the Document has a Page? - if (document().page()) { - if (auto user_style_source = document().page()->user_style(); user_style_source.has_value()) { - m_user_style_sheet = JS::make_handle(parse_css_stylesheet(CSS::Parser::ParsingContext(document()), user_style_source.value())); - } + if (auto user_style_source = document().page().user_style(); user_style_source.has_value()) { + m_user_style_sheet = JS::make_handle(parse_css_stylesheet(CSS::Parser::ParsingContext(document()), user_style_source.value())); } m_author_rule_cache = make_rule_cache_for_cascade_origin(CascadeOrigin::Author); diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.cpp index 47e3aa74bc..1de5c776bd 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.cpp @@ -217,10 +217,7 @@ Color IdentifierStyleValue::to_color(Optional node if (id() == CSS::ValueID::LibwebLink || id() == ValueID::Linktext) return document.link_color(); - if (!document.page()) - return {}; - - auto palette = document.page()->palette(); + auto palette = document.page().palette(); switch (id()) { case CSS::ValueID::LibwebPaletteDesktopBackground: return palette.color(ColorRole::DesktopBackground); diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp index 6daab94929..cd18bb7e7f 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp @@ -32,7 +32,7 @@ void ImageStyleValue::load_any_resources(DOM::Document& document) return; m_document = &document; - m_image_request = HTML::SharedImageRequest::get_or_create(document.realm(), *document.page(), m_url); + m_image_request = HTML::SharedImageRequest::get_or_create(document.realm(), document.page(), m_url); m_image_request->add_callbacks( [this, weak_this = make_weak_ptr()] { if (!weak_this) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index b86e365163..8df330c9c4 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -803,10 +803,8 @@ WebIDL::ExceptionOr Document::set_title(String const& title) return {}; } - if (auto* page = this->page()) { - if (browsing_context() == &page->top_level_browsing_context()) - page->client().page_did_change_title(title.to_deprecated_string()); - } + if (browsing_context() == &page().top_level_browsing_context()) + page().client().page_did_change_title(title.to_deprecated_string()); return {}; } @@ -1040,8 +1038,7 @@ void Document::update_layout() navigable()->set_needs_display(); if (navigable()->is_traversable()) { - if (auto* page = this->page()) - page->client().page_did_layout(); + page().client().page_did_layout(); } m_layout_root->recompute_selection_states(); @@ -1909,12 +1906,12 @@ void Document::update_readiness(HTML::DocumentReadyState readiness_value) } } -Page* Document::page() +Page& Document::page() { return m_page; } -Page const* Document::page() const +Page const& Document::page() const { return m_page; } @@ -1977,9 +1974,7 @@ void Document::completely_finish_loading() String Document::cookie(Cookie::Source source) { - if (auto* page = this->page()) - return MUST(String::from_deprecated_string(page->client().page_did_request_cookie(m_url, source))); - return String {}; + return MUST(String::from_deprecated_string(page().client().page_did_request_cookie(m_url, source))); } void Document::set_cookie(StringView cookie_string, Cookie::Source source) @@ -1988,8 +1983,7 @@ void Document::set_cookie(StringView cookie_string, Cookie::Source source) if (!cookie.has_value()) return; - if (auto* page = this->page()) - page->client().page_did_set_cookie(m_url, cookie.value(), source); + page().client().page_did_set_cookie(m_url, cookie.value(), source); } String Document::dump_dom_tree_as_json() const @@ -2372,8 +2366,7 @@ void Document::increment_number_of_things_delaying_the_load_event(Badgepage()) - page->client().page_did_update_resource_count(m_number_of_things_delaying_the_load_event); + page().client().page_did_update_resource_count(m_number_of_things_delaying_the_load_event); } void Document::decrement_number_of_things_delaying_the_load_event(Badge) @@ -2381,8 +2374,7 @@ void Document::decrement_number_of_things_delaying_the_load_event(Badgepage()) - page->client().page_did_update_resource_count(m_number_of_things_delaying_the_load_event); + page().client().page_did_update_resource_count(m_number_of_things_delaying_the_load_event); } bool Document::anything_is_delaying_the_load_event() const @@ -2726,7 +2718,7 @@ void Document::run_unloading_cleanup_steps() // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document void Document::destroy() { - page()->client().page_did_destroy_document(*this); + 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. diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index 962fde87c1..97126d384e 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -181,8 +181,8 @@ public: void set_browsing_context(HTML::BrowsingContext*); - Page* page(); - Page const* page() const; + Page& page(); + Page const& page() const; Color background_color() const; Vector const* background_layers() const; diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 7cdc6aa951..998972c1a6 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -1188,22 +1188,16 @@ void Element::set_scroll_left(double x) // 8. If the element is the root element invoke scroll() on window with x as first argument and scrollY on window as second argument, and terminate these steps. if (document.document_element() == this) { // FIXME: Implement this in terms of invoking scroll() on window. - if (auto* page = document.page()) { - if (document.browsing_context() == &page->top_level_browsing_context()) - page->client().page_did_request_scroll_to({ static_cast(x), static_cast(window->scroll_y()) }); - } - + if (document.browsing_context() == &document.page().top_level_browsing_context()) + document.page().client().page_did_request_scroll_to({ static_cast(x), static_cast(window->scroll_y()) }); return; } // 9. If the element is the body element, document is in quirks mode, and the element is not potentially scrollable, invoke scroll() on window with x as first argument and scrollY on window as second argument, and terminate these steps. if (document.body() == this && document.in_quirks_mode() && !is_potentially_scrollable()) { // FIXME: Implement this in terms of invoking scroll() on window. - if (auto* page = document.page()) { - if (document.browsing_context() == &page->top_level_browsing_context()) - page->client().page_did_request_scroll_to({ static_cast(x), static_cast(window->scroll_y()) }); - } - + if (document.browsing_context() == &document.page().top_level_browsing_context()) + document.page().client().page_did_request_scroll_to({ static_cast(x), static_cast(window->scroll_y()) }); return; } @@ -1256,22 +1250,16 @@ void Element::set_scroll_top(double y) // 8. If the element is the root element invoke scroll() on window with scrollX on window as first argument and y as second argument, and terminate these steps. if (document.document_element() == this) { // FIXME: Implement this in terms of invoking scroll() on window. - if (auto* page = document.page()) { - if (document.browsing_context() == &page->top_level_browsing_context()) - page->client().page_did_request_scroll_to({ static_cast(window->scroll_x()), static_cast(y) }); - } - + if (document.browsing_context() == &document.page().top_level_browsing_context()) + document.page().client().page_did_request_scroll_to({ static_cast(window->scroll_x()), static_cast(y) }); return; } // 9. If the element is the body element, document is in quirks mode, and the element is not potentially scrollable, invoke scroll() on window with scrollX as first argument and y as second argument, and terminate these steps. if (document.body() == this && document.in_quirks_mode() && !is_potentially_scrollable()) { // FIXME: Implement this in terms of invoking scroll() on window. - if (auto* page = document.page()) { - if (document.browsing_context() == &page->top_level_browsing_context()) - page->client().page_did_request_scroll_to({ static_cast(window->scroll_x()), static_cast(y) }); - } - + if (document.browsing_context() == &document.page().top_level_browsing_context()) + document.page().client().page_did_request_scroll_to({ static_cast(window->scroll_x()), static_cast(y) }); return; } diff --git a/Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp b/Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp index 1e0c176def..6decedc4a6 100644 --- a/Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp +++ b/Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp @@ -1425,10 +1425,7 @@ WebIDL::ExceptionOr> http_network_or_cache_fet auto document = Bindings::host_defined_environment_settings_object(realm).responsible_document(); if (!document) return DeprecatedString::empty(); - auto* page = document->page(); - if (!page) - return DeprecatedString::empty(); - return page->client().page_did_request_cookie(http_request->current_url(), Cookie::Source::Http); + return document->page().client().page_did_request_cookie(http_request->current_url(), Cookie::Source::Http); })(); // 2. If cookies is not the empty string, then append (`Cookie`, cookies) to httpRequest’s header list. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index 89b8d081ab..e2dc91aed6 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -57,7 +57,7 @@ void HTMLImageElement::initialize(JS::Realm& realm) Base::initialize(realm); set_prototype(&Bindings::ensure_web_prototype(realm, "HTMLImageElement"_fly_string)); - m_current_request = ImageRequest::create(realm, *document().page()); + m_current_request = ImageRequest::create(realm, document().page()); } void HTMLImageElement::adopted_from(DOM::Document& old_document) @@ -385,7 +385,7 @@ ErrorOr HTMLImageElement::update_the_image_data(bool restart_animations, b m_pending_request = nullptr; // 4. Let current request be a new image request whose image data is that of the entry and whose state is completely available. - m_current_request = ImageRequest::create(realm(), *document().page()); + m_current_request = ImageRequest::create(realm(), document().page()); m_current_request->set_image_data(entry->image_data); m_current_request->set_state(ImageRequest::State::CompletelyAvailable); @@ -512,7 +512,7 @@ after_step_7: // multiple image elements (as well as CSS background-images, etc.) // 16. Set image request to a new image request whose current URL is urlString. - auto image_request = ImageRequest::create(realm(), *document().page()); + auto image_request = ImageRequest::create(realm(), document().page()); image_request->set_current_url(realm(), url_string); // 17. If current request's state is unavailable or broken, then set the current request to image request. @@ -707,7 +707,7 @@ void HTMLImageElement::react_to_changes_in_the_environment() key.origin = document().origin(); // 11. ⌛ Let image request be a new image request whose current URL is urlString - auto image_request = ImageRequest::create(realm(), *document().page()); + auto image_request = ImageRequest::create(realm(), document().page()); image_request->set_current_url(realm(), url_string); // 12. ⌛ Let the element's pending request be image request. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp index de6cdbf857..d05bf64505 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp @@ -70,7 +70,7 @@ void HTMLLinkElement::inserted() ResourceLoader::the().preconnect(document().parse_url(deprecated_attribute(HTML::AttributeNames::href))); } else if (m_relationship & Relationship::Icon) { auto favicon_url = document().parse_url(href()); - auto favicon_request = LoadRequest::create_for_url_on_page(favicon_url, document().page()); + auto favicon_request = LoadRequest::create_for_url_on_page(favicon_url, &document().page()); set_resource(ResourceLoader::the().load_resource(Resource::Type::Generic, favicon_request)); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp index 0176db94bd..477ca2aca6 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLMetaElement.cpp @@ -57,9 +57,6 @@ void HTMLMetaElement::inserted() auto name = attribute(AttributeNames::name); auto content = attribute(AttributeNames::content); if (name.has_value() && name->bytes_as_string_view().equals_ignoring_ascii_case("theme-color"sv) && content.has_value()) { - auto* page = document().page(); - if (!page) - return; auto context = CSS::Parser::ParsingContext { document() }; // 2. For each element in candidate elements: @@ -82,7 +79,7 @@ void HTMLMetaElement::inserted() auto color = css_value->as_color().color(); // 4. If color is not failure, then return color. - page->client().page_did_change_theme_color(color); + document().page().client().page_did_change_theme_color(color); return; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp index c5724a6585..eeb80a2aac 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp @@ -138,7 +138,7 @@ void HTMLObjectElement::queue_element_task_to_run_object_representation_steps() } // 4. Let request be a new request whose URL is the resulting URL record, client is the element's node document's relevant settings object, destination is "object", credentials mode is "include", mode is "navigate", and whose use-URL-credentials flag is set. - auto request = LoadRequest::create_for_url_on_page(url, document().page()); + auto request = LoadRequest::create_for_url_on_page(url, &document().page()); // 5. Fetch request, with processResponseEndOfBody given response res set to finalize and report timing with res, the element's node document's relevant global object, and "object". // Fetching the resource must delay the load event of the element's node document until the task that is queued by the networking task source once the resource has been fetched (defined next) has been run. @@ -316,7 +316,7 @@ void HTMLObjectElement::load_image() // NOTE: This currently reloads the image instead of reusing the resource we've already downloaded. auto data = deprecated_attribute(HTML::AttributeNames::data); auto url = document().parse_url(data); - m_image_request = HTML::SharedImageRequest::get_or_create(realm(), *document().page(), url); + m_image_request = HTML::SharedImageRequest::get_or_create(realm(), document().page(), url); m_image_request->add_callbacks( [this] { run_object_representation_completed_steps(Representation::Image); diff --git a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp index db67476a54..d34c9a10ad 100644 --- a/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp +++ b/Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp @@ -69,9 +69,8 @@ WebIDL::ExceptionOr NavigableContainer::create_new_child_navigable() VERIFY(group); // 3. Let browsingContext and document be the result of creating a new browsing context and document given element's node document, element, and group. - auto* page = document().page(); - VERIFY(page); - auto [browsing_context, document] = TRY(BrowsingContext::create_a_new_browsing_context_and_document(*page, this->document(), *this, *group)); + auto& page = document().page(); + auto [browsing_context, document] = TRY(BrowsingContext::create_a_new_browsing_context_and_document(page, this->document(), *this, *group)); // 4. Let targetName be null. Optional target_name; diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp index f2c1f74f05..4d7635bb2c 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp @@ -292,7 +292,7 @@ bool EnvironmentSettingsObject::is_scripting_enabled() const // The user has not disabled scripting for settings at this time. (User agents may provide users with the option to disable scripting globally, or in a finer-grained manner, e.g., on a per-origin basis, down to the level of individual environment settings objects.) auto document = const_cast(*this).responsible_document(); VERIFY(document); - if (!document->page() || !document->page()->is_scripting_enabled()) + if (!document->page().is_scripting_enabled()) return false; // FIXME: Either settings's global object is not a Window object, or settings's global object's associated Document's active sandboxing flag set does not have its sandboxed scripts browsing context flag set. diff --git a/Userland/Libraries/LibWeb/HTML/Window.cpp b/Userland/Libraries/LibWeb/HTML/Window.cpp index 66310e4800..f7523f6bab 100644 --- a/Userland/Libraries/LibWeb/HTML/Window.cpp +++ b/Userland/Libraries/LibWeb/HTML/Window.cpp @@ -429,12 +429,12 @@ bool Window::dispatch_event(DOM::Event& event) Page* Window::page() { - return associated_document().page(); + return &associated_document().page(); } Page const* Window::page() const { - return associated_document().page(); + return &associated_document().page(); } Optional Window::query_media_feature(CSS::MediaFeatureID media_feature) const diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 6aeff57fcd..3130a720dd 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -752,7 +752,7 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) VERIFY_NOT_REACHED(); }; - border.width = snap_a_length_as_a_border_width(document().page()->client().device_pixels_per_css_pixel(), resolve_border_width()); + border.width = snap_a_length_as_a_border_width(document().page().client().device_pixels_per_css_pixel(), resolve_border_width()); } }; diff --git a/Userland/Libraries/LibWeb/SVG/SVGTitleElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGTitleElement.cpp index e9790ce112..efb93c979f 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGTitleElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGTitleElement.cpp @@ -33,16 +33,14 @@ void SVGTitleElement::children_changed() { Base::children_changed(); - auto* page = document().page(); - if (!page) - return; - if (document().browsing_context() != &page->top_level_browsing_context()) + auto& page = document().page(); + if (document().browsing_context() != &page.top_level_browsing_context()) return; auto* document_element = document().document_element(); if (document_element == parent() && is(document_element)) - page->client().page_did_change_title(document().title().to_deprecated_string()); + page.client().page_did_change_title(document().title().to_deprecated_string()); } }