eliott/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird synced 2026-04-26 01:35:08 +02:00
Code Issues Packages Projects Releases Wiki Activity

LibWeb: Remove ::spin_until() from unloading a doc with descendants

Browse Source 
Rename DocumentDestructionState into a more generic
DocumentLifecycleState so it can be reused for the document unloading
logic. Instead of using ::spin_until(), use a callback mechanism to run
code once all child navigables are unloaded.
This commit is contained in:
Jelle Raaijmakers
2026-03-13 16:45:30 +01:00
committed by Tim Flynn
parent 0a81470bff
commit a095e4cd58
Notes: github-actions[bot] 2026-03-15 13:04:42 +00:00 
Author: https://github.com/gmta
Commit: https://github.com/LadybirdBrowser/ladybird/commit/a095e4cd58a
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/8397
Reviewed-by: https://github.com/trflynn89 ✅
1 changed files with 64 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

126
Libraries/LibWeb/DOM/Document.cpp
View File

@@ -4581,104 +4581,103 @@ void Document::make_unsalvageable([[maybe_unused]] String reason)
set_salvageable(false);
}
struct DocumentDestructionState : public GC::Cell {
GC_CELL(DocumentDestructionState, GC::Cell);
GC_DECLARE_ALLOCATOR(DocumentDestructionState);
struct DocumentLifecycleState : public GC::Cell {
GC_CELL(DocumentLifecycleState, GC::Cell);
GC_DECLARE_ALLOCATOR(DocumentLifecycleState);
static constexpr int TIMEOUT_MS = 15000;
DocumentDestructionState(GC::Ref<Document> document, size_t remaining, GC::Ptr<GC::Function<void()>> after)
DocumentLifecycleState(GC::Ref<Document> document, size_t remaining, GC::Ref<GC::Function<void()>> finish_callback)
: remaining_children(remaining)
, document(document)
, after_all(after)
, finish_callback(finish_callback)
, timeout(Platform::Timer::create_single_shot(heap(), TIMEOUT_MS, GC::create_function(heap(), [this] {
if (remaining_children > 0)
dbgln("FIXME: Document destruction timed out with {} remaining children", remaining_children);
dbgln("FIXME: Document unload/destruction timed out with {} remaining children", remaining_children);
})))
{
timeout->start();
}
virtual void visit_edges(GC::Cell::Visitor& visitor) override
virtual void visit_edges(Visitor& visitor) override
{
Base::visit_edges(visitor);
visitor.visit(document);
visitor.visit(after_all);
visitor.visit(finish_callback);
visitor.visit(timeout);
}
void increment_destroyed()
void did_process_child()
{
--remaining_children;
if (remaining_children > 0)
if (--remaining_children > 0)
return;
timeout->stop();
queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(document), GC::create_function(heap(), [document = move(document), after_all = move(after_all)] {
document->destroy();
if (after_all)
after_all->function()();
}));
queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(document), finish_callback);
}
size_t remaining_children { 0 };
GC::Ref<Document> document;
GC::Ptr<GC::Function<void()>> after_all;
GC::Ref<GC::Function<void()>> finish_callback;
GC::Ref<Platform::Timer> timeout;
};
GC_DEFINE_ALLOCATOR(DocumentDestructionState);
GC_DEFINE_ALLOCATOR(DocumentLifecycleState);
// https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document-and-its-descendants
void Document::destroy_a_document_and_its_descendants(GC::Ptr<GC::Function<void()>> after_all_destruction)
{
// 1. If document is not fully active, then:
if (!is_fully_active()) {
// 1. Let reason be a string from user-agent specific blocking reasons.
// If none apply, then let reason be "masked".
// 1. Let reason be a string from user-agent specific blocking reasons. If none apply, then let reason be
// "masked".
// FIXME: user-agent specific blocking reasons.
auto reason = "masked"_string;
// 2. Make document unsalvageable given document and reason.
make_unsalvageable(reason);
// FIXME: 3. If document's node navigable is a top-level traversable,
// build not restored reasons for a top-level traversable and its descendants given document's node navigable.
// FIXME: 3. If document's node navigable is a top-level traversable, build not restored reasons for a top-level
// traversable and its descendants given document's node navigable.
}
// 2. Let childNavigables be document's child navigables.
IGNORE_USE_IN_ESCAPING_LAMBDA auto child_navigables = document_tree_child_navigables();
IGNORE_USE_IN_ESCAPING_LAMBDA auto child_navigables = navigable()->child_navigables();
// NOTE: Not in the spec but we could avoid allocating destruction state in case there's no child navigables.
// 6. Queue a global task on the navigation and traversal task source given document's relevant global object to
// perform the following steps:
auto finish_callback = GC::create_function(heap(), [document = this, after_all_destruction] {
// 1. Destroy document.
document->destroy();
// 2. If afterAllDestruction was given, then run it.
if (after_all_destruction)
after_all_destruction->function()();
});
// AD-HOC: We avoid allocating a DocumentLifecycleState in case there's no child navigables.
if (child_navigables.is_empty()) {
HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(*this), GC::create_function(heap(), [document = this, after_all_destruction] {
// 1. Destroy document.
document->destroy();
// 2. If afterAllDestruction was given, then run it.
if (after_all_destruction)
after_all_destruction->function()();
}));
HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(*this), finish_callback);
return;
}
// 3. Let numberDestroyed be 0.
auto destruction_state = heap().allocate<DocumentDestructionState>(*this, child_navigables.size(), after_all_destruction);
auto destruction_state = heap().allocate<DocumentLifecycleState>(*this, child_navigables.size(), finish_callback);
// 4. For each childNavigable of childNavigables, 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) {
queue_global_task(HTML::Task::Source::NavigationAndTraversal, *child_navigable->active_window(), GC::create_function(heap(), [&heap = heap(), destruction_state, child_navigable] {
// 1. Let incrementDestroyed be an algorithm step which increments numberDestroyed.
auto increment_destroyed = GC::create_function(heap, [destruction_state] { destruction_state->increment_destroyed(); });
queue_global_task(HTML::Task::Source::NavigationAndTraversal, *child_navigable->active_window(),
GC::create_function(heap(), [&heap = heap(), destruction_state, child_navigable] {
// 1. Let incrementDestroyed be an algorithm step which increments numberDestroyed.
auto increment_destroyed = GC::create_function(heap, [destruction_state] { destruction_state->did_process_child(); });
// 2. Destroy a document and its descendants given childNavigable's active document and incrementDestroyed.
child_navigable->active_document()->destroy_a_document_and_its_descendants(increment_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(increment_destroyed);
}));
}
// NOTE: Both of subsequent steps are handled by DocumentDestructionState.
// 5. Wait until numberDestroyed equals childNavigable's size.
// 6. Queue a global task on the navigation and traversal task source given document's relevant global object to perform the following steps:
// NB: This is handled by destruction_state.
}
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#abort-a-document
@@ -4855,18 +4854,37 @@ void Document::unload_a_document_and_its_descendants(GC::Ptr<Document> new_docum
// queue.
// 2. Let childNavigables be document's child navigables.
auto child_navigables = navigable()->child_navigables();
IGNORE_USE_IN_ESCAPING_LAMBDA auto child_navigables = navigable()->child_navigables();
// 6. Queue a global task on the navigation and traversal task source given document's relevant global object to
// perform the following steps:
auto finish_callback = GC::create_function(heap(), [document = this, new_document, after_all_unloads] {
// FIXME: 1. If firePageSwapSteps is given, then run firePageSwapSteps.
// 2. Unload document, passing along newDocument if it is not null.
document->unload(new_document);
// 3. If afterAllUnloads was given, then run it.
if (after_all_unloads)
after_all_unloads->function()();
});
// AD-HOC: We avoid allocating a DocumentLifecycleState in case there's no child navigables.
if (child_navigables.is_empty()) {
HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, relevant_global_object(*this), finish_callback);
return;
}
// 3. Let numberUnloaded be 0.
IGNORE_USE_IN_ESCAPING_LAMBDA size_t number_unloaded = 0;
auto unload_state = heap().allocate<DocumentLifecycleState>(*this, child_navigables.size(), finish_callback);
// 4. For each childNavigable of childNavigables [[ in what order? ]], 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(),
GC::create_function(heap(), [&heap = heap(), child_navigable = child_navigable.ptr(), &number_unloaded] {
GC::create_function(heap(), [&heap = heap(), unload_state, child_navigable = child_navigable.ptr()] {
// 1. Let incrementUnloaded be an algorithm step which increments numberUnloaded.
auto increment_unloaded = GC::create_function(heap, [&number_unloaded] { ++number_unloaded; });
auto increment_unloaded = GC::create_function(heap, [unload_state] { unload_state->did_process_child(); });
// 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({}, increment_unloaded);
@@ -4874,23 +4892,7 @@ void Document::unload_a_document_and_its_descendants(GC::Ptr<Document> new_docum
}
// 5. Wait until numberUnloaded equals childNavigables's size.
HTML::main_thread_event_loop().spin_until(GC::create_function(heap(), [&] {
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, HTML::relevant_global_object(*this),
GC::create_function(heap(), [this, new_document, after_all_unloads] {
// FIXME: 1. If firePageSwapSteps is given, then run firePageSwapSteps.
// 2. Unload document, passing along newDocument if it is not null.
unload(new_document);
// 3. If afterAllUnloads was given, then run it.
if (after_all_unloads)
after_all_unloads->function()();
}));
// NB: This is handled by unload_state.
}
// https://html.spec.whatwg.org/multipage/iframe-embed-object.html#allowed-to-use