LibWeb/IndexedDB: Remove spin_until from waiting for connection closure

Author: https://github.com/Lubrsi
Commit: 52b53e52fb
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/6182

Libraries/LibWeb/CMakeLists.txt

@@ -682,6 +682,7 @@ set(SOURCES
     IndexedDB/IDBVersionChangeEvent.cpp
     IndexedDB/Internal/Algorithms.cpp
     IndexedDB/Internal/Database.cpp
+    IndexedDB/Internal/IDBDatabaseObserver.cpp
     IndexedDB/Internal/IDBRequestObserver.cpp
     IndexedDB/Internal/Index.cpp
     IndexedDB/Internal/Key.cpp

Libraries/LibWeb/Forward.h

@@ -803,6 +803,7 @@ class Database;
 class IDBCursor;
 class IDBCursorWithValue;
 class IDBDatabase;
+class IDBDatabaseObserver;
 class IDBFactory;
 class IDBIndex;
 class IDBKeyRange;

Libraries/LibWeb/IndexedDB/ConnectionState.h

@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2024-2025, stelar7 <dudedbz@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+namespace Web::IndexedDB {
+
+enum ConnectionState {
+    Open,
+    Closed,
+};
+
+}

Libraries/LibWeb/IndexedDB/IDBDatabase.cpp

@@ -11,6 +11,7 @@
 #include <LibWeb/IndexedDB/IDBDatabase.h>
 #include <LibWeb/IndexedDB/IDBObjectStore.h>
 #include <LibWeb/IndexedDB/Internal/Algorithms.h>
+#include <LibWeb/IndexedDB/Internal/IDBDatabaseObserver.h>
 
 namespace Web::IndexedDB {
 
@@ -42,6 +43,7 @@ void IDBDatabase::initialize(JS::Realm& realm)
 void IDBDatabase::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
+    visitor.visit(m_database_observers_being_notified);
     visitor.visit(m_object_store_set);
     visitor.visit(m_associated_database);
     visitor.visit(m_transactions);
@@ -241,4 +243,24 @@ WebIDL::ExceptionOr<GC::Ref<IDBTransaction>> IDBDatabase::transaction(Variant<St
     return transaction;
 }
 
+void IDBDatabase::register_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver& database_observer)
+{
+    auto result = m_database_observers.set(database_observer);
+    VERIFY(result == AK::HashSetResult::InsertedNewEntry);
+}
+
+void IDBDatabase::unregister_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver& database_observer)
+{
+    bool was_removed = m_database_observers.remove(database_observer);
+    VERIFY(was_removed);
+}
+
+void IDBDatabase::set_state(ConnectionState state)
+{
+    m_state = state;
+    notify_each_database_observer([](IDBDatabaseObserver const& request_observer) {
+        return request_observer.connection_state_changed_observer();
+    });
+}
+
 }

Libraries/LibWeb/IndexedDB/IDBDatabase.h

@@ -10,6 +10,7 @@
 #include <LibWeb/Bindings/IDBDatabasePrototype.h>
 #include <LibWeb/DOM/EventTarget.h>
 #include <LibWeb/HTML/DOMStringList.h>
+#include <LibWeb/IndexedDB/ConnectionState.h>
 #include <LibWeb/IndexedDB/IDBRequest.h>
 #include <LibWeb/IndexedDB/IDBTransaction.h>
 #include <LibWeb/IndexedDB/Internal/Database.h>
@@ -37,11 +38,6 @@ class IDBDatabase : public DOM::EventTarget {
     WEB_PLATFORM_OBJECT(IDBDatabase, DOM::EventTarget);
     GC_DECLARE_ALLOCATOR(IDBDatabase);
 
-    enum ConnectionState {
-        Open,
-        Closed,
-    };
-
 public:
     virtual ~IDBDatabase() override;
 
@@ -49,7 +45,7 @@ public:
 
     void set_version(u64 version) { m_version = version; }
     void set_close_pending(bool close_pending) { m_close_pending = close_pending; }
-    void set_state(ConnectionState state) { m_state = state; }
+    void set_state(ConnectionState state);
 
     [[nodiscard]] String uuid() const { return m_uuid; }
     [[nodiscard]] String name() const { return m_name; }
@@ -84,6 +80,9 @@ public:
     void set_onversionchange(WebIDL::CallbackType*);
     WebIDL::CallbackType* onversionchange();
 
+    void register_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver&);
+    void unregister_database_observer(Badge<IDBDatabaseObserver>, IDBDatabaseObserver&);
+
 protected:
     explicit IDBDatabase(JS::Realm&, Database&);
 
@@ -91,6 +90,26 @@ protected:
     virtual void visit_edges(Visitor& visitor) override;
 
 private:
