Browse Source

refactor: port window-setup to use ctx bridge instead of being run in the main world (#23302)

* refactor: port parts of window-setup to use ctx bridge instead of being run in the main world (#23194)

* refactor: port parts of window-setup to use ctx bridge instead of being run in the main world

* chore: update ctx bridge specs for new base numbers

* 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

* s/gin/mate

* fix: do not inject in content scripts and do not override window.history because it does not work
Samuel Attard 5 years ago
parent
commit
045331666c

+ 1 - 3
filenames.auto.gni

@@ -177,11 +177,8 @@ auto_filenames = {
   isolated_bundle_deps = [
     "lib/common/electron-binding-setup.ts",
     "lib/isolated_renderer/init.js",
-    "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",
@@ -191,6 +188,7 @@ auto_filenames = {
     "lib/common/electron-binding-setup.ts",
     "lib/common/webpack-globals-provider.ts",
     "lib/content_script/init.js",
+    "lib/renderer/api/context-bridge.ts",
     "lib/renderer/chrome-api.ts",
     "lib/renderer/extensions/event.ts",
     "lib/renderer/extensions/i18n.ts",

+ 5 - 0
lib/browser/api/web-contents.js

@@ -429,6 +429,11 @@ WebContents.prototype._init = function () {
       const referrer = { url: '', policy: 'default' };
       internalWindowOpen(event, url, referrer, frameName, disposition, options);
     });
+
+    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) => {

+ 0 - 12
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 } = isolatedWorldArgs;
-  const { windowSetup } = require('@electron/internal/renderer/window-setup');
-  windowSetup(guestInstanceId, openerId, isHiddenPage, usesNativeWindowOpen);
-}

+ 15 - 0
lib/renderer/api/context-bridge.ts

@@ -18,3 +18,18 @@ const contextBridge = {
 if (!binding._debugGCMaps) delete contextBridge.debugGC;
 
 export default contextBridge;
+
+export const internalContextBridge = {
+  contextIsolationEnabled,
+  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);
+  },
+  isInMainWorld: () => binding._isCalledFromMainWorld() as boolean,
+  isInIsolatedWorld: () => binding._isCalledFromIsolatedWorld() as boolean
+};

+ 96 - 23
lib/renderer/window-setup.ts

@@ -1,11 +1,11 @@
 import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal';
 import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils';
+import { internalContextBridge } from '@electron/internal/renderer/api/context-bridge';
 
-// This file implements the following APIs:
-// - window.history.back()
-// - window.history.forward()
-// - window.history.go()
-// - window.history.length
+const { contextIsolationEnabled, isInIsolatedWorld } = internalContextBridge;
+const shouldUseContextBridge = contextIsolationEnabled && isInIsolatedWorld();
+
+// This file implements the following APIs over the ctx bridge:
 // - window.open()
 // - window.opener.blur()
 // - window.opener.close()
@@ -13,7 +13,12 @@ import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-inte
 // - window.opener.focus()
 // - window.opener.location
 // - window.opener.print()
+// - window.opener.closed
 // - window.opener.postMessage()
+// - window.history.back()
+// - window.history.forward()
+// - window.history.go()
+// - window.history.length
 // - window.prompt()
 // - document.hidden
 // - document.visibilityState
@@ -30,13 +35,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) => {
@@ -64,6 +69,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] : '';
@@ -82,6 +89,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
@@ -114,6 +145,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
 
@@ -124,7 +166,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) {
     url = resolveURL(url, this.location.href);
@@ -141,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);
   }
 
@@ -182,9 +245,11 @@ export const windowSetup = (
     window.close = function () {
       ipcRendererInternal.sendSync('ELECTRON_BROWSER_WINDOW_CLOSE');
     };
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['close'], window.close);
   }
 
   if (!usesNativeWindowOpen) {
+    // TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
     // Make the browser window or guest view emit "new-window" event.
     (window as any).open = function (url?: string, frameName?: string, features?: string) {
       if (url != null && url !== '') {
@@ -197,16 +262,19 @@ export const windowSetup = (
         return null;
       }
     };
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['open'], window.open);
   }
 
   if (openerId != null) {
     window.opener = getOrCreateProxy(openerId);
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['opener'], window.opener);
   }
 
   // But we do not support prompt().
   window.prompt = function () {
     throw new Error('prompt() is and will not be supported.');
   };
+  if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['prompt'], window.prompt);
 
   if (!usesNativeWindowOpen || openerId != null) {
     ipcRendererInternal.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (
@@ -233,20 +301,25 @@ export const windowSetup = (
     window.history.back = function () {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK');
     };
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'back'], window.history.back);
 
     window.history.forward = function () {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD');
     };
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'forward'], window.history.forward);
 
     window.history.go = function (offset: number) {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset);
     };
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalValueFromIsolatedWorld(['history', 'go'], window.history.go);
 
+    const getHistoryLength = () => ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH');
     Object.defineProperty(window.history, 'length', {
-      get: function () {
-        return ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH');
-      }
+      get: getHistoryLength,
+      set () {}
     });
