LibWeb/IndexedDB: Remove spin_until from checking finished transactions

Author: Lubrsi

Libraries/LibWeb/CMakeLists.txt

@@ -684,6 +684,7 @@ set(SOURCES
     IndexedDB/Internal/Database.cpp
     IndexedDB/Internal/IDBDatabaseObserver.cpp
     IndexedDB/Internal/IDBRequestObserver.cpp
+    IndexedDB/Internal/IDBTransactionObserver.cpp
     IndexedDB/Internal/Index.cpp
     IndexedDB/Internal/Key.cpp
     IndexedDB/Internal/ObjectStore.cpp

Libraries/LibWeb/Forward.h

@@ -813,6 +813,7 @@ class IDBRecord;
 class IDBRequest;
 class IDBRequestObserver;
 class IDBTransaction;
+class IDBTransactionObserver;
 class IDBVersionChangeEvent;
 class Index;
 class ObjectStore;

Libraries/LibWeb/IndexedDB/IDBDatabase.cpp

@@ -12,10 +12,12 @@
 #include <LibWeb/IndexedDB/IDBObjectStore.h>
 #include <LibWeb/IndexedDB/Internal/Algorithms.h>
 #include <LibWeb/IndexedDB/Internal/IDBDatabaseObserver.h>
+#include <LibWeb/IndexedDB/Internal/IDBTransactionObserver.h>
 
 namespace Web::IndexedDB {
 
 GC_DEFINE_ALLOCATOR(IDBDatabase);
+GC_DEFINE_ALLOCATOR(IDBDatabase::TransactionFinishState);
 
 IDBDatabase::IDBDatabase(JS::Realm& realm, Database& db)
     : EventTarget(realm)
@@ -47,6 +49,7 @@ void IDBDatabase::visit_edges(Visitor& visitor)
     visitor.visit(m_object_store_set);
     visitor.visit(m_associated_database);
     visitor.visit(m_transactions);
+    visitor.visit(m_transaction_finish_queue);
 }
 
 void IDBDatabase::set_onabort(WebIDL::CallbackType* event_handler)
@@ -263,4 +266,60 @@ void IDBDatabase::set_state(ConnectionState state)
     });
 }
 
+void IDBDatabase::wait_for_transactions_to_finish(ReadonlySpan<GC::Ref<IDBTransaction>> transactions, GC::Ref<GC::Function<void()>> on_complete)
+{
+    GC::Ptr<TransactionFinishState> transaction_finish_state;
+
+    for (auto const& entry : transactions) {
+        if (!entry->is_finished()) {
+            if (!transaction_finish_state) {
+                transaction_finish_state = heap().allocate<TransactionFinishState>();
+            }
+
+            transaction_finish_state->add_transaction_to_observe(entry);
+        }
+    }
+
+    if (transaction_finish_state) {
+        transaction_finish_state->after_all = GC::create_function(heap(), [this, transaction_finish_state, on_complete] {
+            bool was_removed = m_transaction_finish_queue.remove_first_matching([transaction_finish_state](GC::Ref<TransactionFinishState> pending_transaction_finish_state) {
+                return pending_transaction_finish_state == transaction_finish_state;
+            });
+            VERIFY(was_removed);
+            queue_a_database_task(on_complete);
+        });
+        m_transaction_finish_queue.append(transaction_finish_state.as_nonnull());
+    } else {
+        queue_a_database_task(on_complete);
+    }
+}
+
+void IDBDatabase::TransactionFinishState::visit_edges(Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(transaction_observers);
+    visitor.visit(after_all);
+}
+
+void IDBDatabase::TransactionFinishState::add_transaction_to_observe(GC::Ref<IDBTransaction> transaction)
+{
+    auto transaction_observer = heap().allocate<IDBTransactionObserver>(transaction);
+    transaction_observer->set_transaction_finished_observer(GC::create_function(heap(), [this] {
+        transaction_observers.remove_all_matching([](GC::Ref<IDBTransactionObserver> const& transaction_observer) {
+            if (transaction_observer->transaction()->is_finished()) {
+                transaction_observer->unobserve();
+                return true;
+            }
+
+            return false;
+        });
+
+        if (transaction_observers.is_empty()) {
+            queue_a_database_task(after_all.as_nonnull());
+        }
+    }));
+
+    transaction_observers.append(transaction_observer);
+}
+
 }

Libraries/LibWeb/IndexedDB/IDBDatabase.h

@@ -83,6 +83,8 @@ class IDBDatabase : public DOM::EventTarget {
     void register_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver&);
     void unregister_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver&);
 
+    void wait_for_transactions_to_finish(ReadonlySpan<GC::Ref<IDBTransaction>>, GC::Ref<GC::Function<void()>> on_complete);
+
 protected:
     explicit IDBDatabase(JS::Realm&, Database&);
 