+    template<typename GetNotifier, typename... Args>
+    void notify_each_database_observer(GetNotifier&& get_notifier, Args&&... args)
+    {
+        ScopeGuard guard { [&]() { m_database_observers_being_notified.clear_with_capacity(); } };
+        m_database_observers_being_notified.ensure_capacity(m_database_observers.size());
+
+        for (auto observer : m_database_observers)
+            m_database_observers_being_notified.unchecked_append(observer);
+
+        for (auto database_observer : m_database_observers_being_notified) {
+            if (auto notifier = get_notifier(*database_observer))
+                notifier->function()(forward<Args>(args)...);
+        }
+    }
+
+    // IDBDatabase should not visit IDBDatabaseObserver to avoid leaks.
+    // It's responsibility of object that requires IDBDatabaseObserver to keep it alive.
+    HashTable<GC::RawRef<IDBDatabaseObserver>> m_database_observers;
+    Vector<GC::Ref<IDBDatabaseObserver>> m_database_observers_being_notified;
+
     u64 m_version { 0 };
     String m_name;
 

Libraries/LibWeb/IndexedDB/IDBRequest.cpp

@@ -128,7 +128,7 @@ void IDBRequest::unregister_request_observer(Badge<IDBRequestObserver>, IDBReque
 void IDBRequest::set_processed(bool processed)
 {
     m_processed = processed;
-    notify_each_request_observer([&](IDBRequestObserver const& request_observer) {
+    notify_each_request_observer([](IDBRequestObserver const& request_observer) {
         return request_observer.request_processed_changed_observer();
     });
 }

Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp

@@ -164,7 +164,7 @@ void open_a_database_connection(JS::Realm& realm, StorageAPI::StorageKey storage
                 // 4. If any of the connections in openConnections are still not closed,
                 //    queue a database task to fire a version change event named blocked at request with db’s version and version.
                 for (auto const& entry : open_connections) {
-                    if (entry->state() != IDBDatabase::ConnectionState::Closed) {
+                    if (entry->state() != ConnectionState::Closed) {
                         queue_a_database_task(GC::create_function(realm.vm().heap(), [&realm, entry, db, version]() {
                             fire_a_version_change_event(realm, HTML::EventNames::blocked, *entry, db->version(), version);
                         }));
@@ -172,29 +172,20 @@ void open_a_database_connection(JS::Realm& realm, StorageAPI::StorageKey storage
                 }
 
                 // 5. Wait until all connections in openConnections are closed.
-                HTML::main_thread_event_loop().spin_until(GC::create_function(realm.vm().heap(), [open_connections]() {
                 if constexpr (IDB_DEBUG) {
                     dbgln("open_a_database_connection: waiting for step 10.5");
                     dbgln("open connections: {}", open_connections.size());
-                        for (auto const& connection : open_connections) {
-                            dbgln("  - {}", connection->uuid());
+                    for (auto const& open_connection : open_connections) {
+                        dbgln("  - {}", open_connection->uuid());
                     }
                 }
 
-                    for (auto const& entry : open_connections) {
-                        if (entry->state() != IDBDatabase::ConnectionState::Closed) {
-                            return false;
-                        }
-                    }
-
-                    return true;
-                }));
-
+                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() == IDBDatabase::ConnectionState::Closed) {
+                    if (connection->state() == ConnectionState::Closed) {
                         on_complete->function()(WebIDL::AbortError::create(realm, "Connection was closed"_utf16));
                         return;
                     }
@@ -209,6 +200,7 @@ void open_a_database_connection(JS::Realm& realm, StorageAPI::StorageKey storage
 
                     // 11. Return connection.
                     on_complete->function()(connection);
+                }));
             });
 
             if (task_counter_state) {
@@ -396,7 +388,7 @@ void close_a_database_connection(GC::Ref<IDBDatabase> connection, bool forced)
         return true;
     }));
 
-    connection->set_state(IDBDatabase::ConnectionState::Closed);
+    connection->set_state(ConnectionState::Closed);
 
     // 4. If the forced flag is true, then fire an event named close at connection.
     if (forced)
@@ -535,7 +527,7 @@ void delete_a_database(JS::Realm& realm, StorageAPI::StorageKey storage_key, Str
         auto after_all = GC::create_function(realm.heap(), [&realm, open_connections, db, storage_key = move(storage_key), name = move(name), on_complete] {
             // 8. If any of the connections in openConnections are still not closed, queue a database task to fire a version change event named blocked at request with db’s version and null.
             for (auto const& entry : open_connections) {
-                if (entry->state() != IDBDatabase::ConnectionState::Closed) {
+                if (entry->state() != ConnectionState::Closed) {
                     queue_a_database_task(GC::create_function(realm.vm().heap(), [&realm, entry, db]() {
                         fire_a_version_change_event(realm, HTML::EventNames::blocked, *entry, db->version(), {});
                     }));
@@ -543,7 +535,6 @@ void delete_a_database(JS::Realm& realm, StorageAPI::StorageKey storage_key, Str
             }
 
             // 9. Wait until all connections in openConnections are closed.
-            HTML::main_thread_event_loop().spin_until(GC::create_function(realm.vm().heap(), [open_connections]() {
             if constexpr (IDB_DEBUG) {
                 dbgln("delete_a_database: waiting for step 9");
                 dbgln("open connections: {}", open_connections.size());
@@ -552,15 +543,7 @@ void delete_a_database(JS::Realm& realm, StorageAPI::StorageKey storage_key, Str
                 }
             }
 
-                for (auto const& entry : open_connections) {
-                    if (entry->state() != IDBDatabase::ConnectionState::Closed) {
-                        return false;
-                    }
-                }
-
-                return true;
-            }));
-
+            db->wait_for_connections_to_close(open_connections, GC::create_function(realm.heap(), [&realm, db, storage_key = move(storage_key), name = move(name), on_complete] {
                 // 10. Let version be db’s version.
                 auto version = db->version();
 
@@ -573,6 +556,7 @@ void delete_a_database(JS::Realm& realm, StorageAPI::StorageKey storage_key, Str
 
                 // 12. Return version.
                 on_complete->function()(version);
+            }));
         });
 
         if (task_counter_state) {

Libraries/LibWeb/IndexedDB/Internal/Database.cpp

@@ -7,6 +7,7 @@
 #include <LibWeb/IndexedDB/IDBTransaction.h>
 #include <LibWeb/IndexedDB/Internal/ConnectionQueueHandler.h>
 #include <LibWeb/IndexedDB/Internal/Database.h>
+#include <LibWeb/IndexedDB/Internal/IDBDatabaseObserver.h>
 #include <LibWeb/IndexedDB/Internal/RequestList.h>
 
 namespace Web::IndexedDB {
@@ -24,6 +25,7 @@ void Database::for_each_database(AK::Function<void(GC::Root<Database> const&)> c
 }
 
 GC_DEFINE_ALLOCATOR(Database);
+GC_DEFINE_ALLOCATOR(Database::ConnectionCloseState);
 
 Database::~Database() = default;
 
@@ -36,6 +38,7 @@ void Database::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_associated_connections);
+    visitor.visit(m_pending_connection_close_queue);
     visitor.visit(m_upgrade_transaction);
     visitor.visit(m_object_stores);
 }
@@ -113,4 +116,81 @@ ErrorOr<void> Database::delete_for_key_and_name(StorageAPI::StorageKey const& ke
     return {};
 }
 
+Vector<GC::Root<IDBDatabase>> Database::associated_connections()
+{
+    Vector<GC::Root<IDBDatabase>> connections;
+    connections.ensure_capacity(m_associated_connections.size());
+    for (auto& associated_connection : m_associated_connections) {
+        connections.unchecked_append(associated_connection);
+    }
+    return connections;
+}
+
+Vector<GC::Root<IDBDatabase>> Database::associated_connections_except(IDBDatabase& connection)
+{
+    Vector<GC::Root<IDBDatabase>> connections;
+    for (auto& associated_connection : m_associated_connections) {
+        if (associated_connection != &connection)
+            connections.append(associated_connection);
+    }
+    return connections;
+}
+
+void Database::ConnectionCloseState::visit_edges(Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(database_observers);
+    visitor.visit(after_all);
+}
+
+void Database::ConnectionCloseState::add_connection_to_observe(GC::Ref<IDBDatabase> database)
+{
+    auto database_observer = heap().allocate<IDBDatabaseObserver>(database);
+    database_observer->set_connection_state_changed_observer(GC::create_function(heap(), [this] {
+        VERIFY(!database_observers.is_empty());
+        database_observers.remove_all_matching([](GC::Ref<IDBDatabaseObserver> const& database_observer) {
+            if (database_observer->database()->state() == ConnectionState::Closed) {
+                database_observer->unobserve();
+                return true;
+            }
+
+            return false;
+        });
+
+        if (database_observers.is_empty()) {
+            queue_a_database_task(after_all.as_nonnull());
+        }
+    }));
+
+    database_observers.append(database_observer);
+}
+
+void Database::wait_for_connections_to_close(ReadonlySpan<GC::Root<IDBDatabase>> connections, GC::Ref<GC::Function<void()>> after_all)
+{
+    GC::Ptr<ConnectionCloseState> connection_close_state;
+
+    for (auto const& entry : connections) {
+        if (entry->state() != ConnectionState::Closed) {
+            if (!connection_close_state) {
+                connection_close_state = heap().allocate<ConnectionCloseState>();
+            }
+
+            connection_close_state->add_connection_to_observe(*entry.cell());
+        }
+    }
+
+    if (connection_close_state) {
+        connection_close_state->after_all = GC::create_function(heap(), [this, connection_close_state, after_all] {
+            bool was_removed = m_pending_connection_close_queue.remove_first_matching([connection_close_state](GC::Ref<ConnectionCloseState> pending_connection_close_state) {
+                return pending_connection_close_state == connection_close_state;
+            });
+            VERIFY(was_removed);
+            queue_a_database_task(after_all);
+        });
+        m_pending_connection_close_queue.append(connection_close_state.as_nonnull());
+    } else {
+        queue_a_database_task(after_all);
+    }
+}
+
 }

Libraries/LibWeb/IndexedDB/Internal/Database.h

@@ -29,16 +29,8 @@ public:
     [[nodiscard]] GC::Ptr<IDBTransaction> upgrade_transaction() { return m_upgrade_transaction; }
 
     void associate(GC::Ref<IDBDatabase> connection) { m_associated_connections.append(connection); }
-    ReadonlySpan<GC::Ref<IDBDatabase>> associated_connections() { return m_associated_connections; }
-    Vector<GC::Root<IDBDatabase>> associated_connections_except(IDBDatabase& connection)
-    {
-        Vector<GC::Root<IDBDatabase>> connections;
-        for (auto& associated_connection : m_associated_connections) {
-            if (associated_connection != &connection)
-                connections.append(associated_connection);
-        }
-        return connections;
-    }
+    Vector<GC::Root<IDBDatabase>> associated_connections();
+    Vector<GC::Root<IDBDatabase>> associated_connections_except(IDBDatabase& connection);
 
     ReadonlySpan<GC::Ref<ObjectStore>> object_stores() { return m_object_stores; }
     GC::Ptr<ObjectStore> object_store_with_name(String const& name) const;
@@ -58,6 +50,8 @@ public:
     [[nodiscard]] static GC::Ref<Database> create(JS::Realm&, String const&);
     virtual ~Database();
 
+    void wait_for_connections_to_close(ReadonlySpan<GC::Root<IDBDatabase>> connections, GC::Ref<GC::Function<void()>> after_all);
+
 protected:
     explicit Database(IDBDatabase& database);
 
@@ -70,7 +64,20 @@ protected:
     virtual void visit_edges(Visitor&) override;
 
 private:
+    struct ConnectionCloseState final : public GC::Cell {
+        GC_CELL(ConnectionCloseState, GC::Cell);
+        GC_DECLARE_ALLOCATOR(ConnectionCloseState);
+
+        virtual void visit_edges(Visitor& visitor) override;
+
+        void add_connection_to_observe(GC::Ref<IDBDatabase> database);
+
+        Vector<GC::Ref<IDBDatabaseObserver>> database_observers;
+        GC::Ptr<GC::Function<void()>> after_all;
+    };
+
     Vector<GC::Ref<IDBDatabase>> m_associated_connections;
+    Vector<GC::Ref<ConnectionCloseState>> m_pending_connection_close_queue;
 
     // A database has a name which identifies it within a specific storage key.
     String m_name;

Libraries/LibWeb/IndexedDB/Internal/IDBDatabaseObserver.cpp

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

Libraries/LibWeb/IndexedDB/Internal/IDBDatabaseObserver.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2025, Luke Wilde <luke@ladybird.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <LibGC/Cell.h>
+#include <LibGC/CellAllocator.h>
+#include <LibWeb/Forward.h>
+#include <LibWeb/IndexedDB/ConnectionState.h>
+
+namespace Web::IndexedDB {
+
+class IDBDatabaseObserver final : public GC::Cell {
+    GC_CELL(IDBDatabaseObserver, GC::Cell);
+    GC_DECLARE_ALLOCATOR(IDBDatabaseObserver);
+
+public:
+    virtual ~IDBDatabaseObserver();
+
+    [[nodiscard]] GC::Ptr<GC::Function<void()>> connection_state_changed_observer() const { return m_connection_state_changed_observer; }
+    void set_connection_state_changed_observer(GC::Ptr<GC::Function<void()>> callback) { m_connection_state_changed_observer = callback; }
+
+    GC::Ref<IDBDatabase> database() const { return m_database; }
+
+    void unobserve();
+
+private:
+    explicit IDBDatabaseObserver(IDBDatabase&);
+
+    virtual void visit_edges(Cell::Visitor&) override;
+    virtual void finalize() override;
+
+    bool m_observing { false };
+
+    GC::Ref<IDBDatabase> m_database;
+    GC::Ptr<GC::Function<void()>> m_connection_state_changed_observer;
+};
+
+}