fix: <webview> not working with contextIsolation + sandbox (electron#16469)

miniak · Cheng Zhao · commit b965e54efc1d · 2019-01-22T10:08:16.000+09:00
diff --git a/atom/renderer/atom_sandboxed_renderer_client.cc b/atom/renderer/atom_sandboxed_renderer_client.cc
@@ -202,6 +202,30 @@ void AtomSandboxedRendererClient::DidCreateScriptContext(
       &preload_bundle_params, &preload_bundle_args, nullptr);
 }
 
+void AtomSandboxedRendererClient::SetupMainWorldOverrides(
+    v8::Handle<v8::Context> context,
+    content::RenderFrame* render_frame) {
+  // Setup window overrides in the main world context
+  // Wrap the bundle into a function that receives the isolatedWorld as
+  // an argument.
+  auto* isolate = context->GetIsolate();
+
+  mate::Dictionary process = mate::Dictionary::CreateEmpty(isolate);
+  process.SetMethod("binding", GetBinding);
+
+  std::vector<v8::Local<v8::String>> isolated_bundle_params = {
+      node::FIXED_ONE_BYTE_STRING(isolate, "nodeProcess"),
+      node::FIXED_ONE_BYTE_STRING(isolate, "isolatedWorld")};
+
+  std::vector<v8::Local<v8::Value>> isolated_bundle_args = {
+      process.GetHandle(),
+      GetContext(render_frame->GetWebFrame(), isolate)->Global()};
+
+  node::per_process::native_module_loader.CompileAndCall(
+      context, "electron/js2c/isolated_bundle", &isolated_bundle_params,
+      &isolated_bundle_args, nullptr);
+}
+
 void AtomSandboxedRendererClient::WillReleaseScriptContext(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
diff --git a/atom/renderer/atom_sandboxed_renderer_client.h b/atom/renderer/atom_sandboxed_renderer_client.h
@@ -29,7 +29,7 @@ class AtomSandboxedRendererClient : public RendererClientBase {
   void WillReleaseScriptContext(v8::Handle<v8::Context> context,
                                 content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context,
-                               content::RenderFrame* render_frame) override {}
+                               content::RenderFrame* render_frame) override;
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;
diff --git a/filenames.gni b/filenames.gni
@@ -74,6 +74,7 @@ filenames = {
     "lib/renderer/web-view/web-view-constants.js",
     "lib/renderer/web-view/web-view-element.js",
     "lib/renderer/web-view/web-view-impl.js",
+    "lib/renderer/web-view/web-view-init.js",
     "lib/renderer/api/exports/electron.js",
     "lib/renderer/api/crash-reporter.js",
     "lib/renderer/api/ipc-renderer.js",
diff --git a/lib/isolated_renderer/init.js b/lib/isolated_renderer/init.js
@@ -2,16 +2,21 @@
 
 /* global nodeProcess, isolatedWorld */
 
-// Note: Don't use "process", as it will be replaced by browserify's one.
-const v8Util = nodeProcess.atomBinding('v8_util')
+const atomBinding = require('@electron/internal/common/atom-binding-setup')(nodeProcess.binding, 'renderer')
 
-const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')
-const { webViewImpl, ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
+const v8Util = atomBinding('v8_util')
+
+const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl')
 
 if (webViewImpl) {
   // Must setup the WebView element in main world.
   const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
   setupWebView(v8Util, webViewImpl)
 }
 
-require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
+const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args')
+
+if (isolatedWorldArgs) {
+  const { ipcRenderer, guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen } = isolatedWorldArgs
+  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
+}
diff --git a/lib/renderer/init.js b/lib/renderer/init.js
@@ -58,33 +58,29 @@ if (preloadScript) {
   preloadScripts.push(preloadScript)
 }
 
-if (window.location.protocol === 'chrome-devtools:') {
-  // Override some inspector APIs.
-  require('@electron/internal/renderer/inspector')
-} else if (window.location.protocol === 'chrome-extension:') {
-  // Add implementations of chrome API.
-  require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
-} else if (window.location.protocol === 'chrome:') {
-  // Disable node integration for chrome UI scheme.
-} else {
-  // Override default web functions.
-  require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
-
-  // Inject content scripts.
-  require('@electron/internal/renderer/content-scripts-injector')
+switch (window.location.protocol) {
+  case 'chrome-devtools:': {
+    // Override some inspector APIs.
+    require('@electron/internal/renderer/inspector')
+    break
+  }
+  case 'chrome-extension:': {
+    // Inject the chrome.* APIs that chrome extensions require
+    require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
+    break
+  }
+  default: {
+    // Override default web functions.
+    require('@electron/internal/renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen)
 
-  // Load webview tag implementation.
-  if (webviewTag && guestInstanceId == null) {
-    const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
-    if (contextIsolation) {
-      Object.assign(isolatedWorldArgs, { webViewImpl })
-    } else {
-      const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
-      setupWebView(v8Util, webViewImpl)
-    }
+    // Inject content scripts.
+    require('@electron/internal/renderer/content-scripts-injector')
   }
 }
 
+// Load webview tag implementation.
+require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, webviewTag, guestInstanceId)
+
 // Pass the arguments to isolatedWorld.
 if (contextIsolation) {
   v8Util.setHiddenValue(global, 'isolated-world-args', isolatedWorldArgs)
@@ -163,15 +159,3 @@ for (const preloadScript of preloadScripts) {
 
 // Warn about security issues
 require('@electron/internal/renderer/security-warnings')(nodeIntegration)
-
-// Report focus/blur events of webview to browser.
-// Note that while Chromium content APIs have observer for focus/blur, they
-// unfortunately do not work for webview.
-if (guestInstanceId) {
-  window.addEventListener('focus', () => {
-    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId)
-  })
-  window.addEventListener('blur', () => {
-    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId)
-  })
-}
diff --git a/lib/renderer/web-view/web-view-init.js b/lib/renderer/web-view/web-view-init.js
@@ -0,0 +1,35 @@
+'use strict'
+
+const ipcRenderer = require('@electron/internal/renderer/ipc-renderer-internal')
+const v8Util = process.atomBinding('v8_util')
+
+function handleFocusBlur (guestInstanceId) {
+  // Note that while Chromium content APIs have observer for focus/blur, they
+  // unfortunately do not work for webview.
+
+  window.addEventListener('focus', () => {
+    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', true, guestInstanceId)
+  })
+
+  window.addEventListener('blur', () => {
+    ipcRenderer.send('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', false, guestInstanceId)
+  })
+}
+
+module.exports = function (contextIsolation, webviewTag, guestInstanceId) {
+  // Load webview tag implementation.
+  if (webviewTag && guestInstanceId == null) {
+    const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
+    if (contextIsolation) {
+      v8Util.setHiddenValue(window, 'web-view-impl', webViewImpl)
+    } else {
+      const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
+      setupWebView(v8Util, webViewImpl)
+    }
+  }
+
+  if (guestInstanceId) {
+    // Report focus/blur events of webview to browser.
+    handleFocusBlur(guestInstanceId)
+  }
+}
diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js
@@ -105,8 +105,12 @@ function preloadRequire (module) {
   throw new Error('module not found')
 }
 
+// Process command line arguments.
 const { hasSwitch } = process.atomBinding('command_line')
 
+const isBackgroundPage = hasSwitch('background-page')
+const contextIsolation = hasSwitch('context-isolation')
+
 switch (window.location.protocol) {
   case 'chrome-devtools:': {
     // Override some inspector APIs.
@@ -115,22 +119,15 @@ switch (window.location.protocol) {
   }
   case 'chrome-extension:': {
     // Inject the chrome.* APIs that chrome extensions require
-    const isBackgroundPage = hasSwitch('background-page')
     require('@electron/internal/renderer/chrome-api').injectTo(window.location.hostname, isBackgroundPage, window)
     break
   }
 }
 
-if (binding.guestInstanceId) {
-  process.guestInstanceId = parseInt(binding.guestInstanceId)
-}
+const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanceId)
 
-// Don't allow recursive `<webview>`.
-if (!process.guestInstanceId && isWebViewTagEnabled) {
-  const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl')
-  const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element')
-  setupWebView(v8Util, webViewImpl)
-}
+// Load webview tag implementation.
+require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, isWebViewTagEnabled, guestInstanceId)
 
 const errorUtils = require('@electron/internal/common/error-utils')
 
diff --git a/spec/webview-spec.js b/spec/webview-spec.js
@@ -76,6 +76,19 @@ describe('<webview> tag', function () {
     await emittedOnce(ipcMain, 'pong')
   })
 
+  it('works with sandbox', async () => {
+    const w = await openTheWindow({
+      show: false,
+      webPreferences: {
+        webviewTag: true,
+        nodeIntegration: true,
+        sandbox: true
+      }
+    })
+    w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
+    await emittedOnce(ipcMain, 'pong')
+  })
+
   it('works with contextIsolation', async () => {
     const w = await openTheWindow({
       show: false,
@@ -89,6 +102,20 @@ describe('<webview> tag', function () {
     await emittedOnce(ipcMain, 'pong')
   })
 
+  it('works with contextIsolation + sandbox', async () => {
+    const w = await openTheWindow({
+      show: false,
+      webPreferences: {
+        webviewTag: true,
+        nodeIntegration: true,
+        contextIsolation: true,
+        sandbox: true
+      }
+    })
+    w.loadFile(path.join(fixtures, 'pages', 'webview-isolated.html'))
+    await emittedOnce(ipcMain, 'pong')
+  })
+
   it('is disabled by default', async () => {
     const w = await openTheWindow({
       show: false,
@@ -1349,47 +1376,56 @@ describe('<webview> tag', function () {
       if (div != null) div.remove()
     })
 
-    it('emits resize events', async () => {
-      const firstResizeSignal = waitForEvent(webview, 'resize')
-      const domReadySignal = waitForEvent(webview, 'dom-ready')
+    const generateSpecs = (description, sandbox) => {
+      describe(description, () => {
+        it('emits resize events', async () => {
+          const firstResizeSignal = waitForEvent(webview, 'resize')
+          const domReadySignal = waitForEvent(webview, 'dom-ready')
 
-      webview.src = `file://${fixtures}/pages/a.html`
-      div.appendChild(webview)
-      document.body.appendChild(div)
+          webview.src = `file://${fixtures}/pages/a.html`
+          webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}`
+          div.appendChild(webview)
+          document.body.appendChild(div)
 
-      const firstResizeEvent = await firstResizeSignal
-      expect(firstResizeEvent.target).to.equal(webview)
-      expect(firstResizeEvent.newWidth).to.equal(100)
-      expect(firstResizeEvent.newHeight).to.equal(10)
+          const firstResizeEvent = await firstResizeSignal
+          expect(firstResizeEvent.target).to.equal(webview)
+          expect(firstResizeEvent.newWidth).to.equal(100)
+          expect(firstResizeEvent.newHeight).to.equal(10)
 
-      await domReadySignal
+          await domReadySignal
 
-      const secondResizeSignal = waitForEvent(webview, 'resize')
+          const secondResizeSignal = waitForEvent(webview, 'resize')
 
-      const newWidth = 1234
-      const newHeight = 789
-      div.style.width = `${newWidth}px`
-      div.style.height = `${newHeight}px`
+          const newWidth = 1234
+          const newHeight = 789
+          div.style.width = `${newWidth}px`
+          div.style.height = `${newHeight}px`
 
-      const secondResizeEvent = await secondResizeSignal
-      expect(secondResizeEvent.target).to.equal(webview)
-      expect(secondResizeEvent.newWidth).to.equal(newWidth)
-      expect(secondResizeEvent.newHeight).to.equal(newHeight)
-    })
+          const secondResizeEvent = await secondResizeSignal
+          expect(secondResizeEvent.target).to.equal(webview)
+          expect(secondResizeEvent.newWidth).to.equal(newWidth)
+          expect(secondResizeEvent.newHeight).to.equal(newHeight)
+        })
 
-    it('emits focus event', async () => {
-      const domReadySignal = waitForEvent(webview, 'dom-ready')
-      webview.src = `file://${fixtures}/pages/a.html`
-      document.body.appendChild(webview)
+        it('emits focus event', async () => {
+          const domReadySignal = waitForEvent(webview, 'dom-ready')
+          webview.src = `file://${fixtures}/pages/a.html`
+          webview.webpreferences = `sandbox=${sandbox ? 'yes' : 'no'}`
+          document.body.appendChild(webview)
 
-      await domReadySignal
+          await domReadySignal
 
-      // If this test fails, check if webview.focus() still works.
-      const focusSignal = waitForEvent(webview, 'focus')
-      webview.focus()
+          // If this test fails, check if webview.focus() still works.
+          const focusSignal = waitForEvent(webview, 'focus')
+          webview.focus()
 
-      await focusSignal
-    })
+          await focusSignal
+        })
+      })
+    }
+
+    generateSpecs('without sandbox', false)
+    generateSpecs('with sandbox', true)
   })
 
   describe('zoom behavior', () => {