@@ -110,6 +112,20 @@ class IDBDatabase : public DOM::EventTarget {
     HashTable<GC::RawRef<IDBDatabaseObserver>> m_database_observers;
     Vector<GC::Ref<IDBDatabaseObserver>> m_database_observers_being_notified;
 
+    struct TransactionFinishState final : public GC::Cell {
+        GC_CELL(TransactionFinishState, GC::Cell);
+        GC_DECLARE_ALLOCATOR(TransactionFinishState);
+
+        virtual void visit_edges(Visitor& visitor) override;
+
+        void add_transaction_to_observe(GC::Ref<IDBTransaction> transaction);
+
+        Vector<GC::Ref<IDBTransactionObserver>> transaction_observers;
+        GC::Ptr<GC::Function<void()>> after_all;
+    };
+
+    Vector<GC::Ref<TransactionFinishState>> m_transaction_finish_queue;
+
     u64 m_version { 0 };
     String m_name;
 

Libraries/LibWeb/IndexedDB/IDBTransaction.cpp

@@ -11,6 +11,7 @@
 #include <LibWeb/IndexedDB/IDBObjectStore.h>
 #include <LibWeb/IndexedDB/IDBTransaction.h>
 #include <LibWeb/IndexedDB/Internal/Algorithms.h>
+#include <LibWeb/IndexedDB/Internal/IDBTransactionObserver.h>
 
 namespace Web::IndexedDB {
 
@@ -43,6 +44,7 @@ void IDBTransaction::initialize(JS::Realm& realm)
 void IDBTransaction::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
+    visitor.visit(m_transaction_observers_being_notified);
     visitor.visit(m_connection);
     visitor.visit(m_error);
     visitor.visit(m_associated_request);
@@ -147,4 +149,27 @@ WebIDL::ExceptionOr<GC::Ref<IDBObjectStore>> IDBTransaction::object_store(String
     return IDBObjectStore::create(realm, *store, *this);
 }
 
+void IDBTransaction::register_transaction_observer(Badge<IDBTransactionObserver>, IDBTransactionObserver& database_observer)
+{
+    auto result = m_transaction_observers.set(database_observer);
+    VERIFY(result == AK::HashSetResult::InsertedNewEntry);
+}
+
+void IDBTransaction::unregister_transaction_observer(Badge<IDBTransactionObserver>, IDBTransactionObserver& database_observer)
+{
+    bool was_removed = m_transaction_observers.remove(database_observer);
+    VERIFY(was_removed);
+}
+
+void IDBTransaction::set_state(TransactionState state)
+{
+    m_state = state;
+
+    if (m_state == TransactionState::Finished) {
+        notify_each_transaction_observer([](IDBTransactionObserver const& transaction_observer) {
+            return transaction_observer.transaction_finished_observer();
+        });
+    }
+}
+
 }

Libraries/LibWeb/IndexedDB/IDBTransaction.h

@@ -54,7 +54,7 @@ class IDBTransaction : public DOM::EventTarget {
     void set_associated_request(GC::Ptr<IDBRequest> request) { m_associated_request = request; }
     void set_aborted(bool aborted) { m_aborted = aborted; }
     void set_cleanup_event_loop(GC::Ptr<HTML::EventLoop> event_loop) { m_cleanup_event_loop = event_loop; }
-    void set_state(TransactionState state) { m_state = state; }
+    void set_state(TransactionState state);
 
     [[nodiscard]] bool is_upgrade_transaction() const { return m_mode == Bindings::IDBTransactionMode::Versionchange; }
     [[nodiscard]] bool is_readonly() const { return m_mode == Bindings::IDBTransactionMode::Readonly; }
@@ -78,12 +78,35 @@ class IDBTransaction : public DOM::EventTarget {
     void set_onerror(WebIDL::CallbackType*);
     WebIDL::CallbackType* onerror();
 
+    void register_transaction_observer(Badge<IDBTransactionObserver>, IDBTransactionObserver&);
+    void unregister_transaction_observer(Badge<IDBTransactionObserver>, IDBTransactionObserver&);
+
 protected:
     explicit IDBTransaction(JS::Realm&, GC::Ref<IDBDatabase>, Bindings::IDBTransactionMode, Bindings::IDBTransactionDurability, Vector<GC::Ref<ObjectStore>>);
     virtual void initialize(JS::Realm&) override;
     virtual void visit_edges(Visitor& visitor) override;
 
 private:
+    template<typename GetNotifier, typename... Args>
+    void notify_each_transaction_observer(GetNotifier&& get_notifier, Args&&... args)
+    {
+        ScopeGuard guard { [&]() { m_transaction_observers_being_notified.clear_with_capacity(); } };
+        m_transaction_observers_being_notified.ensure_capacity(m_transaction_observers.size());
+
+        for (auto observer : m_transaction_observers)
+            m_transaction_observers_being_notified.unchecked_append(observer);
+
+        for (auto transaction_observer : m_transaction_observers) {
+            if (auto notifier = get_notifier(*transaction_observer))
+                notifier->function()(forward<Args>(args)...);
+        }
+    }
+
+    // IDBTransaction should not visit IDBTransactionObserver to avoid leaks.
+    // It's responsibility of object that requires IDBTransactionObserver to keep it alive.
+    HashTable<GC::RawRef<IDBTransactionObserver>> m_transaction_observers;
+    Vector<GC::Ref<IDBTransactionObserver>> m_transaction_observers_being_notified;
+
     // AD-HOC: The transaction has a connection
     GC::Ref<IDBDatabase> m_connection;
 

Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp

@@ -182,24 +182,25 @@ void open_a_database_connection(JS::Realm& realm, StorageAPI::StorageKey storage
 
                 db->wait_for_connections_to_close(open_connections, GC::create_function(realm.heap(), [&realm, connection, version, request, on_complete] {
                     // 6. Run upgrade a database using connection, version and request.
-                    upgrade_a_database(realm, connection, version, request);
-
-                    // 7. If connection was closed, return a newly created "AbortError" DOMException and abort these steps.
-                    if (connection->state() == ConnectionState::Closed) {
-                        on_complete->function()(WebIDL::AbortError::create(realm, "Connection was closed"_utf16));
-                        return;
-                    }
-
-                    // 8. If request's error is set, run the steps to close a database connection with connection,
-                    //    return a newly created "AbortError" DOMException and abort these steps.
-                    if (request->has_error()) {
-                        close_a_database_connection(*connection);
-                        on_complete->function()(WebIDL::AbortError::create(realm, "Upgrade transaction was aborted"_utf16));
-                        return;
-                    }
-
-                    // 11. Return connection.
-                    on_complete->function()(connection);
+                    upgrade_a_database(realm, connection, version, request, GC::create_function(realm.heap(), [&realm, connection, request, on_complete] {
+                        // 7. If connection was closed, return a newly created "AbortError" DOMException and abort these steps.
+                        if (connection->state() == ConnectionState::Closed) {
+                            on_complete->function()(WebIDL::AbortError::create(realm, "Connection was closed"_utf16));
+                            return;
+                        }
+
+                        // 8. If request's error is set, run the steps to close a database connection with connection,
+                        //    return a newly created "AbortError" DOMException and abort these steps.
+                        if (request->has_error()) {
+                            close_a_database_connection(*connection, GC::create_function(realm.heap(), [&realm, on_complete] {
+                                on_complete->function()(WebIDL::AbortError::create(realm, "Upgrade transaction was aborted"_utf16));
+                            }));
+                            return;
+                        }
+
+                        // 11. Return connection.
+                        on_complete->function()(connection);
+                    }));
                 }));
             });
 
@@ -356,7 +357,7 @@ WebIDL::ExceptionOr<GC::Ref<Key>> convert_a_value_to_a_key(JS::Realm& realm, JS:
 }
 
 // https://w3c.github.io/IndexedDB/#close-a-database-connection
-void close_a_database_connection(GC::Ref<IDBDatabase> connection, bool forced)
+void close_a_database_connection(GC::Ref<IDBDatabase> connection, GC::Ptr<GC::Function<void()>> on_complete, bool forced)
 {
     auto& realm = connection->realm();
 
@@ -371,32 +372,28 @@ void close_a_database_connection(GC::Ref<IDBDatabase> connection, bool forced)
     }
 
     // 3. Wait for all transactions created using connection to complete. Once they are complete, connection is closed.
-    HTML::main_thread_event_loop().spin_until(GC::create_function(realm.vm().heap(), [connection]() {
-        if constexpr (IDB_DEBUG) {
-            dbgln("close_a_database_connection: waiting for step 3");
-            dbgln("transactions created using connection:");
-            for (auto const& transaction : connection->transactions()) {
-                dbgln("  - {} - {}", transaction->uuid(), (u8)transaction->state());
-            }
-        }
-
+    if constexpr (IDB_DEBUG) {
+        dbgln("close_a_database_connection: waiting for step 3");
+        dbgln("transactions created using connection:");
         for (auto const& transaction : connection->transactions()) {
-            if (!transaction->is_finished())
-                return false;
+            dbgln("  - {} - {}", transaction->uuid(), (u8)transaction->state());
         }
+    }
 
-        return true;
-    }));
+    connection->wait_for_transactions_to_finish(connection->transactions(), GC::create_function(realm.heap(), [&realm, connection, forced, on_complete] {
+        connection->set_state(ConnectionState::Closed);
 
-    connection->set_state(ConnectionState::Closed);
+        // 4. If the forced flag is true, then fire an event named close at connection.
+        if (forced)
+            connection->dispatch_event(DOM::Event::create(realm, HTML::EventNames::close));
 
-    // 4. If the forced flag is true, then fire an event named close at connection.
-    if (forced)
-        connection->dispatch_event(DOM::Event::create(realm, HTML::EventNames::close));
+        if (on_complete)
+            queue_a_database_task(on_complete.as_nonnull());
+    }));
 }
 
 // https://w3c.github.io/IndexedDB/#upgrade-a-database
-void upgrade_a_database(JS::Realm& realm, GC::Ref<IDBDatabase> connection, u64 version, GC::Ref<IDBRequest> request)
+void upgrade_a_database(JS::Realm& realm, GC::Ref<IDBDatabase> connection, u64 version, GC::Ref<IDBRequest> request, GC::Ref<GC::Function<void()>> on_complete)
 {
     // 1. Let db be connection’s database.
     auto db = connection->associated_database();
@@ -463,9 +460,9 @@ void upgrade_a_database(JS::Realm& realm, GC::Ref<IDBDatabase> connection, u64 v
     }));
 
     // 11. Wait for transaction to finish.
