Browse Source

refactor: take advantage of structured clone algorithm in the remote module (#20427)

Milan Burda 5 years ago
parent
commit
b92163d226

+ 4 - 14
filenames.auto.gni

@@ -138,9 +138,7 @@ auto_filenames = {
     "lib/common/crash-reporter.js",
     "lib/common/define-properties.ts",
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
-    "lib/common/remote/buffer-utils.ts",
-    "lib/common/remote/is-promise.ts",
+    "lib/common/remote/type-utils.ts",
     "lib/common/web-view-methods.ts",
     "lib/renderer/api/crash-reporter.js",
     "lib/renderer/api/desktop-capturer.ts",
@@ -175,7 +173,6 @@ auto_filenames = {
 
   isolated_bundle_deps = [
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
     "lib/isolated_renderer/init.js",
     "lib/renderer/ipc-renderer-internal-utils.ts",
     "lib/renderer/ipc-renderer-internal.ts",
@@ -189,7 +186,6 @@ auto_filenames = {
 
   content_script_bundle_deps = [
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
     "lib/content_script/init.js",
     "lib/renderer/chrome-api.ts",
     "lib/renderer/extensions/event.ts",
@@ -272,11 +268,9 @@ auto_filenames = {
     "lib/common/crash-reporter.js",
     "lib/common/define-properties.ts",
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
     "lib/common/init.ts",
     "lib/common/parse-features-string.js",
-    "lib/common/remote/buffer-utils.ts",
-    "lib/common/remote/is-promise.ts",
+    "lib/common/remote/type-utils.ts",
     "lib/common/reset-search-paths.ts",
     "lib/common/web-view-methods.ts",
     "lib/renderer/ipc-renderer-internal-utils.ts",
@@ -297,10 +291,8 @@ auto_filenames = {
     "lib/common/crash-reporter.js",
     "lib/common/define-properties.ts",
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
     "lib/common/init.ts",
-    "lib/common/remote/buffer-utils.ts",
-    "lib/common/remote/is-promise.ts",
+    "lib/common/remote/type-utils.ts",
     "lib/common/reset-search-paths.ts",
     "lib/common/web-view-methods.ts",
     "lib/renderer/api/crash-reporter.js",
@@ -347,10 +339,8 @@ auto_filenames = {
     "lib/common/crash-reporter.js",
     "lib/common/define-properties.ts",
     "lib/common/electron-binding-setup.ts",
-    "lib/common/error-utils.ts",
     "lib/common/init.ts",
-    "lib/common/remote/buffer-utils.ts",
-    "lib/common/remote/is-promise.ts",
+    "lib/common/remote/type-utils.ts",
     "lib/common/reset-search-paths.ts",
     "lib/renderer/api/crash-reporter.js",
     "lib/renderer/api/desktop-capturer.ts",

+ 0 - 1
lib/browser/api/web-contents.js

@@ -10,7 +10,6 @@ const { app, ipcMain, session, deprecate } = electron
 const NavigationController = require('@electron/internal/browser/navigation-controller')
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal')
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils')
-const errorUtils = require('@electron/internal/common/error-utils')
 
 // session is not used here, the purpose is to make sure session is initalized
 // before the webContents module.

+ 2 - 3
lib/browser/ipc-main-internal-utils.ts

@@ -1,5 +1,4 @@
 import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal'
-import * as errorUtils from '@electron/internal/common/error-utils'
 
 type IPCHandler = (event: Electron.IpcMainInvokeEvent, ...args: any[]) => any
 
@@ -8,7 +7,7 @@ export const handleSync = function <T extends IPCHandler> (channel: string, hand
     try {
       event.returnValue = [null, await handler(event, ...args)]
     } catch (error) {
-      event.returnValue = [errorUtils.serialize(error)]
+      event.returnValue = [error]
     }
   })
 }
@@ -30,7 +29,7 @@ export function invokeInWebContents<T> (sender: Electron.WebContentsInternal, se
       ipcMainInternal.removeListener(channel, handler)
 
       if (error) {
-        reject(errorUtils.deserialize(error))
+        reject(error)
       } else {
         resolve(result)
       }

+ 21 - 49
lib/browser/remote/server.ts

@@ -2,12 +2,10 @@
 
 import * as electron from 'electron'
 import { EventEmitter } from 'events'
-import { isBuffer, bufferToMeta, BufferMeta, metaToBuffer } from '@electron/internal/common/remote/buffer-utils'
 import objectsRegistry from './objects-registry'
 import { ipcMainInternal } from '../ipc-main-internal'
 import * as guestViewManager from '@electron/internal/browser/guest-view-manager'
-import * as errorUtils from '@electron/internal/common/error-utils'
-import { isPromise } from '@electron/internal/common/remote/is-promise'
+import { isPromise, isSerializableObject } from '@electron/internal/common/remote/type-utils'
 
 const v8Util = process.electronBinding('v8_util')
 const eventBinding = process.electronBinding('event')
@@ -105,16 +103,14 @@ type MetaType = {
   value: any,
 } | {
   type: 'buffer',
-  value: BufferMeta,
+  value: Uint8Array,
 } | {
   type: 'array',
   members: MetaType[]
 } | {
   type: 'error',
+  value: Error,
   members: ObjectMember[]
-} | {
-  type: 'date',
-  value: number
 } | {
   type: 'promise',
   then: MetaType
@@ -126,16 +122,14 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
   let type: MetaType['type'] = typeof value
   if (type === 'object') {
     // Recognize certain types of objects.
-    if (value === null) {
-      type = 'value'
-    } else if (isBuffer(value)) {
+    if (value instanceof Buffer) {
       type = 'buffer'
     } else if (Array.isArray(value)) {
       type = 'array'
     } else if (value instanceof Error) {
       type = 'error'
-    } else if (value instanceof Date) {
-      type = 'date'
+    } else if (isSerializableObject(value)) {
+      type = 'value'
     } else if (isPromise(value)) {
       type = 'promise'
     } else if (hasProp.call(value, 'callee') && value.length != null) {
@@ -165,7 +159,7 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
       proto: getObjectPrototype(value)
     }
   } else if (type === 'buffer') {
-    return { type, value: bufferToMeta(value) }
+    return { type, value }
   } else if (type === 'promise') {
     // Add default handler to prevent unhandled rejections in main process
     // Instead they should appear in the renderer process
@@ -178,16 +172,14 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
       })
     }
   } else if (type === 'error') {
-    const members = plainObjectToMeta(value)
-
-    // Error.name is not part of own properties.
-    members.push({
-      name: 'name',
-      value: value.name
-    })
-    return { type, members }
-  } else if (type === 'date') {
-    return { type, value: value.getTime() }
+    return {
+      type,
+      value,
+      members: Object.keys(value).map(name => ({
+        name,
+        value: valueToMeta(sender, contextId, value[name])
+      }))
+    }
   } else {
     return {
       type: 'value',
@@ -196,24 +188,6 @@ const valueToMeta = function (sender: electron.WebContents, contextId: string, v
   }
 }
 
-// Convert object to meta by value.
-const plainObjectToMeta = function (obj: any): ObjectMember[] {
-  return Object.getOwnPropertyNames(obj).map(function (name) {
-    return {
-      name: name,
-      value: obj[name]
-    }
-  })
-}
-
-// Convert Error into meta data.
-const exceptionToMeta = function (error: Error) {
-  return {
-    type: 'exception',
-    value: errorUtils.serialize(error)
-  }
-}
-
 const throwRPCError = function (message: string) {
   const error = new Error(message) as Error & {code: string, errno: number}
   error.code = 'EBADRPC'
@@ -253,10 +227,7 @@ type MetaTypeFromRenderer = {
   value: MetaTypeFromRenderer[]
 } | {
   type: 'buffer',
-  value: BufferMeta
-} | {
-  type: 'date',
-  value: number
+  value: Uint8Array
 } | {
   type: 'promise',
   then: MetaTypeFromRenderer
@@ -285,9 +256,7 @@ const unwrapArgs = function (sender: electron.WebContents, frameId: number, cont
       case 'array':
         return unwrapArgs(sender, frameId, contextId, meta.value)
       case 'buffer':
-        return metaToBuffer(meta.value)
-      case 'date':
-        return new Date(meta.value)
+        return Buffer.from(meta.value.buffer, meta.value.byteOffset, meta.value.byteLength)
       case 'promise':
         return Promise.resolve({
           then: metaToValue(meta.then)
@@ -365,7 +334,10 @@ const handleRemoteCommand = function (channel: string, handler: (event: Electron
     try {
       returnValue = handler(event, contextId, ...args)
     } catch (error) {
-      returnValue = exceptionToMeta(error)
+      returnValue = {
+        type: 'exception',
+        value: valueToMeta(event.sender, contextId, error)
+      }
     }
 
     if (returnValue !== undefined) {

+ 3 - 4
lib/browser/rpc-server.js

@@ -11,7 +11,6 @@ const { crashReporterInit } = require('@electron/internal/browser/crash-reporter
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal')
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils')
 const guestViewManager = require('@electron/internal/browser/guest-view-manager')
-const errorUtils = require('@electron/internal/common/error-utils')
 const clipboardUtils = require('@electron/internal/common/clipboard-utils')
 
 const emitCustomEvent = function (contents, eventName, ...args) {
@@ -91,8 +90,8 @@ const getPreloadScript = async function (preloadPath) {
   let preloadError = null
   try {
     preloadSrc = (await fs.promises.readFile(preloadPath)).toString()
-  } catch (err) {
-    preloadError = errorUtils.serialize(err)
+  } catch (error) {
+    preloadError = error
   }
   return { preloadPath, preloadSrc, preloadError }
 }
@@ -130,5 +129,5 @@ ipcMainUtils.handleSync('ELECTRON_BROWSER_SANDBOX_LOAD', async function (event)
 })
 
 ipcMainInternal.on('ELECTRON_BROWSER_PRELOAD_ERROR', function (event, preloadPath, error) {
-  event.sender.emit('preload-error', event, preloadPath, errorUtils.deserialize(error))
+  event.sender.emit('preload-error', event, preloadPath, error)
 })

+ 0 - 37
lib/common/error-utils.ts

@@ -1,37 +0,0 @@
-const constructors = new Map([
-  [Error.name, Error],
-  [EvalError.name, EvalError],
-  [RangeError.name, RangeError],
-  [ReferenceError.name, ReferenceError],
-  [SyntaxError.name, SyntaxError],
-  [TypeError.name, TypeError],
-  [URIError.name, URIError]
-])
-
-export function deserialize (error: Electron.SerializedError): Electron.ErrorWithCause {
-  if (error && error.__ELECTRON_SERIALIZED_ERROR__ && constructors.has(error.name)) {
-    const constructor = constructors.get(error.name)
-    const deserializedError = new constructor!(error.message) as Electron.ErrorWithCause
-    deserializedError.stack = error.stack
-    deserializedError.from = error.from
-    deserializedError.cause = exports.deserialize(error.cause)
-    return deserializedError
-  }
-  return error
-}
-
-export function serialize (error: Electron.ErrorWithCause): Electron.SerializedError {
-  if (error instanceof Error) {
-    // Errors get lost, because: JSON.stringify(new Error('Message')) === {}
-    // Take the serializable properties and construct a generic object
-    return {
-      message: error.message,
-      stack: error.stack,
-      name: error.name,
-      from: process.type as Electron.ProcessType,
-      cause: exports.serialize(error.cause),
-      __ELECTRON_SERIALIZED_ERROR__: true
-    }
-  }
-  return error
-}

+ 0 - 71
lib/common/remote/buffer-utils.ts

@@ -1,71 +0,0 @@
-import { Buffer } from 'buffer'
-
-const typedArrays: Record<string, Function> = {
-  Buffer,
-  ArrayBuffer,
-  Int8Array,
-  Uint8Array,
-  Uint8ClampedArray,
-  Int16Array,
-  Uint16Array,
-  Int32Array,
-  Uint32Array,
-  Float32Array,
-  Float64Array
-}
-
-type BufferLike = Buffer | ArrayBuffer | ArrayBufferView
-
-function getType (value: BufferLike) {
-  for (const type of Object.keys(typedArrays)) {
-    if (value instanceof typedArrays[type]) {
-      return type
-    }
-  }
-  throw new Error('Invalid buffer')
-}
-
-function getBuffer (value: BufferLike) {
-  if (value instanceof Buffer) {
-    return value
-  } else if (value instanceof ArrayBuffer) {
-    return Buffer.from(value)
-  } else {
-    return Buffer.from(value.buffer, value.byteOffset, value.byteLength)
-  }
-}
-
-export function isBuffer (value: BufferLike) {
-  return ArrayBuffer.isView(value) || value instanceof ArrayBuffer
-}
-
-export interface BufferMeta {
-  type: keyof typeof typedArrays;
-  data: Buffer;
-  length: number | undefined;
-}
-
-export function bufferToMeta (value: BufferLike): BufferMeta {
-  return {
-    type: getType(value),
-    data: getBuffer(value),
-    // NB. We only use length when decoding Int8Array and friends.
-    // For other buffer-like types this is expected to be undefined.
-    length: (value as Buffer).length
-  }
-}
-
-export function metaToBuffer (value: BufferMeta) {
-  const constructor = typedArrays[value.type]
-  const data = getBuffer(value.data)
-
-  if (constructor === Buffer) {
-    return data
-  } else if (constructor === ArrayBuffer) {
-    return data.buffer
-  } else if (constructor) {
-    return new (constructor as any)(data.buffer, data.byteOffset, value.length)
-  } else {
-    return data
-  }
-}

+ 13 - 0
lib/common/remote/is-promise.ts → lib/common/remote/type-utils.ts

@@ -10,3 +10,16 @@ export function isPromise (val: any) {
     val.constructor.resolve instanceof Function
   )
 }
+
+const serializableTypes = [
+  Boolean,
+  Number,
+  String,
+  Date,
+  RegExp,
+  ArrayBuffer
+]
+
+export function isSerializableObject (value: any) {
+  return value === null || ArrayBuffer.isView(value) || serializableTypes.some(type => value instanceof type)
+}

+ 15 - 20
lib/renderer/api/remote.js

@@ -4,9 +4,7 @@ const v8Util = process.electronBinding('v8_util')
 const { hasSwitch } = process.electronBinding('command_line')
 
 const { CallbacksRegistry } = require('@electron/internal/renderer/remote/callbacks-registry')
-const bufferUtils = require('@electron/internal/common/remote/buffer-utils')
-const errorUtils = require('@electron/internal/common/error-utils')
-const { isPromise } = require('@electron/internal/common/remote/is-promise')
+const { isPromise, isSerializableObject } = require('@electron/internal/common/remote/type-utils')
 const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal')
 
 const callbacksRegistry = new CallbacksRegistry()
@@ -43,17 +41,17 @@ function wrapArgs (args, visited = new Set()) {
       }
       visited.delete(value)
       return meta
-    } else if (bufferUtils.isBuffer(value)) {
+    } else if (value instanceof Buffer) {
       return {
         type: 'buffer',
-        value: bufferUtils.bufferToMeta(value)
+        value
       }
-    } else if (value instanceof Date) {
+    } else if (isSerializableObject(value)) {
       return {
-        type: 'date',
-        value: value.getTime()
+        type: 'value',
+        value
       }
-    } else if ((value != null) && typeof value === 'object') {
+    } else if (typeof value === 'object') {
       if (isPromise(value)) {
         return {
           type: 'promise',
@@ -97,7 +95,7 @@ function wrapArgs (args, visited = new Set()) {
     } else {
       return {
         type: 'value',
-        value: value
+        value
       }
     }
   }
@@ -215,11 +213,10 @@ function metaToValue (meta) {
   const types = {
     value: () => meta.value,
     array: () => meta.members.map((member) => metaToValue(member)),
-    buffer: () => bufferUtils.metaToBuffer(meta.value),
+    buffer: () => Buffer.from(meta.value.buffer, meta.value.byteOffset, meta.value.byteLength),
     promise: () => Promise.resolve({ then: metaToValue(meta.then) }),
-    error: () => metaToPlainObject(meta),
-    date: () => new Date(meta.value),
-    exception: () => { throw errorUtils.deserialize(meta.value) }
+    error: () => metaToError(meta),
+    exception: () => { throw metaToError(meta.value) }
   }
 
   if (meta.type in types) {
@@ -261,12 +258,10 @@ function metaToValue (meta) {
   }
 }
 
-// Construct a plain object from the meta.
-function metaToPlainObject (meta) {
-  const obj = (() => meta.type === 'error' ? new Error() : {})()
-  for (let i = 0; i < meta.members.length; i++) {
-    const { name, value } = meta.members[i]
-    obj[name] = value
+function metaToError (meta) {
+  const obj = meta.value
+  for (const { name, value } of meta.members) {
+    obj[name] = metaToValue(value)
   }
   return obj
 }

+ 1 - 3
lib/renderer/init.ts

@@ -196,8 +196,6 @@ if (nodeIntegration) {
   }
 }
 
-const errorUtils = require('@electron/internal/common/error-utils')
-
 // Load the preload scripts.
 for (const preloadScript of preloadScripts) {
   try {
@@ -206,7 +204,7 @@ for (const preloadScript of preloadScripts) {
     console.error(`Unable to load preload script: ${preloadScript}`)
     console.error(error)
 
-    ipcRendererInternal.send('ELECTRON_BROWSER_PRELOAD_ERROR', preloadScript, errorUtils.serialize(error))
+    ipcRendererInternal.send('ELECTRON_BROWSER_PRELOAD_ERROR', preloadScript, error)
   }
 }
 

+ 2 - 3
lib/renderer/ipc-renderer-internal-utils.ts

@@ -1,5 +1,4 @@
 import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal'
-import * as errorUtils from '@electron/internal/common/error-utils'
 
 type IPCHandler = (event: Electron.IpcRendererEvent, ...args: any[]) => any
 
@@ -9,7 +8,7 @@ export const handle = function <T extends IPCHandler> (channel: string, handler:
     try {
       event.sender.send(replyChannel, null, await handler(event, ...args))
     } catch (error) {
-      event.sender.send(replyChannel, errorUtils.serialize(error))
+      event.sender.send(replyChannel, error)
     }
   })
 }
@@ -18,7 +17,7 @@ export function invokeSync<T> (command: string, ...args: any[]): T {
   const [ error, result ] = ipcRendererInternal.sendSync(command, ...args)
 
   if (error) {
-    throw errorUtils.deserialize(error)
+    throw error
   } else {
     return result
   }

+ 2 - 4
lib/sandboxed_renderer/init.js

@@ -140,8 +140,6 @@ if (process.isMainFrame) {
   webViewInit(contextIsolation, isWebViewTagEnabled, guestInstanceId)
 }
 
-const errorUtils = require('@electron/internal/common/error-utils')
-
 // Wrap the script into a function executed in global scope. It won't have
 // access to the current scope, so we'll expose a few objects as arguments:
 //
@@ -166,13 +164,13 @@ for (const { preloadPath, preloadSrc, preloadError } of preloadScripts) {
     if (preloadSrc) {
       runPreloadScript(preloadSrc)
     } else if (preloadError) {
-      throw errorUtils.deserialize(preloadError)
+      throw preloadError
     }
   } catch (error) {
     console.error(`Unable to load preload script: ${preloadPath}`)
     console.error(error)
 
-    ipcRendererInternal.send('ELECTRON_BROWSER_PRELOAD_ERROR', preloadPath, errorUtils.serialize(error))
+    ipcRendererInternal.send('ELECTRON_BROWSER_PRELOAD_ERROR', preloadPath, error)
   }
 }
 

+ 24 - 1
spec/api-remote-spec.js

@@ -256,12 +256,36 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => {
       expect(printName.print(buf)).to.equal('Buffer')
     })
 
+    it('supports instanceof Boolean', () => {
+      const obj = Boolean(true)
+      expect(printName.print(obj)).to.equal('Boolean')
+      expect(printName.echo(obj)).to.deep.equal(obj)
+    })
+
+    it('supports instanceof Number', () => {
+      const obj = Number(42)
+      expect(printName.print(obj)).to.equal('Number')
+      expect(printName.echo(obj)).to.deep.equal(obj)
+    })
+
+    it('supports instanceof String', () => {
+      const obj = String('Hello World!')
+      expect(printName.print(obj)).to.equal('String')
+      expect(printName.echo(obj)).to.deep.equal(obj)
+    })
+
     it('supports instanceof Date', () => {
       const now = new Date()
       expect(printName.print(now)).to.equal('Date')
       expect(printName.echo(now)).to.deep.equal(now)
     })
 
+    it('supports instanceof RegExp', () => {
+      const regexp = RegExp('.*')
+      expect(printName.print(regexp)).to.equal('RegExp')
+      expect(printName.echo(regexp)).to.deep.equal(regexp)
+    })
+
     it('supports instanceof Buffer', () => {
       const buffer = Buffer.from('test')
       expect(buffer.equals(printName.echo(buffer))).to.be.true()
@@ -484,7 +508,6 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => {
       try {
         throwFunction(err)
       } catch (error) {
-        expect(error.from).to.equal('browser')
         expect(error.cause).to.deep.equal(...resolveGetters(err))
       }
     })