LibWeb: Catch up with the spec on document destroy, abort and unload

These changes do not solve hanging `location.reload()` and
`location.go()` but only align implementation with the latest edits in
the specification.

`WindowProxy-Get-after-detaching-from-browsing-context` test output is
affected because `iframe.remove();` no longer synchronously does
destruction of a document, but queues a task on event loop.

Co-Authored-By: Andrew Kaster <akaster@serenityos.org>

Tests/LibWeb/Text/expected/HTML/WindowProxy-Get-after-detaching-from-browsing-context.txt

@@ -1,3 +1,3 @@
  
-null
+
 PASS (didn't crash)

Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -2974,12 +2974,10 @@ void Document::destroy()
     // 6. Set document's browsing context to null.
     m_browsing_context = nullptr;
 
-    // When a frame element stops being an active frame element, the user agent must destroy a child navigable given the element.
-    // A frame element is said to be an active frame element when it is in a document tree and its node document's browsing context is non-null.
+    // Not in the spec:
     for (auto& navigable_container : HTML::NavigableContainer::all_instances()) {
-        if (&navigable_container->document() == this) {
-            navigable_container->destroy_the_child_navigable();
-        }
+        if (&navigable_container->document() == this)
+            HTML::all_navigables().remove(navigable_container->content_navigable());
     }
 
     // 7. Set document's node navigable's active session history entry's document state's document to null.
@@ -2992,37 +2990,61 @@ void Document::destroy()
     // FIXME: 9. For each workletGlobalScope in document's worklet global scopes, terminate workletGlobalScope.
 }
 
-// https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
-void Document::abort()
+// https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document-and-its-descendants
+void Document::destroy_a_document_and_its_descendants(JS::SafeFunction<void()> after_all_destruction)
 {
-    // 1. Abort the active documents of each of document's descendant navigables.
-    //    If this results in any of those Document objects having their salvageable state set to false,
-    //    then set document's salvageable state to false also.
-    for (auto navigable : descendant_navigables()) {
-        if (auto document = navigable->active_document()) {
-            // NOTE: This is not in the spec but we need to abort ongoing navigations in all descendandt navigables.
-            //       See https://github.com/whatwg/html/issues/9711
-            navigable->set_ongoing_navigation({});
+    // 1. Let childNavigables be document's child navigables.
+    auto child_navigables = document_tree_child_navigables();
 
-            document->abort();
-            if (!document->m_salvageable)
-                m_salvageable = false;
-        }
+    // 3. Let numberDestroyed be 0.
+    size_t number_destroyed = 0;
+
+    // 3. For each childNavigable of childNavigable's, queue a global task on the navigation and traversal task source
+    //    given childNavigable's active window to perform the following steps:
+    for (auto& child_navigable : child_navigables) {
+        HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, *child_navigable->active_window(), [&number_destroyed, child_navigable = child_navigable.ptr()] {
+            // 1. Let incrementDestroyed be an algorithm step which increments numberDestroyed.
+            auto increment_destroyed = [&number_destroyed] { ++number_destroyed; };
+
+            // 2. Destroy a document and its descendants given childNavigable's active document and incrementDestroyed.
+            child_navigable->active_document()->destroy_a_document_and_its_descendants(move(increment_destroyed));
+        });
     }
 
+    // 4. Wait until numberDestroyed equals childNavigable's size.
+    HTML::main_thread_event_loop().spin_until([&] {
+        return number_destroyed == child_navigables.size();
+    });
+
+    // 4. Queue a global task on the navigation and traversal task source given document's relevant global object to perform the following steps:
+    HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(*this), [after_all_destruction = move(after_all_destruction), this] {
+        // 1. Destroy document.
+        destroy();
+
+        // 2. If afterAllDestruction was given, then run it.
+        if (after_all_destruction)
+            after_all_destruction();
+    });
+}
+
+// https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
+void Document::abort()
+{
+    // 1. Assert: this is running as part of a task queued on document's relevant agent's event loop.
+
     // FIXME: 2. Cancel any instances of the fetch algorithm in the context of document,
     //           discarding any tasks queued for them, and discarding any further data received from the network for them.
     //           If this resulted in any instances of the fetch algorithm being canceled
     //           or any queued tasks or any network data getting discarded,
     //           then set document's salvageable state to false.
 
-    // 3. If document's navigation id is non-null, then:
+    // 3. If document's during-loading navigation ID for WebDriver BiDi is non-null, then:
     if (m_navigation_id.has_value()) {
         // 1. FIXME: Invoke WebDriver BiDi navigation aborted with document's browsing context,
         //           and new WebDriver BiDi navigation status whose whose id is document's navigation id,
         //           status is "canceled", and url is document's URL.
 
-        // 2. Set document's navigation id to null.
+        // 2. Set document's during-loading navigation ID for WebDriver BiDi to null.
         m_navigation_id = {};
     }
 
@@ -3039,6 +3061,34 @@ void Document::abort()
     }
 }
 
