瀏覽代碼

LibWeb: Prevent world leak when activating event handler

Since SafeFunction strongly protects all of its captures, we can't
capture `this` when activating an event handler since that creates a
reference cycle and we end up leaking the entire world.
Andreas Kling 2 年之前
父節點
當前提交
8875cd0c83
共有 1 個文件被更改,包括 7 次插入7 次删除
  1. 7 7
      Userland/Libraries/LibWeb/DOM/EventTarget.cpp

+ 7 - 7
Userland/Libraries/LibWeb/DOM/EventTarget.cpp

@@ -535,13 +535,11 @@ void EventTarget::activate_event_handler(FlyString const& name, HTML::EventHandl
     // 4. Let callback be the result of creating a Web IDL EventListener instance representing a reference to a function of one argument that executes the steps of the event handler processing algorithm, given eventTarget, name, and its argument.
     // 4. Let callback be the result of creating a Web IDL EventListener instance representing a reference to a function of one argument that executes the steps of the event handler processing algorithm, given eventTarget, name, and its argument.
     //    The EventListener's callback context can be arbitrary; it does not impact the steps of the event handler processing algorithm. [DOM]
     //    The EventListener's callback context can be arbitrary; it does not impact the steps of the event handler processing algorithm. [DOM]
 
 
-    // NOTE: The callback must keep `this` alive. For example:
-    //          document.body.onunload = () => { console.log("onunload called!"); }
-    //          document.body.remove();
-    //          location.reload();
-    //       The body element is no longer in the DOM and there is no variable holding onto it. However, the onunload handler is still called, meaning the callback keeps the body element alive.
+    // NOTE: We *don't* capture `this` here, since that would create a reference cycle
+    //       between the NativeFunction's SafeFunction captures (always strong)
+    //       and the registered event listener on this EventTarget.
     auto callback_function = JS::NativeFunction::create(
     auto callback_function = JS::NativeFunction::create(
-        realm, [event_target = JS::make_handle(*this), name](JS::VM& vm) mutable -> JS::ThrowCompletionOr<JS::Value> {
+        realm, [name](JS::VM& vm) mutable -> JS::ThrowCompletionOr<JS::Value> {
             // The event dispatcher should only call this with one argument.
             // The event dispatcher should only call this with one argument.
             VERIFY(vm.argument_count() == 1);
             VERIFY(vm.argument_count() == 1);
 
 
@@ -550,7 +548,9 @@ void EventTarget::activate_event_handler(FlyString const& name, HTML::EventHandl
             VERIFY(event_wrapper_argument.is_object());
             VERIFY(event_wrapper_argument.is_object());
             auto& event = verify_cast<DOM::Event>(event_wrapper_argument.as_object());
             auto& event = verify_cast<DOM::Event>(event_wrapper_argument.as_object());
 
 
-            TRY(event_target->process_event_handler_for_event(name, event));
+            auto& this_value = verify_cast<EventTarget>(vm.this_value().as_object());
+
+            TRY(this_value.process_event_handler_for_event(name, event));
             return JS::js_undefined();
             return JS::js_undefined();
         },
         },
         0, "", &realm);
         0, "", &realm);