Skip to content

Commit

Permalink
LibWeb: Overhaul document unloading/destruction/aborting
Browse files Browse the repository at this point in the history 
See whatwg/html#9907

As far as I understand these changes do not fix bugs in our
implementation but allows us to remove some hacks and be aligned with
the spec.
  • Loading branch information
@kalenikaliaksandr
kalenikaliaksandr committed Nov 3, 2023
1 parent f9cab32 commit 9ecf124
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 62 deletions.
124 changes: 97 additions & 27 deletions Userland/Libraries/LibWeb/DOM/Document.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2696,14 +2696,15 @@ void Document::destroy()
{
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.
// FIXME: 1. Assert: this is running as part of a task queued on document's relevant agent's event loop.

// 2. Abort document.
abort();

// 2. Set document's salvageable state to false.
// 3. Set document's salvageable state to false.
m_salvageable = false;

// 3. Run any unloading document cleanup steps for document that are defined by this specification and other applicable specifications.
// 4. Run any unloading document cleanup steps for document that are defined by this specification and other applicable specifications.
run_unloading_cleanup_steps();

// 5. Remove any tasks whose document is document from any task queue (without running those tasks).
Expand All @@ -2714,14 +2715,6 @@ 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.
for (auto& navigable_container : HTML::NavigableContainer::all_instances()) {
if (&navigable_container->document() == this) {
navigable_container->destroy_the_child_navigable();
}
}

// 7. Set document's node navigable's active session history entry's document state's document to null.
if (navigable()) {
navigable()->active_session_history_entry()->document_state->set_document(nullptr);
Expand All @@ -2732,35 +2725,50 @@ void Document::destroy()
// FIXME: 9. For each workletGlobalScope in document's worklet global scopes, terminate workletGlobalScope.
}

void Document::destroy_document_and_its_descendants(JS::NonnullGCPtr<Document> document)
{
// 1. Let childNavigables be document's child navigables.
auto child_navigables = document->document_tree_child_navigables();

// 2. 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(), [&] {
// 1. Destroy childNavigable's active document.
child_navigable->active_document()->destroy();

// 2. Increment numberDestroyed.
number_destroyed++;
});
}

// 4. Wait until numberDestroyed equals childNavigable's size.
HTML::main_thread_event_loop().spin_until([&] {
return number_destroyed == child_navigables.size();
});
}

// https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
void Document::abort()
{
// 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({});

document->abort();
if (!document->m_salvageable)
m_salvageable = false;
}
}

// 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.
// FIXME: 1. Invoke WebDriver BiDi navigation aborted with document's browsing context, and new WebDriver BiDi navigation
// status whose whose id is document's during-loading navigation ID for WebDriver BiDi, status is "canceled",
// and url is document's URL.

// 2. Set document's navigation id to null.
m_navigation_id = {};
Expand All @@ -2779,6 +2787,31 @@ void Document::abort()
}
}

void Document::abort_document_and_its_descendants(JS::NonnullGCPtr<Document> document)
{
// 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 = document->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(), [descendant_navigable] {
// 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) {
descendant_navigable->active_document()->set_salvageable(false);
}
});
}

// 4. Abort document.
document->abort();
}

// https://html.spec.whatwg.org/multipage/dom.html#active-parser
JS::GCPtr<HTML::HTMLParser> Document::active_parser()
{
Expand Down Expand Up @@ -2879,6 +2912,43 @@ void Document::unload(JS::GCPtr<Document>)
did_stop_being_active_document_in_navigable();
}

void Document::unload_a_document_and_its_descendants(JS::NonnullGCPtr<Document> document, JS::GCPtr<Document> new_document, JS::SafeFunction<void()> after_all_unloads)
{
// FIXME: 1. 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->document_tree_child_navigables();

// 3. Let numberUnloaded be 0.
size_t number_unloaded = 0;

// 4. 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(), [child_navigable, &number_unloaded] {
// 1. Unload childNavigable's active document.
child_navigable->active_document()->unload();

// 2. Increment numberUnloaded.
number_unloaded++;
});
}

// 5. Wait until numberUnloaded equals childNavigable's size.
HTML::main_thread_event_loop().spin_until([&] {
return number_unloaded == child_navigables.size();
});

