feat: promisify executeJavaScript (#17312)

atom/renderer/api/atom_api_web_frame.cc

@@ -14,6 +14,7 @@
 #include "atom/common/native_mate_converters/gfx_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "atom/common/node_includes.h"
+#include "atom/common/promise_util.h"
 #include "atom/renderer/api/atom_api_spell_check_client.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "content/public/renderer/render_frame.h"
@@ -92,23 +93,20 @@ class RenderFrameStatus : public content::RenderFrameObserver {
 
 class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
  public:
-  using CompletionCallback =
-      base::Callback<void(const v8::Local<v8::Value>& result)>;
-
-  explicit ScriptExecutionCallback(const CompletionCallback& callback)
-      : callback_(callback) {}
+  explicit ScriptExecutionCallback(atom::util::Promise promise)
+      : promise_(std::move(promise)) {}
   ~ScriptExecutionCallback() override {}
 
   void Completed(
       const blink::WebVector<v8::Local<v8::Value>>& result) override {
-    if (!callback_.is_null() && !result.empty() && !result[0].IsEmpty())
+    if (!result.empty() && !result[0].IsEmpty())
       // Right now only single results per frame is supported.
-      callback_.Run(result[0]);
+      promise_.Resolve(result[0]);
     delete this;
   }
 
  private:
-  CompletionCallback callback_;
+  atom::util::Promise promise_;
 
   DISALLOW_COPY_AND_ASSIGN(ScriptExecutionCallback);
 };
@@ -322,25 +320,32 @@ void InsertCSS(v8::Local<v8::Value> window, const std::string& css) {
   }
 }
 
-void ExecuteJavaScript(mate::Arguments* args,
-                       v8::Local<v8::Value> window,
-                       const base::string16& code) {
+v8::Local<v8::Promise> ExecuteJavaScript(mate::Arguments* args,
+                                         v8::Local<v8::Value> window,
+                                         const base::string16& code) {
+  v8::Isolate* isolate = args->isolate();
+  util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
   bool has_user_gesture = false;
   args->GetNext(&has_user_gesture);
-  ScriptExecutionCallback::CompletionCallback completion_callback;
-  args->GetNext(&completion_callback);
-  std::unique_ptr<blink::WebScriptExecutionCallback> callback(
-      new ScriptExecutionCallback(completion_callback));
+
   GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptAndReturnValue(
       blink::WebScriptSource(blink::WebString::FromUTF16(code)),
-      has_user_gesture, callback.release());
+      has_user_gesture, new ScriptExecutionCallback(std::move(promise)));
+
+  return handle;
 }
 