+    // TODO(MarshallOfSound): Fix so that the internal context bridge can override a non-configurable property
+    // if (shouldUseContextBridge) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['history', 'length'], getHistoryLength);
   }
 
   if (guestInstanceId != null) {
@@ -268,16 +341,16 @@ export const windowSetup = (
     });
 
     // Make document.hidden and document.visibilityState return the correct value.
+    const getDocumentHidden = () => cachedVisibilityState !== 'visible';
     Object.defineProperty(document, 'hidden', {
-      get: function () {
-        return cachedVisibilityState !== 'visible';
-      }
+      get: getDocumentHidden
     });
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'hidden'], getDocumentHidden);
 
+    const getDocumentVisibilityState = () => cachedVisibilityState;
     Object.defineProperty(document, 'visibilityState', {
-      get: function () {
-        return cachedVisibilityState;
-      }
+      get: getDocumentVisibilityState
     });
+    if (shouldUseContextBridge) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'visibilityState'], getDocumentVisibilityState);
   }
 };

+ 181 - 16
shell/renderer/api/electron_api_context_bridge.cc

@@ -148,6 +148,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);
@@ -181,7 +182,8 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     {
       v8::Local<v8::Value> proxy_func = BindRepeatingFunctionToV8(
           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);
@@ -211,7 +213,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;
@@ -232,7 +234,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;
@@ -278,7 +280,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>();
 
@@ -296,9 +298,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());
@@ -327,6 +329,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
 v8::Local<v8::Value> ProxyFunctionWrapper(
     context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
+    bool support_dynamic_properties,
     mate::Arguments* args) {
   // Context the proxy function was called from
   v8::Local<v8::Context> calling_context = args->isolate()->GetCurrentContext();
@@ -346,7 +349,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());
@@ -384,9 +388,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();
@@ -399,6 +404,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) {
   mate::Dictionary api(source_context->GetIsolate(), api_object);
   v8::Context::Scope destination_context_scope(destination_context);
@@ -423,13 +429,54 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
       if (!mate::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;
+        mate::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());
@@ -474,8 +521,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();
@@ -486,6 +534,115 @@ void ExposeAPIInMainWorld(const std::string& key,
   }
 }
 
+mate::Dictionary TraceKeyPath(const mate::Dictionary& start,
+                              const std::vector<std::string>& key_path) {
+  mate::Dictionary current = start;
+  for (size_t i = 0; i < key_path.size() - 1; i++) {
+    CHECK(current.Get(key_path[i], &current));
+  }
+  return current;
+}
+
+void OverrideGlobalValueFromIsolatedWorld(
+    const std::vector<std::string>& key_path,
+    v8::Local<v8::Object> value,
+    bool support_dynamic_properties) {
+  if (key_path.size() == 0)
+    return;
+
+  auto* render_frame = GetRenderFrame(value);
+  CHECK(render_frame);
+  context_bridge::RenderFrameFunctionStore* store =
+      GetOrCreateStore(render_frame);
+  auto* frame = render_frame->GetWebFrame();
+  CHECK(frame);
+  v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
+  mate::Dictionary global(main_context->GetIsolate(), main_context->Global());
+
+  const std::string final_key = key_path[key_path.size() - 1];
+  mate::Dictionary target_object = TraceKeyPath(global, key_path);
+
+  {
+    v8::Context::Scope main_context_scope(main_context);
+    context_bridge::ObjectCache object_cache;
+    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();
+
+    target_object.Set(final_key, proxy);
+  }
+}
+
+bool OverrideGlobalPropertyFromIsolatedWorld(
+    const std::vector<std::string>& key_path,
+    v8::Local<v8::Object> getter,
+    v8::Local<v8::Value> setter,
+    mate::Arguments* args) {
+  if (key_path.size() == 0)
+    return false;
+
+  auto* render_frame = GetRenderFrame(getter);
+  CHECK(render_frame);
+  context_bridge::RenderFrameFunctionStore* store =
+      GetOrCreateStore(render_frame);
+  auto* frame = render_frame->GetWebFrame();
+  CHECK(frame);
+  v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
+  mate::Dictionary global(main_context->GetIsolate(), main_context->Global());
+
+  const std::string final_key = key_path[key_path.size() - 1];
+  v8::Local<v8::Object> target_object =
+      TraceKeyPath(global, key_path).GetHandle();
+
+  {
+    v8::Context::Scope main_context_scope(main_context);
+    context_bridge::ObjectCache object_cache;
+    v8::Local<v8::Value> getter_proxy;
+    v8::Local<v8::Value> setter_proxy;
+    if (!getter->IsNullOrUndefined()) {
+      v8::MaybeLocal<v8::Value> maybe_getter_proxy =
+          PassValueToOtherContext(getter->CreationContext(), main_context,
+                                  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, false, 1);
+      DCHECK(!maybe_setter_proxy.IsEmpty());
+      setter_proxy = maybe_setter_proxy.ToLocalChecked();
+    }
+
+    v8::PropertyDescriptor desc(getter_proxy, setter_proxy);
+    bool success = mate::internal::IsTrue(target_object->DefineProperty(
+        main_context, gin::StringToV8(args->isolate(), final_key), desc));
+    DCHECK(success);
+    return success;
+  }
+}
+
+bool IsCalledFromMainWorld(v8::Isolate* isolate) {
+  auto* render_frame = GetRenderFrame(isolate->GetCurrentContext()->Global());
+  CHECK(render_frame);
+  auto* frame = render_frame->GetWebFrame();
+  CHECK(frame);
+  v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
+  return isolate->GetCurrentContext() == main_context;
+}
+
+bool IsCalledFromIsolatedWorld(v8::Isolate* isolate) {
+  auto* render_frame = GetRenderFrame(isolate->GetCurrentContext()->Global());
+  CHECK(render_frame);
+  auto* frame = render_frame->GetWebFrame();
+  CHECK(frame);
+  v8::Local<v8::Context> isolated_context =
+      frame->WorldScriptContext(isolate, World::ISOLATED_WORLD);
+  return isolate->GetCurrentContext() == isolated_context;
+}
+
 }  // namespace api
 
 }  // namespace electron
