Explorar o código

LibWeb: Don't store JS::Handle<JS::Promise> in EnvironmentSettingsObject

Now that the ESO is a JS::Cell, we can just store them as NonnullGCPtr
and mark them in visit_edges().
Andreas Kling %!s(int64=2) %!d(string=hai) anos
pai
achega
18a5c56f14

+ 2 - 2
Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp

@@ -106,12 +106,12 @@ JS::VM& main_thread_vm()
             case JS::Promise::RejectionOperation::Reject:
                 // 4. If operation is "reject",
                 //    1. Add promise to settings object's about-to-be-notified rejected promises list.
-                settings_object.push_onto_about_to_be_notified_rejected_promises_list(JS::make_handle(&promise));
+                settings_object.push_onto_about_to_be_notified_rejected_promises_list(promise);
                 break;
             case JS::Promise::RejectionOperation::Handle: {
                 // 5. If operation is "handle",
                 //    1. If settings object's about-to-be-notified rejected promises list contains promise, then remove promise from that list and return.
-                bool removed_about_to_be_notified_rejected_promise = settings_object.remove_from_about_to_be_notified_rejected_promises_list(&promise);
+                bool removed_about_to_be_notified_rejected_promise = settings_object.remove_from_about_to_be_notified_rejected_promises_list(promise);
                 if (removed_about_to_be_notified_rejected_promise)
                     return;
 

+ 18 - 12
Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp

@@ -29,6 +29,13 @@ EnvironmentSettingsObject::~EnvironmentSettingsObject()
     responsible_event_loop().unregister_environment_settings_object({}, *this);
 }
 
+void EnvironmentSettingsObject::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    for (auto& promise : m_about_to_be_notified_rejected_promises_list)
+        visitor.visit(promise);
+}
+
 JS::ExecutionContext& EnvironmentSettingsObject::realm_execution_context()
 {
     // NOTE: All environment settings objects are created with a realm execution context, so it's stored and returned here in the base class.
@@ -173,15 +180,15 @@ bool EnvironmentSettingsObject::remove_from_outstanding_rejected_promises_weak_s
     });
 }
 
-void EnvironmentSettingsObject::push_onto_about_to_be_notified_rejected_promises_list(JS::Handle<JS::Promise> promise)
+void EnvironmentSettingsObject::push_onto_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise> promise)
 {
     m_about_to_be_notified_rejected_promises_list.append(move(promise));
 }
 
-bool EnvironmentSettingsObject::remove_from_about_to_be_notified_rejected_promises_list(JS::Promise* promise)
+bool EnvironmentSettingsObject::remove_from_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise> promise)
 {
-    return m_about_to_be_notified_rejected_promises_list.remove_first_matching([&](JS::Handle<JS::Promise> promise_in_list) {
-        return promise == promise_in_list.cell();
+    return m_about_to_be_notified_rejected_promises_list.remove_first_matching([&](auto& promise_in_list) {
+        return promise == promise_in_list;
     });
 }
 
@@ -204,11 +211,10 @@ void EnvironmentSettingsObject::notify_about_rejected_promises(Badge<EventLoop>)
     // 5. Queue a global task on the DOM manipulation task source given global to run the following substep:
     queue_global_task(Task::Source::DOMManipulation, global, [this, &global, list = move(list)]() mutable {
         // 1. For each promise p in list:
-        for (auto promise_handle : list) {
-            auto& promise = *promise_handle.cell();
+        for (auto promise : list) {
 
             // 1. If p's [[PromiseIsHandled]] internal slot is true, continue to the next iteration of the loop.
-            if (promise.is_handled())
+            if (promise->is_handled())
                 continue;
 
             // 2. Let notHandled be the result of firing an event named unhandledrejection at global, using PromiseRejectionEvent, with the cancelable attribute initialized to true,
@@ -220,8 +226,8 @@ void EnvironmentSettingsObject::notify_about_rejected_promises(Badge<EventLoop>)
                     .composed = false,
                 },
                 // Sadly we can't use .promise and .reason here, as we can't use the designator on the initialization of DOM::EventInit above.
-                /* .promise = */ promise_handle,
-                /* .reason = */ promise.result(),
+                /* .promise = */ JS::make_handle(*promise),
+                /* .reason = */ promise->result(),
             };
             // FIXME: This currently assumes that global is a WindowObject.
             auto& window = verify_cast<HTML::Window>(global);
@@ -233,13 +239,13 @@ void EnvironmentSettingsObject::notify_about_rejected_promises(Badge<EventLoop>)
             // 3. If notHandled is false, then the promise rejection is handled. Otherwise, the promise rejection is not handled.
 
             // 4. If p's [[PromiseIsHandled]] internal slot is false, add p to settings object's outstanding rejected promises weak set.
-            if (!promise.is_handled())
-                m_outstanding_rejected_promises_weak_set.append(&promise);
+            if (!promise->is_handled())
+                m_outstanding_rejected_promises_weak_set.append(promise);
 
             // This algorithm results in promise rejections being marked as handled or not handled. These concepts parallel handled and not handled script errors.
             // If a rejection is still not handled after this, then the rejection may be reported to a developer console.
             if (not_handled)
-                HTML::report_exception_to_console(promise.result(), realm(), ErrorInPromise::Yes);
+                HTML::report_exception_to_console(promise->result(), realm(), ErrorInPromise::Yes);
         }
     });
 }

+ 5 - 3
Userland/Libraries/LibWeb/HTML/Scripting/Environments.h

@@ -98,10 +98,10 @@ struct EnvironmentSettingsObject
     // Returns true if removed, false otherwise.
     bool remove_from_outstanding_rejected_promises_weak_set(JS::Promise*);
 
-    void push_onto_about_to_be_notified_rejected_promises_list(JS::Handle<JS::Promise>);
+    void push_onto_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise>);
 
     // Returns true if removed, false otherwise.
-    bool remove_from_about_to_be_notified_rejected_promises_list(JS::Promise*);
+    bool remove_from_about_to_be_notified_rejected_promises_list(JS::NonnullGCPtr<JS::Promise>);
 
     void notify_about_rejected_promises(Badge<EventLoop>);
 
@@ -113,6 +113,8 @@ struct EnvironmentSettingsObject
 protected:
     explicit EnvironmentSettingsObject(NonnullOwnPtr<JS::ExecutionContext>);
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
 private:
     NonnullOwnPtr<JS::ExecutionContext> m_realm_execution_context;
     ModuleMap m_module_map;
@@ -124,7 +126,7 @@ private:
     Vector<JS::Promise*> m_outstanding_rejected_promises_weak_set;
 
     // https://html.spec.whatwg.org/multipage/webappapis.html#about-to-be-notified-rejected-promises-list
-    Vector<JS::Handle<JS::Promise>> m_about_to_be_notified_rejected_promises_list;
+    Vector<JS::NonnullGCPtr<JS::Promise>> m_about_to_be_notified_rejected_promises_list;
 };
 
 EnvironmentSettingsObject& incumbent_settings_object();