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

filenames.auto.gni

@@ -182,6 +182,7 @@ 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",
@@ -196,6 +197,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",

lib/renderer/api/context-bridge.ts

@@ -18,3 +18,14 @@ const contextBridge = {
 if (!binding._debugGCMaps) delete contextBridge.debugGC;
 
 export default contextBridge;
+
+export const internalContextBridge = {
+  contextIsolationEnabled,
+  overrideGlobalMethodFromIsolatedWorld: (keys: string[], method: Function) => {
+    return binding._overrideGlobalMethodFromIsolatedWorld(keys, method);
+  },
+  overrideGlobalPropertyFromIsolatedWorld: (keys: string[], getter: Function, setter?: Function) => {
+    return binding._overrideGlobalPropertyFromIsolatedWorld(keys, getter, setter || null);
+  },
+  isInMainWorld: () => binding._isCalledFromMainWorld() as boolean
+};

lib/renderer/window-setup.ts

@@ -1,11 +1,15 @@
 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:
+const inMainWorld = internalContextBridge.isInMainWorld();
-// - window.history.back()
+const { contextIsolationEnabled } = internalContextBridge;
-// - window.history.forward()
+
-// - window.history.go()
+// Should we inject APIs into this world, if ctx isolation is enabled then only inject in the isolated world
-// - window.history.length
+// else inject everywhere
+const shouldInjectGivenContextIsolationIsMaybeEnabled = contextIsolationEnabled ? !inMainWorld : true;
+
+// This file implements the following APIs Directly:
 // - window.open()
 // - window.opener.blur()
 // - window.opener.close()
@@ -14,6 +18,12 @@ import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-inte
 // - window.opener.location
 // - window.opener.print()
 // - window.opener.postMessage()
+
+// And the following APIs over the ctx bridge:
+// - window.history.back()
+// - window.history.forward()
+// - window.history.go()
+// - window.history.length
 // - window.prompt()
 // - document.hidden
 // - document.visibilityState
@@ -178,14 +188,16 @@ class BrowserWindowProxy {
 export const windowSetup = (
   guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean, rendererProcessReuseEnabled: boolean
 ) => {
-  if (!process.sandboxed && guestInstanceId == null) {
+  if (!process.sandboxed && guestInstanceId == null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
     // Override default window.close.
     window.close = function () {
       ipcRendererInternal.send('ELECTRON_BROWSER_WINDOW_CLOSE');
     };
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['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 !== '') {
@@ -201,15 +213,20 @@ export const windowSetup = (
   }
 
   if (openerId != null) {
+    // TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
     window.opener = getOrCreateProxy(openerId);
   }
 
   // But we do not support prompt().
-  window.prompt = function () {
+  if (shouldInjectGivenContextIsolationIsMaybeEnabled) {
-    throw new Error('prompt() is and will not be supported.');
+    window.prompt = function () {
-  };
+      throw new Error('prompt() is and will not be supported.');
+    };
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['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
     ) {
@@ -230,28 +247,31 @@ export const windowSetup = (
     });
   }
 
-  if (!process.sandboxed && !rendererProcessReuseEnabled) {
+  if (!process.sandboxed && !rendererProcessReuseEnabled && shouldInjectGivenContextIsolationIsMaybeEnabled) {
     window.history.back = function () {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK');
     };
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['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);
 
     window.history.go = function (offset: number) {
       ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset);
     };
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'go'], window.history.go);
 
+    const getHistoryLength = () => ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH') + 104;
     Object.defineProperty(window.history, 'length', {
-      get: function () {
+      get: getHistoryLength,
-        return ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH');
-      },
       set () {}
     });
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['history', 'length'], getHistoryLength);
   }
 
-  if (guestInstanceId != null) {
+  if (guestInstanceId != null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
     // Webview `document.visibilityState` tracks window visibility (and ignores
     // the actual <webview> element visibility) for backwards compatibility.
     // See discussion in #9178.
@@ -270,16 +290,16 @@ export const windowSetup = (
     });
 
     // Make document.hidden and document.visibilityState return the correct value.
+    const getDocumentHidden = () => cachedVisibilityState !== 'visible';
     Object.defineProperty(document, 'hidden', {
-      get: function () {
+      get: getDocumentHidden
-        return cachedVisibilityState !== 'visible';
-      }
     });
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'hidden'], getDocumentHidden);
 
+    const getDocumentVisibilityState = () => cachedVisibilityState;
     Object.defineProperty(document, 'visibilityState', {
-      get: function () {
+      get: getDocumentVisibilityState
-        return cachedVisibilityState;
-      }
     });
+    if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'visibilityState'], getDocumentVisibilityState);
   }
 };

