From 9697ad635fc67cce667dc14d658fcdedba9ede47 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 11 Aug 2020 08:59:00 -0700 Subject: [PATCH] fix(chromium): handle the case when new pending comes before old commit (#3370) --- src/frames.ts | 35 +++++++++++++++++++++++++++-------- test/proxy.spec.js | 13 ++++++++++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/frames.ts b/src/frames.ts index 15d35164b3..1d94cfc170 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -20,7 +20,7 @@ import * as util from 'util'; import { ConsoleMessage } from './console'; import * as dom from './dom'; import { Events } from './events'; -import { assert, helper, RegisteredListener, assertMaxArguments, debugAssert } from './helper'; +import { assert, helper, RegisteredListener, assertMaxArguments } from './helper'; import * as js from './javascript'; import * as network from './network'; import { Page } from './page'; @@ -172,14 +172,31 @@ export class FrameManager { this.removeChildFramesRecursively(frame); frame._url = url; frame._name = name; - if (frame._pendingDocument && frame._pendingDocument.documentId === undefined) - frame._pendingDocument.documentId = documentId; - debugAssert(!frame._pendingDocument || frame._pendingDocument.documentId === documentId); - if (frame._pendingDocument && frame._pendingDocument.documentId === documentId) - frame._currentDocument = frame._pendingDocument; - else + + let keepPending: DocumentInfo | undefined; + if (frame._pendingDocument) { + if (frame._pendingDocument.documentId === undefined) { + // Pending with unknown documentId - assume it is the one being committed. + frame._pendingDocument.documentId = documentId; + } + if (frame._pendingDocument.documentId === documentId) { + // Committing a pending document. + frame._currentDocument = frame._pendingDocument; + } else { + // Sometimes, we already have a new pending when the old one commits. + // An example would be Chromium error page followed by a new navigation request, + // where the error page commit arrives after Network.requestWillBeSent for the + // new navigation. + // We commit, but keep the pending request since it's not done yet. + keepPending = frame._pendingDocument; + frame._currentDocument = { documentId, request: undefined }; + } + frame._pendingDocument = undefined; + } else { + // No pending - just commit a new document. frame._currentDocument = { documentId, request: undefined }; - frame._pendingDocument = undefined; + } + frame._onClearLifecycle(); const navigationEvent: NavigationEvent = { url, name, newDocument: frame._currentDocument }; frame._eventEmitter.emit(kNavigationEvent, navigationEvent); @@ -187,6 +204,8 @@ export class FrameManager { this._page._logger.info(` navigated to "${url}"`); this._page.emit(Events.Page.FrameNavigated, frame); } + // Restore pending if any - see comments above about keepPending. + frame._pendingDocument = keepPending; } frameCommittedSameDocumentNavigation(frameId: string, url: string) { diff --git a/test/proxy.spec.js b/test/proxy.spec.js index 12582a41ea..96b2529168 100644 --- a/test/proxy.spec.js +++ b/test/proxy.spec.js @@ -17,7 +17,7 @@ require('./base.fixture'); const socks = require('socksv5'); const utils = require('./utils'); -const {FFOX, CHROMIUM, WEBKIT, MAC} = testOptions; +const {FFOX, CHROMIUM, WEBKIT, MAC, HEADLESS} = testOptions; it('should use proxy', async ({browserType, defaultBrowserOptions, server}) => { server.setRoute('/target.html', async (req, res) => { @@ -40,7 +40,7 @@ it('should authenticate', async ({browserType, defaultBrowserOptions, server}) = res.writeHead(407, 'Proxy Authentication Required', { 'Proxy-Authenticate': 'Basic realm="Access to internal site"' }); - res.end(); + res.end(); } else { res.end(`${auth}`); } @@ -55,7 +55,8 @@ it('should authenticate', async ({browserType, defaultBrowserOptions, server}) = await browser.close(); }); -it('should exclude patterns', async ({browserType, defaultBrowserOptions, server}) => { +it.fail(CHROMIUM && !HEADLESS)('should exclude patterns', async ({browserType, defaultBrowserOptions, server}) => { + // Chromium headful crashes with CHECK(!in_frame_tree_) in RenderFrameImpl::OnDeleteFrame. server.setRoute('/target.html', async (req, res) => { res.end('Served by the proxy'); }); @@ -83,6 +84,12 @@ it('should exclude patterns', async ({browserType, defaultBrowserOptions, server expect(error.message).toBeTruthy(); } + if (CHROMIUM) { + // Should successfully navigate to the error page. + await page.waitForEvent('framenavigated', frame => frame.url() === 'chrome-error://chromewebdata/'); + expect(page.url()).toBe('chrome-error://chromewebdata/'); + } + await browser.close(); });