From c8d77cae4ac828552736d9c9f2c4c6dabca2197d Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Tue, 24 Nov 2020 22:11:39 +0100 Subject: [PATCH] refactor: replace V8 hidden values with WeakMap / WeakSet (#26659) --- lib/browser/remote/objects-registry.ts | 10 +++++----- lib/browser/remote/server.ts | 6 ++++-- lib/renderer/api/remote.ts | 15 +++++++++------ lib/renderer/remote/callbacks-registry.ts | 17 +++++++++++------ 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/browser/remote/objects-registry.ts b/lib/browser/remote/objects-registry.ts index 5bf2d976fe..32a05018ae 100644 --- a/lib/browser/remote/objects-registry.ts +++ b/lib/browser/remote/objects-registry.ts @@ -1,11 +1,11 @@ import { WebContents } from 'electron/main'; -const v8Util = process._linkedBinding('electron_common_v8_util'); - const getOwnerKey = (webContents: WebContents, contextId: string) => { return `${webContents.id}-${contextId}`; }; +const electronIds = new WeakMap(); + class ObjectsRegistry { private nextId: number = 0 @@ -81,14 +81,14 @@ class ObjectsRegistry { // Private: Saves the object into storage and assigns an ID for it. saveToStorage (object: any) { - let id: number = v8Util.getHiddenValue(object, 'electronId'); + let id = electronIds.get(object); if (!id) { id = ++this.nextId; this.storage[id] = { count: 0, object: object }; - v8Util.setHiddenValue(object, 'electronId', id); + electronIds.set(object, id); } return id; } @@ -101,7 +101,7 @@ class ObjectsRegistry { } pointer.count -= 1; if (pointer.count === 0) { - v8Util.deleteHiddenValue(pointer.object, 'electronId'); + electronIds.delete(pointer.object); delete this.storage[id]; } } diff --git a/lib/browser/remote/server.ts b/lib/browser/remote/server.ts index dc505d1e38..71cbc0fb25 100644 --- a/lib/browser/remote/server.ts +++ b/lib/browser/remote/server.ts @@ -56,6 +56,8 @@ function setCachedRendererFunction (id: RendererFunctionId, wc: electron.WebCont return value; } +const locationInfo = new WeakMap(); + // Return the description of object's members: const getObjectMembers = function (object: any): ObjectMember[] { let names = Object.getOwnPropertyNames(object); @@ -186,7 +188,7 @@ const throwRPCError = function (message: string) { }; const removeRemoteListenersAndLogWarning = (sender: any, callIntoRenderer: (...args: any[]) => void) => { - const location = v8Util.getHiddenValue(callIntoRenderer, 'location'); + const location = locationInfo.get(callIntoRenderer); let message = 'Attempting to call a function in a renderer window that has been closed or released.' + `\nFunction provided here: ${location}`; @@ -269,7 +271,7 @@ const unwrapArgs = function (sender: electron.WebContents, frameId: number, cont removeRemoteListenersAndLogWarning(this, callIntoRenderer); } }; - v8Util.setHiddenValue(callIntoRenderer, 'location', meta.location); + locationInfo.set(callIntoRenderer, meta.location); Object.defineProperty(callIntoRenderer, 'length', { value: meta.length }); setCachedRendererFunction(objectId, sender, frameId, callIntoRenderer); diff --git a/lib/renderer/api/remote.ts b/lib/renderer/api/remote.ts index a2231aed96..c62ef81391 100644 --- a/lib/renderer/api/remote.ts +++ b/lib/renderer/api/remote.ts @@ -23,6 +23,9 @@ const finalizationRegistry = new (window as any).FinalizationRegistry((id: numbe } }); +const electronIds = new WeakMap(); +const isReturnValue = new WeakSet(); + function getCachedRemoteObject (id: number) { const ref = remoteObjectCache.get(id); if (ref !== undefined) { @@ -90,10 +93,10 @@ function wrapArgs (args: any[], visited = new Set()): any { value.then(onFulfilled, onRejected); }) }; - } else if (v8Util.getHiddenValue(value, 'electronId')) { + } else if (electronIds.has(value)) { return { type: 'remote-object', - id: v8Util.getHiddenValue(value, 'electronId') + id: electronIds.get(value) }; } @@ -111,7 +114,7 @@ function wrapArgs (args: any[], visited = new Set()): any { } visited.delete(value); return meta; - } else if (typeof value === 'function' && v8Util.getHiddenValue(value, 'returnValue')) { + } else if (typeof value === 'function' && isReturnValue.has(value)) { return { type: 'function-with-return-value', value: valueToMeta(value()) @@ -120,7 +123,7 @@ function wrapArgs (args: any[], visited = new Set()): any { return { type: 'function', id: callbacksRegistry.add(value), - location: v8Util.getHiddenValue(value, 'location'), + location: callbacksRegistry.getLocation(value), length: value.length }; } else { @@ -287,7 +290,7 @@ function metaToValue (meta: MetaType): any { } // Track delegate obj's lifetime & tell browser to clean up when object is GCed. - v8Util.setHiddenValue(ret, 'electronId', meta.id); + electronIds.set(ret, meta.id); setCachedRemoteObject(meta.id, ret); return ret; } @@ -373,7 +376,7 @@ Object.defineProperty(exports, 'process', { // Create a function that will return the specified value when called in browser. export function createFunctionWithReturnValue (returnValue: T): () => T { const func = () => returnValue; - v8Util.setHiddenValue(func, 'returnValue', true); + isReturnValue.add(func); return func; } diff --git a/lib/renderer/remote/callbacks-registry.ts b/lib/renderer/remote/callbacks-registry.ts index 07b030f21b..c36d84c7a3 100644 --- a/lib/renderer/remote/callbacks-registry.ts +++ b/lib/renderer/remote/callbacks-registry.ts @@ -1,4 +1,5 @@ -const v8Util = process._linkedBinding('electron_common_v8_util'); +const callbackIds = new WeakMap(); +const locationInfo = new WeakMap(); export class CallbacksRegistry { private nextId: number = 0 @@ -6,7 +7,7 @@ export class CallbacksRegistry { add (callback: Function) { // The callback is already added. - let id = v8Util.getHiddenValue(callback, 'callbackId'); + let id = callbackIds.get(callback); if (id != null) return id; id = this.nextId += 1; @@ -17,7 +18,7 @@ export class CallbacksRegistry { const stackString = (new Error()).stack; if (!stackString) return; - let filenameAndLine; + let filenameAndLine: string; let match; while ((match = regexp.exec(stackString)) !== null) { @@ -32,8 +33,8 @@ export class CallbacksRegistry { } this.callbacks.set(id, callback); - v8Util.setHiddenValue(callback, 'callbackId', id); - v8Util.setHiddenValue(callback, 'location', filenameAndLine); + callbackIds.set(callback, id); + locationInfo.set(callback, filenameAndLine!); return id; } @@ -41,6 +42,10 @@ export class CallbacksRegistry { return this.callbacks.get(id) || function () {}; } + getLocation (callback: Function) { + return locationInfo.get(callback); + } + apply (id: number, ...args: any[]) { return this.get(id).apply(global, ...args); } @@ -48,7 +53,7 @@ export class CallbacksRegistry { remove (id: number) { const callback = this.callbacks.get(id); if (callback) { - v8Util.deleteHiddenValue(callback, 'callbackId'); + callbackIds.delete(callback); this.callbacks.delete(id); } }