@@ -499,6 +656,14 @@ void Initialize(v8::Local<v8::Object> exports,
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
   dict.SetMethod("exposeAPIInMainWorld", &electron::api::ExposeAPIInMainWorld);
+  dict.SetMethod("_overrideGlobalValueFromIsolatedWorld",
+                 &electron::api::OverrideGlobalValueFromIsolatedWorld);
+  dict.SetMethod("_overrideGlobalPropertyFromIsolatedWorld",
+                 &electron::api::OverrideGlobalPropertyFromIsolatedWorld);
+  dict.SetMethod("_isCalledFromMainWorld",
+                 &electron::api::IsCalledFromMainWorld);
+  dict.SetMethod("_isCalledFromIsolatedWorld",
+                 &electron::api::IsCalledFromIsolatedWorld);
 #ifdef DCHECK_IS_ON
   dict.SetMethod("_debugGCMaps", &electron::api::DebugGC);
 #endif

+ 2 - 0
shell/renderer/api/electron_api_context_bridge.h

@@ -21,6 +21,7 @@ class RenderFrameFunctionStore;
 v8::Local<v8::Value> ProxyFunctionWrapper(
     context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
+    bool support_dynamic_properties,
     mate::Arguments* args);
 
 v8::MaybeLocal<v8::Object> CreateProxyForAPI(
@@ -29,6 +30,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

+ 3 - 0
shell/renderer/electron_renderer_client.cc

@@ -214,6 +214,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.

+ 4 - 0
shell/renderer/electron_sandboxed_renderer_client.cc

@@ -237,6 +237,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.

+ 21 - 18
spec-main/api-context-bridge-spec.ts

@@ -340,19 +340,22 @@ describe('contextBridge', () => {
             require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
             contextBridge.exposeInMainWorld('example', {
               getFunction: () => () => 123
-            })
-          })
-          expect((await getGCInfo()).functionCount).to.equal(2)
+            });
+          });
           await callWithBindings(async (root: any) => {
-            root.x = [root.example.getFunction()]
-          })
-          expect((await getGCInfo()).functionCount).to.equal(3)
+            root.GCRunner.run();
+          });
+          const baseValue = (await getGCInfo()).functionCount;
           await callWithBindings(async (root: any) => {
-            root.x = []
-            root.GCRunner.run()
-          })
-          expect((await getGCInfo()).functionCount).to.equal(2)
-        })
+            root.x = [root.example.getFunction()];
+          });
+          expect((await getGCInfo()).functionCount).to.equal(baseValue + 1);
+          await callWithBindings(async (root: any) => {
+            root.x = [];
+            root.GCRunner.run();
+          });
+          expect((await getGCInfo()).functionCount).to.equal(baseValue);
+        });
       }
 
       if (useSandbox) {
@@ -361,19 +364,19 @@ describe('contextBridge', () => {
             require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
             contextBridge.exposeInMainWorld('example', {
               getFunction: () => () => 123
-            })
-            require('electron').ipcRenderer.send('window-ready-for-tasking')
-          })
-          const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking')
-          expect((await getGCInfo()).functionCount).to.equal(1)
+            });
+            require('electron').ipcRenderer.send('window-ready-for-tasking');
+          });
+          const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking');
+          const baseValue = (await getGCInfo()).functionCount;
           await callWithBindings((root: any) => {
             root.location.reload()
           })
           await loadPromise
           // If this is ever "2" it means we leaked the exposed function and
           // therefore the entire context after a reload
-          expect((await getGCInfo()).functionCount).to.equal(1)
-        })
+          expect((await getGCInfo()).functionCount).to.equal(baseValue);
+        });
       }
 
       it('it should not let you overwrite existing exposed things', async () => {

+ 1 - 8
spec/chromium-spec.js

@@ -70,13 +70,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) => {
@@ -203,8 +196,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();
       };

+ 1 - 1
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", "*")