Skip to content

Commit 96bf9ce

Browse files
MarshallOfSoundMarshallOfSound
authored
refactor: port parts of window-setup to use ctx bridge instead of being run in the main world (electron#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
1 parent 9d60cfa commit 96bf9ce

File tree

5 files changed

+167
-25
lines changed

5 files changed

+167
-25
lines changed

‎filenames.auto.gni

+2
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ auto_filenames = {
182182
isolated_bundle_deps = [
183183
"lib/common/electron-binding-setup.ts",
184184
"lib/isolated_renderer/init.js",
185+
"lib/renderer/api/context-bridge.ts",
185186
"lib/renderer/ipc-renderer-internal-utils.ts",
186187
"lib/renderer/ipc-renderer-internal.ts",
187188
"lib/renderer/web-view/web-view-constants.ts",
@@ -196,6 +197,7 @@ auto_filenames = {
196197
"lib/common/electron-binding-setup.ts",
197198
"lib/common/webpack-globals-provider.ts",
198199
"lib/content_script/init.js",
200+
"lib/renderer/api/context-bridge.ts",
199201
"lib/renderer/chrome-api.ts",
200202
"lib/renderer/extensions/event.ts",
201203
"lib/renderer/extensions/i18n.ts",

‎lib/renderer/api/context-bridge.ts

+11
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,14 @@ const contextBridge = {
1818
if (!binding._debugGCMaps) delete contextBridge.debugGC;
1919

2020
export default contextBridge;
21+
22+
export const internalContextBridge = {
23+
contextIsolationEnabled,
24+
overrideGlobalMethodFromIsolatedWorld: (keys: string[], method: Function) => {
25+
return binding._overrideGlobalMethodFromIsolatedWorld(keys, method);
26+
},
27+
overrideGlobalPropertyFromIsolatedWorld: (keys: string[], getter: Function, setter?: Function) => {
28+
return binding._overrideGlobalPropertyFromIsolatedWorld(keys, getter, setter || null);
29+
},
30+
isInMainWorld: () => binding._isCalledFromMainWorld() as boolean
31+
};

‎lib/renderer/window-setup.ts

+40-20
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal';
22
import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils';
3+
import { internalContextBridge } from '@electron/internal/renderer/api/context-bridge';
34

4-
// This file implements the following APIs:
5-
// - window.history.back()
6-
// - window.history.forward()
7-
// - window.history.go()
8-
// - window.history.length
5+
const inMainWorld = internalContextBridge.isInMainWorld();
6+
const { contextIsolationEnabled } = internalContextBridge;
7+
8+
// Should we inject APIs into this world, if ctx isolation is enabled then only inject in the isolated world
9+
// else inject everywhere
10+
const shouldInjectGivenContextIsolationIsMaybeEnabled = contextIsolationEnabled ? !inMainWorld : true;
11+
12+
// This file implements the following APIs Directly:
913
// - window.open()
1014
// - window.opener.blur()
1115
// - window.opener.close()
@@ -14,6 +18,12 @@ import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-inte
1418
// - window.opener.location
1519
// - window.opener.print()
1620
// - window.opener.postMessage()
21+
22+
// And the following APIs over the ctx bridge:
23+
// - window.history.back()
24+
// - window.history.forward()
25+
// - window.history.go()
26+
// - window.history.length
1727
// - window.prompt()
1828
// - document.hidden
1929
// - document.visibilityState
@@ -178,14 +188,16 @@ class BrowserWindowProxy {
178188
export const windowSetup = (
179189
guestInstanceId: number, openerId: number, isHiddenPage: boolean, usesNativeWindowOpen: boolean, rendererProcessReuseEnabled: boolean
180190
) => {
181-
if (!process.sandboxed && guestInstanceId == null) {
191+
if (!process.sandboxed && guestInstanceId == null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
182192
// Override default window.close.
183193
window.close = function () {
184194
ipcRendererInternal.send('ELECTRON_BROWSER_WINDOW_CLOSE');
185195
};
196+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['close'], window.close);
186197
}
187198

188199
if (!usesNativeWindowOpen) {
200+
// TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
189201
// Make the browser window or guest view emit "new-window" event.
190202
(window as any).open = function (url?: string, frameName?: string, features?: string) {
191203
if (url != null && url !== '') {
@@ -201,15 +213,20 @@ export const windowSetup = (
201213
}
202214

203215
if (openerId != null) {
216+
// TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
204217
window.opener = getOrCreateProxy(openerId);
205218
}
206219

207220
// But we do not support prompt().
208-
window.prompt = function () {
209-
throw new Error('prompt() is and will not be supported.');
210-
};
221+
if (shouldInjectGivenContextIsolationIsMaybeEnabled) {
222+
window.prompt = function () {
223+
throw new Error('prompt() is and will not be supported.');
224+
};
225+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['prompt'], window.prompt);
226+
}
211227

212228
if (!usesNativeWindowOpen || openerId != null) {
229+
// TODO(MarshallOfSound): Make compatible with ctx isolation without hole-punch
213230
ipcRendererInternal.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (
214231
_event, sourceId: number, message: any, sourceOrigin: string
215232
) {
@@ -230,28 +247,31 @@ export const windowSetup = (
230247
});
231248
}
232249

233-
if (!process.sandboxed && !rendererProcessReuseEnabled) {
250+
if (!process.sandboxed && !rendererProcessReuseEnabled && shouldInjectGivenContextIsolationIsMaybeEnabled) {
234251
window.history.back = function () {
235252
ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_BACK');
236253
};
254+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'back'], window.history.back);
237255

238256
window.history.forward = function () {
239257
ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_FORWARD');
240258
};
259+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'forward'], window.history.forward);
241260

242261
window.history.go = function (offset: number) {
243262
ipcRendererInternal.send('ELECTRON_NAVIGATION_CONTROLLER_GO_TO_OFFSET', +offset);
244263
};
264+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalMethodFromIsolatedWorld(['history', 'go'], window.history.go);
245265

266+
const getHistoryLength = () => ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH') + 104;
246267
Object.defineProperty(window.history, 'length', {
247-
get: function () {
248-
return ipcRendererInternal.sendSync('ELECTRON_NAVIGATION_CONTROLLER_LENGTH');
249-
},
268+
get: getHistoryLength,
250269
set () {}
251270
});
271+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['history', 'length'], getHistoryLength);
252272
}
253273

254-
if (guestInstanceId != null) {
274+
if (guestInstanceId != null && shouldInjectGivenContextIsolationIsMaybeEnabled) {
255275
// Webview `document.visibilityState` tracks window visibility (and ignores
256276
// the actual <webview> element visibility) for backwards compatibility.
257277
// See discussion in #9178.
@@ -270,16 +290,16 @@ export const windowSetup = (
270290
});
271291

272292
// Make document.hidden and document.visibilityState return the correct value.
293+
const getDocumentHidden = () => cachedVisibilityState !== 'visible';
273294
Object.defineProperty(document, 'hidden', {
274-
get: function () {
275-
return cachedVisibilityState !== 'visible';
276-
}
295+
get: getDocumentHidden
277296
});
297+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'hidden'], getDocumentHidden);
278298

299+
const getDocumentVisibilityState = () => cachedVisibilityState;
279300
Object.defineProperty(document, 'visibilityState', {
280-
get: function () {
281-
return cachedVisibilityState;
282-
}
301+
get: getDocumentVisibilityState
283302
});
303+
if (contextIsolationEnabled) internalContextBridge.overrideGlobalPropertyFromIsolatedWorld(['document', 'visibilityState'], getDocumentVisibilityState);
284304
}
285305
};

‎shell/renderer/api/electron_api_context_bridge.cc

+106
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,106 @@ void ExposeAPIInMainWorld(const std::string& key,
484484
}
485485
}
486486

487+
gin_helper::Dictionary TraceKeyPath(const gin_helper::Dictionary& start,
488+
const std::vector<std::string>& key_path) {
489+
gin_helper::Dictionary current = start;
490+
for (size_t i = 0; i < key_path.size() - 1; i++) {
491+
CHECK(current.Get(key_path[i], &current));
492+
}
493+
return current;
494+
}
495+
496+
void OverrideGlobalMethodFromIsolatedWorld(
497+
const std::vector<std::string>& key_path,
498+
v8::Local<v8::Function> method) {
499+
if (key_path.size() == 0)
500+
return;
501+
502+
auto* render_frame = GetRenderFrame(method);
503+
CHECK(render_frame);
504+
context_bridge::RenderFrameFunctionStore* store =
505+
GetOrCreateStore(render_frame);
506+
auto* frame = render_frame->GetWebFrame();
507+
CHECK(frame);
508+
v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
509+
gin_helper::Dictionary global(main_context->GetIsolate(),
510+
main_context->Global());
511+
512+
const std::string final_key = key_path[key_path.size() - 1];
513+
gin_helper::Dictionary target_object = TraceKeyPath(global, key_path);
514+
515+
{
516+
v8::Context::Scope main_context_scope(main_context);
517+
context_bridge::ObjectCache object_cache;
518+
v8::MaybeLocal<v8::Value> maybe_proxy =
519+
PassValueToOtherContext(method->CreationContext(), main_context, method,
520+
store, &object_cache, 1);
521+
DCHECK(!maybe_proxy.IsEmpty());
522+
auto proxy = maybe_proxy.ToLocalChecked();
523+
524+
target_object.Set(final_key, proxy);
525+
}
526+
}
527+
528+
bool OverrideGlobalPropertyFromIsolatedWorld(
529+
const std::vector<std::string>& key_path,
530+
v8::Local<v8::Object> getter,
531+
v8::Local<v8::Value> setter,
532+
gin_helper::Arguments* args) {
533+
if (key_path.size() == 0)
534+
return false;
535+
536+
auto* render_frame = GetRenderFrame(getter);
537+
CHECK(render_frame);
538+
context_bridge::RenderFrameFunctionStore* store =
539+
GetOrCreateStore(render_frame);
540+
auto* frame = render_frame->GetWebFrame();
541+
CHECK(frame);
542+
v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
543+
gin_helper::Dictionary global(main_context->GetIsolate(),
544+
main_context->Global());
545+
546+
const std::string final_key = key_path[key_path.size() - 1];
547+
v8::Local<v8::Object> target_object =
548+
TraceKeyPath(global, key_path).GetHandle();
549+
550+
{
551+
v8::Context::Scope main_context_scope(main_context);
552+
context_bridge::ObjectCache object_cache;
553+
v8::Local<v8::Value> getter_proxy;
554+
v8::Local<v8::Value> setter_proxy;
555+
if (!getter->IsNullOrUndefined()) {
556+
v8::MaybeLocal<v8::Value> maybe_getter_proxy =
557+
PassValueToOtherContext(getter->CreationContext(), main_context,
558+
getter, store, &object_cache, 1);
559+
DCHECK(!maybe_getter_proxy.IsEmpty());
560+
getter_proxy = maybe_getter_proxy.ToLocalChecked();
561+
}
562+
if (!setter->IsNullOrUndefined() && setter->IsObject()) {
563+
v8::MaybeLocal<v8::Value> maybe_setter_proxy =
564+
PassValueToOtherContext(getter->CreationContext(), main_context,
565+
setter, store, &object_cache, 1);
566+
DCHECK(!maybe_setter_proxy.IsEmpty());
567+
setter_proxy = maybe_setter_proxy.ToLocalChecked();
568+
}
569+
570+
v8::PropertyDescriptor desc(getter_proxy, setter_proxy);
571+
bool success = IsTrue(target_object->DefineProperty(
572+
main_context, gin::StringToV8(args->isolate(), final_key), desc));
573+
DCHECK(success);
574+
return success;
575+
}
576+
}
577+
578+
bool IsCalledFromMainWorld(v8::Isolate* isolate) {
579+
auto* render_frame = GetRenderFrame(isolate->GetCurrentContext()->Global());
580+
CHECK(render_frame);
581+
auto* frame = render_frame->GetWebFrame();
582+
CHECK(frame);
583+
v8::Local<v8::Context> main_context = frame->MainWorldScriptContext();
584+
return isolate->GetCurrentContext() == main_context;
585+
}
586+
487587
} // namespace api
488588

