refactor: deduplicate defineEventHandler util (#12367)

ext/broadcast_channel/01_broadcast_channel.js

@@ -7,7 +7,7 @@
 ((window) => {
   const core = window.Deno.core;
   const webidl = window.__bootstrap.webidl;
-  const { setTarget } = window.__bootstrap.event;
+  const { defineEventHandler, setTarget } = window.__bootstrap.event;
   const { DOMException } = window.__bootstrap.domException;
   const {
     ArrayPrototypeIndexOf,
@@ -15,55 +15,8 @@
     ArrayPrototypePush,
     Symbol,
     Uint8Array,
-    ObjectDefineProperty,
-    Map,
-    MapPrototypeSet,
-    MapPrototypeGet,
-    FunctionPrototypeCall,
   } = window.__bootstrap.primordials;
 
-  const handlerSymbol = Symbol("eventHandlers");
-  function makeWrappedHandler(handler) {
-    function wrappedHandler(...args) {
-      if (typeof wrappedHandler.handler !== "function") {
-        return;
-      }
-      return FunctionPrototypeCall(wrappedHandler.handler, this, ...args);
-    }
-    wrappedHandler.handler = handler;
-    return wrappedHandler;
-  }
-  // TODO(lucacasonato) reuse when we can reuse code between web crates
-  function defineEventHandler(emitter, name) {
-    // HTML specification section 8.1.5.1
-    ObjectDefineProperty(emitter, `on${name}`, {
-      get() {
-        // TODO(bnoordhuis) The "BroadcastChannel should have an onmessage
-        // event" WPT test expects that .onmessage !== undefined. Returning
-        // null makes it pass but is perhaps not exactly in the spirit.
-        if (!this[handlerSymbol]) {
-          return null;
-        }
-        return MapPrototypeGet(this[handlerSymbol], name)?.handler ?? null;
-      },
-      set(value) {
-        if (!this[handlerSymbol]) {
-          this[handlerSymbol] = new Map();
-        }
-        let handlerWrapper = MapPrototypeGet(this[handlerSymbol], name);
-        if (handlerWrapper) {
-          handlerWrapper.handler = value;
-        } else {
-          handlerWrapper = makeWrappedHandler(value);
-          this.addEventListener(name, handlerWrapper);
-        }
-        MapPrototypeSet(this[handlerSymbol], name, handlerWrapper);
-      },
-      configurable: true,
-      enumerable: true,
-    });
-  }
-
   const _name = Symbol("[[name]]");
   const _closed = Symbol("[[closed]]");
 

ext/web/02_event.js

@@ -1226,36 +1226,76 @@
 
   const _eventHandlers = Symbol("eventHandlers");
 
-  function makeWrappedHandler(handler) {
+  function makeWrappedHandler(handler, isSpecialErrorEventHandler) {
-    function wrappedHandler(...args) {
+    function wrappedHandler(evt) {
       if (typeof wrappedHandler.handler !== "function") {
         return;
       }
-      return FunctionPrototypeCall(wrappedHandler.handler, this, ...args);
+
+      if (
+        isSpecialErrorEventHandler &&
+        evt instanceof ErrorEvent && evt.type === "error"
+      ) {
+        const ret = FunctionPrototypeCall(
+          wrappedHandler.handler,
+          this,
+          evt.message,
+          evt.filename,
+          evt.lineno,
+          evt.colno,
+          evt.error,
+        );
+        if (ret === true) {
+          evt.preventDefault();
+        }
+        return;
+      }
+
+      return FunctionPrototypeCall(wrappedHandler.handler, this, evt);
     }
     wrappedHandler.handler = handler;
     return wrappedHandler;
   }
 
-  // TODO(benjamingr) reuse this here and websocket where possible
+  // `init` is an optional function that will be called the first time that the
-  function defineEventHandler(emitter, name, init) {
+  // event handler property is set. It will be called with the object on which
-    // HTML specification section 8.1.5.1
+  // the property is set as its argument.
+  // `isSpecialErrorEventHandler` can be set to true to opt into the special
+  // behavior of event handlers for the "error" event in a global scope.
+  function defineEventHandler(
+    emitter,
+    name,
+    init = undefined,
+    isSpecialErrorEventHandler = false,
+  ) {
+    // HTML specification section 8.1.7.1
     ObjectDefineProperty(emitter, `on${name}`, {
       get() {
-        const map = this[_eventHandlers];
+        if (!this[_eventHandlers]) {
+          return null;
+        }
 
-        if (!map) return undefined;
+        return MapPrototypeGet(this[_eventHandlers], name)?.handler ?? null;
-        return MapPrototypeGet(map, name)?.handler;
       },
       set(value) {
+        // All three Web IDL event handler types are nullable callback functions
+        // with the [LegacyTreatNonObjectAsNull] extended attribute, meaning
+        // anything other than an object is treated as null.
+        if (typeof value !== "object" && typeof value !== "function") {
+          value = null;
+        }
+
         if (!this[_eventHandlers]) {
           this[_eventHandlers] = new Map();
         }
         let handlerWrapper = MapPrototypeGet(this[_eventHandlers], name);
         if (handlerWrapper) {
           handlerWrapper.handler = value;
-        } else {
+        } else if (value !== null) {
-          handlerWrapper = makeWrappedHandler(value);
+          handlerWrapper = makeWrappedHandler(
+            value,
+            isSpecialErrorEventHandler,
+          );
           this.addEventListener(name, handlerWrapper);
           init?.(this);
         }

ext/websocket/01_websocket.js

@@ -9,6 +9,7 @@
   const webidl = window.__bootstrap.webidl;
   const { HTTP_TOKEN_CODE_POINT_RE } = window.__bootstrap.infra;
   const { DOMException } = window.__bootstrap.domException;
+  const { defineEventHandler } = window.__bootstrap.event;
   const { Blob } = globalThis.__bootstrap.file;
   const {
     ArrayBuffer,
@@ -16,16 +17,11 @@
     ArrayPrototypeJoin,
     DataView,
     ErrorPrototypeToString,
-    ObjectDefineProperty,
-    Map,
-    MapPrototypeGet,
-    MapPrototypeSet,
     Set,
     Symbol,
     String,
     StringPrototypeToLowerCase,
     StringPrototypeEndsWith,
-    FunctionPrototypeCall,
     RegExpPrototypeTest,
     ObjectDefineProperties,
     ArrayPrototypeMap,
@@ -65,45 +61,6 @@
   const CLOSING = 2;
   const CLOSED = 3;
 
-  const handlerSymbol = Symbol("eventHandlers");
-  function makeWrappedHandler(handler) {
-    function wrappedHandler(...args) {
-      if (typeof wrappedHandler.handler !== "function") {
-        return;
-      }
-      return FunctionPrototypeCall(wrappedHandler.handler, this, ...args);
-    }
-    wrappedHandler.handler = handler;
-    return wrappedHandler;
-  }
-  // TODO(lucacasonato) reuse when we can reuse code between web crates
-  function defineEventHandler(emitter, name) {
-    // HTML specification section 8.1.5.1
-    ObjectDefineProperty(emitter, `on${name}`, {
-      get() {
-        if (!this[handlerSymbol]) {
-          return null;
-        }
-        return MapPrototypeGet(this[handlerSymbol], name)?.handler;
-      },
-      set(value) {
-        if (!this[handlerSymbol]) {
-          this[handlerSymbol] = new Map();
-        }
-        let handlerWrapper = MapPrototypeGet(this[handlerSymbol], name);
-        if (handlerWrapper) {
-          handlerWrapper.handler = value;
-        } else {
-          handlerWrapper = makeWrappedHandler(value);
-          this.addEventListener(name, handlerWrapper);
-        }
-        MapPrototypeSet(this[handlerSymbol], name, handlerWrapper);
-      },
-      configurable: true,
-      enumerable: true,
-    });
-  }
-
   const _readyState = Symbol("[[readyState]]");
   const _url = Symbol("[[url]]");
   const _rid = Symbol("[[rid]]");

runtime/js/01_web_util.js

@@ -2,15 +2,7 @@
 "use strict";
 
 ((window) => {
-  const {
+  const { TypeError, Symbol } = window.__bootstrap.primordials;
-    FunctionPrototypeCall,
-    Map,
-    MapPrototypeGet,
-    MapPrototypeSet,
-    ObjectDefineProperty,
-    TypeError,
-    Symbol,
-  } = window.__bootstrap.primordials;
   const illegalConstructorKey = Symbol("illegalConstructorKey");
 
   function requiredArguments(
@@ -26,75 +18,8 @@
     }
   }
 
-  const handlerSymbol = Symbol("eventHandlers");
-  function makeWrappedHandler(handler, isSpecialErrorEventHandler) {
-    function wrappedHandler(...args) {
-      if (typeof wrappedHandler.handler !== "function") {
-        return;
-      }
-      if (isSpecialErrorEventHandler) {
-        const evt = args[0];
-        if (evt instanceof ErrorEvent && evt.type === "error") {
-          const ret = FunctionPrototypeCall(
-            wrappedHandler.handler,
-            this,
-            evt.message,
-            evt.filename,
-            evt.lineno,
-            evt.colno,
-            evt.error,
-          );
-          if (ret === true) {
-            evt.preventDefault();
-          }
-          return;
-        }
-      }
-
-      return FunctionPrototypeCall(wrappedHandler.handler, this, ...args);
-    }
-    wrappedHandler.handler = handler;
-    return wrappedHandler;
-  }
-  function defineEventHandler(
-    emitter,
-    name,
-    defaultValue = undefined,
-    isSpecialErrorEventHandler = false,
-  ) {
-    // HTML specification section 8.1.5.1
-    ObjectDefineProperty(emitter, `on${name}`, {
-      get() {
-        if (!this[handlerSymbol]) {
-          return defaultValue;
-        }
-
-        return MapPrototypeGet(this[handlerSymbol], name)?.handler ??
-          defaultValue;
-      },
-      set(value) {
-        if (!this[handlerSymbol]) {
-          this[handlerSymbol] = new Map();
-        }
-        let handlerWrapper = MapPrototypeGet(this[handlerSymbol], name);
-        if (handlerWrapper) {
-          handlerWrapper.handler = value;
-        } else {
-          handlerWrapper = makeWrappedHandler(
-            value,
-            isSpecialErrorEventHandler,
-          );
-          this.addEventListener(name, handlerWrapper);
-        }
-        MapPrototypeSet(this[handlerSymbol], name, handlerWrapper);
-      },
-      configurable: true,
-      enumerable: true,
-    });
-  }
   window.__bootstrap.webUtil = {
     illegalConstructorKey,
     requiredArguments,
-    defineEventHandler,
   };
 })(this);

runtime/js/11_workers.js

@@ -16,7 +16,7 @@
   const { URL } = window.__bootstrap.url;
   const { getLocationHref } = window.__bootstrap.location;
   const { log, pathFromURL } = window.__bootstrap.util;
-  const { defineEventHandler } = window.__bootstrap.webUtil;
+  const { defineEventHandler } = window.__bootstrap.event;
   const { deserializeJsMessageData, serializeJsMessageData } =
     window.__bootstrap.messagePort;
 

runtime/js/99_main.js

@@ -59,7 +59,7 @@ delete Object.prototype.__proto__;
   const errors = window.__bootstrap.errors.errors;
   const webidl = window.__bootstrap.webidl;
   const domException = window.__bootstrap.domException;
-  const { defineEventHandler } = window.__bootstrap.webUtil;
+  const { defineEventHandler } = window.__bootstrap.event;
   const { deserializeJsMessageData, serializeJsMessageData } =
     window.__bootstrap.messagePort;
 
@@ -479,10 +479,6 @@ delete Object.prototype.__proto__;
       enumerable: true,
       get: () => navigator,
     },
-    // TODO(bartlomieju): from MDN docs (https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope)
-    // it seems those two properties should be available to workers as well
-    onload: util.writable(null),
-    onunload: util.writable(null),
     close: util.writable(windowClose),
     closed: util.getterOnly(() => windowIsClosing),
     alert: util.writable(prompt.alert),
@@ -514,8 +510,6 @@ delete Object.prototype.__proto__;
       get: () => workerNavigator,
     },
     self: util.readOnly(globalThis),
-    onmessage: util.writable(null),
-    onerror: util.writable(null),
     // TODO(bartlomieju): should be readonly?
     close: util.nonEnumerable(workerClose),
     postMessage: util.writable(postMessage),
@@ -548,8 +542,8 @@ delete Object.prototype.__proto__;
 
     eventTarget.setEventTargetData(globalThis);
 
-    defineEventHandler(window, "load", null);
+    defineEventHandler(window, "load");
-    defineEventHandler(window, "unload", null);
+    defineEventHandler(window, "unload");
 
     const isUnloadDispatched = SymbolFor("isUnloadDispatched");
     // Stores the flag for checking whether unload is dispatched or not.
@@ -646,8 +640,8 @@ delete Object.prototype.__proto__;
 
     eventTarget.setEventTargetData(globalThis);
 
-    defineEventHandler(self, "message", null);
+    defineEventHandler(self, "message");
-    defineEventHandler(self, "error", null, true);
+    defineEventHandler(self, "error", undefined, true);
 
     runtimeStart(
       runtimeOptions,

tools/wpt/expectation.json

@@ -14979,9 +14979,9 @@
     "constructor.any.html": true,
     "constructor.any.html?wpt_flags=h2": true,
     "constructor.any.html?wss": true,
-    "eventhandlers.any.html": false,
+    "eventhandlers.any.html": true,
-    "eventhandlers.any.html?wpt_flags=h2": false,
+    "eventhandlers.any.html?wpt_flags=h2": true,
-    "eventhandlers.any.html?wss": false,
+    "eventhandlers.any.html?wss": true,
     "referrer.any.html": true,
     "Close-delayed.any.html": false,
     "Close-delayed.any.html?wpt_flags=h2": false,
@@ -15056,10 +15056,7 @@
     "interfaces": {
       "DedicatedWorkerGlobalScope": {
         "EventTarget.worker.html": true,
-        "onmessage.worker.html": [
+        "onmessage.worker.html": true,
-          "Setting onmessage to 1",
-          "Setting onmessage to 1 (again)"
-        ],
         "postMessage": {
           "return-value.worker.html": true
         }