From aae191aa33c88edba97f872707a5d0f9705cb0aa Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 5 Oct 2024 15:16:48 +0200 Subject: [PATCH] LibWeb: Bail from various navigable operations when no active window If we end up in a situation where the navigable no longer has an active window, we can't perform navigation or many other navigable operations. These are all ad-hoc, since the navigables spec is basically all written as if there's always an active window. Unfortunately, the active window comes from the active document's browsing context, which is a nullable concept even in the spec, so we do need to deal with null here. This removes all the locally reproducible crashes when running WPT over the legacy Japanese encoding directory on my computer. Yes, this is a bit of a monkey patch, but it should be harmless since we're (as I understand it) dealing with navigables that are still hanging around with related tasks queued on them. Once all these tasks have been completed, the navigables will go away anyway. --- Userland/Libraries/LibWeb/HTML/Navigable.cpp | 35 ++++++++++++++++--- .../LibWeb/HTML/TraversableNavigable.cpp | 7 ++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/Navigable.cpp b/Userland/Libraries/LibWeb/HTML/Navigable.cpp index e8c505fbf91..a69bfc4212b 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/Navigable.cpp @@ -382,7 +382,7 @@ Navigable::ChosenNavigable Navigable::choose_a_navigable(StringView name, Tokeni else { // --> If current's active window does not have transient activation and the user agent has been configured to // not show popups (i.e., the user agent has a "popup blocker" enabled) - if (!active_window()->has_transient_activation() && traversable_navigable()->page().should_block_pop_ups()) { + if (active_window() && !active_window()->has_transient_activation() && traversable_navigable()->page().should_block_pop_ups()) { // FIXME: The user agent may inform the user that a popup has been blocked. dbgln("Pop-up blocked!"); } @@ -590,6 +590,7 @@ static JS::GCPtr attempt_to_create_a_non_fetch_scheme_document(No static WebIDL::ExceptionOr> create_navigation_params_from_a_srcdoc_resource(JS::GCPtr entry, JS::GCPtr navigable, TargetSnapshotParams const& target_snapshot_params, Optional navigation_id) { auto& vm = navigable->vm(); + VERIFY(navigable->active_window()); auto& realm = navigable->active_window()->realm(); // 1. Let documentResource be entry's document state's resource. @@ -670,6 +671,7 @@ static WebIDL::ExceptionOr> create_navigation static WebIDL::ExceptionOr create_navigation_params_by_fetching(JS::GCPtr entry, JS::GCPtr navigable, SourceSnapshotParams const& source_snapshot_params, TargetSnapshotParams const& target_snapshot_params, CSPNavigationType csp_navigation_type, Optional navigation_id) { auto& vm = navigable->vm(); + VERIFY(navigable->active_window()); auto& realm = navigable->active_window()->realm(); auto& active_document = *navigable->active_document(); @@ -1045,6 +1047,10 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( bool allow_POST, JS::GCPtr> completion_steps) { + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) + return {}; + // FIXME: 1. Assert: this is running in parallel. // 2. Assert: if navigationParams is non-null, then navigationParams's response is non-null. @@ -1093,8 +1099,8 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( } } - // NOTE: Not in the spec but queuing task on the next step will fail because active_window() does not exist for destroyed navigable. - if (has_been_destroyed()) + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) return {}; // 6. Queue a global task on the navigation and traversal task source, given navigable's active window, to run these steps: @@ -1206,6 +1212,10 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( // https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) { + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) + return {}; + auto const& url = params.url; auto source_document = params.source_document; auto const& document_resource = params.document_resource; @@ -1318,6 +1328,7 @@ WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) // 18. If url's scheme is "javascript", then: if (url.scheme() == "javascript"sv) { // 1. Queue a global task on the navigation and traversal task source given navigable's active window to navigate to a javascript: URL given navigable, url, historyHandling, initiatorOriginSnapshot, and cspNavigationType. + VERIFY(active_window()); queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), JS::create_heap_function(heap(), [this, url, history_handling, initiator_origin_snapshot, csp_navigation_type, navigation_id] { (void)navigate_to_a_javascript_url(url, to_history_handling_behavior(history_handling), initiator_origin_snapshot, csp_navigation_type, navigation_id); })); @@ -1334,6 +1345,7 @@ WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) // then: if (user_involvement != UserNavigationInvolvement::BrowserUI && active_document.origin().is_same_origin_domain(source_document->origin()) && !active_document.is_initial_about_blank() && Fetch::Infrastructure::is_fetch_scheme(url.scheme())) { // 1. Let navigation be navigable's active window's navigation API. + VERIFY(active_window()); auto navigation = active_window()->navigation(); // 2. Let entryListForFiring be formDataEntryList if documentResource is a POST resource; otherwise, null. @@ -1363,8 +1375,8 @@ WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) // 20. In parallel, run these steps: Platform::EventLoopPlugin::the().deferred_invoke([this, source_snapshot_params, target_snapshot_params, csp_navigation_type, document_resource, url, navigation_id, referrer_policy, initiator_origin_snapshot, response, history_handling, initiator_base_url_snapshot] { - // NOTE: Not in the spec but subsequent steps will fail because destroyed navigable does not have active document. - if (has_been_destroyed()) { + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) { set_delaying_load_events(false); return; } @@ -1381,6 +1393,12 @@ WebIDL::ExceptionOr Navigable::navigate(NavigateParams params) return; } + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) { + set_delaying_load_events(false); + return; + } + // 3. Queue a global task on the navigation and traversal task source given navigable's active window to abort a document and its descendants given navigable's active document. queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), JS::create_heap_function(heap(), [this] { VERIFY(this->active_document()); @@ -1455,6 +1473,7 @@ WebIDL::ExceptionOr Navigable::navigate_to_a_fragment(URL::URL const& url, (void)navigation_id; // 1. Let navigation be navigable's active window's navigation API. + VERIFY(active_window()); auto navigation = active_window()->navigation(); // 2. Let destinationNavigationAPIState be navigable's active session history entry's navigation API state. @@ -1540,6 +1559,7 @@ WebIDL::ExceptionOr Navigable::navigate_to_a_fragment(URL::URL const& url, WebIDL::ExceptionOr> Navigable::evaluate_javascript_url(URL::URL const& url, URL::Origin const& new_document_origin, String navigation_id) { auto& vm = this->vm(); + VERIFY(active_window()); auto& realm = active_window()->realm(); // 1. Let urlString be the result of running the URL serializer on url. @@ -2058,8 +2078,13 @@ void Navigable::inform_the_navigation_api_about_aborting_navigation() // FIXME: 1. If this algorithm is running on navigable's active window's relevant agent's event loop, then continue on to the following steps. // Otherwise, queue a global task on the navigation and traversal task source given navigable's active window to run the following steps. + // AD-HOC: Not in the spec but subsequent steps will fail if the navigable doesn't have an active window. + if (!active_window()) + return; + queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), JS::create_heap_function(heap(), [this] { // 2. Let navigation be navigable's active window's navigation API. + VERIFY(active_window()); auto navigation = active_window()->navigation(); // 3. If navigation's ongoing navigate event is null, then return. diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 54ee97d77b4..3570b5c193c 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -497,6 +497,8 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ // 12. For each navigable of changingNavigables, queue a global task on the navigation and traversal task source of navigable's active window to run the steps: for (auto& navigable : changing_navigables) { + if (!navigable->active_window()) + continue; queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), JS::create_heap_function(heap(), [&] { // NOTE: This check is not in the spec but we should not continue navigation if navigable has been destroyed. if (navigable->has_been_destroyed()) { @@ -627,6 +629,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ // navigable's active window to run afterDocumentPopulated. Platform::EventLoopPlugin::the().deferred_invoke([populated_target_entry, potentially_target_specific_source_snapshot_params, target_snapshot_params, this, allow_POST, navigable, after_document_populated = JS::create_heap_function(this->heap(), move(after_document_populated))] { navigable->populate_session_history_entry_document(populated_target_entry, *potentially_target_specific_source_snapshot_params, target_snapshot_params, {}, Empty {}, CSPNavigationType::Other, allow_POST, JS::create_heap_function(this->heap(), [this, after_document_populated, populated_target_entry]() mutable { + VERIFY(active_window()); queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), JS::create_heap_function(this->heap(), [after_document_populated, populated_target_entry]() mutable { after_document_populated->function()(true, populated_target_entry); })); @@ -753,6 +756,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ navigable->set_ongoing_navigation({}); // 2. Queue a global task on the navigation and traversal task source given navigable's active window to perform afterPotentialUnloads. + VERIFY(navigable->active_window()); queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), after_potential_unload); } // 11. Otherwise: @@ -787,6 +791,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ continue; } + VERIFY(navigable->active_window()); queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), JS::create_heap_function(heap(), [&] { // NOTE: This check is not in the spec but we should not continue navigation if navigable has been destroyed. if (navigable->has_been_destroyed()) { @@ -880,6 +885,7 @@ TraversableNavigable::CheckIfUnloadingIsCanceledResult TraversableNavigable::che } // 5. Queue a global task on the navigation and traversal task source given traversable's active window to perform the following steps: + VERIFY(traversable->active_window()); queue_global_task(Task::Source::NavigationAndTraversal, *traversable->active_window(), JS::create_heap_function(heap(), [&] { // 1. if needsBeforeunload is true, then: if (needs_beforeunload) { @@ -900,6 +906,7 @@ TraversableNavigable::CheckIfUnloadingIsCanceledResult TraversableNavigable::che return; // 3. Let navigation be traversable's active window's navigation API. + VERIFY(traversable->active_window()); auto navigation = traversable->active_window()->navigation(); // 4. Let navigateEventResult be the result of firing a traverse navigate event at navigation given targetEntry and userInvolvementForNavigateEvent.