shell/renderer/api/electron_api_context_bridge.cc

@@ -484,6 +484,106 @@ void ExposeAPIInMainWorld(const std::string& key,
   }
 }
 
+gin_helper::Dictionary TraceKeyPath(const gin_helper::Dictionary& start,
+                                    const std::vector<std::string>& key_path) {
+  gin_helper::Dictionary current = start;
+  for (size_t i = 0; i < key_path.size() - 1; i++) {
+    CHECK(current.Get(key_path[i], &current));
+  }
+  return current;
+}
+
+void OverrideGlobalMethodFromIsolatedWorld(
+    const std::vector<std::string>& key_path,
+    v8::Local<v8::Function> method) {
+  if (key_path.size() == 0)
+    return;
+
+  auto* render_frame = GetRenderFrame(method);
+  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();
+  gin_helper::Dictionary global(main_context->GetIsolate(),
+                                main_context->Global());
+
+  const std::string final_key = key_path[key_path.size() - 1];
+  gin_helper::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(method->CreationContext(), main_context, method,
+                                store, &object_cache, 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,
+    gin_helper::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();
+  gin_helper::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, 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);
+      DCHECK(!maybe_setter_proxy.IsEmpty());
+      setter_proxy = maybe_setter_proxy.ToLocalChecked();
+    }
+
+    v8::PropertyDescriptor desc(getter_proxy, setter_proxy);
+    bool success = 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;
+}
+
 }  // namespace api
 
 }  // namespace electron
@@ -497,6 +597,12 @@ 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("_overrideGlobalPropertyFromIsolatedWorld",
+                 &electron::api::OverrideGlobalPropertyFromIsolatedWorld);
+  dict.SetMethod("_isCalledFromMainWorld",
+                 &electron::api::IsCalledFromMainWorld);
 #ifdef DCHECK_IS_ON
   dict.SetMethod("_debugGCMaps", &electron::api::DebugGC);
 #endif

spec-main/api-context-bridge-spec.ts

@@ -346,16 +346,19 @@ describe('contextBridge', () => {
               getFunction: () => () => 123
             });
           });
-          expect((await getGCInfo()).functionCount).to.equal(2);
+          await callWithBindings(async (root: any) => {
+            root.GCRunner.run();
+          });
+          const baseValue = (await getGCInfo()).functionCount;
           await callWithBindings(async (root: any) => {
             root.x = [root.example.getFunction()];
           });
-          expect((await getGCInfo()).functionCount).to.equal(3);
+          expect((await getGCInfo()).functionCount).to.equal(baseValue + 1);
           await callWithBindings(async (root: any) => {
             root.x = [];
             root.GCRunner.run();
           });
-          expect((await getGCInfo()).functionCount).to.equal(2);
+          expect((await getGCInfo()).functionCount).to.equal(baseValue);
         });
       }
 
@@ -369,14 +372,14 @@ describe('contextBridge', () => {
             require('electron').ipcRenderer.send('window-ready-for-tasking');
           });
           const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking');
-          expect((await getGCInfo()).functionCount).to.equal(1);
+          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);
         });
       }