From f53559cb55f1936756481c5a499d72e06c43ef5d Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 11 Jun 2025 18:51:22 +0200 Subject: [PATCH] LibWeb: Change Storage{Bottle,Bucket,Shelf} to be GC-allocated In upcoming changes StorageBottle will own pointers to GC-allocated objects, so it needs to be a GC-allocated object itself to avoid introducing more GC roots. --- Libraries/LibWeb/Forward.h | 6 ++-- Libraries/LibWeb/HTML/Storage.cpp | 10 ++++-- Libraries/LibWeb/HTML/Storage.h | 7 ++-- .../LibWeb/HTML/TraversableNavigable.cpp | 2 ++ Libraries/LibWeb/HTML/TraversableNavigable.h | 2 +- Libraries/LibWeb/HTML/Window.cpp | 4 +-- Libraries/LibWeb/StorageAPI/StorageBottle.cpp | 27 ++++++++++----- Libraries/LibWeb/StorageAPI/StorageBottle.h | 33 ++++++++++++++----- Libraries/LibWeb/StorageAPI/StorageShed.cpp | 21 ++++++++---- Libraries/LibWeb/StorageAPI/StorageShed.h | 18 +++++++--- Libraries/LibWeb/StorageAPI/StorageShelf.cpp | 11 ++++++- Libraries/LibWeb/StorageAPI/StorageShelf.h | 20 ++++++++--- 12 files changed, 117 insertions(+), 44 deletions(-) diff --git a/Libraries/LibWeb/Forward.h b/Libraries/LibWeb/Forward.h index 399e7400cb3..1542d4cece5 100644 --- a/Libraries/LibWeb/Forward.h +++ b/Libraries/LibWeb/Forward.h @@ -917,13 +917,13 @@ struct UnderlyingSource; namespace Web::StorageAPI { class NavigatorStorage; +class StorageBottle; +class StorageBucket; class StorageManager; class StorageShed; +class StorageShelf; -struct StorageBottle; -struct StorageBucket; struct StorageEndpoint; -struct StorageShelf; } diff --git a/Libraries/LibWeb/HTML/Storage.cpp b/Libraries/LibWeb/HTML/Storage.cpp index 0665ee235ba..4347b3a7367 100644 --- a/Libraries/LibWeb/HTML/Storage.cpp +++ b/Libraries/LibWeb/HTML/Storage.cpp @@ -25,12 +25,12 @@ static HashTable>& all_storages() return storages; } -GC::Ref Storage::create(JS::Realm& realm, Type type, NonnullRefPtr storage_bottle) +GC::Ref Storage::create(JS::Realm& realm, Type type, GC::Ref storage_bottle) { return realm.create(realm, type, move(storage_bottle)); } -Storage::Storage(JS::Realm& realm, Type type, NonnullRefPtr storage_bottle) +Storage::Storage(JS::Realm& realm, Type type, GC::Ref storage_bottle) : Bindings::PlatformObject(realm) , m_type(type) , m_storage_bottle(move(storage_bottle)) @@ -65,6 +65,12 @@ void Storage::finalize() all_storages().remove(*this); } +void Storage::visit_edges(GC::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_storage_bottle); +} + // https://html.spec.whatwg.org/multipage/webstorage.html#dom-storage-length size_t Storage::length() const { diff --git a/Libraries/LibWeb/HTML/Storage.h b/Libraries/LibWeb/HTML/Storage.h index 84d09695f7c..7aac4e581a9 100644 --- a/Libraries/LibWeb/HTML/Storage.h +++ b/Libraries/LibWeb/HTML/Storage.h @@ -27,7 +27,7 @@ public: Session, }; - [[nodiscard]] static GC::Ref create(JS::Realm&, Type, NonnullRefPtr); + [[nodiscard]] static GC::Ref create(JS::Realm&, Type, GC::Ref); ~Storage(); @@ -44,10 +44,11 @@ public: void dump() const; private: - Storage(JS::Realm&, Type, NonnullRefPtr); + Storage(JS::Realm&, Type, GC::Ref); virtual void initialize(JS::Realm&) override; virtual void finalize() override; + virtual void visit_edges(GC::Cell::Visitor&) override; // ^PlatformObject virtual Optional item_value(size_t index) const override; @@ -61,7 +62,7 @@ private: void broadcast(Optional const& key, Optional const& old_value, Optional const& new_value); Type m_type {}; - NonnullRefPtr m_storage_bottle; + GC::Ref m_storage_bottle; u64 m_stored_bytes { 0 }; }; diff --git a/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 9865b4b4bdc..db000cacf2d 100644 --- a/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -50,6 +50,7 @@ static RefPtr get_skia_backend_context() TraversableNavigable::TraversableNavigable(GC::Ref page) : Navigable(page) + , m_storage_shed(StorageAPI::StorageShed::create(page->heap())) , m_session_history_traversal_queue(vm().heap().allocate()) { if (!page->client().is_svg_page_client()) { @@ -75,6 +76,7 @@ void TraversableNavigable::visit_edges(Cell::Visitor& visitor) Base::visit_edges(visitor); visitor.visit(m_session_history_entries); visitor.visit(m_session_history_traversal_queue); + visitor.visit(m_storage_shed); } static OrderedHashTable& user_agent_top_level_traversable_set() diff --git a/Libraries/LibWeb/HTML/TraversableNavigable.h b/Libraries/LibWeb/HTML/TraversableNavigable.h index a63cd50f728..adfd243a729 100644 --- a/Libraries/LibWeb/HTML/TraversableNavigable.h +++ b/Libraries/LibWeb/HTML/TraversableNavigable.h @@ -164,7 +164,7 @@ private: // https://storage.spec.whatwg.org/#traversable-navigable-storage-shed // A traversable navigable holds a storage shed, which is a storage shed. A traversable navigable’s storage shed holds all session storage data. - StorageAPI::StorageShed m_storage_shed; + GC::Ref m_storage_shed; GC::Ref m_session_history_traversal_queue; diff --git a/Libraries/LibWeb/HTML/Window.cpp b/Libraries/LibWeb/HTML/Window.cpp index ce4a17bf2eb..97cd3cff6d3 100644 --- a/Libraries/LibWeb/HTML/Window.cpp +++ b/Libraries/LibWeb/HTML/Window.cpp @@ -471,7 +471,7 @@ WebIDL::ExceptionOr> Window::local_storage() return WebIDL::SecurityError::create(realm, "localStorage is not available"_string); // 4. Let storage be a new Storage object whose map is map. - auto storage = Storage::create(realm, Storage::Type::Session, map.release_nonnull()); + auto storage = Storage::create(realm, Storage::Type::Session, *map); // 5. Set this's associated Document's local storage holder to storage. associated_document.set_local_storage_holder(storage); @@ -498,7 +498,7 @@ WebIDL::ExceptionOr> Window::session_storage() return WebIDL::SecurityError::create(realm, "sessionStorage is not available"_string); // 4. Let storage be a new Storage object whose map is map. - auto storage = Storage::create(realm, Storage::Type::Session, map.release_nonnull()); + auto storage = Storage::create(realm, Storage::Type::Session, *map); // 5. Set this's associated Document's session storage holder to storage. associated_document.set_session_storage_holder(storage); diff --git a/Libraries/LibWeb/StorageAPI/StorageBottle.cpp b/Libraries/LibWeb/StorageAPI/StorageBottle.cpp index 6136d9893a5..4f64cfc0391 100644 --- a/Libraries/LibWeb/StorageAPI/StorageBottle.cpp +++ b/Libraries/LibWeb/StorageAPI/StorageBottle.cpp @@ -13,6 +13,15 @@ namespace Web::StorageAPI { +GC_DEFINE_ALLOCATOR(StorageBottle); +GC_DEFINE_ALLOCATOR(StorageBucket); + +void StorageBucket::visit_edges(GC::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_bottle_map); +} + StorageBucket::StorageBucket(StorageType type) { // 1. Let bucket be null. @@ -24,21 +33,21 @@ StorageBucket::StorageBucket(StorageType type) // 4. For each endpoint of registered storage endpoints whose types contain type, set bucket’s bottle map[endpoint’s identifier] to a new storage bottle whose quota is endpoint’s quota. for (auto const& endpoint : StorageEndpoint::registered_endpoints()) { if (endpoint.type == type) - bottle_map.set(endpoint.identifier, StorageBottle::create(endpoint.quota)); + m_bottle_map.set(endpoint.identifier, StorageBottle::create(heap(), endpoint.quota)); } // 5. Return bucket. } // https://storage.spec.whatwg.org/#obtain-a-storage-bottle-map -RefPtr obtain_a_storage_bottle_map(StorageType type, HTML::EnvironmentSettingsObject& environment, StringView identifier) +GC::Ptr obtain_a_storage_bottle_map(StorageType type, HTML::EnvironmentSettingsObject& environment, StringView identifier) { // 1. Let shed be null. - StorageShed* shed = nullptr; + GC::Ptr shed = nullptr; // 2. If type is "local", then set shed to the user agent’s storage shed. if (type == StorageType::Local) { - shed = &user_agent_storage_shed(); + shed = user_agent_storage_shed(environment.heap()); } // 3. Otherwise: else { @@ -54,14 +63,14 @@ RefPtr obtain_a_storage_bottle_map(StorageType type, HTML::Enviro auto shelf = shed->obtain_a_storage_shelf(environment, type); // 5. If shelf is failure, then return failure. - if (!shelf.has_value()) + if (!shelf) return {}; // 6. Let bucket be shelf’s bucket map["default"]. - auto& bucket = shelf->bucket_map.get("default"sv).value(); + auto& bucket = shelf->bucket_map().get("default"sv).value(); // 7. Let bottle be bucket’s bottle map[identifier]. - auto bottle = bucket.bottle_map.get(identifier).value(); + auto bottle = bucket->bottle_map().get(identifier).value(); // 8. Let proxyMap be a new storage proxy map whose backing map is bottle’s map. // 9. Append proxyMap to bottle’s proxy map reference set. @@ -70,7 +79,7 @@ RefPtr obtain_a_storage_bottle_map(StorageType type, HTML::Enviro } // https://storage.spec.whatwg.org/#obtain-a-session-storage-bottle-map -RefPtr obtain_a_session_storage_bottle_map(HTML::EnvironmentSettingsObject& environment, StringView identifier) +GC::Ptr obtain_a_session_storage_bottle_map(HTML::EnvironmentSettingsObject& environment, StringView identifier) { // To obtain a session storage bottle map, given an environment settings object environment and storage identifier identifier, // return the result of running obtain a storage bottle map with "session", environment, and identifier. @@ -78,7 +87,7 @@ RefPtr obtain_a_session_storage_bottle_map(HTML::EnvironmentSetti } // https://storage.spec.whatwg.org/#obtain-a-local-storage-bottle-map -RefPtr obtain_a_local_storage_bottle_map(HTML::EnvironmentSettingsObject& environment, StringView identifier) +GC::Ptr obtain_a_local_storage_bottle_map(HTML::EnvironmentSettingsObject& environment, StringView identifier) { // To obtain a local storage bottle map, given an environment settings object environment and storage identifier identifier, // return the result of running obtain a storage bottle map with "local", environment, and identifier. diff --git a/Libraries/LibWeb/StorageAPI/StorageBottle.h b/Libraries/LibWeb/StorageAPI/StorageBottle.h index f1b3f65532d..c2564ce16ee 100644 --- a/Libraries/LibWeb/StorageAPI/StorageBottle.h +++ b/Libraries/LibWeb/StorageAPI/StorageBottle.h @@ -14,14 +14,17 @@ namespace Web::StorageAPI { // https://storage.spec.whatwg.org/#storage-bottle -struct StorageBottle : public RefCounted { - static NonnullRefPtr create(Optional quota) { return adopt_ref(*new StorageBottle(quota)); } +class StorageBottle : public GC::Cell { + GC_CELL(StorageBottle, GC::Cell); + GC_DECLARE_ALLOCATOR(StorageBottle); + + static GC::Ref create(GC::Heap& heap, Optional quota) { return heap.allocate(quota); } // A storage bottle has a map, which is initially an empty map OrderedHashMap map; // A storage bottle also has a proxy map reference set, which is initially an empty set - NonnullRefPtr proxy() { return *this; } + GC::Ref proxy() { return *this; } // A storage bottle also has a quota, which is null or a number representing a conservative estimate of // the total amount of bytes it can hold. Null indicates the lack of a limit. @@ -34,19 +37,31 @@ private: } }; -using BottleMap = OrderedHashMap>; +using BottleMap = OrderedHashMap>; // https://storage.spec.whatwg.org/#storage-bucket // A storage bucket is a place for storage endpoints to store data. -struct StorageBucket { +class StorageBucket : public GC::Cell { + GC_CELL(StorageBucket, GC::Cell); + GC_DECLARE_ALLOCATOR(StorageBucket); + +public: + static GC::Ref create(GC::Heap& heap, StorageType type) { return heap.allocate(type); } + + BottleMap& bottle_map() { return m_bottle_map; } + BottleMap const& bottle_map() const { return m_bottle_map; } + + virtual void visit_edges(GC::Cell::Visitor& visitor) override; + +private: explicit StorageBucket(StorageType); // A storage bucket has a bottle map of storage identifiers to storage bottles. - BottleMap bottle_map; + BottleMap m_bottle_map; }; -RefPtr obtain_a_session_storage_bottle_map(HTML::EnvironmentSettingsObject&, StringView storage_identifier); -RefPtr obtain_a_local_storage_bottle_map(HTML::EnvironmentSettingsObject&, StringView storage_identifier); -RefPtr obtain_a_storage_bottle_map(StorageType, HTML::EnvironmentSettingsObject&, StringView storage_identifier); +GC::Ptr obtain_a_session_storage_bottle_map(HTML::EnvironmentSettingsObject&, StringView storage_identifier); +GC::Ptr obtain_a_local_storage_bottle_map(HTML::EnvironmentSettingsObject&, StringView storage_identifier); +GC::Ptr obtain_a_storage_bottle_map(StorageType, HTML::EnvironmentSettingsObject&, StringView storage_identifier); } diff --git a/Libraries/LibWeb/StorageAPI/StorageShed.cpp b/Libraries/LibWeb/StorageAPI/StorageShed.cpp index cdce960aeb3..d98055eee7d 100644 --- a/Libraries/LibWeb/StorageAPI/StorageShed.cpp +++ b/Libraries/LibWeb/StorageAPI/StorageShed.cpp @@ -4,13 +4,22 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include namespace Web::StorageAPI { +GC_DEFINE_ALLOCATOR(StorageShed); + +void StorageShed::visit_edges(GC::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_data); +} + // https://storage.spec.whatwg.org/#obtain-a-storage-shelf -Optional StorageShed::obtain_a_storage_shelf(HTML::EnvironmentSettingsObject const& environment, StorageType type) +GC::Ptr StorageShed::obtain_a_storage_shelf(HTML::EnvironmentSettingsObject const& environment, StorageType type) { // 1. Let key be the result of running obtain a storage key with environment. auto key = obtain_a_storage_key(environment); @@ -21,18 +30,18 @@ Optional StorageShed::obtain_a_storage_shelf(HTML::EnvironmentSet // 3. If shed[key] does not exist, then set shed[key] to the result of running create a storage shelf with type. // 4. Return shed[key]. - return m_data.ensure(key.value(), [type] { - return StorageShelf { type }; + return m_data.ensure(key.value(), [type, &heap = this->heap()] { + return StorageShelf::create(heap, type); }); } // https://storage.spec.whatwg.org/#user-agent-storage-shed -StorageShed& user_agent_storage_shed() +GC::Ref user_agent_storage_shed(GC::Heap& heap) { // A user agent holds a storage shed, which is a storage shed. A user agent’s storage shed holds all local storage data. // FIXME: Storing this statically in memory is not the correct place or way of doing this! - static StorageShed storage_shed; - return storage_shed; + static GC::Root storage_shed = GC::make_root(StorageShed::create(heap)); + return *storage_shed; } } diff --git a/Libraries/LibWeb/StorageAPI/StorageShed.h b/Libraries/LibWeb/StorageAPI/StorageShed.h index 806199367e7..ecacd5be710 100644 --- a/Libraries/LibWeb/StorageAPI/StorageShed.h +++ b/Libraries/LibWeb/StorageAPI/StorageShed.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -16,14 +17,23 @@ namespace Web::StorageAPI { // https://storage.spec.whatwg.org/#storage-shed // A storage shed is a map of storage keys to storage shelves. It is initially empty. -class StorageShed { +class StorageShed : public GC::Cell { + GC_CELL(StorageShed, GC::Cell); + GC_DECLARE_ALLOCATOR(StorageShed); + public: - Optional obtain_a_storage_shelf(HTML::EnvironmentSettingsObject const&, StorageType); + static GC::Ref create(GC::Heap& heap) { return heap.allocate(); } + + GC::Ptr obtain_a_storage_shelf(HTML::EnvironmentSettingsObject const&, StorageType); + + virtual void visit_edges(GC::Cell::Visitor& visitor) override; private: - OrderedHashMap m_data; + StorageShed() = default; + + OrderedHashMap> m_data; }; -StorageShed& user_agent_storage_shed(); +GC::Ref user_agent_storage_shed(GC::Heap&); } diff --git a/Libraries/LibWeb/StorageAPI/StorageShelf.cpp b/Libraries/LibWeb/StorageAPI/StorageShelf.cpp index 09f51c1eaec..b6ac559fc09 100644 --- a/Libraries/LibWeb/StorageAPI/StorageShelf.cpp +++ b/Libraries/LibWeb/StorageAPI/StorageShelf.cpp @@ -4,16 +4,25 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include namespace Web::StorageAPI { +GC_DEFINE_ALLOCATOR(StorageShelf); + +void StorageShelf::visit_edges(GC::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_bucket_map); +} + // https://storage.spec.whatwg.org/#create-a-storage-shelf StorageShelf::StorageShelf(StorageType type) { // 1. Let shelf be a new storage shelf. // 2. Set shelf’s bucket map["default"] to the result of running create a storage bucket with type. - bucket_map.set("default"_string, StorageBucket { type }); + m_bucket_map.set("default"_string, StorageBucket::create(heap(), type)); // 3. Return shelf. } diff --git a/Libraries/LibWeb/StorageAPI/StorageShelf.h b/Libraries/LibWeb/StorageAPI/StorageShelf.h index 67004d04be7..a14343a3c88 100644 --- a/Libraries/LibWeb/StorageAPI/StorageShelf.h +++ b/Libraries/LibWeb/StorageAPI/StorageShelf.h @@ -8,7 +8,7 @@ #include #include -#include +#include #include #include @@ -16,12 +16,24 @@ namespace Web::StorageAPI { // https://storage.spec.whatwg.org/#storage-shelf // A storage shelf exists for each storage key within a storage shed. It holds a bucket map, which is a map of strings to storage buckets. -using BucketMap = OrderedHashMap; +using BucketMap = OrderedHashMap>; -struct StorageShelf { +class StorageShelf : public GC::Cell { + GC_CELL(StorageShelf, GC::Cell); + GC_DECLARE_ALLOCATOR(StorageShelf); + +public: + static GC::Ref create(GC::Heap& heap, StorageType type) { return heap.allocate(type); } + + BucketMap& bucket_map() { return m_bucket_map; } + BucketMap const& bucket_map() const { return m_bucket_map; } + + virtual void visit_edges(GC::Cell::Visitor& visitor) override; + +private: explicit StorageShelf(StorageType); - BucketMap bucket_map; + BucketMap m_bucket_map; }; }