489589
} // namespace electron
@@ -497,6 +597,12 @@ void Initialize(v8::Local<v8::Object> exports,
497597
v8::Isolate* isolate = context->GetIsolate();
498598
gin_helper::Dictionary dict(isolate, exports);
499599
dict.SetMethod("exposeAPIInMainWorld", &electron::api::ExposeAPIInMainWorld);
600+
dict.SetMethod("_overrideGlobalMethodFromIsolatedWorld",
601+
&electron::api::OverrideGlobalMethodFromIsolatedWorld);
602+
dict.SetMethod("_overrideGlobalPropertyFromIsolatedWorld",
603+
&electron::api::OverrideGlobalPropertyFromIsolatedWorld);
604+
dict.SetMethod("_isCalledFromMainWorld",
605+
&electron::api::IsCalledFromMainWorld);
500606
#ifdef DCHECK_IS_ON
501607
dict.SetMethod("_debugGCMaps", &electron::api::DebugGC);
502608
#endif

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

+8-5
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,19 @@ describe('contextBridge', () => {
346346
getFunction: () => () => 123
347347
});
348348
});
349-
expect((await getGCInfo()).functionCount).to.equal(2);
349+
await callWithBindings(async (root: any) => {
350+
root.GCRunner.run();
351+
});
352+
const baseValue = (await getGCInfo()).functionCount;
350353
await callWithBindings(async (root: any) => {
351354
root.x = [root.example.getFunction()];
352355
});
353-
expect((await getGCInfo()).functionCount).to.equal(3);
356+
expect((await getGCInfo()).functionCount).to.equal(baseValue + 1);
354357
await callWithBindings(async (root: any) => {
355358
root.x = [];
356359
root.GCRunner.run();
357360
});
358-
expect((await getGCInfo()).functionCount).to.equal(2);
361+
expect((await getGCInfo()).functionCount).to.equal(baseValue);
359362
});
360363
}
361364

@@ -369,14 +372,14 @@ describe('contextBridge', () => {
369372
require('electron').ipcRenderer.send('window-ready-for-tasking');
370373
});
371374
const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking');
372-
expect((await getGCInfo()).functionCount).to.equal(1);
375+
const baseValue = (await getGCInfo()).functionCount;
373376
await callWithBindings((root: any) => {
374377
root.location.reload();
375378
});
376379
await loadPromise;
377380
// If this is ever "2" it means we leaked the exposed function and
378381
// therefore the entire context after a reload
379-
expect((await getGCInfo()).functionCount).to.equal(1);
382+
expect((await getGCInfo()).functionCount).to.equal(baseValue);
380383
});
381384
}
382385

0 commit comments

Comments
 (0)