+// https://html.spec.whatwg.org/multipage/document-lifecycle.html#abort-a-document-and-its-descendants
+void Document::abort_a_document_and_its_descendants()
+{
+    // FIXME 1. Assert: this is running as part of a task queued on document's relevant agent's event loop.
+
+    // 2. Let descendantNavigables be document's descendant navigables.
+    auto descendant_navigables = this->descendant_navigables();
+
+    // 3. For each descendantNavigable of descendantNavigables, queue a global task on the navigation and traversal task source given descendantNavigable's active window to perform the following steps:
+    for (auto& descendant_navigable : descendant_navigables) {
+        HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, *descendant_navigable->active_window(), [this, descendant_navigable = descendant_navigable.ptr()] {
+            // NOTE: This is not in the spec but we need to abort ongoing navigations in all descendant navigables.
+            //       See https://github.com/whatwg/html/issues/9711
+            descendant_navigable->set_ongoing_navigation({});
+
+            // 1. Abort descendantNavigable's active document.
+            descendant_navigable->active_document()->abort();
+
+            // 2. If descendantNavigable's active document's salvageable is false, then set document's salvageable to false.
+            if (!descendant_navigable->active_document()->m_salvageable)
+                m_salvageable = false;
+        });
+    }
+
+    // 4. Abort document.
+    abort();
+}
+
 // https://html.spec.whatwg.org/multipage/dom.html#active-parser
 JS::GCPtr<HTML::HTMLParser> Document::active_parser()
 {
@@ -3139,6 +3189,46 @@ void Document::unload(JS::GCPtr<Document>)
     did_stop_being_active_document_in_navigable();
 }
 
+// https://html.spec.whatwg.org/multipage/document-lifecycle.html#unload-a-document-and-its-descendants
+void Document::unload_a_document_and_its_descendants(JS::GCPtr<Document> new_document, JS::SafeFunction<void()> after_all_unloads)
+{
+    // 1. FIXME: Assert: this is running within document's node navigable's traversable navigable's session history traversal queue.
+
+    // 2. Let childNavigables be document's child navigables.
+    auto child_navigables = document_tree_child_navigables();
+
+    // 2. Let numberUnloaded be 0.
+    size_t number_unloaded = 0;
+
+    // Spec FIXME: in what order?
+    // 3. For each childNavigable of childNavigable's, queue a global task on the navigation and traversal task source
+    //    given childNavigable's active window to perform the following steps:
+    for (auto& child_navigable : child_navigables) {
+        HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, *child_navigable->active_window(), [&number_unloaded, child_navigable = child_navigable.ptr()] {
+            // 1. Let incrementUnloaded be an algorithm step which increments numberUnloaded.
+            auto increment_unloaded = [&number_unloaded] { ++number_unloaded; };
+
+            // 2. Unload a document and its descendants given childNavigable's active document, null, and incrementUnloaded.
+            child_navigable->active_document()->unload_a_document_and_its_descendants(nullptr, move(increment_unloaded));
+        });
+    }
+
+    // 4. Wait until numberUnloaded equals childNavigable's size.
+    HTML::main_thread_event_loop().spin_until([&] {
+        return number_unloaded == child_navigables.size();
+    });
+
+    // 5. Queue a global task on the navigation and traversal task source given document's relevant global object to perform the following steps:
+    HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, HTML::relevant_global_object(*this), [this, new_document, after_all_unloads = move(after_all_unloads)] {
+        // 1. Unload document, passing along newDocument if it is not null.
+        unload(new_document);
+
+        // 2. If afterAllUnloads was given, then run it.
+        if (after_all_unloads)
+            after_all_unloads();
+    });
+}
+
 // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#allowed-to-use
 bool Document::is_allowed_to_use_feature(PolicyControlledFeature feature) const
 {

Userland/Libraries/LibWeb/DOM/Document.h

@@ -489,13 +489,20 @@ public:
     Vector<JS::Handle<HTML::Navigable>> inclusive_ancestor_navigables();
     Vector<JS::Handle<HTML::Navigable>> document_tree_child_navigables();
 
+    // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document
     void destroy();
+    // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document-and-its-descendants
+    void destroy_a_document_and_its_descendants(JS::SafeFunction<void()> after_all_destruction = {});
 
     // https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
     void abort();
+    // https://html.spec.whatwg.org/multipage/document-lifecycle.html#abort-a-document-and-its-descendants
+    void abort_a_document_and_its_descendants();
 
     // https://html.spec.whatwg.org/multipage/document-lifecycle.html#unload-a-document
     void unload(JS::GCPtr<Document> new_document = nullptr);
+    // https://html.spec.whatwg.org/multipage/document-lifecycle.html#unload-a-document-and-its-descendants
+    void unload_a_document_and_its_descendants(JS::GCPtr<Document> new_document, JS::SafeFunction<void()> after_all_unloads = {});
 
     // https://html.spec.whatwg.org/multipage/dom.html#active-parser
     JS::GCPtr<HTML::HTMLParser> active_parser();

Userland/Libraries/LibWeb/HTML/Navigable.cpp

@@ -1371,10 +1371,10 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
             return;
         }
 