-void ExecuteJavaScriptInIsolatedWorld(
+v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
     mate::Arguments* args,
     v8::Local<v8::Value> window,
     int world_id,
     const std::vector<mate::Dictionary>& scripts) {
+  v8::Isolate* isolate = args->isolate();
+  util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
   std::vector<blink::WebScriptSource> sources;
 
   for (const auto& script : scripts) {
@@ -351,8 +356,8 @@ void ExecuteJavaScriptInIsolatedWorld(
     script.Get("startLine", &start_line);
 
     if (!script.Get("code", &code)) {
-      args->ThrowError("Invalid 'code'");
-      return;
+      promise.RejectWithErrorMessage("Invalid 'code'");
+      return handle;
     }
 
     sources.emplace_back(
@@ -367,14 +372,11 @@ void ExecuteJavaScriptInIsolatedWorld(
       blink::WebLocalFrame::kSynchronous;
   args->GetNext(&scriptExecutionType);
 
-  ScriptExecutionCallback::CompletionCallback completion_callback;
-  args->GetNext(&completion_callback);
-  std::unique_ptr<blink::WebScriptExecutionCallback> callback(
-      new ScriptExecutionCallback(completion_callback));
-
   GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
       world_id, &sources.front(), sources.size(), has_user_gesture,
-      scriptExecutionType, callback.release());
+      scriptExecutionType, new ScriptExecutionCallback(std::move(promise)));
+
+  return handle;
 }
 
 void SetIsolatedWorldInfo(v8::Local<v8::Value> window,

docs/api/promisification.md

@@ -11,16 +11,13 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate)
 - [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox)
 - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog)
-- [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript)
 - [contents.print([options], [callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#print)
-- [webFrame.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScript)
-- [webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScriptInIsolatedWorld)
-- [webviewTag.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#executeJavaScript)
 
 ### Converted Functions
 
 - [app.getFileIcon(path[, options], callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#getFileIcon)
 - [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage)
+- [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript)
 - [contents.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#printToPDF)
 - [contents.savePage(fullPath, saveType, callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#savePage)
 - [contentTracing.getCategories(callback)](https://github.com/electron/electron/blob/master/docs/api/content-tracing.md#getCategories)
@@ -48,6 +45,9 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [ses.clearCache(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#clearCache)
 - [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData)
 - [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal)
+- [webFrame.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScript)
+- [webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScriptInIsolatedWorld)
 - [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage)
+- [webviewTag.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#executeJavaScript)
 - [webviewTag.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#printToPDF)
 - [win.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/browser-window.md#capturePage)

docs/api/web-contents.md

@@ -973,9 +973,28 @@ In the browser window some HTML APIs like `requestFullScreen` can only be
 invoked by a gesture from the user. Setting `userGesture` to `true` will remove
 this limitation.
 
-If the result of the executed code is a promise the callback result will be the
-resolved value of the promise. We recommend that you use the returned Promise
-to handle code that results in a Promise.
+```js
+contents.executeJavaScript('fetch("https://jsonplaceholder.typicode.com/users/1").then(resp => resp.json())', true)
+  .then((result) => {
+    console.log(result) // Will be the JSON object from the fetch call
+  })
+```
+
+**[Deprecated Soon](promisification.md)**
+
+#### `contents.executeJavaScript(code[, userGesture])`
+
+* `code` String
+* `userGesture` Boolean (optional) - Default is `false`.
+
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
+Evaluates `code` in page.
+
+In the browser window some HTML APIs like `requestFullScreen` can only be
+invoked by a gesture from the user. Setting `userGesture` to `true` will remove
+this limitation.
 
 ```js
 contents.executeJavaScript('fetch("https://jsonplaceholder.typicode.com/users/1").then(resp => resp.json())', true)

docs/api/web-frame.md

@@ -123,6 +123,22 @@ In the browser window some HTML APIs like `requestFullScreen` can only be
 invoked by a gesture from the user. Setting `userGesture` to `true` will remove
 this limitation.
 
+**[Deprecated Soon](promisification.md)**
+
+### `webFrame.executeJavaScript(code[, userGesture])`
+
+* `code` String
+* `userGesture` Boolean (optional) - Default is `false`.
+
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
+Evaluates `code` in page.
+
+In the browser window some HTML APIs like `requestFullScreen` can only be
+invoked by a gesture from the user. Setting `userGesture` to `true` will remove
+this limitation.
+
 ### `webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])`
 
 * `worldId` Integer - The ID of the world to run the javascript in, `0` is the default world, `999` is the world used by Electrons `contextIsolation` feature. Chrome extensions reserve the range of IDs in `[1 << 20, 1 << 29)`. You can provide any integer here.
@@ -131,7 +147,23 @@ this limitation.
 * `callback` Function (optional) - Called after script has been executed.
   * `result` Any
 
-Work like `executeJavaScript` but evaluates `scripts` in an isolated context.
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
+Works like `executeJavaScript` but evaluates `scripts` in an isolated context.
+
+**[Deprecated Soon](promisification.md)**
+
+### `webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture])`
+
+* `worldId` Integer - The ID of the world to run the javascript in, `0` is the default world, `999` is the world used by Electrons `contextIsolation` feature.  You can provide any integer here.
+* `scripts` [WebSource[]](structures/web-source.md)
+* `userGesture` Boolean (optional) - Default is `false`.
+
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
+Works like `executeJavaScript` but evaluates `scripts` in an isolated context.
 
 ### `webFrame.setIsolatedWorldContentSecurityPolicy(worldId, csp)` _(Deprecated)_
 

docs/api/webview-tag.md

@@ -374,6 +374,23 @@ Injects CSS into the guest page.
 * `callback` Function (optional) - Called after script has been executed.
   * `result` Any
 
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
+Evaluates `code` in page. If `userGesture` is set, it will create the user
+gesture context in the page. HTML APIs like `requestFullScreen`, which require
+user action, can take advantage of this option for automation.
+
+**[Deprecated Soon](promisification.md)**
+
+### `<webview>.executeJavaScript(code[, userGesture])`
+
+* `code` String
+* `userGesture` Boolean (optional) - Default `false`.
+
+Returns `Promise<any>` - A promise that resolves with the result of the executed code
+or is rejected if the result of the code is a rejected promise.
+
 Evaluates `code` in page. If `userGesture` is set, it will create the user
 gesture context in the page. HTML APIs like `requestFullScreen`, which require
 user action, can take advantage of this option for automation.

lib/browser/api/web-contents.js

@@ -178,47 +178,25 @@ const webFrameMethods = [
   'setVisualZoomLevelLimits'
 ]
 
-const asyncWebFrameMethods = function (requestId, method, callback, ...args) {
-  return new Promise((resolve, reject) => {
-    ipcMainInternal.once(`ELECTRON_INTERNAL_BROWSER_ASYNC_WEB_FRAME_RESPONSE_${requestId}`, function (event, error, result) {
-      if (error == null) {
-        if (typeof callback === 'function') callback(result)
-        resolve(result)
-      } else {
-        reject(errorUtils.deserialize(error))
-      }
-    })
-    this._sendInternal('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', requestId, method, args)
-  })
-}
-
 for (const method of webFrameMethods) {
   WebContents.prototype[method] = function (...args) {
     ipcMainUtils.invokeInWebContents(this, 'ELECTRON_INTERNAL_RENDERER_WEB_FRAME_METHOD', method, ...args)
   }
 }
 
+const executeJavaScript = (contents, code, hasUserGesture) => {
+  return ipcMainUtils.invokeInWebContents(contents, 'ELECTRON_INTERNAL_RENDERER_WEB_FRAME_METHOD', 'executeJavaScript', code, hasUserGesture)
+}
+
 // Make sure WebContents::executeJavaScript would run the code only when the
 // WebContents has been loaded.
-WebContents.prototype.executeJavaScript = function (code, hasUserGesture, callback) {
-  const requestId = getNextId()
-
-  if (typeof hasUserGesture === 'function') {
-    // Shift.
-    callback = hasUserGesture
-    hasUserGesture = null
-  }
-
-  if (hasUserGesture == null) {
-    hasUserGesture = false
-  }
-
+WebContents.prototype.executeJavaScript = function (code, hasUserGesture) {
   if (this.getURL() && !this.isLoadingMainFrame()) {
-    return asyncWebFrameMethods.call(this, requestId, 'executeJavaScript', callback, code, hasUserGesture)
+    return executeJavaScript(this, code, hasUserGesture)
   } else {
     return new Promise((resolve, reject) => {
       this.once('did-stop-loading', () => {
-        asyncWebFrameMethods.call(this, requestId, 'executeJavaScript', callback, code, hasUserGesture).then(resolve).catch(reject)
+        executeJavaScript(this, code, hasUserGesture).then(resolve, reject)
       })
     })
   }
@@ -345,6 +323,7 @@ WebContents.prototype.loadFile = function (filePath, options = {}) {
 }
 
 WebContents.prototype.capturePage = deprecate.promisify(WebContents.prototype.capturePage)
+WebContents.prototype.executeJavaScript = deprecate.promisify(WebContents.prototype.executeJavaScript)
 WebContents.prototype.printToPDF = deprecate.promisify(WebContents.prototype.printToPDF)
 WebContents.prototype.savePage = deprecate.promisify(WebContents.prototype.savePage)
 

lib/common/api/deprecate.ts

@@ -94,10 +94,12 @@ const deprecate: ElectronInternal.DeprecationUtil = {
           process.nextTick(() => {
             cb!.length === 2 ? cb!(null, res) : cb!(res)
           })
+          return res
         }, (err: Error) => {
           process.nextTick(() => {
             cb!.length === 2 ? cb!(err) : cb!()
           })
+          throw err
         })
     } as T
   },

lib/renderer/api/web-frame.ts

@@ -85,6 +85,15 @@ function getWebFrame (context: Window) {
   return context ? new WebFrame(context) : null
 }
 
+const promisifiedMethods = new Set<string>([
+  'executeJavaScript',
+  'executeJavaScriptInIsolatedWorld'
+])
+
+for (const method of promisifiedMethods) {
+  (WebFrame as any).prototype[method] = deprecate.promisify((WebFrame as any).prototype[method])
+}
+
 const _webFrame = new WebFrame(window)
 
 export default _webFrame

lib/renderer/security-warnings.ts

@@ -64,16 +64,14 @@ const getIsRemoteProtocol = function () {
  * @returns {boolean} Is a CSP with `unsafe-eval` set?
  */
 const isUnsafeEvalEnabled = function () {
-  return new Promise((resolve) => {
-    webFrame.executeJavaScript(`(${(() => {
-      try {
-        new Function('') // eslint-disable-line no-new,no-new-func
-      } catch {
-        return false
-      }
-      return true
-    }).toString()})()`, false, resolve)
-  })
+  return webFrame.executeJavaScript(`(${(() => {
+    try {
+      new Function('') // eslint-disable-line no-new,no-new-func
+    } catch {
+      return false
+    }
+    return true
+  }).toString()})()`, false)
 }
 
 const moreInformation = `\nFor more information and help, consult

lib/renderer/web-frame-init.ts

@@ -1,7 +1,5 @@
 import { webFrame, WebFrame } from 'electron'
-import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal'
 import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils'
-import * as errorUtils from '@electron/internal/common/error-utils'
 
 // All keys of WebFrame that extend Function
 type WebFrameMethod = {
@@ -19,21 +17,4 @@ export const webFrameInit = () => {
     // will be caught by "keyof WebFrameMethod" though.
     return (webFrame[method] as any)(...args)
   })
-
-  ipcRendererInternal.on('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', (
-    event, requestId: number, method: keyof WebFrameMethod, args: any[]
-  ) => {
-    new Promise(resolve =>
-      // The TypeScript compiler cannot handle the sheer number of
-      // call signatures here and simply gives up. Incorrect invocations
-      // will be caught by "keyof WebFrameMethod" though.
-      (webFrame[method] as any)(...args, resolve)
-    ).then(result => {
-      return [null, result]
-    }, error => {
-      return [errorUtils.serialize(error)]
-    }).then(responseArgs => {
-      event.sender.send(`ELECTRON_INTERNAL_BROWSER_ASYNC_WEB_FRAME_RESPONSE_${requestId}`, ...responseArgs)
-    })
-  })
 }

spec/api-web-contents-spec.js

@@ -1075,7 +1075,21 @@ describe('webContents module', () => {
   })
 
   describe('webframe messages in sandboxed contents', () => {
-    it('responds to executeJavaScript', (done) => {
+    it('responds to executeJavaScript', async () => {
+      w.destroy()
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          sandbox: true
+        }
+      })
+      await w.loadURL('about:blank')
+      const result = await w.webContents.executeJavaScript('37 + 5')
+      assert.strictEqual(result, 42)
+    })
+
+    // TODO(miniak): remove when promisification is complete
+    it('responds to executeJavaScript (callback)', (done) => {
       w.destroy()
       w = new BrowserWindow({
         show: false,

spec/ts-smoke/electron/main.ts

@@ -97,6 +97,7 @@ app.on('ready', () => {
   mainWindow.webContents.printToPDF({}, (err, data) => console.log(err))
 
   mainWindow.webContents.executeJavaScript('return true;').then((v: boolean) => console.log(v))
+  mainWindow.webContents.executeJavaScript('return true;', true).then((v: boolean) => console.log(v))
   mainWindow.webContents.executeJavaScript('return true;', true)
   mainWindow.webContents.executeJavaScript('return true;', true, (result: boolean) => console.log(result))
   mainWindow.webContents.insertText('blah, blah, blah')

spec/ts-smoke/electron/renderer.ts

@@ -72,9 +72,10 @@ webFrame.setSpellCheckProvider('en-US', {
 
 webFrame.insertText('text')
 
-webFrame.executeJavaScript('JSON.stringify({})', false, (result) => {
-  console.log(result)
-}).then((result: string) => console.log('OK:' + result))
+webFrame.executeJavaScript('return true;').then((v: boolean) => console.log(v))
+webFrame.executeJavaScript('return true;', true).then((v: boolean) => console.log(v))
+webFrame.executeJavaScript('return true;', true)
+webFrame.executeJavaScript('return true;', true, (result: boolean) => console.log(result))
 
 console.log(webFrame.getResourceUsage())
 webFrame.clearCache()

spec/webview-spec.js

@@ -789,11 +789,7 @@ describe('<webview> tag', function () {
       const devtools = webview2.getWebContents()
       assert.ok(devtools.getURL().startsWith('chrome-devtools://devtools'))
 
-      const name = await new Promise((resolve) => {
-        devtools.executeJavaScript('InspectorFrontendHost.constructor.name', (name) => {
-          resolve(name)
-        })
-      })
+      const name = await devtools.executeJavaScript('InspectorFrontendHost.constructor.name')
       document.body.removeChild(webview2)
 
       expect(name).to.be.equal('InspectorFrontendHostImpl')
@@ -1001,6 +997,19 @@ describe('<webview> tag', function () {
       const jsScript = "'4'+2"
       const expectedResult = '42'
 
+      const result = await webview.executeJavaScript(jsScript)
+      assert.strictEqual(result, expectedResult)
+    })
+
+    // TODO(miniak): remove when promisification is complete
+    it('can return the result of the executed script (callback)', async () => {
+      await loadWebView(webview, {
+        src: 'about:blank'
+      })
+
+      const jsScript = "'4'+2"
+      const expectedResult = '42'
+
       const result = await new Promise((resolve) => {
         webview.executeJavaScript(jsScript, false, (result) => {
           resolve(result)