LibWeb: Make Storage GC-allocated

Author: https://github.com/awesomekling
Commit: https://github.com/SerenityOS/serenity/commit/2ac8e3db3a
Pull-request: https://github.com/SerenityOS/serenity/pull/14816
Reviewed-by: https://github.com/ADKaster
Reviewed-by: https://github.com/linusg ✅

Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator/IDLGenerators.cpp

@@ -157,6 +157,8 @@ static bool impl_is_wrapper(Type const& type)
         return true;
     if (type.name == "Path2D"sv)
         return true;
+    if (type.name == "Storage"sv)
+        return true;
 
     return false;
 }

Userland/Libraries/LibWeb/Forward.h

@@ -466,7 +466,6 @@ class OptionConstructor;
 class RangePrototype;
 class ResizeObserverWrapper;
 class SelectionWrapper;
-class StorageWrapper;
 class SubtleCryptoWrapper;
 class TextDecoderWrapper;
 class TextEncoderWrapper;

Userland/Libraries/LibWeb/HTML/Storage.cpp

@@ -6,15 +6,20 @@
 
 #include <AK/String.h>
 #include <LibWeb/HTML/Storage.h>
+#include <LibWeb/HTML/Window.h>
 
 namespace Web::HTML {
 
-NonnullRefPtr<Storage> Storage::create()
+JS::NonnullGCPtr<Storage> Storage::create(HTML::Window& window)
 {
-    return adopt_ref(*new Storage);
+    return *window.heap().allocate<Storage>(window.realm(), window);
 }
 
-Storage::Storage() = default;
+Storage::Storage(HTML::Window& window)
+    : PlatformObject(window.realm())
+{
+    set_prototype(&window.cached_web_prototype("Storage"));
+}
 
 Storage::~Storage() = default;
 

Userland/Libraries/LibWeb/HTML/Storage.h

@@ -7,20 +7,16 @@
 #pragma once
 
 #include <AK/HashMap.h>
-#include <AK/RefCounted.h>
-#include <LibWeb/Bindings/Wrappable.h>
+#include <LibWeb/Bindings/PlatformObject.h>
 #include <LibWeb/DOM/ExceptionOr.h>
-#include <LibWeb/Forward.h>
 
 namespace Web::HTML {
 
-class Storage
-    : public RefCounted<Storage>
-    , public Bindings::Wrappable {
-public:
-    using WrapperType = Bindings::StorageWrapper;
+class Storage : public Bindings::PlatformObject {
+    WEB_PLATFORM_OBJECT(Storage, Bindings::PlatformObject);
 
-    static NonnullRefPtr<Storage> create();
+public:
+    static JS::NonnullGCPtr<Storage> create(HTML::Window&);
     ~Storage();
 
     size_t length() const;
@@ -37,7 +33,7 @@ public:
     void dump() const;
 
 private:
-    Storage();
+    explicit Storage(HTML::Window&);
 
     void reorder();
     void broadcast(String const& key, String const& old_value, String const& new_value);
@@ -47,8 +43,4 @@ private:
 
 }
 
-namespace Web::Bindings {
-
-StorageWrapper* wrap(JS::Realm&, HTML::Storage&);
-
-}
+WRAPPER_HACK(Storage, Web::HTML)

Userland/Libraries/LibWeb/HTML/Window.cpp

@@ -25,7 +25,6 @@
 #include <LibWeb/Bindings/NavigatorObject.h>
 #include <LibWeb/Bindings/Replaceable.h>
 #include <LibWeb/Bindings/SelectionWrapper.h>
-#include <LibWeb/Bindings/StorageWrapper.h>
 #include <LibWeb/Bindings/WindowObjectHelper.h>
 #include <LibWeb/Bindings/WindowPrototype.h>
 #include <LibWeb/CSS/MediaQueryList.h>
@@ -564,25 +563,27 @@ Selection::Selection* Window::get_selection_impl()
 }
 
 // https://html.spec.whatwg.org/multipage/webstorage.html#dom-localstorage
-RefPtr<HTML::Storage> Window::local_storage()
+JS::NonnullGCPtr<HTML::Storage> Window::local_storage()
 {
     // FIXME: Implement according to spec.
 
-    static HashMap<Origin, NonnullRefPtr<HTML::Storage>> local_storage_per_origin;
-    return local_storage_per_origin.ensure(associated_document().origin(), [] {
-        return HTML::Storage::create();
+    static HashMap<Origin, JS::Handle<HTML::Storage>> local_storage_per_origin;
+    auto storage = local_storage_per_origin.ensure(associated_document().origin(), [this]() -> JS::Handle<HTML::Storage> {
+        return *HTML::Storage::create(*this);
     });
+    return *storage;
 }
 
 // https://html.spec.whatwg.org/multipage/webstorage.html#dom-sessionstorage
-RefPtr<HTML::Storage> Window::session_storage()
+JS::NonnullGCPtr<HTML::Storage> Window::session_storage()
 {
     // FIXME: Implement according to spec.
 
-    static HashMap<Origin, NonnullRefPtr<HTML::Storage>> session_storage_per_origin;
-    return session_storage_per_origin.ensure(associated_document().origin(), [] {
-        return HTML::Storage::create();
+    static HashMap<Origin, JS::Handle<HTML::Storage>> session_storage_per_origin;
+    auto storage = session_storage_per_origin.ensure(associated_document().origin(), [this]() -> JS::Handle<HTML::Storage> {
+        return *HTML::Storage::create(*this);
     });
+    return *storage;
 }
 
 // https://html.spec.whatwg.org/multipage/browsers.html#dom-parent
@@ -1408,18 +1409,14 @@ JS_DEFINE_NATIVE_FUNCTION(Window::origin_getter)
 
 JS_DEFINE_NATIVE_FUNCTION(Window::local_storage_getter)
 {
-    auto& realm = *vm.current_realm();
     auto* impl = TRY(impl_from(vm));
-    // FIXME: localStorage may throw. We have to deal with that here.
-    return wrap(realm, *impl->local_storage());
+    return impl->local_storage();
 }
 
 JS_DEFINE_NATIVE_FUNCTION(Window::session_storage_getter)
 {
-    auto& realm = *vm.current_realm();
     auto* impl = TRY(impl_from(vm));
-    // FIXME: sessionStorage may throw. We have to deal with that here.
-    return wrap(realm, *impl->session_storage());
+    return impl->session_storage();
 }
 
 JS_DEFINE_NATIVE_FUNCTION(Window::name_getter)

Userland/Libraries/LibWeb/HTML/Window.h

@@ -100,8 +100,8 @@ public:
 
     Selection::Selection* get_selection_impl();
 
-    RefPtr<HTML::Storage> local_storage();
-    RefPtr<HTML::Storage> session_storage();
+    JS::NonnullGCPtr<HTML::Storage> local_storage();
+    JS::NonnullGCPtr<HTML::Storage> session_storage();
 
     Window* parent();
 

Userland/Libraries/LibWeb/idl_files.cmake

@@ -147,7 +147,7 @@ libweb_js_wrapper(HTML/MessagePort NO_INSTANCE)
 libweb_js_wrapper(HTML/PageTransitionEvent NO_INSTANCE)
 libweb_js_wrapper(HTML/Path2D NO_INSTANCE)
 libweb_js_wrapper(HTML/PromiseRejectionEvent NO_INSTANCE)
-libweb_js_wrapper(HTML/Storage)
+libweb_js_wrapper(HTML/Storage NO_INSTANCE)
 libweb_js_wrapper(HTML/SubmitEvent NO_INSTANCE)
 libweb_js_wrapper(HTML/TextMetrics NO_INSTANCE)
 libweb_js_wrapper(HTML/Worker NO_INSTANCE)