From d48f9bcf7fd912c5a139a6331162c6a276a0d66a Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 1 Oct 2018 03:07:50 +0200 Subject: [PATCH] refactor: implement methods via dedicated IPCs without the remote module (#14377) --- lib/browser/rpc-server.js | 34 +++++++--- lib/renderer/web-view/web-view-attributes.js | 14 +++- lib/renderer/web-view/web-view.js | 71 ++++++++++---------- spec/webview-spec.js | 2 - 4 files changed, 73 insertions(+), 48 deletions(-) diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 3bff8e471e4c..c8e99b8c8f75 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -373,19 +373,37 @@ handleRemoteCommand('ELECTRON_BROWSER_GUEST_WEB_CONTENTS', function (event, cont return valueToMeta(event.sender, contextId, guestViewManager.getGuest(guestInstanceId)) }) -ipcMain.on('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', function (event, contextId, requestId, guestInstanceId, method, ...args) { +ipcMain.on('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', function (event, requestId, guestInstanceId, method, args, hasCallback) { + new Promise(resolve => { + let guestViewManager = require('./guest-view-manager') + let guest = guestViewManager.getGuest(guestInstanceId) + if (guest.hostWebContents !== event.sender) { + throw new Error('Access denied') + } + if (hasCallback) { + guest[method](...args, resolve) + } else { + resolve(guest[method](...args)) + } + }).then(result => { + return [null, result] + }, error => { + return [errorUtils.serialize(error)] + }).then(responseArgs => { + event.sender.send(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, ...responseArgs) + }) +}) + +ipcMain.on('ELECTRON_BROWSER_SYNC_CALL_TO_GUEST_VIEW', function (event, guestInstanceId, method, args) { try { let guestViewManager = require('@electron/internal/browser/guest-view-manager') let guest = guestViewManager.getGuest(guestInstanceId) - if (requestId) { - const responseCallback = function (result) { - event.sender.send(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, result) - } - args.push(responseCallback) + if (guest.hostWebContents !== event.sender) { + throw new Error('Access denied') } - guest[method].apply(guest, args) + event.returnValue = [null, guest[method].apply(guest, args)] } catch (error) { - event.returnValue = exceptionToMeta(event.sender, contextId, error) + event.returnValue = [errorUtils.serialize(error)] } }) diff --git a/lib/renderer/web-view/web-view-attributes.js b/lib/renderer/web-view/web-view-attributes.js index d5d6f1bb8219..6c51e77bf020 100644 --- a/lib/renderer/web-view/web-view-attributes.js +++ b/lib/renderer/web-view/web-view-attributes.js @@ -1,8 +1,9 @@ 'use strict' +const { ipcRenderer } = require('electron') const WebViewImpl = require('@electron/internal/renderer/web-view/web-view') const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants') -const { remote } = require('electron') +const errorUtils = require('@electron/internal/common/error-utils') // Helper function to resolve url set in attribute. const a = document.createElement('a') @@ -180,8 +181,15 @@ class SrcAttribute extends WebViewAttribute { if (useragent) { opts.userAgent = useragent } - const guestContents = remote.getGuestWebContents(this.webViewImpl.guestInstanceId) - guestContents.loadURL(this.getValue(), opts) + + const guestInstanceId = this.webViewImpl.guestInstanceId + const method = 'loadURL' + const args = [this.getValue(), opts] + + const [error] = ipcRenderer.sendSync('ELECTRON_BROWSER_SYNC_CALL_TO_GUEST_VIEW', guestInstanceId, method, args) + if (error) { + throw errorUtils.deserialize(error) + } } } diff --git a/lib/renderer/web-view/web-view.js b/lib/renderer/web-view/web-view.js index 7e3bbfbf4457..56bdb21b5f63 100644 --- a/lib/renderer/web-view/web-view.js +++ b/lib/renderer/web-view/web-view.js @@ -5,9 +5,7 @@ const { ipcRenderer, remote, webFrame } = require('electron') const v8Util = process.atomBinding('v8_util') const guestViewInternal = require('@electron/internal/renderer/web-view/guest-view-internal') const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants') - -// An unique ID that can represent current context. -const contextId = v8Util.getHiddenValue(global, 'contextId') +const errorUtils = require('@electron/internal/common/error-utils') // ID generator. let nextId = 0 @@ -66,7 +64,6 @@ class WebViewImpl { this.guestInstanceId = void 0 } - this.webContents = null this.beforeFirstNavigation = true this.attributes[webViewConstants.ATTRIBUTE_PARTITION].validPartitionId = true @@ -188,7 +185,6 @@ class WebViewImpl { } this.internalInstanceId = getNextId() this.guestInstanceId = guestInstanceId - this.webContents = remote.getGuestWebContents(this.guestInstanceId) guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams(), this.internalElement.contentWindow) // ResizeObserver is a browser global not recognized by "standard". /* globals ResizeObserver */ @@ -277,14 +273,9 @@ const registerWebViewElement = function () { 'stopFindInPage', 'downloadURL', 'inspectServiceWorker', - 'print', - 'printToPDF', 'showDefinitionForSelection', - 'capturePage', 'setZoomFactor', - 'setZoomLevel', - 'getZoomLevel', - 'getZoomFactor' + 'setZoomLevel' ] const nonblockMethods = [ 'insertCSS', @@ -292,17 +283,32 @@ const registerWebViewElement = function () { 'send', 'sendInputEvent', 'setLayoutZoomLevelLimits', - 'setVisualZoomLevelLimits' + 'setVisualZoomLevelLimits', + // with callback + 'capturePage', + 'executeJavaScript', + 'getZoomFactor', + 'getZoomLevel', + 'print', + 'printToPDF' ] + const getGuestInstanceId = function (self) { + const internal = v8Util.getHiddenValue(self, 'internal') + if (!internal.guestInstanceId) { + throw new Error('The WebView must be attached to the DOM and the dom-ready event emitted before this method can be called.') + } + return internal.guestInstanceId + } + // Forward proto.foo* method calls to WebViewImpl.foo*. - const createBlockHandler = function (m) { + const createBlockHandler = function (method) { return function (...args) { - const internal = v8Util.getHiddenValue(this, 'internal') - if (internal.webContents) { - return internal.webContents[m](...args) + const [error, result] = ipcRenderer.sendSync('ELECTRON_BROWSER_SYNC_CALL_TO_GUEST_VIEW', getGuestInstanceId(this), method, args) + if (error) { + throw errorUtils.deserialize(error) } else { - throw new Error(`Cannot call ${m} because the webContents is unavailable. The WebView must be attached to the DOM and the dom-ready event emitted before this method can be called.`) + return result } } } @@ -310,36 +316,31 @@ const registerWebViewElement = function () { proto[method] = createBlockHandler(method) } - const createNonBlockHandler = function (m) { + const createNonBlockHandler = function (method) { return function (...args) { - const internal = v8Util.getHiddenValue(this, 'internal') - ipcRenderer.send('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', contextId, null, internal.guestInstanceId, m, ...args) + const callback = (typeof args[args.length - 1] === 'function') ? args.pop() : null + const requestId = getNextId() + ipcRenderer.once(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, function (event, error, result) { + if (error == null) { + if (callback) callback(result) + } else { + throw errorUtils.deserialize(error) + } + }) + ipcRenderer.send('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', requestId, getGuestInstanceId(this), method, args, callback != null) } } for (const method of nonblockMethods) { proto[method] = createNonBlockHandler(method) } - proto.executeJavaScript = function (code, hasUserGesture, callback) { - const internal = v8Util.getHiddenValue(this, 'internal') - if (typeof hasUserGesture === 'function') { - callback = hasUserGesture - hasUserGesture = false - } - const requestId = getNextId() - ipcRenderer.send('ELECTRON_BROWSER_ASYNC_CALL_TO_GUEST_VIEW', contextId, requestId, internal.guestInstanceId, 'executeJavaScript', code, hasUserGesture) - ipcRenderer.once(`ELECTRON_RENDERER_ASYNC_CALL_TO_GUEST_VIEW_RESPONSE_${requestId}`, function (event, result) { - if (callback) callback(result) - }) - } - // WebContents associated with this webview. proto.getWebContents = function () { const internal = v8Util.getHiddenValue(this, 'internal') - if (!internal.webContents) { + if (!internal.guestInstanceId) { internal.createGuestSync() } - return internal.webContents + return remote.getGuestWebContents(internal.guestInstanceId) } // Focusing the webview should move page focus to the underlying iframe. diff --git a/spec/webview-spec.js b/spec/webview-spec.js index ecfae8813e67..173c3de742b9 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -890,7 +890,6 @@ describe(' tag', function () { it('throws a custom error when an API method is called before the event is emitted', () => { const expectedErrorMessage = - 'Cannot call stop because the webContents is unavailable. ' + 'The WebView must be attached to the DOM ' + 'and the dom-ready event emitted before this method can be called.' expect(() => { webview.stop() }).to.throw(expectedErrorMessage) @@ -1179,7 +1178,6 @@ describe(' tag', function () { loadWebView(webview) setTimeout(() => { const expectedErrorMessage = - 'Cannot call stop because the webContents is unavailable. ' + 'The WebView must be attached to the DOM ' + 'and the dom-ready event emitted before this method can be called.' expect(() => { webview.stop() }).to.throw(expectedErrorMessage)