From 6ed2bf2bb1c26a8a7746f3622a9e60b56408e280 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Mon, 9 Dec 2024 19:47:09 -0700 Subject: [PATCH] LibWeb: Mark local variables captured in GC functions as ignored These variables are all captured in queued events or other event loop tasks, but are all guarded by event loop spins later in the function. The IGNORE_USE_IN_ESCAPING_LAMBDA will soon be required for all locals that are captured by ref in GC::Function as well as AK::Function. --- Libraries/LibWeb/DOM/Document.cpp | 4 ++-- Libraries/LibWeb/HTML/EventSource.cpp | 2 +- Libraries/LibWeb/HTML/Scripting/Fetching.cpp | 3 ++- Libraries/LibWeb/HTML/TraversableNavigable.cpp | 16 ++++++++-------- .../LibWeb/IndexedDB/Internal/Algorithms.cpp | 4 ++-- Libraries/LibWeb/XHR/XMLHttpRequest.cpp | 4 ++-- 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index d5e3fce1324..5a4eb93b8b8 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -3470,7 +3470,7 @@ void Document::destroy_a_document_and_its_descendants(GC::Ptr new_docum descendant_navigables.append(other_navigable); } - auto unloaded_documents_count = descendant_navigables.size() + 1; + IGNORE_USE_IN_ESCAPING_LAMBDA auto unloaded_documents_count = descendant_navigables.size() + 1; HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, HTML::relevant_global_object(*this), GC::create_function(heap(), [&number_unloaded, this, new_document] { unload(new_document); diff --git a/Libraries/LibWeb/HTML/EventSource.cpp b/Libraries/LibWeb/HTML/EventSource.cpp index cd91883c6f8..79ffec187c5 100644 --- a/Libraries/LibWeb/HTML/EventSource.cpp +++ b/Libraries/LibWeb/HTML/EventSource.cpp @@ -280,7 +280,7 @@ void EventSource::announce_the_connection() // https://html.spec.whatwg.org/multipage/server-sent-events.html#reestablish-the-connection void EventSource::reestablish_the_connection() { - bool initial_task_has_run { false }; + IGNORE_USE_IN_ESCAPING_LAMBDA bool initial_task_has_run { false }; // 1. Queue a task to run the following steps: HTML::queue_a_task(HTML::Task::Source::RemoteEvent, nullptr, nullptr, GC::create_function(heap(), [&]() { diff --git a/Libraries/LibWeb/HTML/Scripting/Fetching.cpp b/Libraries/LibWeb/HTML/Scripting/Fetching.cpp index 66cb8d3730a..3658e0d8efb 100644 --- a/Libraries/LibWeb/HTML/Scripting/Fetching.cpp +++ b/Libraries/LibWeb/HTML/Scripting/Fetching.cpp @@ -475,7 +475,7 @@ WebIDL::ExceptionOr> fetch_a_classic_worker_imported_scri auto& vm = realm.vm(); // 1. Let response be null. - GC::Ptr response = nullptr; + IGNORE_USE_IN_ESCAPING_LAMBDA GC::Ptr response = nullptr; // 2. Let bodyBytes be null. Fetch::Infrastructure::FetchAlgorithms::BodyBytes body_bytes; @@ -510,6 +510,7 @@ WebIDL::ExceptionOr> fetch_a_classic_worker_imported_scri } // 5. Pause until response is not null. + // FIXME: Consider using a "response holder" to avoid needing to annotate response as IGNORE_USE_IN_ESCAPING_LAMBDA. auto& event_loop = settings_object.responsible_event_loop(); event_loop.spin_until(GC::create_function(vm.heap(), [&]() -> bool { return response; diff --git a/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 9dd5d2b9f70..132594d789c 100644 --- a/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -429,7 +429,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ bool check_for_cancelation, IGNORE_USE_IN_ESCAPING_LAMBDA Optional source_snapshot_params, GC::Ptr initiator_to_check, - Optional user_involvement_for_navigate_events, + IGNORE_USE_IN_ESCAPING_LAMBDA Optional user_involvement_for_navigate_events, IGNORE_USE_IN_ESCAPING_LAMBDA Optional navigation_type, IGNORE_USE_IN_ESCAPING_LAMBDA SynchronousNavigation synchronous_navigation) { @@ -487,7 +487,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ } // 9. Let totalChangeJobs be the size of changingNavigables. - auto total_change_jobs = changing_navigables.size(); + IGNORE_USE_IN_ESCAPING_LAMBDA auto total_change_jobs = changing_navigables.size(); // 10. Let completedChangeJobs be 0. IGNORE_USE_IN_ESCAPING_LAMBDA size_t completed_change_jobs = 0; @@ -799,7 +799,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_ })); // 15. Let totalNonchangingJobs be the size of nonchangingNavigablesThatStillNeedUpdates. - auto total_non_changing_jobs = non_changing_navigables_that_still_need_updates.size(); + IGNORE_USE_IN_ESCAPING_LAMBDA auto total_non_changing_jobs = non_changing_navigables_that_still_need_updates.size(); // 16. Let completedNonchangingJobs be 0. IGNORE_USE_IN_ESCAPING_LAMBDA auto completed_non_changing_jobs = 0u; @@ -880,10 +880,10 @@ TraversableNavigable::CheckIfUnloadingIsCanceledResult TraversableNavigable::che documents_to_fire_beforeunload.append(navigable->active_document()); // 2. Let unloadPromptShown be false. - auto unload_prompt_shown = false; + IGNORE_USE_IN_ESCAPING_LAMBDA auto unload_prompt_shown = false; // 3. Let finalStatus be "continue". - auto final_status = CheckIfUnloadingIsCanceledResult::Continue; + IGNORE_USE_IN_ESCAPING_LAMBDA auto final_status = CheckIfUnloadingIsCanceledResult::Continue; // 4. If traversable was given, then: if (traversable) { @@ -900,7 +900,7 @@ TraversableNavigable::CheckIfUnloadingIsCanceledResult TraversableNavigable::che VERIFY(user_involvement_for_navigate_events.has_value()); // 2. Let eventsFired be false. - auto events_fired = false; + IGNORE_USE_IN_ESCAPING_LAMBDA auto events_fired = false; // 3. Let needsBeforeunload be true if navigablesThatNeedBeforeUnload contains traversable; otherwise false. auto it = navigables_that_need_before_unload.find_if([&traversable](GC::Root navigable) { @@ -964,10 +964,10 @@ TraversableNavigable::CheckIfUnloadingIsCanceledResult TraversableNavigable::che } // 5. Let totalTasks be the size of documentsThatNeedBeforeunload. - auto total_tasks = documents_to_fire_beforeunload.size(); + IGNORE_USE_IN_ESCAPING_LAMBDA auto total_tasks = documents_to_fire_beforeunload.size(); // 6. Let completedTasks be 0. - size_t completed_tasks = 0; + IGNORE_USE_IN_ESCAPING_LAMBDA size_t completed_tasks = 0; // 7. For each document of documents, queue a global task on the navigation and traversal task source given document's relevant global object to run the steps: for (auto& document : documents_to_fire_beforeunload) { diff --git a/Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp b/Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp index 2a533256e46..69cc8b227f0 100644 --- a/Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp +++ b/Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp @@ -79,8 +79,8 @@ WebIDL::ExceptionOr> open_a_database_connection(JS::Realm& // 2. For each entry of openConnections that does not have its close pending flag set to true, // queue a task to fire a version change event named versionchange at entry with db’s version and version. - u32 events_to_fire = open_connections.size(); - u32 events_fired = 0; + IGNORE_USE_IN_ESCAPING_LAMBDA u32 events_to_fire = open_connections.size(); + IGNORE_USE_IN_ESCAPING_LAMBDA u32 events_fired = 0; for (auto& entry : open_connections) { if (!entry->close_pending()) { HTML::queue_a_task(HTML::Task::Source::DatabaseAccess, nullptr, nullptr, GC::create_function(realm.vm().heap(), [&realm, entry, db, version, &events_fired]() { diff --git a/Libraries/LibWeb/XHR/XMLHttpRequest.cpp b/Libraries/LibWeb/XHR/XMLHttpRequest.cpp index 2423c890788..da3fdea6e8e 100644 --- a/Libraries/LibWeb/XHR/XMLHttpRequest.cpp +++ b/Libraries/LibWeb/XHR/XMLHttpRequest.cpp @@ -892,7 +892,7 @@ WebIDL::ExceptionOr XMLHttpRequest::send(Optional response, Variant null_or_failure_or_bytes) { @@ -927,7 +927,7 @@ WebIDL::ExceptionOr XMLHttpRequest::send(Optional