From 74e5b68682d4d2503e4af5bac3b98067bc58f275 Mon Sep 17 00:00:00 2001 From: Andreu Botella Date: Fri, 8 Oct 2021 09:53:31 +0200 Subject: [PATCH] refactor: deduplicate `defineEventHandler` util (#12367) --- ext/broadcast_channel/01_broadcast_channel.js | 49 +----------- ext/web/02_event.js | 62 ++++++++++++--- ext/websocket/01_websocket.js | 45 +---------- runtime/js/01_web_util.js | 77 +------------------ runtime/js/11_workers.js | 2 +- runtime/js/99_main.js | 16 ++-- tools/wpt/expectation.json | 11 +-- 7 files changed, 64 insertions(+), 198 deletions(-) diff --git a/ext/broadcast_channel/01_broadcast_channel.js b/ext/broadcast_channel/01_broadcast_channel.js index 42112c014d..d1970b1eb6 100644 --- a/ext/broadcast_channel/01_broadcast_channel.js +++ b/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]]"); diff --git a/ext/web/02_event.js b/ext/web/02_event.js index 790bb35ea8..5ca36300c6 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -1226,36 +1226,76 @@ const _eventHandlers = Symbol("eventHandlers"); - function makeWrappedHandler(handler) { - function wrappedHandler(...args) { + function makeWrappedHandler(handler, isSpecialErrorEventHandler) { + 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 - function defineEventHandler(emitter, name, init) { - // HTML specification section 8.1.5.1 + // `init` is an optional function that will be called the first time that the + // event handler property is set. It will be called with the object on which + // 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(map, name)?.handler; + return MapPrototypeGet(this[_eventHandlers], name)?.handler ?? null; }, 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 { - handlerWrapper = makeWrappedHandler(value); + } else if (value !== null) { + handlerWrapper = makeWrappedHandler( + value, + isSpecialErrorEventHandler, + ); this.addEventListener(name, handlerWrapper); init?.(this); } diff --git a/ext/websocket/01_websocket.js b/ext/websocket/01_websocket.js index 54e05c4085..200bb86590 100644 --- a/ext/websocket/01_websocket.js +++ b/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]]"); diff --git a/runtime/js/01_web_util.js b/runtime/js/01_web_util.js index 9b51021f9f..ca3d748269 100644 --- a/runtime/js/01_web_util.js +++ b/runtime/js/01_web_util.js @@ -2,15 +2,7 @@ "use strict"; ((window) => { - const { - FunctionPrototypeCall, - Map, - MapPrototypeGet, - MapPrototypeSet, - ObjectDefineProperty, - TypeError, - Symbol, - } = window.__bootstrap.primordials; + const { 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); diff --git a/runtime/js/11_workers.js b/runtime/js/11_workers.js index 59ae7371da..53b2563347 100644 --- a/runtime/js/11_workers.js +++ b/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; diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 4f21655d68..117200a287 100644 --- a/runtime/js/99_main.js +++ b/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, "unload", null); + defineEventHandler(window, "load"); + 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, "error", null, true); + defineEventHandler(self, "message"); + defineEventHandler(self, "error", undefined, true); runtimeStart( runtimeOptions, diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json index a0f53e9eb6..514731f502 100644 --- a/tools/wpt/expectation.json +++ b/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?wpt_flags=h2": false, - "eventhandlers.any.html?wss": false, + "eventhandlers.any.html": true, + "eventhandlers.any.html?wpt_flags=h2": true, + "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": [ - "Setting onmessage to 1", - "Setting onmessage to 1 (again)" - ], + "onmessage.worker.html": true, "postMessage": { "return-value.worker.html": true }