From 359cb3a740987b35b384d4470a03149af547f1aa Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 18 May 2020 17:13:51 -0700 Subject: [PATCH] fix(oopif): adopt main requests into oopifs (#2284) Main request for an OOPIF starts in the parent session, and the oopif session is create only after the response has been received. Therefore, we should adopt the request after oopif session is created. --- src/chromium/crNetworkManager.ts | 29 ++++++++++++++++++++++--- src/chromium/crPage.ts | 11 +++++----- test/chromium/oopif.spec.js | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/chromium/crNetworkManager.ts b/src/chromium/crNetworkManager.ts index c4c4585ebc..e390348273 100644 --- a/src/chromium/crNetworkManager.ts +++ b/src/chromium/crNetworkManager.ts @@ -28,6 +28,7 @@ import { logError } from '../logger'; export class CRNetworkManager { private _client: CRSession; private _page: Page; + private _parentManager: CRNetworkManager | null; private _requestIdToRequest = new Map(); private _requestIdToRequestWillBeSentEvent = new Map(); private _credentials: {username: string, password: string} | null = null; @@ -37,9 +38,10 @@ export class CRNetworkManager { private _requestIdToRequestPausedEvent = new Map(); private _eventListeners: RegisteredListener[]; - constructor(client: CRSession, page: Page) { + constructor(client: CRSession, page: Page, parentManager: CRNetworkManager | null) { this._client = client; this._page = page; + this._parentManager = parentManager; this._eventListeners = this.instrumentNetworkEvents(client); } @@ -235,7 +237,9 @@ export class CRNetworkManager { } _onLoadingFinished(event: Protocol.Network.loadingFinishedPayload) { - const request = this._requestIdToRequest.get(event.requestId); + let request = this._requestIdToRequest.get(event.requestId); + if (!request) + request = this._maybeAdoptMainRequest(event.requestId); // For certain requestIds we never receive requestWillBeSent event. // @see https://crbug.com/750469 if (!request) @@ -253,7 +257,9 @@ export class CRNetworkManager { } _onLoadingFailed(event: Protocol.Network.loadingFailedPayload) { - const request = this._requestIdToRequest.get(event.requestId); + let request = this._requestIdToRequest.get(event.requestId); + if (!request) + request = this._maybeAdoptMainRequest(event.requestId); // For certain requestIds we never receive requestWillBeSent event. // @see https://crbug.com/750469 if (!request) @@ -267,6 +273,23 @@ export class CRNetworkManager { request.request._setFailureText(event.errorText); this._page._frameManager.requestFailed(request.request, !!event.canceled); } + + private _maybeAdoptMainRequest(requestId: Protocol.Network.RequestId): InterceptableRequest | undefined { + // OOPIF has a main request that starts in the parent session but finishes in the child session. + if (!this._parentManager) + return; + const request = this._parentManager._requestIdToRequest.get(requestId); + // Main requests have matching loaderId and requestId. + if (!request || request._documentId !== requestId) + return; + this._requestIdToRequest.set(requestId, request); + this._parentManager._requestIdToRequest.delete(requestId); + if (request._interceptionId && this._parentManager._attemptedAuthentications.has(request._interceptionId)) { + this._parentManager._attemptedAuthentications.delete(request._interceptionId); + this._attemptedAuthentications.add(request._interceptionId); + } + return request; + } } class InterceptableRequest implements network.RouteDelegate { diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index 1a7cd8e386..31cd76b617 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -65,7 +65,7 @@ export class CRPage implements PageDelegate { this._coverage = new CRCoverage(client, browserContext); this._browserContext = browserContext; this._page = new Page(this, browserContext); - this._mainFrameSession = new FrameSession(this, client, targetId); + this._mainFrameSession = new FrameSession(this, client, targetId, null); this._sessions.set(targetId, this._mainFrameSession); client.once(CRSessionEvents.Disconnected, () => this._page._didDisconnect()); this._pagePromise = this._mainFrameSession._initialize(hasUIWindow).then(() => this._initializedPage = this._page).catch(e => e); @@ -75,7 +75,7 @@ export class CRPage implements PageDelegate { await Promise.all(Array.from(this._sessions.values()).map(frame => cb(frame))); } - private _sessionForFrame(frame: frames.Frame): FrameSession { + _sessionForFrame(frame: frames.Frame): FrameSession { // Frame id equals target id. while (!this._sessions.has(frame._id)) { const parent = frame.parentFrame(); @@ -95,8 +95,9 @@ export class CRPage implements PageDelegate { // Frame id equals target id. const frame = this._page._frameManager.frame(targetId); assert(frame); + const parentSession = this._sessionForFrame(frame); this._page._frameManager.removeChildFramesRecursively(frame); - const frameSession = new FrameSession(this, session, targetId); + const frameSession = new FrameSession(this, session, targetId, parentSession); this._sessions.set(targetId, frameSession); frameSession._initialize(false).catch(e => e); } @@ -330,12 +331,12 @@ class FrameSession { private _firstNonInitialNavigationCommittedReject = (e: Error) => {}; private _windowId: number | undefined; - constructor(crPage: CRPage, client: CRSession, targetId: string) { + constructor(crPage: CRPage, client: CRSession, targetId: string, parentSession: FrameSession | null) { this._client = client; this._crPage = crPage; this._page = crPage._page; this._targetId = targetId; - this._networkManager = new CRNetworkManager(client, this._page); + this._networkManager = new CRNetworkManager(client, this._page, parentSession ? parentSession._networkManager : null); this._firstNonInitialNavigationCommittedPromise = new Promise((f, r) => { this._firstNonInitialNavigationCommittedFulfill = f; this._firstNonInitialNavigationCommittedReject = r; diff --git a/test/chromium/oopif.spec.js b/test/chromium/oopif.spec.js index db916f738f..6f52ee2155 100644 --- a/test/chromium/oopif.spec.js +++ b/test/chromium/oopif.spec.js @@ -151,6 +151,43 @@ describe('OOPIF', function() { await page.goto(server.PREFIX + '/dynamic-oopif.html'); expect(await countOOPIFs(browser)).toBe(1); }); + it('should report main requests', async function({browser, page, server, context}) { + const requestFrames = []; + page.on('request', r => requestFrames.push(r.frame())); + const finishedFrames = []; + page.on('requestfinished', r => finishedFrames.push(r.frame())); + + await page.goto(server.PREFIX + '/empty.html'); + const main = page.mainFrame(); + + await main.evaluate(url => { + const iframe = document.createElement('iframe'); + iframe.src = url; + document.body.appendChild(iframe); + return new Promise(f => iframe.onload = f); + }, server.CROSS_PROCESS_PREFIX + '/empty.html'); + expect(page.frames().length).toBe(2); + const child = main.childFrames()[0]; + await child.waitForLoadState('domcontentloaded'); + + await child.evaluate(url => { + const iframe = document.createElement('iframe'); + iframe.src = url; + document.body.appendChild(iframe); + return new Promise(f => iframe.onload = f); + }, server.PREFIX + '/empty.html'); + expect(page.frames().length).toBe(3); + const grandChild = child.childFrames()[0]; + await grandChild.waitForLoadState('domcontentloaded'); + + expect(await countOOPIFs(browser)).toBe(2); + expect(requestFrames[0]).toBe(main); + expect(finishedFrames[0]).toBe(main); + expect(requestFrames[1]).toBe(child); + expect(finishedFrames[1]).toBe(child); + expect(requestFrames[2]).toBe(grandChild); + expect(finishedFrames[2]).toBe(grandChild); + }); // @see https://github.com/microsoft/playwright/issues/1240 it('should click a button when it overlays oopif', async function({browser, page, server, context}) { await page.goto(server.PREFIX + '/button-overlay-oopif.html');