-        // 3. Queue a global task on the navigation and traversal task source given navigable's active window to abort navigable's active document.
+        // 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(), [this] {
             VERIFY(this->active_document());
-            this->active_document()->abort();
+            this->active_document()->abort_a_document_and_its_descendants();
         });
 
         // 4. Let documentState be a new document state with

Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp

@@ -20,6 +20,7 @@
 #include <LibWeb/HTML/Origin.h>
 #include <LibWeb/HTML/Scripting/WindowEnvironmentSettingsObject.h>
 #include <LibWeb/HTML/TraversableNavigable.h>
+#include <LibWeb/HTML/Window.h>
 #include <LibWeb/HighResolutionTime/TimeOrigin.h>
 #include <LibWeb/Page/Page.h>
 
@@ -263,8 +264,12 @@ void NavigableContainer::destroy_the_child_navigable()
 
     // FIXME: 4. Inform the navigation API about child navigable destruction given navigable.
 
-    // 5. Destroy navigable's active document.
-    navigable->active_document()->destroy();
+    // 5. Destroy a document and its descendants given navigable's active document.
+    navigable->active_document()->destroy_a_document_and_its_descendants({});
+
+    // Not in the spec
+    navigable->set_has_been_destroyed();
+    HTML::all_navigables().remove(navigable);
 
     // 6. Let parentDocState be container's node navigable's active session history entry's document state.
     auto parent_doc_state = this->navigable()->active_session_history_entry()->document_state();
@@ -277,13 +282,9 @@ void NavigableContainer::destroy_the_child_navigable()
     // 8. Let traversable be container's node navigable's traversable navigable.
     auto traversable = this->navigable()->traversable_navigable();
 
-    // Not in the spec
-    navigable->set_has_been_destroyed();
-    HTML::all_navigables().remove(navigable);
-
     // 9. Append the following session history traversal steps to traversable:
     traversable->append_session_history_traversal_steps([traversable] {
-        // 1. Apply pending history changes to traversable.
+        // 1. Update for navigable creation/destruction given traversable.
         traversable->update_for_navigable_creation_or_destruction();
     });
 }

Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp

@@ -334,6 +334,49 @@ Vector<JS::Handle<Navigable>> TraversableNavigable::get_all_navigables_that_migh
     return results;
 }
 
