chore: apply PR feedback

electron · Jun 24, 2020 · 3e2502a · 3e2502a
1 parent 5bc26a2
commit 3e2502a
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 6 deletions.
diff --git a/lib/renderer/api/web-frame.ts b/lib/renderer/api/web-frame.ts
@@ -48,7 +48,7 @@ class WebFrame extends EventEmitter {
   }
 }
 
-const { hasSwitch } = process.electronBinding('command_line');
+const { hasSwitch } = process._linkedBinding('electron_common_command_line');
 const worldSafeJS = hasSwitch('world-safe-execute-javascript') && hasSwitch('context-isolation');
 
 // Populate the methods.

diff --git a/shell/renderer/api/electron_api_web_frame.cc b/shell/renderer/api/electron_api_web_frame.cc
@@ -134,7 +134,12 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
     v8::Isolate* isolate = v8::Isolate::GetCurrent();
     if (!result.empty()) {
       if (!result[0].IsEmpty()) {
-        if (!world_safe_result_) {
+        // Either world safe results are disabled or the result was created in
+        // the same world as the caller
+        if (!world_safe_result_ ||
+            (result[0]->IsObject() &&
+             promise_.GetContext() ==
+                 result[0].As<v8::Object>()->CreationContext())) {
           // Right now only single results per frame is supported.
           if (!callback_.is_null())
             std::move(callback_).Run(result[0], v8::Undefined(isolate));
@@ -143,16 +148,31 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
           // Serializable objects
           blink::CloneableMessage ret;
           bool success;
+          std::string error_message;
           {
             v8::TryCatch try_catch(isolate);
             success = gin::ConvertFromV8(isolate, result[0], &ret);
+            if (try_catch.HasCaught()) {
+              auto message = try_catch.Message();
+
+              if (message.IsEmpty() ||
+                  !gin::ConvertFromV8(isolate, message->Get(),
+                                      &error_message)) {
+                error_message =
+                    "An unknown exception occurred while getting the result of "
+                    "the script";
+              }
+            }
           }
           if (!success) {
             // Failed convert so we send undefined everywhere
             if (!callback_.is_null())
-              std::move(callback_).Run(v8::Undefined(isolate),
-                                       v8::Undefined(isolate));
-            promise_.Resolve(v8::Undefined(isolate));
+              std::move(callback_).Run(
+                  v8::Undefined(isolate),
+                  v8::Exception::Error(
+                      v8::String::NewFromUtf8(isolate, error_message.c_str())
+                          .ToLocalChecked()));
+            promise_.RejectWithErrorMessage(error_message);
           } else {
             v8::Local<v8::Context> context = promise_.GetContext();
             v8::Context::Scope context_scope(context);

diff --git a/spec-main/api-web-frame-spec.ts b/spec-main/api-web-frame-spec.ts
@@ -21,12 +21,28 @@ describe('webFrame module', () => {
         }
       });
       const isSafe = emittedOnce(ipcMain, 'executejs-safe');
-      w.loadFile(path.join(fixtures, 'pages', 'blank.html'));
+      w.loadURL('about:blank');
       const [, wasSafe] = await isSafe;
       expect(wasSafe).to.equal(worldSafe);
     });
   }
 
+  it('can use executeJavaScript with world safe mode enabled and catch conversion errors', async () => {
+    const w = new BrowserWindow({
+      show: true,
+      webPreferences: {
+        nodeIntegration: true,
+        contextIsolation: true,
+        worldSafeExecuteJavaScript: true,
+        preload: path.join(fixtures, 'pages', 'world-safe-preload-error.js')
+      }
+    });
+    const execError = emittedOnce(ipcMain, 'executejs-safe');
+    w.loadURL('about:blank');
+    const [, error] = await execError;
+    expect(error).to.have.property('message', 'Uncaught Error: An object could not be cloned.');
+  });
+
   it('calls a spellcheck provider', async () => {
     const w = new BrowserWindow({
       show: false,

diff --git a/spec/fixtures/pages/world-safe-preload-error.js b/spec/fixtures/pages/world-safe-preload-error.js
@@ -0,0 +1,8 @@
+const { ipcRenderer, webFrame } = require('electron');
+
+webFrame.executeJavaScript(`(() => {
+  return Symbol('a');
+})()`).catch((err) => {
+  // Considered safe if the object is constructed in this world
+  ipcRenderer.send('executejs-safe', err);
+});