refactor: port window.open and window.opener to use ctx bridge instead of hole punching (#23235)

* refactor: port window.open and window.opener to use ctx bridge instead of hole punching

* refactor: only run the isolated init bundle when webview is enabled

filenames.auto.gni

@@ -182,12 +182,8 @@ auto_filenames = {
   isolated_bundle_deps = [
     "lib/common/electron-binding-setup.ts",
     "lib/isolated_renderer/init.js",
-    "lib/renderer/api/context-bridge.ts",
-    "lib/renderer/ipc-renderer-internal-utils.ts",
-    "lib/renderer/ipc-renderer-internal.ts",
     "lib/renderer/web-view/web-view-constants.ts",
     "lib/renderer/web-view/web-view-element.ts",
-    "lib/renderer/window-setup.ts",
     "package.json",
     "tsconfig.electron.json",
     "tsconfig.json",

lib/browser/api/web-contents.js

@@ -548,6 +548,11 @@ WebContents.prototype._init = function () {
 
       internalWindowOpen(event, url, referrer, frameName, disposition, mergedOptions, additionalFeatures, postData);
     });
+
+    const prefs = this.getWebPreferences() || {};
+    if (prefs.webviewTag && prefs.contextIsolation) {
+      electron.deprecate.log('Security Warning: A WebContents was just created with both webviewTag and contextIsolation enabled.  This combination is fundamentally less secure and effectively bypasses the protections of contextIsolation.  We strongly recommend you move away from webviews to OOPIF or BrowserView in order for your app to be more secure');
+    }
   }
 
   this.on('login', (event, ...args) => {

lib/isolated_renderer/init.js

@@ -6,10 +6,6 @@ process.electronBinding = require('@electron/internal/common/electron-binding-se
 
 const v8Util = process.electronBinding('v8_util');
 
-// The `lib/renderer/ipc-renderer-internal.js` module looks for the ipc object in the
-// "ipc-internal" hidden value
-v8Util.setHiddenValue(global, 'ipc-internal', v8Util.getHiddenValue(isolatedWorld, 'ipc-internal'));
-
 const webViewImpl = v8Util.getHiddenValue(isolatedWorld, 'web-view-impl');
 
 if (webViewImpl) {
@@ -17,11 +13,3 @@ if (webViewImpl) {
   const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element');
   setupWebView(v8Util, webViewImpl);
 }
-
-const isolatedWorldArgs = v8Util.getHiddenValue(isolatedWorld, 'isolated-world-args');
-
-if (isolatedWorldArgs) {
-  const { guestInstanceId, isHiddenPage, openerId, usesNativeWindowOpen, rendererProcessReuseEnabled } = isolatedWorldArgs;
-  const { windowSetup } = require('@electron/internal/renderer/window-setup');
-  windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen, rendererProcessReuseEnabled);
-}

lib/renderer/api/context-bridge.ts

@@ -21,8 +21,11 @@ export default contextBridge;
 
 export const internalContextBridge = {
   contextIsolationEnabled,
-  overrideGlobalMethodFromIsolatedWorld: (keys: string[], method: Function) => {
-    return binding._overrideGlobalMethodFromIsolatedWorld(keys, method);
+  overrideGlobalValueFromIsolatedWorld: (keys: string[], value: any) => {
+    return binding._overrideGlobalValueFromIsolatedWorld(keys, value, false);
+  },
+  overrideGlobalValueWithDynamicPropsFromIsolatedWorld: (keys: string[], value: any) => {
+    return binding._overrideGlobalValueFromIsolatedWorld(keys, value, true);
   },
   overrideGlobalPropertyFromIsolatedWorld: (keys: string[], getter: Function, setter?: Function) => {
     return binding._overrideGlobalPropertyFromIsolatedWorld(keys, getter, setter || null);

lib/renderer/window-setup.ts

@@ -2,14 +2,9 @@ import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-in
 import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils';
 import { internalContextBridge } from '@electron/internal/renderer/api/context-bridge';
 
-const inMainWorld = internalContextBridge.isInMainWorld();
 const { contextIsolationEnabled } = internalContextBridge;
 
-// Should we inject APIs into this world, if ctx isolation is enabled then only inject in the isolated world
-// else inject everywhere
-const shouldInjectGivenContextIsolationIsMaybeEnabled = contextIsolationEnabled ? !inMainWorld : true;
-
-// This file implements the following APIs Directly:
+// This file implements the following APIs over the ctx bridge:
 // - window.open()
 // - window.opener.blur()
 // - window.opener.close()
@@ -17,9 +12,8 @@ const shouldInjectGivenContextIsolationIsMaybeEnabled = contextIsolationEnabled
 // - window.opener.focus()
 // - window.opener.location
 // - window.opener.print()
+// - window.opener.closed
 // - window.opener.postMessage()
-
-// And the following APIs over the ctx bridge:
 // - window.history.back()
 // - window.history.forward()
 // - window.history.go()
@@ -40,13 +34,13 @@ const toString = (value: any) => {
 
 const windowProxies = new Map<number, BrowserWindowProxy>();
 
-const getOrCreateProxy = (guestId: number) => {
+const getOrCreateProxy = (guestId: number): SafelyBoundBrowserWindowProxy => {
   let proxy = windowProxies.get(guestId);
   if (proxy == null) {
     proxy = new BrowserWindowProxy(guestId);
     windowProxies.set(guestId, proxy);
   }
-  return proxy;
+  return proxy.getSafe();
 };
 
 const removeProxy = (guestId: number) => {
@@ -74,6 +68,8 @@ class LocationProxy {
    */
   private static ProxyProperty<T> (target: LocationProxy, propertyKey: LocationProperties) {
     Object.defineProperty(target, propertyKey, {
+      enumerable: true,
+      configurable: true,
       get: function (this: LocationProxy): T | string {
         const guestURL = this.getGuestURL();
         const value = guestURL ? guestURL[propertyKey] : '';
@@ -92,6 +88,30 @@ class LocationProxy {
     });
   }
 
+  public getSafe = () => {
+    const that = this;
+    return {
+      get href () { return that.href; },
+      set href (newValue) { that.href = newValue; },
+      get hash () { return that.hash; },
+      set hash (newValue) { that.hash = newValue; },
+      get host () { return that.host; },
+      set host (newValue) { that.host = newValue; },
+      get hostname () { return that.hostname; },
+      set hostname (newValue) { that.hostname = newValue; },
+      get origin () { return that.origin; },
+      set origin (newValue) { that.origin = newValue; },
+      get pathname () { return that.pathname; },
+      set pathname (newValue) { that.pathname = newValue; },
+      get port () { return that.port; },
+      set port (newValue) { that.port = newValue; },
+      get protocol () { return that.protocol; },
+      set protocol (newValue) { that.protocol = newValue; },
+      get search () { return that.search; },
+      set search (newValue) { that.search = newValue; }
+    };
+  }
+
   constructor (guestId: number) {
     // eslint will consider the constructor "useless"
     // unless we assign them in the body. It's fine, that's what
@@ -124,6 +144,17 @@ class LocationProxy {
   }
 }
 
+interface SafelyBoundBrowserWindowProxy {
+  location: WindowProxy['location'];
+  blur: WindowProxy['blur'];
+  close: WindowProxy['close'];
+  eval: typeof eval; // eslint-disable-line no-eval
+  focus: WindowProxy['focus'];
+  print: WindowProxy['print'];
+  postMessage: WindowProxy['postMessage'];
+  closed: boolean;
+}
+
 class BrowserWindowProxy {
   public closed: boolean = false
 
@@ -134,7 +165,7 @@ class BrowserWindowProxy {
   // so for now, we'll have to make do with an "any" in the mix.
   // https://github.com/Microsoft/TypeScript/issues/2521
   public get location (): LocationProxy | any {
-    return this._location;
+    return this._location.getSafe();
   }
 
   public set location (url: string | any) {
@@ -152,27 +183,48 @@ class BrowserWindowProxy {
     });
   }
 
-  public close () {
+  public getSafe = (): SafelyBoundBrowserWindowProxy => {
+    const that = this;
+    return {
+      postMessage: this.postMessage,
+      blur: this.blur,
+      close: this.close,
+      focus: this.focus,
+      print: this.print,
+      eval: this.eval,
+      get location () {
+        return that.location;
+      },
+      set location (url: string | any) {
+        that.location = url;
+      },
+      get closed () {
+        return that.closed;
+      }
+    };
+  }
+
+  public close = () => {
     this._invokeWindowMethod('destroy');
   }
 
-  public focus () {
+  public focus = () => {
     this._invokeWindowMethod('focus');
   }
 
-  public blur () {
+  public blur = () => {
     this._invokeWindowMethod('blur');
   }
 
-  public print () {
+  public print = () => {
     this._invokeWebContentsMethod('print');
   }
 
-  public postMessage (message: any, targetOrigin: string) {
+  public postMessage = (message: any, targetOrigin: string) => {
     ipcRendererInternal.invoke('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_POSTMESSAGE', this.guestId, message, toString(targetOrigin), window.location.origin);
   }
 
-  public eval (code: string) {
+  public eval = (code: string) => {
     this._invokeWebContentsMethod('executeJavaScript', code);
   }
 
@@ -188,12 +240,12 @@ class BrowserWindowProxy {
 export const windowSetup = (
   guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean, rendererProcessReuseEnabled: boolean
 ) => {
-  if (!process.sandboxed && guestInstanceId == null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
+  if (!process.sandboxed && guestInstanceId == null) {
     // Override default window.close.
     window.close = function () {
       ipcRendererInternal.send('ELECTRON_BROWSER_WINDOW_CLOSE');
     };
-    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['close'], window.close);
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['close'], window.close);
   }
 
   if (!usesNativeWindowOpen) {
@@ -210,23 +262,21 @@ export const windowSetup = (
         return null;
       }
     };
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['open'], window.open);
   }
 
   if (openerId != null) {
-    // TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
     window.opener = getOrCreateProxy(openerId);
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['opener'], window.opener);
   }
 
   // But we do not support prompt().
-  if (shouldInjectGivenContextIsolationIsMaybeEnabled) {
-    window.prompt = function () {
-      throw new Error('prompt() is and will not be supported.');
-    };
-    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['prompt'], window.prompt);
-  }
+  window.prompt = function () {
+    throw new Error('prompt() is and will not be supported.');
+  };
+  if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['prompt'], window.prompt);
 
   if (!usesNativeWindowOpen || openerId != null) {
-    // TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
     ipcRendererInternal.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (
       _event, sourceId: number, message: any, sourceOrigin: string
     ) {
@@ -247,21 +297,21 @@ export const windowSetup = (
     });
   }
 
-  if (!process.sandboxed && !rendererProcessReuseEnabled && shouldInjectGivenContextIsolationIsMaybeEnabled) {
+  if (!process.sandboxed && !rendererProcessReuseEnabled) {
     window.history.back = function () {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK');
     };
-    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'back'], window.history.back);
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'back'], window.history.back);
 
     window.history.forward = function () {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD');
     };
-    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'forward'], window.history.forward);
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'forward'], window.history.forward);
 
     window.history.go = function (offset: number) {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset);
     };
-    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'go'], window.history.go);
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'go'], window.history.go);
 
     const getHistoryLength = () => ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH') + 104;
     Object.defineProperty(window.history, 'length', {
@@ -271,7 +321,7 @@ export const windowSetup = (
     if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['history', 'length'], getHistoryLength);
   }
 
-  if (guestInstanceId != null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
+  if (guestInstanceId != null) {
     // Webview `document.visibilityState` tracks window visibility (and ignores
     // the actual <webview> element visibility) for backwards compatibility.
     // See discussion in #9178.

shell/renderer/api/electron_api_context_bridge.cc

@@ -146,6 +146,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     v8::Local<v8::Value> value,
     context_bridge::RenderFrameFunctionStore* store,
     context_bridge::ObjectCache* object_cache,
+    bool support_dynamic_properties,
     int recursion_depth) {
   if (recursion_depth >= kMaxRecursion) {
     v8::Context::Scope source_scope(source_context);
@@ -179,7 +180,8 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     {
       v8::Local<v8::Value> proxy_func = gin_helper::CallbackToV8Leaked(
           destination_context->GetIsolate(),
-          base::BindRepeating(&ProxyFunctionWrapper, store, func_id));
+          base::BindRepeating(&ProxyFunctionWrapper, store, func_id,
+                              support_dynamic_properties));
       FunctionLifeMonitor::BindTo(destination_context->GetIsolate(),
                                   v8::Local<v8::Object>::Cast(proxy_func),
                                   store->GetWeakPtr(), func_id);
@@ -209,7 +211,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
             auto val =
                 PassValueToOtherContext(global_source_context.Get(isolate),
                                         global_destination_context.Get(isolate),
-                                        result, store, &object_cache, 0);
+                                        result, store, &object_cache, false, 0);
             if (!val.IsEmpty())
               proxied_promise->Resolve(val.ToLocalChecked());
             delete proxied_promise;
@@ -230,7 +232,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
             auto val =
                 PassValueToOtherContext(global_source_context.Get(isolate),
                                         global_destination_context.Get(isolate),
-                                        result, store, &object_cache, 0);
+                                        result, store, &object_cache, false, 0);
             if (!val.IsEmpty())
               proxied_promise->Reject(val.ToLocalChecked());
             delete proxied_promise;
@@ -276,7 +278,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
         auto value_for_array = PassValueToOtherContext(
             source_context, destination_context,
             arr->Get(source_context, i).ToLocalChecked(), store, object_cache,
-            recursion_depth + 1);
+            support_dynamic_properties, recursion_depth + 1);
         if (value_for_array.IsEmpty())
           return v8::MaybeLocal<v8::Value>();
 
@@ -293,9 +295,9 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
   // Proxy all objects
   if (IsPlainObject(value)) {
     auto object_value = v8::Local<v8::Object>::Cast(value);
-    auto passed_value =
-        CreateProxyForAPI(object_value, source_context, destination_context,
-                          store, object_cache, recursion_depth + 1);
+    auto passed_value = CreateProxyForAPI(
+        object_value, source_context, destination_context, store, object_cache,
+        support_dynamic_properties, recursion_depth + 1);
     if (passed_value.IsEmpty())
       return v8::MaybeLocal<v8::Value>();
     return v8::MaybeLocal<v8::Value>(passed_value.ToLocalChecked());
@@ -324,6 +326,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
 v8::Local<v8::Value> ProxyFunctionWrapper(
     context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
+    bool support_dynamic_properties,
     gin_helper::Arguments* args) {
   // Context the proxy function was called from
   v8::Local<v8::Context> calling_context = args->isolate()->GetCurrentContext();
@@ -343,7 +346,8 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
 
     for (auto value : original_args) {
       auto arg = PassValueToOtherContext(calling_context, func_owning_context,
-                                         value, store, &object_cache, 0);
+                                         value, store, &object_cache,
+                                         support_dynamic_properties, 0);
       if (arg.IsEmpty())
         return v8::Undefined(args->isolate());
       proxied_args.push_back(arg.ToLocalChecked());
@@ -381,9 +385,10 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
     if (maybe_return_value.IsEmpty())
       return v8::Undefined(args->isolate());
 
-    auto ret = PassValueToOtherContext(func_owning_context, calling_context,
-                                       maybe_return_value.ToLocalChecked(),
-                                       store, &object_cache, 0);
+    auto ret =
+        PassValueToOtherContext(func_owning_context, calling_context,
+                                maybe_return_value.ToLocalChecked(), store,
+                                &object_cache, support_dynamic_properties, 0);
     if (ret.IsEmpty())
       return v8::Undefined(args->isolate());
     return ret.ToLocalChecked();
@@ -396,6 +401,7 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Context>& destination_context,
     context_bridge::RenderFrameFunctionStore* store,
     context_bridge::ObjectCache* object_cache,
+    bool support_dynamic_properties,
     int recursion_depth) {
   gin_helper::Dictionary api(source_context->GetIsolate(), api_object);
   v8::Context::Scope destination_context_scope(destination_context);
@@ -420,13 +426,54 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
       if (!gin::ConvertFromV8(api.isolate(), key, &key_str)) {
         continue;
       }
+      if (support_dynamic_properties) {
+        v8::Context::Scope source_context_scope(source_context);
+        auto maybe_desc = api.GetHandle()->GetOwnPropertyDescriptor(
+            source_context, v8::Local<v8::Name>::Cast(key));
+        v8::Local<v8::Value> desc_value;
+        if (!maybe_desc.ToLocal(&desc_value) || !desc_value->IsObject())
+          continue;
+        gin_helper::Dictionary desc(api.isolate(), desc_value.As<v8::Object>());
+        if (desc.Has("get") || desc.Has("set")) {
+          v8::Local<v8::Value> getter;
+          v8::Local<v8::Value> setter;
+          desc.Get("get", &getter);
+          desc.Get("set", &setter);
+
+          {
+            v8::Context::Scope destination_context_scope(destination_context);
+            v8::Local<v8::Value> getter_proxy;
+            v8::Local<v8::Value> setter_proxy;
+            if (!getter.IsEmpty()) {
+              if (!PassValueToOtherContext(source_context, destination_context,
+                                           getter, store, object_cache, false,
+                                           1)
+                       .ToLocal(&getter_proxy))
+                continue;
+            }
+            if (!setter.IsEmpty()) {
+              if (!PassValueToOtherContext(source_context, destination_context,
+                                           setter, store, object_cache, false,
+                                           1)
+                       .ToLocal(&setter_proxy))
+                continue;
+            }
+
+            v8::PropertyDescriptor desc(getter_proxy, setter_proxy);
+            ignore_result(proxy.GetHandle()->DefineProperty(
+                destination_context, gin::StringToV8(api.isolate(), key_str),
+                desc));
+          }
+          continue;
+        }
+      }
       v8::Local<v8::Value> value;
       if (!api.Get(key_str, &value))
         continue;
 
-      auto passed_value =
-          PassValueToOtherContext(source_context, destination_context, value,
-                                  store, object_cache, recursion_depth + 1);
+      auto passed_value = PassValueToOtherContext(
+          source_context, destination_context, value, store, object_cache,
+          support_dynamic_properties, recursion_depth + 1);
       if (passed_value.IsEmpty())
         return v8::MaybeLocal<v8::Object>();
       proxy.Set(key_str, passed_value.ToLocalChecked());
@@ -472,8 +519,9 @@ void ExposeAPIInMainWorld(const std::string& key,
   context_bridge::ObjectCache object_cache;
   v8::Context::Scope main_context_scope(main_context);
   {
-    v8::MaybeLocal<v8::Object> maybe_proxy = CreateProxyForAPI(
-        api_object, isolated_context, main_context, store, &object_cache, 0);
+    v8::MaybeLocal<v8::Object> maybe_proxy =
+        CreateProxyForAPI(api_object, isolated_context, main_context, store,
+                          &object_cache, false, 0);
     if (maybe_proxy.IsEmpty())
       return;
     auto proxy = maybe_proxy.ToLocalChecked();
@@ -493,13 +541,14 @@ gin_helper::Dictionary TraceKeyPath(const gin_helper::Dictionary& start,
   return current;
 }
 
-void OverrideGlobalMethodFromIsolatedWorld(
+void OverrideGlobalValueFromIsolatedWorld(
     const std::vector<std::string>& key_path,
-    v8::Local<v8::Function> method) {
+    v8::Local<v8::Object> value,
+    bool support_dynamic_properties) {
   if (key_path.size() == 0)
     return;
 
-  auto* render_frame = GetRenderFrame(method);
+  auto* render_frame = GetRenderFrame(value);
   CHECK(render_frame);
   context_bridge::RenderFrameFunctionStore* store =
       GetOrCreateStore(render_frame);
@@ -515,9 +564,9 @@ void OverrideGlobalMethodFromIsolatedWorld(
   {
     v8::Context::Scope main_context_scope(main_context);
     context_bridge::ObjectCache object_cache;
-    v8::MaybeLocal<v8::Value> maybe_proxy =
-        PassValueToOtherContext(method->CreationContext(), main_context, method,
-                                store, &object_cache, 1);
+    v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
+        value->CreationContext(), main_context, value, store, &object_cache,
+        support_dynamic_properties, 1);
     DCHECK(!maybe_proxy.IsEmpty());
     auto proxy = maybe_proxy.ToLocalChecked();
 
@@ -555,14 +604,14 @@ bool OverrideGlobalPropertyFromIsolatedWorld(
     if (!getter->IsNullOrUndefined()) {
       v8::MaybeLocal<v8::Value> maybe_getter_proxy =
           PassValueToOtherContext(getter->CreationContext(), main_context,
-                                  getter, store, &object_cache, 1);
+                                  getter, store, &object_cache, false, 1);
       DCHECK(!maybe_getter_proxy.IsEmpty());
       getter_proxy = maybe_getter_proxy.ToLocalChecked();
     }
     if (!setter->IsNullOrUndefined() && setter->IsObject()) {
       v8::MaybeLocal<v8::Value> maybe_setter_proxy =
           PassValueToOtherContext(getter->CreationContext(), main_context,
-                                  setter, store, &object_cache, 1);
+                                  setter, store, &object_cache, false, 1);
       DCHECK(!maybe_setter_proxy.IsEmpty());
       setter_proxy = maybe_setter_proxy.ToLocalChecked();
     }
@@ -597,8 +646,8 @@ void Initialize(v8::Local<v8::Object> exports,
   v8::Isolate* isolate = context->GetIsolate();
   gin_helper::Dictionary dict(isolate, exports);
   dict.SetMethod("exposeAPIInMainWorld", &electron::api::ExposeAPIInMainWorld);
-  dict.SetMethod("_overrideGlobalMethodFromIsolatedWorld",
-                 &electron::api::OverrideGlobalMethodFromIsolatedWorld);
+  dict.SetMethod("_overrideGlobalValueFromIsolatedWorld",
+                 &electron::api::OverrideGlobalValueFromIsolatedWorld);
   dict.SetMethod("_overrideGlobalPropertyFromIsolatedWorld",
                  &electron::api::OverrideGlobalPropertyFromIsolatedWorld);
   dict.SetMethod("_isCalledFromMainWorld",

shell/renderer/api/electron_api_context_bridge.h

@@ -23,6 +23,7 @@ class RenderFrameFunctionStore;
 v8::Local<v8::Value> ProxyFunctionWrapper(
     context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
+    bool support_dynamic_properties,
     gin_helper::Arguments* args);
 
 v8::MaybeLocal<v8::Object> CreateProxyForAPI(
@@ -31,6 +32,7 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Context>& target_context,
     context_bridge::RenderFrameFunctionStore* store,
     context_bridge::ObjectCache* object_cache,
+    bool support_dynamic_properties,
     int recursion_depth);
 
 }  // namespace api

shell/renderer/electron_renderer_client.cc

@@ -218,6 +218,9 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
 void ElectronRendererClient::SetupMainWorldOverrides(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
+  // We only need to run the isolated bundle if webview is enabled
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kWebviewTag))
+    return;
   // Setup window overrides in the main world context
   // Wrap the bundle into a function that receives the isolatedWorld as
   // an argument.

shell/renderer/electron_sandboxed_renderer_client.cc

@@ -232,6 +232,10 @@ void ElectronSandboxedRendererClient::DidCreateScriptContext(
 void ElectronSandboxedRendererClient::SetupMainWorldOverrides(
     v8::Handle<v8::Context> context,
     content::RenderFrame* render_frame) {
+  // We only need to run the isolated bundle if webview is enabled
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kWebviewTag))
+    return;
+
   // Setup window overrides in the main world context
   // Wrap the bundle into a function that receives the isolatedWorld as
   // an argument.

spec/chromium-spec.js

@@ -65,13 +65,6 @@ describe('chromium feature', () => {
   });
 
   describe('window.open', () => {
-    it('returns a BrowserWindowProxy object', () => {
-      const b = window.open('about:blank', '', 'show=no');
-      expect(b.closed).to.be.false();
-      expect(b.constructor.name).to.equal('BrowserWindowProxy');
-      b.close();
-    });
-
     it('accepts "nodeIntegration" as feature', (done) => {
       let b = null;
       listener = (event) => {
@@ -198,8 +191,8 @@ describe('chromium feature', () => {
       let b = null;
       listener = (event) => {
         window.removeEventListener('message', listener);
+        expect(event.source).to.deep.equal(b);
         b.close();
-        expect(event.source).to.equal(b);
         expect(event.origin).to.equal('file://');
         done();
       };

spec/fixtures/pages/window-open-postMessage.html

@@ -5,7 +5,7 @@
     window.opener.postMessage(JSON.stringify({
       origin: e.origin,
       data: e.data,
-      sourceEqualsOpener: e.source === window.opener
+      sourceEqualsOpener: e.source.location.href === window.opener.location.href
     }), '*');
   });
   window.opener.postMessage("ready", "*")