fix contextIsolation issue while webPreference sandbox is on

contextIsolation didn't work while sandbox is on. The fix is contextIsolation picked up while sandbox on

atom/renderer/atom_renderer_client.cc

@@ -41,8 +41,6 @@ AtomRendererClient::AtomRendererClient()
     : node_integration_initialized_(false),
       node_bindings_(NodeBindings::Create(NodeBindings::RENDERER)),
       atom_bindings_(new AtomBindings(uv_default_loop())) {
-  isolated_world_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kContextIsolation);
 }
 
 AtomRendererClient::~AtomRendererClient() {
@@ -171,14 +169,6 @@ void AtomRendererClient::WillDestroyWorkerContextOnWorkerThread(
   }
 }
 
-v8::Local<v8::Context> AtomRendererClient::GetContext(
-    blink::WebFrame* frame, v8::Isolate* isolate) {
-  if (isolated_world())
-    return frame->WorldScriptContext(isolate, World::ISOLATED_WORLD);
-  else
-    return frame->MainWorldScriptContext();
-}
-
 void AtomRendererClient::SetupMainWorldOverrides(
     v8::Handle<v8::Context> context) {
   // Setup window overrides in the main world context

atom/renderer/atom_renderer_client.h

@@ -20,10 +20,6 @@ class AtomRendererClient : public RendererClientBase {
   AtomRendererClient();
   virtual ~AtomRendererClient();
 
-  // Get the context that the Electron API is running in.
-  v8::Local<v8::Context> GetContext(
-      blink::WebFrame* frame, v8::Isolate* isolate);
-
   // atom::RendererClientBase:
   void DidCreateScriptContext(
       v8::Handle<v8::Context> context,
@@ -32,7 +28,6 @@ class AtomRendererClient : public RendererClientBase {
       v8::Handle<v8::Context> context,
       content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override;
-  bool isolated_world() override { return isolated_world_; }
 
  private:
   enum NodeIntegration {
@@ -64,7 +59,6 @@ class AtomRendererClient : public RendererClientBase {
 
   std::unique_ptr<NodeBindings> node_bindings_;
   std::unique_ptr<AtomBindings> atom_bindings_;
-  bool isolated_world_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomRendererClient);
 };

atom/renderer/atom_sandboxed_renderer_client.h

@@ -27,7 +27,6 @@ class AtomSandboxedRendererClient : public RendererClientBase {
       v8::Handle<v8::Context> context,
       content::RenderFrame* render_frame) override;
   void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override { }
-  bool isolated_world() override { return false; }
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;

atom/renderer/renderer_client_base.cc

@@ -69,6 +69,8 @@ RendererClientBase::RendererClientBase() {
       ParseSchemesCLISwitch(switches::kStandardSchemes);
   for (const std::string& scheme : standard_schemes_list)
     url::AddStandardScheme(scheme.c_str(), url::SCHEME_WITHOUT_PORT);
+    isolated_world_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kContextIsolation);
 }
 
 RendererClientBase::~RendererClientBase() {
@@ -197,4 +199,12 @@ void RendererClientBase::AddSupportedKeySystems(
   AddChromeKeySystems(key_systems);
 }
 
+v8::Local<v8::Context> RendererClientBase::GetContext(
+	blink::WebFrame* frame, v8::Isolate* isolate) {
+	if (isolated_world())
+		return frame->worldScriptContext(isolate, World::ISOLATED_WORLD);
+	else
+		return frame->mainWorldScriptContext();
+}
+
 }  // namespace atom

atom/renderer/renderer_client_base.h

@@ -25,7 +25,12 @@ class RendererClientBase : public content::ContentRendererClient {
       v8::Handle<v8::Context> context, content::RenderFrame* render_frame) = 0;
   virtual void DidClearWindowObject(content::RenderFrame* render_frame);
   virtual void SetupMainWorldOverrides(v8::Handle<v8::Context> context) = 0;
-  virtual bool isolated_world() = 0;
+
+  bool isolated_world() { return isolated_world_; }
+
+  // Get the context that the Electron API is running in.
+  v8::Local<v8::Context> GetContext(
+	  blink::WebFrame* frame, v8::Isolate* isolate);
 
  protected:
   void AddRenderBindings(v8::Isolate* isolate,
@@ -51,6 +56,7 @@ class RendererClientBase : public content::ContentRendererClient {
 
  private:
   std::unique_ptr<PreferencesManager> preferences_manager_;
+  bool isolated_world_;
 };
 
 }  // namespace atom

spec/api-browser-window-spec.js

@@ -17,6 +17,7 @@ const nativeModulesEnabled = remote.getGlobal('nativeModulesEnabled')
 describe('BrowserWindow module', function () {
   var fixtures = path.resolve(__dirname, 'fixtures')
   var w = null
+  var ws = null
   var server, postData
 
   before(function (done) {
@@ -2643,7 +2644,7 @@ describe('BrowserWindow module', function () {
     })
   })
 
-  describe('contextIsolation option', () => {
+  describe('contextIsolation option with and without sandbox option', () => {
     const expectedContextData = {
       preloadContext: {
         preloadProperty: 'number',
@@ -2674,6 +2675,19 @@ describe('BrowserWindow module', function () {
           preload: path.join(fixtures, 'api', 'isolated-preload.js')
         }
       })
+      if (ws != null) ws.destroy()
+      ws = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          sandbox: true,
+          contextIsolation: true,
+          preload: path.join(fixtures, 'api', 'isolated-preload.js')
+        }
+      })
+    })
+
+    afterEach(() => {
+      if (ws != null) ws.destroy()
     })
 
     it('separates the page context from the Electron/preload context', (done) => {
@@ -2702,6 +2716,25 @@ describe('BrowserWindow module', function () {
       })
       w.loadURL('file://' + fixtures + '/pages/window-open.html')
     })
+
+    it('separates the page context from the Electron/preload context with sandbox on', (done) => {
+      ipcMain.once('isolated-sandbox-world', (event, data) => {
+        assert.deepEqual(data, expectedContextData)
+        done()
+      })
+      w.loadURL('file://' + fixtures + '/api/isolated.html')
+    })
+
+    it('recreates the contexts on reload with sandbox on', (done) => {
+      w.webContents.once('did-finish-load', () => {
+        ipcMain.once('isolated-sandbox-world', (event, data) => {
+          assert.deepEqual(data, expectedContextData)
+          done()
+        })
+        w.webContents.reload()
+      })
+      w.loadURL('file://' + fixtures + '/api/isolated.html')
+    })
   })
 
   describe('offscreen rendering', function () {

spec/fixtures/api/isolated-preload.js

@@ -1,14 +1,11 @@
 // Ensure fetch works from isolated world origin
 fetch('http://localhost:1234')
 fetch('https://localhost:1234')
-fetch(`file://${__filename}`)
 
 const {ipcRenderer, webFrame} = require('electron')
 
 window.foo = 3
 
-webFrame.executeJavaScript('window.preloadExecuteJavaScriptProperty = 1234;')
-
 window.addEventListener('message', (event) => {
   ipcRenderer.send('isolated-world', {
     preloadContext: {
@@ -21,4 +18,15 @@ window.addEventListener('message', (event) => {
     },
     pageContext: event.data
   })
+  ipcRenderer.send('isolated-sandbox-world', {
+    preloadContext: {
+      preloadProperty: typeof window.foo,
+      pageProperty: typeof window.hello,
+      typeofRequire: typeof require,
+      typeofProcess: typeof process,
+      typeofArrayPush: typeof Array.prototype.push,
+      typeofFunctionApply: typeof Function.prototype.apply
+    },
+    pageContext: event.data
+  })
 })

spec/fixtures/api/isolated.html

@@ -7,6 +7,7 @@
       window.hello = 'world'
       Array.prototype.push = 3
       Function.prototype.apply = true
+      window.preloadExecuteJavaScriptProperty = 1234;
 
       const opened = window.open()
       opened.close()