From c8c6f0c29a7c5ff1fc1e5cde1789286111331f17 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 17 Feb 2022 23:12:56 +0800 Subject: [PATCH 1/3] lib: update class fields TODO in abort_controller.js https://bugs.chromium.org/p/v8/issues/detail?id=10704 is already fixed, but since AbortController currently throws ERR_INVALID_THIS we'll revert to class fields in a subsequent patch. For now just update the comments. --- lib/internal/abort_controller.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index e0c0d63899972c..7e89e43cc7cead 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -291,9 +291,8 @@ function abortSignal(signal, reason) { signal.dispatchEvent(event); } -// TODO(joyeecheung): V8 snapshot does not support instance member -// initializers for now: -// https://bugs.chromium.org/p/v8/issues/detail?id=10704 +// TODO(joyeecheung): use private fields and we'll get invalid access +// validation from V8 instead of throwing ERR_INVALID_THIS ourselves. const kSignal = Symbol('signal'); function validateAbortController(obj) { From 3e7e7ed6e4a5390ccc467e9233f71dad690c2c35 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 17 Feb 2022 23:15:01 +0800 Subject: [PATCH 2/3] lib: use class fields in Event and EventTarget https://bugs.chromium.org/p/v8/issues/detail?id=10704 is already fixed, so switch back to class fields instead of using symbol properties. --- lib/internal/event_target.js | 48 +++++++++++++++++------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 7c539db2eb3c2a..0f9a896396f79c 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -63,16 +63,7 @@ const kTrustEvent = Symbol('kTrustEvent'); const { now } = require('internal/perf/utils'); -// TODO(joyeecheung): V8 snapshot does not support instance member -// initializers for now: -// https://bugs.chromium.org/p/v8/issues/detail?id=10704 const kType = Symbol('type'); -const kDefaultPrevented = Symbol('defaultPrevented'); -const kCancelable = Symbol('cancelable'); -const kTimestamp = Symbol('timestamp'); -const kBubbles = Symbol('bubbles'); -const kComposed = Symbol('composed'); -const kPropagationStopped = Symbol('propagationStopped'); const isTrustedSet = new SafeWeakSet(); const isTrusted = ObjectGetOwnPropertyDescriptor({ @@ -86,6 +77,13 @@ function isEvent(value) { } class Event { + #cancelable = false; + #bubbles = false; + #composed = false; + #defaultPrevented = false; + #timestamp = now(); + #propagationStopped = false; + /** * @param {string} type * @param {{ @@ -101,13 +99,11 @@ class Event { allowArray: true, allowFunction: true, nullable: true, }); const { cancelable, bubbles, composed } = { ...options }; - this[kCancelable] = !!cancelable; - this[kBubbles] = !!bubbles; - this[kComposed] = !!composed; + this.#cancelable = !!cancelable; + this.#bubbles = !!bubbles; + this.#composed = !!composed; + this[kType] = `${type}`; - this[kDefaultPrevented] = false; - this[kTimestamp] = now(); - this[kPropagationStopped] = false; if (options?.[kTrustEvent]) { isTrustedSet.add(this); } @@ -135,9 +131,9 @@ class Event { return `${name} ${inspect({ type: this[kType], - defaultPrevented: this[kDefaultPrevented], - cancelable: this[kCancelable], - timeStamp: this[kTimestamp], + defaultPrevented: this.#defaultPrevented, + cancelable: this.#cancelable, + timeStamp: this.#timestamp, }, opts)}`; } @@ -150,7 +146,7 @@ class Event { preventDefault() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - this[kDefaultPrevented] = true; + this.#defaultPrevented = true; } /** @@ -195,7 +191,7 @@ class Event { get cancelable() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kCancelable]; + return this.#cancelable; } /** @@ -204,7 +200,7 @@ class Event { get defaultPrevented() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kCancelable] && this[kDefaultPrevented]; + return this.#cancelable && this.#defaultPrevented; } /** @@ -213,7 +209,7 @@ class Event { get timeStamp() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kTimestamp]; + return this.#timestamp; } @@ -244,7 +240,7 @@ class Event { get bubbles() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kBubbles]; + return this.#bubbles; } /** @@ -253,7 +249,7 @@ class Event { get composed() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kComposed]; + return this.#composed; } /** @@ -271,7 +267,7 @@ class Event { get cancelBubble() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - return this[kPropagationStopped]; + return this.#propagationStopped; } /** @@ -288,7 +284,7 @@ class Event { stopPropagation() { if (!isEvent(this)) throw new ERR_INVALID_THIS('Event'); - this[kPropagationStopped] = true; + this.#propagationStopped = true; } static NONE = 0; From ccbb05fe61de1bf944af4dd3e3778de55be2aee6 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 17 Feb 2022 23:15:40 +0800 Subject: [PATCH 3/3] lib: use class fields in observe.js https://bugs.chromium.org/p/v8/issues/detail?id=10704 is already fixed, so switch back to class fields instead of using symbol properties. --- lib/internal/perf/observe.js | 75 +++++++++++++++++------------------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/lib/internal/perf/observe.js b/lib/internal/perf/observe.js index 2f9506ed28e3eb..ca9e68a96258ab 100644 --- a/lib/internal/perf/observe.js +++ b/lib/internal/perf/observe.js @@ -61,13 +61,9 @@ const { const { inspect } = require('util'); -const kBuffer = Symbol('kBuffer'); -const kCallback = Symbol('kCallback'); const kDispatch = Symbol('kDispatch'); -const kEntryTypes = Symbol('kEntryTypes'); const kMaybeBuffer = Symbol('kMaybeBuffer'); const kDeprecatedFields = Symbol('kDeprecatedFields'); -const kType = Symbol('kType'); const kDeprecationMessage = 'Custom PerformanceEntry accessors are deprecated. ' + @@ -151,20 +147,22 @@ function maybeIncrementObserverCount(type) { } class PerformanceObserverEntryList { + #buffer = []; + constructor(entries) { - this[kBuffer] = ArrayPrototypeSort(entries, (first, second) => { + this.#buffer = ArrayPrototypeSort(entries, (first, second) => { return first.startTime - second.startTime; }); } getEntries() { - return ArrayPrototypeSlice(this[kBuffer]); + return ArrayPrototypeSlice(this.#buffer); } getEntriesByType(type) { type = `${type}`; return ArrayPrototypeFilter( - this[kBuffer], + this.#buffer, (entry) => entry.entryType === type); } @@ -172,11 +170,11 @@ class PerformanceObserverEntryList { name = `${name}`; if (type != null /** not nullish */) { return ArrayPrototypeFilter( - this[kBuffer], + this.#buffer, (entry) => entry.name === name && entry.entryType === type); } return ArrayPrototypeFilter( - this[kBuffer], + this.#buffer, (entry) => entry.name === name); } @@ -188,20 +186,19 @@ class PerformanceObserverEntryList { depth: options.depth == null ? null : options.depth - 1 }; - return `PerformanceObserverEntryList ${inspect(this[kBuffer], opts)}`; + return `PerformanceObserverEntryList ${inspect(this.#buffer, opts)}`; } } class PerformanceObserver { + #buffer = []; + #entryTypes = new SafeSet(); + #type; + #callback; + constructor(callback) { - // TODO(joyeecheung): V8 snapshot does not support instance member - // initializers for now: - // https://bugs.chromium.org/p/v8/issues/detail?id=10704 - this[kBuffer] = []; - this[kEntryTypes] = new SafeSet(); - this[kType] = undefined; validateFunction(callback, 'callback'); - this[kCallback] = callback; + this.#callback = callback; } observe(options = {}) { @@ -219,10 +216,10 @@ class PerformanceObserver { 'options.entryTypes can not set with ' + 'options.type together'); - switch (this[kType]) { + switch (this.#type) { case undefined: - if (entryTypes !== undefined) this[kType] = kTypeMultiple; - if (type !== undefined) this[kType] = kTypeSingle; + if (entryTypes !== undefined) this.#type = kTypeMultiple; + if (type !== undefined) this.#type = kTypeSingle; break; case kTypeSingle: if (entryTypes !== undefined) @@ -238,53 +235,53 @@ class PerformanceObserver { break; } - if (this[kType] === kTypeMultiple) { + if (this.#type === kTypeMultiple) { if (!ArrayIsArray(entryTypes)) { throw new ERR_INVALID_ARG_TYPE( 'options.entryTypes', 'string[]', entryTypes); } - maybeDecrementObserverCounts(this[kEntryTypes]); - this[kEntryTypes].clear(); + maybeDecrementObserverCounts(this.#entryTypes); + this.#entryTypes.clear(); for (let n = 0; n < entryTypes.length; n++) { if (ArrayPrototypeIncludes(kSupportedEntryTypes, entryTypes[n])) { - this[kEntryTypes].add(entryTypes[n]); + this.#entryTypes.add(entryTypes[n]); maybeIncrementObserverCount(entryTypes[n]); } } } else { if (!ArrayPrototypeIncludes(kSupportedEntryTypes, type)) return; - this[kEntryTypes].add(type); + this.#entryTypes.add(type); maybeIncrementObserverCount(type); if (buffered) { const entries = filterBufferMapByNameAndType(undefined, type); - ArrayPrototypePushApply(this[kBuffer], entries); + ArrayPrototypePushApply(this.#buffer, entries); kPending.add(this); if (kPending.size) queuePending(); } } - if (this[kEntryTypes].size) + if (this.#entryTypes.size) kObservers.add(this); else this.disconnect(); } disconnect() { - maybeDecrementObserverCounts(this[kEntryTypes]); + maybeDecrementObserverCounts(this.#entryTypes); kObservers.delete(this); kPending.delete(this); - this[kBuffer] = []; - this[kEntryTypes].clear(); - this[kType] = undefined; + this.#buffer = []; + this.#entryTypes.clear(); + this.#type = undefined; } takeRecords() { - const list = this[kBuffer]; - this[kBuffer] = []; + const list = this.#buffer; + this.#buffer = []; return list; } @@ -293,17 +290,17 @@ class PerformanceObserver { } [kMaybeBuffer](entry) { - if (!this[kEntryTypes].has(entry.entryType)) + if (!this.#entryTypes.has(entry.entryType)) return; - ArrayPrototypePush(this[kBuffer], entry); + ArrayPrototypePush(this.#buffer, entry); kPending.add(this); if (kPending.size) queuePending(); } [kDispatch]() { - this[kCallback](new PerformanceObserverEntryList(this.takeRecords()), - this); + this.#callback(new PerformanceObserverEntryList(this.takeRecords()), + this); } [kInspect](depth, options) { @@ -317,8 +314,8 @@ class PerformanceObserver { return `PerformanceObserver ${inspect({ connected: kObservers.has(this), pending: kPending.has(this), - entryTypes: ArrayFrom(this[kEntryTypes]), - buffer: this[kBuffer], + entryTypes: ArrayFrom(this.#entryTypes), + buffer: this.#buffer, }, opts)}`; } }