mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
refactor: remove dead named-window lookup from guest-window-manager (#50498)
The frameNamesToWindow map was a holdover from the BrowserWindowProxy IPC shim. Since nativeWindowOpen became the only code path, Blink's FrameTree::FindOrCreateFrameForNavigation resolves named window targets directly in the renderer, scoped to the opener's browsing context group. When a matching named window exists, Blink navigates it without ever sending a CreateNewWindow IPC to the browser, so this map was never consulted in the legitimate same-opener case. The only time the map found a match was when two unrelated renderers happened to use the same target name, in which case openGuestWindow would short-circuit before consuming the guest WebContents that Chromium had already created for the new window, leaking it. Adds a test verifying Blink handles same-opener named-target reuse end-to-end without any browser-side tracking. Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>
This commit is contained in:
@@ -17,11 +17,6 @@ export type WindowOpenArgs = {
|
||||
features: string,
|
||||
}
|
||||
|
||||
const frameNamesToWindow = new Map<string, WebContents>();
|
||||
const registerFrameNameToGuestWindow = (name: string, webContents: WebContents) => frameNamesToWindow.set(name, webContents);
|
||||
const unregisterFrameName = (name: string) => frameNamesToWindow.delete(name);
|
||||
const getGuestWebContentsByFrameName = (name: string) => frameNamesToWindow.get(name);
|
||||
|
||||
/**
|
||||
* `openGuestWindow` is called to create and setup event handling for the new
|
||||
* window.
|
||||
@@ -47,20 +42,6 @@ export function openGuestWindow ({ embedder, guest, referrer, disposition, postD
|
||||
...overrideBrowserWindowOptions
|
||||
};
|
||||
|
||||
// To spec, subsequent window.open calls with the same frame name (`target` in
|
||||
// spec parlance) will reuse the previous window.
|
||||
// https://html.spec.whatwg.org/multipage/window-object.html#apis-for-creating-and-navigating-browsing-contexts-by-name
|
||||
const existingWebContents = getGuestWebContentsByFrameName(frameName);
|
||||
if (existingWebContents) {
|
||||
if (existingWebContents.isDestroyed()) {
|
||||
// FIXME(t57ser): The webContents is destroyed for some reason, unregister the frame name
|
||||
unregisterFrameName(frameName);
|
||||
} else {
|
||||
existingWebContents.loadURL(url);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if (createWindow) {
|
||||
const webContents = createWindow({
|
||||
webContents: guest,
|
||||
@@ -72,7 +53,7 @@ export function openGuestWindow ({ embedder, guest, referrer, disposition, postD
|
||||
throw new Error('Invalid webContents. Created window should be connected to webContents passed with options object.');
|
||||
}
|
||||
|
||||
handleWindowLifecycleEvents({ embedder, frameName, guest, outlivesOpener });
|
||||
handleWindowLifecycleEvents({ embedder, guest, outlivesOpener });
|
||||
}
|
||||
|
||||
return;
|
||||
@@ -96,7 +77,7 @@ export function openGuestWindow ({ embedder, guest, referrer, disposition, postD
|
||||
});
|
||||
}
|
||||
|
||||
handleWindowLifecycleEvents({ embedder, frameName, guest: window.webContents, outlivesOpener });
|
||||
handleWindowLifecycleEvents({ embedder, guest: window.webContents, outlivesOpener });
|
||||
|
||||
embedder.emit('did-create-window', window, { url, frameName, options: browserWindowOptions, disposition, referrer, postData });
|
||||
}
|
||||
@@ -107,10 +88,9 @@ export function openGuestWindow ({ embedder, guest, referrer, disposition, postD
|
||||
* too is the guest destroyed; this is Electron convention and isn't based in
|
||||
* browser behavior.
|
||||
*/
|
||||
const handleWindowLifecycleEvents = function ({ embedder, guest, frameName, outlivesOpener }: {
|
||||
const handleWindowLifecycleEvents = function ({ embedder, guest, outlivesOpener }: {
|
||||
embedder: WebContents,
|
||||
guest: WebContents,
|
||||
frameName: string,
|
||||
outlivesOpener: boolean
|
||||
}) {
|
||||
const closedByEmbedder = function () {
|
||||
@@ -128,13 +108,6 @@ const handleWindowLifecycleEvents = function ({ embedder, guest, frameName, outl
|
||||
embedder.once('current-render-view-deleted' as any, closedByEmbedder);
|
||||
}
|
||||
guest.once('destroyed', closedByUser);
|
||||
|
||||
if (frameName) {
|
||||
registerFrameNameToGuestWindow(frameName, guest);
|
||||
guest.once('destroyed', function () {
|
||||
unregisterFrameName(frameName);
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
// Security options that child windows will always inherit from parent windows
|
||||
|
||||
@@ -186,6 +186,39 @@ describe('webContents.setWindowOpenHandler', () => {
|
||||
await once(browserWindow.webContents, 'did-create-window');
|
||||
});
|
||||
|
||||
it('reuses an existing window when window.open is called with the same frame name', async () => {
|
||||
let handlerCallCount = 0;
|
||||
browserWindow.webContents.setWindowOpenHandler(() => {
|
||||
handlerCallCount++;
|
||||
return { action: 'allow' };
|
||||
});
|
||||
|
||||
const didCreateWindow = once(browserWindow.webContents, 'did-create-window') as Promise<[BrowserWindow, Electron.DidCreateWindowDetails]>;
|
||||
await browserWindow.webContents.executeJavaScript("window.open('about:blank?one', 'named-target', 'show=no') && true");
|
||||
const [childWindow] = await didCreateWindow;
|
||||
expect(handlerCallCount).to.equal(1);
|
||||
expect(childWindow.webContents.getURL()).to.equal('about:blank?one');
|
||||
|
||||
browserWindow.webContents.on('did-create-window', () => {
|
||||
assert.fail('did-create-window should not fire when reusing a named window');
|
||||
});
|
||||
|
||||
const didNavigate = once(childWindow.webContents, 'did-navigate');
|
||||
const sameWindow = await browserWindow.webContents.executeJavaScript(`
|
||||
(() => {
|
||||
const first = window.open('about:blank?one', 'named-target', 'show=no');
|
||||
const second = window.open('about:blank?two', 'named-target', 'show=no');
|
||||
return first === second;
|
||||
})()
|
||||
`);
|
||||
await didNavigate;
|
||||
|
||||
expect(sameWindow).to.be.true('window.open with matching frame name should return the same window proxy');
|
||||
expect(handlerCallCount).to.equal(1, 'setWindowOpenHandler should not be called when Blink resolves the named target');
|
||||
expect(childWindow.webContents.getURL()).to.equal('about:blank?two');
|
||||
expect(BrowserWindow.getAllWindows()).to.have.lengthOf(2);
|
||||
});
|
||||
|
||||
it('can change webPreferences of child windows', async () => {
|
||||
browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { defaultFontSize: 30 } } }));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user