-    HTML::main_thread_event_loop().spin_until(GC::create_function(realm.vm().heap(), [transaction]() {
-        dbgln_if(IDB_DEBUG, "upgrade_a_database: waiting for step 11");
-        return transaction->is_finished();
+    dbgln_if(IDB_DEBUG, "upgrade_a_database: waiting for step 11");
+    connection->wait_for_transactions_to_finish({ &transaction, 1 }, GC::create_function(realm.heap(), [on_complete] {
+        queue_a_database_task(on_complete);
     }));
 }
 

Libraries/LibWeb/IndexedDB/Internal/Algorithms.h

@@ -30,8 +30,8 @@ using RecordSource = Variant<GC::Ref<ObjectStore>, GC::Ref<Index>>;
 void open_a_database_connection(JS::Realm&, StorageAPI::StorageKey, String, Optional<u64>, GC::Ref<IDBRequest>, GC::Ref<GC::Function<void(WebIDL::ExceptionOr<GC::Ref<IDBDatabase>>)>>);
 bool fire_a_version_change_event(JS::Realm&, FlyString const&, GC::Ref<DOM::EventTarget>, u64, Optional<u64>);
 WebIDL::ExceptionOr<GC::Ref<Key>> convert_a_value_to_a_key(JS::Realm&, JS::Value, Vector<JS::Value> = {});
-void close_a_database_connection(GC::Ref<IDBDatabase>, bool forced = false);
-void upgrade_a_database(JS::Realm&, GC::Ref<IDBDatabase>, u64, GC::Ref<IDBRequest>);
+void close_a_database_connection(GC::Ref<IDBDatabase>, GC::Ptr<GC::Function<void()>> on_complete = nullptr, bool forced = false);
+void upgrade_a_database(JS::Realm&, GC::Ref<IDBDatabase>, u64, GC::Ref<IDBRequest>, GC::Ref<GC::Function<void()>>);
 void delete_a_database(JS::Realm&, StorageAPI::StorageKey, String, GC::Ref<IDBRequest>, GC::Ref<GC::Function<void(WebIDL::ExceptionOr<u64>)>>);
 void abort_a_transaction(GC::Ref<IDBTransaction>, GC::Ptr<WebIDL::DOMException>);
 JS::Value convert_a_key_to_a_value(JS::Realm&, GC::Ref<Key>);

Libraries/LibWeb/IndexedDB/Internal/IDBTransactionObserver.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2025, Luke Wilde <luke@ladybird.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibWeb/IndexedDB/IDBTransaction.h>
+#include <LibWeb/IndexedDB/Internal/IDBTransactionObserver.h>
+
+namespace Web::IndexedDB {
+
+GC_DEFINE_ALLOCATOR(IDBTransactionObserver);
+
+IDBTransactionObserver::IDBTransactionObserver(IDBTransaction& transaction)
+    : m_transaction(transaction)
+{
+    m_transaction->register_transaction_observer({}, *this);
+    m_observing = true;
+}
+
+IDBTransactionObserver::~IDBTransactionObserver() = default;
+
+void IDBTransactionObserver::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_transaction);
+    visitor.visit(m_transaction_finished_observer);
+}
+
+void IDBTransactionObserver::finalize()
+{
+    Base::finalize();
+    unobserve();
+}
+
+void IDBTransactionObserver::unobserve()
+{
+    if (!m_observing)
+        return;
+
+    m_transaction->unregister_transaction_observer({}, *this);
+    m_observing = false;
+}
+
+}