// 6. 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(*document), [document, after_all_unloads = move(after_all_unloads)] {
// 1. Unload document, passing along newDocument if it is not null.
document->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
{
Expand Down
4 changes: 4 additions & 0 deletions Userland/Libraries/LibWeb/DOM/Document.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,10 @@ class Document
// https://html.spec.whatwg.org/multipage/document-lifecycle.html#unload-a-document
void unload(JS::GCPtr<Document> new_document = nullptr);

static void destroy_document_and_its_descendants(JS::NonnullGCPtr<Document>);
static void abort_document_and_its_descendants(JS::NonnullGCPtr<Document>);
static void unload_a_document_and_its_descendants(JS::NonnullGCPtr<Document> document, 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();

Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/HTML/Navigable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1251,10 +1251,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();
DOM::Document::abort_document_and_its_descendants(*this->active_document());
});

// 4. Let documentState be a new document state with
Expand Down
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/HTML/NavigableContainer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ void NavigableContainer::destroy_the_child_navigable()
// 3. Set container's content navigable to null.
m_content_navigable = nullptr;

// 4. Destroy navigable's active document.
navigable->active_document()->destroy();
// 4. Destroy a document and its descendants given navigable's active document.
DOM::Document::destroy_document_and_its_descendants(*navigable->active_document());

// 5. 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;
Expand Down
70 changes: 39 additions & 31 deletions Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,43 +416,51 @@ void TraversableNavigable::apply_the_history_step(int step, Optional<SourceSnaps

// FIXME: 9. Append navigable to navigablesThatMustWaitBeforeHandlingSyncNavigation.

// 10. 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(), [&, target_entry, navigable, displayed_document, update_only = changing_navigable_continuation.update_only, script_history_length, script_history_index] {
// 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;
// FIXME: 10. Let entriesForNavigationAPI be the result of getting session history entries for the navigation API given navigable and targetStep.

// 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_state->document().ptr() != displayed_document.ptr()) {
// 1. Unload displayedDocument given targetEntry's document.
displayed_document->unload(target_entry->document_state->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();
});
}
}
// 11. If changingNavigableContinuation's update-only is false, and targetEntry's document does not equal displayedDocument, then unload a document
// and its descendants given displayedDocument, targetEntry's document, and afterPotentialUnloads.
auto update_only = changing_navigable_continuation.update_only;

// 3. Activate history entry targetEntry for navigable.
JS::SafeFunction<void()> after_potential_unloads = [update_only, navigable, target_entry, &completed_change_jobs, script_history_length, script_history_index, displayed_document] {
// 1. If changingNavigableContinuation's update-only is false, then activate history entry targetEntry for navigable.
if (!update_only) {
navigable->activate_history_entry(*target_entry);
}

// FIXME: 2. If targetEntry's document is not equal to displayedDocument, then queue a global task on the navigation and traversal task source given targetEntry's document's
// relevant global object to perform the following step. Otherwise, continue onward to perform the following step within the currently-queued task.
// FIXME: 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.

// 3. Update document for history step application given targetEntry's document, targetEntry, changingNavigableContinuation's update-only, scriptHistoryLength, and
// scriptHistoryIndex and entriesForNavigationAPI.
// FIXME: Pass entriesForNavigationAPI
target_entry->document_state->document()->update_for_history_step_application(*target_entry, update_only, script_history_length, script_history_index);
// 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 = [target_entry, update_only, script_history_length, script_history_index] {
target_entry->document_state->document()->update_for_history_step_application(*target_entry, update_only, script_history_length, script_history_index);
};

// 4. Increment completedChangeJobs.
// 4. If targetEntry's document is equal to displayedDocument, then perform updateDocument.
if (target_entry->document_state->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_state->document()), move(update_document));
}

// 6. Increment completedChangeJobs.
completed_change_jobs++;
});
};

// 10. If changingNavigableContinuation's update-only is false, and targetEntry's document does not equal displayedDocument, then unload a document and its
// descendants given displayedDocument, targetEntry's document, and afterPotentialUnloads.
if (!update_only && target_entry->document_state->document().ptr() != displayed_document.ptr()) {
DOM::Document::unload_a_document_and_its_descendants(*displayed_document.ptr(), target_entry->document_state->document(), move(after_potential_unloads));
} else {
// Otherwise, 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(), [after_potential_unloads = move(after_potential_unloads)] {
after_potential_unloads();
});
}
}

// FIXME: 15. Let totalNonchangingJobs be the size of nonchangingNavigablesThatStillNeedUpdates.
Expand Down Expand Up @@ -591,9 +599,9 @@ void TraversableNavigable::destroy_top_level_traversable()
// 1. Let document be historyEntry's document.
auto document = history_entry->document_state->document();

// 2. If document is not null, then destroy document.
// 2. If document is not null, then destroy document and its descendants given document.
if (document)
document->destroy();
DOM::Document::destroy_a_document_and_its_descendants(*document);
}

// 3. Remove browsingContext.
Expand Down

0 comments on commit 9ecf124

Please sign in to comment.