Merge pull request #694 from lightpanda-io/add_event_listener_object

AddEventListener object listener

src/browser/dom/event_target.zig

@@ -41,79 +41,46 @@ pub const EventTarget = struct {
 
     // JS funcs
     // --------
+    pub fn _addEventListener(
+        self: *parser.EventTarget,
+        typ: []const u8,
+        listener: EventHandler.Listener,
+        opts: ?EventHandler.Opts,
+        page: *Page,
+    ) !void {
+        _ = try EventHandler.register(page.arena, self, typ, listener, opts);
+    }
 
-    const AddEventListenerOpts = union(enum) {
+    const RemoveEventListenerOpts = union(enum) {
         opts: Opts,
         capture: bool,
 
         const Opts = struct {
             capture: ?bool,
-            once: ?bool, // currently does nothing
-            passive: ?bool, // currently does nothing
-            signal: ?bool, // currently does nothing
         };
     };
 
-    pub fn _addEventListener(
-        self: *parser.EventTarget,
-        typ: []const u8,
-        cbk: Env.Function,
-        opts_: ?AddEventListenerOpts,
-        page: *Page,
-    ) !void {
-        var capture = false;
-        if (opts_) |opts| {
-            switch (opts) {
-                .capture => |c| capture = c,
-                .opts => |o| {
-                    // Done this way so that, for common cases that _only_ set
-                    // capture, i.e. {captrue: true}, it works.
-                    // But for any case that sets any of the other flags, we
-                    // error. If we don't error, this function call would succeed
-                    // but the behavior might be wrong. At this point, it's
-                    // better to be explicit and error.
-                    if (o.once orelse false) return error.NotImplemented;
-                    if (o.signal orelse false) return error.NotImplemented;
-                    if (o.passive orelse false) return error.NotImplemented;
-                    capture = o.capture orelse false;
-                },
-            }
-        }
-
-        // check if event target has already this listener
-        const lst = try parser.eventTargetHasListener(
-            self,
-            typ,
-            capture,
-            cbk.id,
-        );
-        if (lst != null) {
-            return;
-        }
-
-        const eh = try EventHandler.init(page.arena, try cbk.withThis(self));
-
-        try parser.eventTargetAddEventListener(
-            self,
-            typ,
-            &eh.node,
-            capture,
-        );
-    }
-
     pub fn _removeEventListener(
         self: *parser.EventTarget,
         typ: []const u8,
-        cbk: Env.Function,
+        listener: EventHandler.Listener,
-        capture: ?bool,
+        opts_: ?RemoveEventListenerOpts,
-        // TODO: hanle EventListenerOptions
-        // see #https://github.com/lightpanda-io/jsruntime-lib/issues/114
     ) !void {
+        var capture = false;
+        if (opts_) |opts| {
+            capture = switch (opts) {
+                .capture => |c| c,
+                .opts => |o| o.capture orelse false,
+            };
+        }
+
+        const cbk = (try listener.callback(self)) orelse return;
+
         // check if event target has already this listener
         const lst = try parser.eventTargetHasListener(
             self,
             typ,
-            capture orelse false,
+            capture,
             cbk.id,
         );
         if (lst == null) {
@@ -125,7 +92,7 @@ pub const EventTarget = struct {
             self,
             typ,
             lst.?,
-            capture orelse false,
+            capture,
         );
     }
 
@@ -244,4 +211,15 @@ test "Browser.DOM.EventTarget" {
         .{ "phase", "3" },
         .{ "cur.getAttribute('id')", "content" },
     }, .{});
+
+    try runner.testCases(&.{
+        .{ "const obj1 = {calls: 0, handleEvent: function() { this.calls += 1; } };", null },
+        .{ "content.addEventListener('he', obj1);", null },
+        .{ "content.dispatchEvent(new Event('he'));", null },
+        .{ "obj1.calls", "1" },
+
+        .{ "content.removeEventListener('he', obj1);", null },
+        .{ "content.dispatchEvent(new Event('he'));", null },
+        .{ "obj1.calls", "1" },
+    }, .{});
 }

src/browser/dom/mutation_observer.zig

@@ -63,13 +63,13 @@ pub const MutationObserver = struct {
 
         // register node's events
         if (options.childList or options.subtree) {
-            try parser.eventTargetAddEventListener(
+            _ = try parser.eventTargetAddEventListener(
                 parser.toEventTarget(parser.Node, node),
                 "DOMNodeInserted",
                 &observer.event_node,
                 false,
             );
-            try parser.eventTargetAddEventListener(
+            _ = try parser.eventTargetAddEventListener(
                 parser.toEventTarget(parser.Node, node),
                 "DOMNodeRemoved",
                 &observer.event_node,
@@ -77,7 +77,7 @@ pub const MutationObserver = struct {
             );
         }
         if (options.attr()) {
-            try parser.eventTargetAddEventListener(
+            _ = try parser.eventTargetAddEventListener(
                 parser.toEventTarget(parser.Node, node),
                 "DOMAttrModified",
                 &observer.event_node,
@@ -85,7 +85,7 @@ pub const MutationObserver = struct {
             );
         }
         if (options.cdata()) {
-            try parser.eventTargetAddEventListener(
+            _ = try parser.eventTargetAddEventListener(
                 parser.toEventTarget(parser.Node, node),
                 "DOMCharacterDataModified",
                 &observer.event_node,
@@ -93,7 +93,7 @@ pub const MutationObserver = struct {
             );
         }
         if (options.subtree) {
-            try parser.eventTargetAddEventListener(
+            _ = try parser.eventTargetAddEventListener(
                 parser.toEventTarget(parser.Node, node),
                 "DOMSubtreeModified",
                 &observer.event_node,

src/browser/events/event.zig

@@ -21,7 +21,6 @@ const Allocator = std.mem.Allocator;
 
 const log = @import("../../log.zig");
 const parser = @import("../netsurf.zig");
-const Function = @import("../env.zig").Function;
 const generate = @import("../../runtime/generate.zig");
 
 const DOMException = @import("../dom/exceptions.zig").DOMException;
@@ -139,18 +138,97 @@ pub const Event = struct {
 };
 
 pub const EventHandler = struct {
+    once: bool,
+    capture: bool,
     callback: Function,
     node: parser.EventNode,
+    listener: *parser.EventListener,
+
+    const Env = @import("../env.zig").Env;
+    const Function = Env.Function;
+
+    pub const Listener = union(enum) {
+        function: Function,
+        object: Env.JsObject,
+
+        pub fn callback(self: Listener, target: *parser.EventTarget) !?Function {
+            return switch (self) {
+                .function => |func| try func.withThis(target),
+                .object => |obj| blk: {
+                    const func = (try obj.getFunction("handleEvent")) orelse return null;
+                    break :blk try func.withThis(try obj.persist());
+                },
+            };
+        }
+    };
+
+    pub const Opts = union(enum) {
+        flags: Flags,
+        capture: bool,
+
+        const Flags = struct {
+            once: ?bool,
+            capture: ?bool,
+            // We ignore this property. It seems to be largely used to help the
+            // browser make certain performance tweaks (i.e. the browser knows
+            // that the listener won't call preventDefault() and thus can safely
+            // run the default as needed).
+            passive: ?bool,
+            signal: ?bool, // currently does nothing
+        };
+    };
+
+    pub fn register(
+        allocator: Allocator,
+        target: *parser.EventTarget,
+        typ: []const u8,
+        listener: Listener,
+        opts_: ?Opts,
+    ) !?*EventHandler {
+        var once = false;
+        var capture = false;
+        if (opts_) |opts| {
+            switch (opts) {
+                .capture => |c| capture = c,
+                .flags => |f| {
+                    // Done this way so that, for common cases that _only_ set
+                    // capture, i.e. {captrue: true}, it works.
+                    // But for any case that sets any of the other flags, we
+                    // error. If we don't error, this function call would succeed
+                    // but the behavior might be wrong. At this point, it's
+                    // better to be explicit and error.
+                    if (f.signal orelse false) return error.NotImplemented;
+                    once = f.once orelse false;
+                    capture = f.capture orelse false;
+                },
+            }
+        }
+
+        const callback = (try listener.callback(target)) orelse return null;
+
+        // check if event target has already this listener
+        if (try parser.eventTargetHasListener(target, typ, capture, callback.id) != null) {
+            return null;
+        }
 
-    pub fn init(allocator: Allocator, callback: Function) !*EventHandler {
         const eh = try allocator.create(EventHandler);
         eh.* = .{
+            .once = once,
+            .capture = capture,
             .callback = callback,
             .node = .{
                 .id = callback.id,
                 .func = handle,
             },
+            .listener = undefined,
         };
+
+        eh.listener = try parser.eventTargetAddEventListener(
+            target,
+            typ,
+            &eh.node,
+            capture,
+        );
         return eh;
     }
 
@@ -165,6 +243,17 @@ pub const EventHandler = struct {
         self.callback.tryCall(void, .{ievent}, &result) catch {
             log.debug(.event, "handle callback error", .{ .err = result.exception, .stack = result.stack });
         };
+
+        if (self.once) {
+            const target = (parser.eventTarget(event) catch return).?;
+            const typ = parser.eventType(event) catch return;
+            parser.eventTargetRemoveEventListener(
+                target,
+                typ,
+                self.listener,
+                self.capture,
+            ) catch {};
+        }
     }
 };
 
@@ -265,4 +354,13 @@ test "Browser.Event" {
         .{ "document.dispatchEvent(new Event('count'))", "true" },
         .{ "nb", "0" },
     }, .{});
+
+    try runner.testCases(&.{
+        .{ "nb = 0; function cbk(event) { nb ++; }", null },
+        .{ "document.addEventListener('count', cbk, {once: true})", null },
+        .{ "document.dispatchEvent(new Event('count'))", "true" },
+        .{ "document.dispatchEvent(new Event('count'))", "true" },
+        .{ "document.dispatchEvent(new Event('count'))", "true" },
+        .{ "nb", "1" },
+    }, .{});
 }

src/browser/netsurf.zig

@@ -615,7 +615,7 @@ pub fn eventTargetAddEventListener(
     typ: []const u8,
     node: *EventNode,
     capture: bool,
-) !void {
+) !*EventListener {
     const event_handler = struct {
         fn handle(event_: ?*Event, ptr_: ?*anyopaque) callconv(.C) void {
             const ptr = ptr_ orelse return;
@@ -634,6 +634,8 @@ pub fn eventTargetAddEventListener(
     const s = try strFromData(typ);
     const err = eventTargetVtable(et).add_event_listener.?(et, s, listener, capture);
     try DOMErr(err);
+
+    return listener.?;
 }
 
 pub fn eventTargetHasListener(

src/browser/page.zig

@@ -273,7 +273,7 @@ pub const Page = struct {
         const doc = parser.documentHTMLToDocument(html_doc);
 
         const document_element = (try parser.documentGetDocumentElement(doc)) orelse return error.DocumentElementError;
-        try parser.eventTargetAddEventListener(
+        _ = try parser.eventTargetAddEventListener(
             parser.toEventTarget(parser.Element, document_element),
             "click",
             &self.window_clicked_event_node,

src/browser/xhr/event_target.zig

@@ -44,17 +44,18 @@ pub const XMLHttpRequestEventTarget = struct {
         self: *XMLHttpRequestEventTarget,
         alloc: std.mem.Allocator,
         typ: []const u8,
-        cbk: Function,
+        listener: EventHandler.Listener,
-    ) !void {
+    ) !?Function {
         const target = @as(*parser.EventTarget, @ptrCast(self));
-        const eh = try EventHandler.init(alloc, try cbk.withThis(target));
+
-        try parser.eventTargetAddEventListener(
+        // The only time this can return null if the listener is already
-            target,
+        // registered. But before calling `register`, all of our functions
-            typ,
+        // remove any existing listener, so it should be impossible to get null
-            &eh.node,
+        // from this function call.
-            false,
+        const eh = (try EventHandler.register(alloc, target, typ, listener, null)) orelse unreachable;
-        );
+        return eh.callback;
     }
+
     fn unregister(self: *XMLHttpRequestEventTarget, typ: []const u8, cbk_id: usize) !void {
         const et = @as(*parser.EventTarget, @ptrCast(self));
         // check if event target has already this listener
@@ -86,34 +87,28 @@ pub const XMLHttpRequestEventTarget = struct {
         return self.onloadend_cbk;
     }
 
-    pub fn set_onloadstart(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_onloadstart(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.onloadstart_cbk) |cbk| try self.unregister("loadstart", cbk.id);
-        try self.register(page.arena, "loadstart", handler);
+        self.onloadstart_cbk = try self.register(page.arena, "loadstart", listener);
-        self.onloadstart_cbk = handler;
     }
-    pub fn set_onprogress(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_onprogress(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.onprogress_cbk) |cbk| try self.unregister("progress", cbk.id);
-        try self.register(page.arena, "progress", handler);
+        self.onprogress_cbk = try self.register(page.arena, "progress", listener);
-        self.onprogress_cbk = handler;
     }
-    pub fn set_onabort(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_onabort(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.onabort_cbk) |cbk| try self.unregister("abort", cbk.id);
-        try self.register(page.arena, "abort", handler);
+        self.onabort_cbk = try self.register(page.arena, "abort", listener);
-        self.onabort_cbk = handler;
     }
-    pub fn set_onload(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_onload(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.onload_cbk) |cbk| try self.unregister("load", cbk.id);
-        try self.register(page.arena, "load", handler);
+        self.onload_cbk = try self.register(page.arena, "load", listener);
-        self.onload_cbk = handler;
     }
-    pub fn set_ontimeout(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_ontimeout(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.ontimeout_cbk) |cbk| try self.unregister("timeout", cbk.id);
-        try self.register(page.arena, "timeout", handler);
+        self.ontimeout_cbk = try self.register(page.arena, "timeout", listener);
-        self.ontimeout_cbk = handler;
     }
-    pub fn set_onloadend(self: *XMLHttpRequestEventTarget, handler: Function, page: *Page) !void {
+    pub fn set_onloadend(self: *XMLHttpRequestEventTarget, listener: EventHandler.Listener, page: *Page) !void {
         if (self.onloadend_cbk) |cbk| try self.unregister("loadend", cbk.id);
-        try self.register(page.arena, "loadend", handler);
+        self.onloadend_cbk = try self.register(page.arena, "loadend", listener);
-        self.onloadend_cbk = handler;
     }
 };

src/runtime/js.zig

@@ -582,7 +582,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
 
             // Given an anytype, turns it into a v8.Object. The anytype could be:
             // 1 - A V8.object already
-            // 2 - Our this JsObject wrapper around a V8.Object
+            // 2 - Our JsObject wrapper around a V8.Object
             // 3 - A zig instance that has previously been given to V8
             //     (i.e., the value has to be known to the executor)
             fn valueToExistingObject(self: *const Scope, value: anytype) !v8.Object {
@@ -951,15 +951,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     if (!js_value.isFunction()) {
                         return null;
                     }
-
+                    return try self.createFunction(js_value);
-                    const func = v8.Persistent(v8.Function).init(self.isolate, js_value.castTo(v8.Function));
-                    try self.trackCallback(func);
-
-                    return .{
-                        .func = func,
-                        .scope = self,
-                        .id = js_value.castTo(v8.Object).getIdentityHash(),
-                    };
                 }
 
                 const js_obj = js_value.castTo(v8.Object);
@@ -996,6 +988,20 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                 return value;
             }
 
+            fn createFunction(self: *Scope, js_value: v8.Value) !Function {
+                // caller should have made sure this was a function
+                std.debug.assert(js_value.isFunction());
+
+                const func = v8.Persistent(v8.Function).init(self.isolate, js_value.castTo(v8.Function));
+                try self.trackCallback(func);
+
+                return .{
+                    .func = func,
+                    .scope = self,
+                    .id = js_value.castTo(v8.Object).getIdentityHash(),
+                };
+            }
+
             // Probing is part of trying to map a JS value to a Zig union. There's
             // a lot of ambiguity in this process, in part because some JS values
             // can almost always be coerced. For example, anything can be coerced
@@ -1234,11 +1240,16 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
             };
 
             pub fn withThis(self: *const Function, value: anytype) !Function {
+                const this_obj = if (@TypeOf(value) == JsObject)
+                    value.js_obj
+                else
+                    try self.scope.valueToExistingObject(value);
+
                 return .{
                     .id = self.id,
+                    .this = this_obj,
                     .func = self.func,
                     .scope = self.scope,
-                    .this = try self.scope.valueToExistingObject(value),
                 };
             }
 
@@ -1375,6 +1386,17 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
                     .js_obj = gop.value_ptr.castToObject(),
                 };
             }
+
+            pub fn getFunction(self: JsObject, name: []const u8) !?Function {
+                const scope = self.scope;
+                const js_name = v8.String.initUtf8(scope.isolate, name);
+
+                const js_value = try self.js_obj.getValue(scope.context, js_name.toName());
+                if (!js_value.isFunction()) {
+                    return null;
+                }
+                return try scope.createFunction(js_value);
+            }
         };
 
         // This only exists so that we know whether a function wants the opaque