+// https://html.spec.whatwg.org/multipage/browsing-the-web.html#deactivate-a-document-for-a-cross-document-navigation
+static void deactivate_a_document_for_cross_document_navigation(JS::NonnullGCPtr<DOM::Document> displayed_document, Optional<UserNavigationInvolvement>, JS::NonnullGCPtr<SessionHistoryEntry> target_entry, JS::SafeFunction<void()> after_potential_unloads)
+{
+    // 1. Let navigable be displayedDocument's node navigable.
+    auto navigable = displayed_document->navigable();
+
+    // 2. Let potentiallyTriggerViewTransition be false.
+    auto potentially_trigger_view_transition = false;
+
+    // FIXME: 3. Let isBrowserUINavigation be true if userNavigationInvolvement is "browser UI"; otherwise false.
+
+    // FIXME: 4. Set potentiallyTriggerViewTransition to the result of calling can navigation trigger a cross-document
+    //           view-transition? given displayedDocument, targetEntry's document, navigationType, and isBrowserUINavigation.
+
+    // 5. If potentiallyTriggerViewTransition is false, then:
+    if (!potentially_trigger_view_transition) {
+        // FIXME 1. Let firePageSwapBeforeUnload be the following step
+        //            1. Fire the pageswap event given displayedDocument, targetEntry, navigationType, and null.
+
+        // 2. Set the ongoing navigation for navigable to null.
+        navigable->set_ongoing_navigation({});
+
+        // 3. Unload a document and its descendants given displayedDocument, targetEntry's document, afterPotentialUnloads, and firePageSwapBeforeUnload.
+        displayed_document->unload_a_document_and_its_descendants(target_entry->document(), move(after_potential_unloads));
+    }
+    // FIXME: 6. Otherwise, queue a global task on the navigation and traversal task source given navigable's active window to run the steps:
+    else {
+        // FIXME: 1. Let proceedWithNavigationAfterViewTransitionCapture be the following step:
+        //            1. Append the following session history traversal steps to navigable's traversable navigable:
+        //               1. Set the ongoing navigation for navigable to null.
+        //               2. Unload a document and its descendants given displayedDocument, targetEntry's document, and afterPotentialUnloads.
+
+        // FIXME: 2. Let viewTransition be the result of setting up a cross-document view-transition given displayedDocument,
+        //           targetEntry's document, navigationType, and proceedWithNavigationAfterViewTransitionCapture.
+
+        // FIXME: 3. Fire the pageswap event given displayedDocument, targetEntry, navigationType, and viewTransition.
+
+        // FIXME: 4. If viewTransition is null, then run proceedWithNavigationAfterViewTransitionCapture.
+
+        TODO();
+    }
+}
+
 // https://html.spec.whatwg.org/multipage/browsing-the-web.html#apply-the-history-step
 TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_step(
     int step,
@@ -598,76 +641,61 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
         if (navigable->has_been_destroyed())
             continue;
 
-        // 7. Set navigable's ongoing navigation to null.
-        navigable->set_ongoing_navigation({});
-
-        // 8. Let (scriptHistoryLength, scriptHistoryIndex) be the result of getting the history object length and index given traversable and targetStep.
+        // 7. Let (scriptHistoryLength, scriptHistoryIndex) be the result of getting the history object length and index given traversable and targetStep.
         auto history_object_length_and_index = get_the_history_object_length_and_index(target_step);
         auto script_history_length = history_object_length_and_index.script_history_length;
         auto script_history_index = history_object_length_and_index.script_history_index;
 
-        // 9. Append navigable to navigablesThatMustWaitBeforeHandlingSyncNavigation.
+        // 8. Append navigable to navigablesThatMustWaitBeforeHandlingSyncNavigation.
         navigables_that_must_wait_before_handling_sync_navigation.append(*navigable);
 
-        // 10. Let entriesForNavigationAPI be the result of getting session history entries for the navigation API given navigable and targetStep.
+        // 9. Let entriesForNavigationAPI be the result of getting session history entries for the navigation API given navigable and targetStep.
         auto entries_for_navigation_api = get_session_history_entries_for_the_navigation_api(*navigable, target_step);
 
-        // 11. Queue a global task on the navigation and traversal task source given navigable's active window to run the steps:
-        queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), [&completed_change_jobs, target_entry, navigable, displayed_document, update_only = changing_navigable_continuation.update_only, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api), user_involvement_for_navigate_events]() mutable {
-            // NOTE: This check is not in the spec but we should not continue navigation if navigable has been destroyed.
-            if (navigable->has_been_destroyed()) {
-                return;
-            }
-
-            // 1. If changingNavigableContinuation's update-only is false, then:
-            if (!update_only) {
-                // 1. If targetEntry's document does not equal displayedDocument, then:
-                if (target_entry->document().ptr() != displayed_document.ptr()) {
-                    // 1. Unload displayedDocument given targetEntry's document.
-                    displayed_document->unload(target_entry->document());
-
-                    // 2. For each childNavigable of displayedDocument's descendant navigables, queue a global task on the navigation and traversal task source given
-                    //    childNavigable's active window to unload childNavigable's active document.
-                    for (auto child_navigable : displayed_document->descendant_navigables()) {
-                        queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), [child_navigable] {
-                            child_navigable->active_document()->unload();
-                        });
-                    }
-                }
-
-                // 3. Activate history entry targetEntry for navigable.
-                navigable->activate_history_entry(*target_entry);
-            }
-
-            // 2. If navigable is not traversable, and targetEntry is not navigable's current session history entry, and targetEntry's document state's origin is the same as
-            //    navigable's current session history entry's document state's origin, then fire a traverse navigate event given targetEntry and userInvolvementForNavigateEvents.
-            auto target_origin = target_entry->document_state()->origin();
-            auto current_origin = navigable->current_session_history_entry()->document_state()->origin();
-            bool const is_same_origin = target_origin.has_value() && current_origin.has_value() && target_origin->is_same_origin(*current_origin);
-            if (!navigable->is_traversable()
-                && target_entry.ptr() != navigable->current_session_history_entry()
-                && is_same_origin) {
-                navigable->active_window()->navigation()->fire_a_traverse_navigate_event(*target_entry, user_involvement_for_navigate_events.value_or(UserNavigationInvolvement::None));
-            }
-
-            // 3. Let updateDocument be an algorithm step which performs update document for history step application given targetEntry's document,
-            //    targetEntry, changingNavigableContinuation's update-only, scriptHistoryLength, scriptHistoryIndex, and entriesForNavigationAPI.
-            auto update_document = JS::SafeFunction<void()>([target_entry, update_only, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api)] {
-                target_entry->document()->update_for_history_step_application(*target_entry, update_only, script_history_length, script_history_index, entries_for_navigation_api);
+        // 12. In both cases, let afterPotentialUnloads be the following steps:
+        auto after_potential_unload = JS::SafeFunction<void()>([changing_navigable_continuation, target_entry, displayed_document, &completed_change_jobs, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api)] {
+            // 1. If changingNavigableContinuation's update-only is false, then activate history entry targetEntry for navigable.
+            if (!changing_navigable_continuation.update_only)
+                changing_navigable_continuation.navigable->activate_history_entry(*changing_navigable_continuation.target_entry);
+
+            // 2. Let updateDocument be an algorithm step which performs update document for history step application given
+            //    targetEntry's document, targetEntry, changingNavigableContinuation's update-only, scriptHistoryLength,
+            //    scriptHistoryIndex, navigationType, entriesForNavigationAPI, and displayedEntry.
+            auto update_document = JS::SafeFunction<void()>([changing_navigable_continuation, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api)] {
+                changing_navigable_continuation.target_entry->document()->update_for_history_step_application(*changing_navigable_continuation.target_entry, changing_navigable_continuation.update_only, script_history_length, script_history_index, entries_for_navigation_api);
             });
 
-            // 4. If targetEntry's document is equal to displayedDocument, then perform updateDocument.
-            if (target_entry->document() == displayed_document.ptr()) {
+            // 3. If targetEntry's document is equal to displayedDocument, then perform updateDocument.
+            if (target_entry->document().ptr() == displayed_document.ptr()) {
                 update_document();
             }
             // 5. Otherwise, queue a global task on the navigation and traversal task source given targetEntry's document's relevant global object to perform updateDocument
             else {
-                queue_global_task(Task::Source::NavigationAndTraversal, relevant_global_object(*target_entry->document()), move(update_document));
+                queue_global_task(Task::Source::NavigationAndTraversal, relevant_global_object(*target_entry->document()), [update_document = move(update_document)]() {
+                    update_document();
+                });
             }
 
             // 6. Increment completedChangeJobs.
             completed_change_jobs++;
         });
+
+        // 10. If changingNavigableContinuation's update-only is true, or targetEntry's document is displayedDocument, then:
+        if (changing_navigable_continuation.update_only || target_entry->document().ptr() == displayed_document.ptr()) {
+            // 1. Set the ongoing navigation for navigable to null.
+            navigable->set_ongoing_navigation({});
+
+            // 2. Queue a global task on the navigation and traversal task source given navigable's active window to perform afterPotentialUnloads.
+            queue_global_task(Task::Source::NavigationAndTraversal, *navigable->active_window(), move(after_potential_unload));
+        }
+        // 11. Otherwise:
+        else {
+            // 1. Assert: navigationType is not null.
+            VERIFY(navigation_type.has_value());
+
+            // 2. Deactivate displayedDocument, given userNavigationInvolvement, targetEntry, navigationType, and afterPotentialUnloads.
+            deactivate_a_document_for_cross_document_navigation(*displayed_document, user_involvement_for_navigate_events, *target_entry, move(after_potential_unload));
+        }
     }
 
     // 15. Let totalNonchangingJobs be the size of nonchangingNavigablesThatStillNeedUpdates.