Skip to content

Commit 044ebd7

Browse files
yury-sdgozman
authored andcommitted
fix: delete contexts from the map on navigation (#602)
1 parent ac2ba3c commit 044ebd7

File tree

3 files changed

+36
-7
lines changed

3 files changed

+36
-7
lines changed

src/firefox/ffPage.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export class FFPage implements PageDelegate {
5252
this._contextIdToContext = new Map();
5353
this._page = new Page(this, browserContext);
5454
this._networkManager = new FFNetworkManager(session, this._page);
55+
this._page.on(Events.Page.FrameDetached, frame => this._removeContextsForFrame(frame));
5556
this._eventListeners = [
5657
helper.addEventListener(this._session, 'Page.eventFired', this._onEventFired.bind(this)),
5758
helper.addEventListener(this._session, 'Page.frameAttached', this._onFrameAttached.bind(this)),
@@ -122,6 +123,13 @@ export class FFPage implements PageDelegate {
122123
context.frame._contextDestroyed(context as dom.FrameExecutionContext);
123124
}
124125

126+
private _removeContextsForFrame(frame: frames.Frame) {
127+
for (const [contextId, context] of this._contextIdToContext) {
128+
if (context.frame === frame)
129+
this._contextIdToContext.delete(contextId);
130+
}
131+
}
132+
125133
_onNavigationStarted() {
126134
}
127135

src/webkit/wkPage.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export class WKPage implements PageDelegate {
5959
this._page = new Page(this, browserContext);
6060
this._workers = new WKWorkers(this._page);
6161
this._session = undefined as any as WKSession;
62+
this._page.on(Events.Page.FrameDetached, frame => this._removeContextsForFrame(frame, false));
6263
}
6364

6465
private async _initializePageProxySession() {
@@ -248,13 +249,8 @@ export class WKPage implements PageDelegate {
248249

249250
private _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
250251
const frame = this._page._frameManager.frame(framePayload.id);
251-
for (const [contextId, context] of this._contextIdToContext) {
252-
if (context.frame === frame) {
253-
(context._delegate as WKExecutionContext)._dispose();
254-
this._contextIdToContext.delete(contextId);
255-
frame._contextDestroyed(context);
256-
}
257-
}
252+
assert(frame);
253+
this._removeContextsForFrame(frame!, true);
258254
if (!framePayload.parentId)
259255
this._workers.clear();
260256
this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
@@ -268,6 +264,17 @@ export class WKPage implements PageDelegate {
268264
this._page._frameManager.frameDetached(frameId);
269265
}
270266

267+
private _removeContextsForFrame(frame: frames.Frame, notifyFrame: boolean) {
268+
for (const [contextId, context] of this._contextIdToContext) {
269+
if (context.frame === frame) {
270+
(context._delegate as WKExecutionContext)._dispose();
271+
this._contextIdToContext.delete(contextId);
272+
if (notifyFrame)
273+
frame._contextDestroyed(context);
274+
}
275+
}
276+
}
277+
271278
private _onExecutionContextCreated(contextPayload : Protocol.Runtime.ExecutionContextDescription) {
272279
if (this._contextIdToContext.has(contextPayload.id))
273280
return;

test/evaluation.spec.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,20 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
331331
expect(await page.frames()[0].evaluate(() => document.body.textContent.trim())).toBe('');
332332
expect(await page.frames()[1].evaluate(() => document.body.textContent.trim())).toBe(`Hi, I'm frame`);
333333
});
334+
it('should dispose context on navigation', async({page, server}) => {
335+
await page.goto(server.PREFIX + '/frames/one-frame.html');
336+
expect(page.frames().length).toBe(2);
337+
expect(page._delegate._contextIdToContext.size).toBe(4);
338+
await page.goto(server.EMPTY_PAGE);
339+
expect(page._delegate._contextIdToContext.size).toBe(2);
340+
});
341+
it('should dispose context on cross-origin navigation', async({page, server}) => {
342+
await page.goto(server.PREFIX + '/frames/one-frame.html');
343+
expect(page.frames().length).toBe(2);
344+
expect(page._delegate._contextIdToContext.size).toBe(4);
345+
await page.goto(server.CROSS_PROCESS_PREFIX + '/empty.html');
346+
expect(page._delegate._contextIdToContext.size).toBe(2);
347+
});
334348
it('should execute after cross-site navigation', async({page, server}) => {
335349
await page.goto(server.EMPTY_PAGE);
336350
const mainFrame = page.mainFrame();

0 commit comments

Comments
 (0)