diff --git a/package.json b/package.json index 310341cb03..3c140737d9 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "main": "index.js", "playwright": { "chromium_revision": "747023", - "firefox_revision": "1032", + "firefox_revision": "1035", "webkit_revision": "1168" }, "scripts": { diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index f76686d68e..d2d47303f5 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -60,10 +60,11 @@ export class FFPage implements PageDelegate { helper.addEventListener(this._session, 'Page.frameDetached', this._onFrameDetached.bind(this)), helper.addEventListener(this._session, 'Page.navigationAborted', this._onNavigationAborted.bind(this)), helper.addEventListener(this._session, 'Page.navigationCommitted', this._onNavigationCommitted.bind(this)), - helper.addEventListener(this._session, 'Page.navigationStarted', this._onNavigationStarted.bind(this)), + helper.addEventListener(this._session, 'Page.navigationStarted', event => this._onNavigationStarted(event.frameId)), helper.addEventListener(this._session, 'Page.sameDocumentNavigation', this._onSameDocumentNavigation.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextCreated', this._onExecutionContextCreated.bind(this)), helper.addEventListener(this._session, 'Runtime.executionContextDestroyed', this._onExecutionContextDestroyed.bind(this)), + helper.addEventListener(this._session, 'Page.linkClicked', event => this._onLinkClicked(event.phase)), helper.addEventListener(this._session, 'Page.uncaughtError', this._onUncaughtError.bind(this)), helper.addEventListener(this._session, 'Runtime.console', this._onConsole.bind(this)), helper.addEventListener(this._session, 'Page.dialogOpened', this._onDialogOpened.bind(this)), @@ -116,7 +117,15 @@ export class FFPage implements PageDelegate { } } - _onNavigationStarted() { + _onLinkClicked(phase: 'before' | 'after') { + if (phase === 'before') + this._page._frameManager.frameWillPotentiallyRequestNavigation(); + else + this._page._frameManager.frameDidPotentiallyRequestNavigation(); + } + + _onNavigationStarted(frameId: string) { + this._page._frameManager.frameRequestedNavigation(frameId); } _onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) { diff --git a/src/frames.ts b/src/frames.ts index 9c9227dad3..a12612ef9f 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -59,7 +59,7 @@ export class FrameManager { private _mainFrame: Frame; readonly _lifecycleWatchers = new Set<() => void>(); readonly _consoleMessageTags = new Map(); - private _navigationRequestCollectors = new Set>(); + private _pendingNavigationBarriers = new Set(); constructor(page: Page) { this._page = page; @@ -108,34 +108,39 @@ export class FrameManager { } } - async waitForNavigationsCreatedBy(action: () => Promise): Promise { - const frameIds = new Set(); - this._navigationRequestCollectors.add(frameIds); + async waitForNavigationsCreatedBy(action: () => Promise, options?: WaitForNavigationOptions): Promise { + const barrier = new PendingNavigationBarrier(options); + this._pendingNavigationBarriers.add(barrier); try { const result = await action(); - if (!frameIds.size) - return result; - const frames = Array.from(frameIds.values()).map(frameId => this._frames.get(frameId)); - await Promise.all(frames.map(frame => frame!.waitForNavigation({ waitUntil: []}))).catch(e => {}); + await barrier.waitFor(); + // Resolve in the next task, after all waitForNavigations. await new Promise(platform.makeWaitForNextTask()); return result; } finally { - this._navigationRequestCollectors.delete(frameIds); + this._pendingNavigationBarriers.delete(barrier); } } - frameRequestedNavigation(frameId: string) { - for (const frameIds of this._navigationRequestCollectors) - frameIds.add(frameId); + frameWillPotentiallyRequestNavigation() { + for (const barrier of this._pendingNavigationBarriers) + barrier.retain(); } - _cancelFrameRequestedNavigation(frameId: string) { - for (const frameIds of this._navigationRequestCollectors) - frameIds.delete(frameId); + frameDidPotentiallyRequestNavigation() { + for (const barrier of this._pendingNavigationBarriers) + barrier.release(); + } + + frameRequestedNavigation(frameId: string) { + const frame = this._frames.get(frameId); + if (!frame) + return; + for (const barrier of this._pendingNavigationBarriers) + barrier.addFrame(frame); } frameCommittedNewDocumentNavigation(frameId: string, url: string, name: string, documentId: string, initial: boolean) { - this._cancelFrameRequestedNavigation(frameId); const frame = this._frames.get(frameId)!; for (const child of frame.childFrames()) this._removeFramesRecursively(child); @@ -150,7 +155,6 @@ export class FrameManager { } frameCommittedSameDocumentNavigation(frameId: string, url: string) { - this._cancelFrameRequestedNavigation(frameId); const frame = this._frames.get(frameId); if (!frame) return; @@ -235,7 +239,6 @@ export class FrameManager { if (request._documentId && frame) { const isCurrentDocument = frame._lastDocumentId === request._documentId; if (!isCurrentDocument) { - this._cancelFrameRequestedNavigation(frame._id); let errorText = request.failure()!.errorText; if (canceled) errorText += '; maybe frame was detached?'; @@ -248,13 +251,11 @@ export class FrameManager { } provisionalLoadFailed(frame: Frame, documentId: string, error: string) { - this._cancelFrameRequestedNavigation(frame._id); for (const watcher of frame._documentWatchers) watcher(documentId, new Error(error)); } private _removeFramesRecursively(frame: Frame) { - this._cancelFrameRequestedNavigation(frame._id); for (const child of frame.childFrames()) this._removeFramesRecursively(child); frame._onDetached(); @@ -1105,3 +1106,42 @@ function selectorToString(selector: string, visibility: types.Visibility): strin } return `${label}${selector}`; } + +class PendingNavigationBarrier { + private _frameIds = new Map(); + private _waitOptions: WaitForNavigationOptions | undefined; + private _protectCount = 0; + private _promise: Promise; + private _promiseCallback = () => {}; + + constructor(options?: WaitForNavigationOptions) { + this._waitOptions = options; + this._promise = new Promise(f => this._promiseCallback = f); + this.retain(); + } + + waitFor(): Promise { + this.release(); + return this._promise; + } + + async addFrame(frame: Frame) { + this.retain(); + await frame.waitForNavigation(this._waitOptions).catch(e => {}); + this.release(); + } + + retain() { + ++this._protectCount; + } + + release() { + --this._protectCount; + this._maybeResolve(); + } + + private async _maybeResolve() { + if (!this._protectCount && !this._frameIds.size) + this._promiseCallback(); + } +} diff --git a/src/platform.ts b/src/platform.ts index 97200f0db9..36ccb12273 100644 --- a/src/platform.ts +++ b/src/platform.ts @@ -277,7 +277,9 @@ export function fetchUrl(url: string): Promise { // See https://joel.tools/microtasks/ export function makeWaitForNextTask() { - assert(isNode, 'Waitng for the next task is only supported in nodejs'); + if (!isNode) + return (func: () => void) => setTimeout(func, 0); + if (parseInt(process.versions.node, 10) >= 11) return setImmediate; diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js index 46ff5a178e..65fb03b415 100644 --- a/test/browsercontext.spec.js +++ b/test/browsercontext.spec.js @@ -344,7 +344,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF expect(error.message).toBe('Function "baz" has been already registered in one of the pages'); await context.close(); }); - it.skip(FFOX)('should be callable from-inside addInitScript', async({browser, server}) => { + it.fail(FFOX)('should be callable from-inside addInitScript', async({browser, server}) => { const context = await browser.newContext(); let args = []; await context.exposeFunction('woof', function(arg) { diff --git a/test/navigation.spec.js b/test/navigation.spec.js index efed78ba1f..b08ee74dd6 100644 --- a/test/navigation.spec.js +++ b/test/navigation.spec.js @@ -561,7 +561,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF return page.setContent(``, { waitUntil: 'networkidle2' }); }, true); }); - it.skip(FFOX)('should wait for networkidle0 in setContent with request from previous navigation', async({page, server}) => { + it.fail(FFOX)('should wait for networkidle0 in setContent with request from previous navigation', async({page, server}) => { // TODO: in Firefox window.stop() does not cancel outstanding requests, and we also lack 'init' lifecycle, // therefore we don't clear inflight requests at the right time. await page.goto(server.EMPTY_PAGE); @@ -571,7 +571,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF return page.setContent(``, { waitUntil: 'networkidle0' }); }, true); }); - it.skip(FFOX)('should wait for networkidle2 in setContent with request from previous navigation', async({page, server}) => { + it.fail(FFOX)('should wait for networkidle2 in setContent with request from previous navigation', async({page, server}) => { // TODO: in Firefox window.stop() does not cancel outstanding requests, and we also lack 'init' lifecycle, // therefore we don't clear inflight requests at the right time. await page.goto(server.EMPTY_PAGE); @@ -800,7 +800,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF }); }); - describe.fail(FFOX)('Page.automaticWaiting', () => { + describe('Page.automaticWaiting', () => { it('clicking anchor should await navigation', async({page, server}) => { const messages = []; server.setRoute('/empty.html', async (req, res) => { @@ -869,7 +869,7 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF ]); expect(messages.join('|')).toBe('route|waitForNavigation|click'); }); - it('assigning to location should await navigation', async({page, server}) => { + it('assigning location should await navigation', async({page, server}) => { const messages = []; server.setRoute('/empty.html', async (req, res) => { messages.push('route'); @@ -881,12 +881,32 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF ]); expect(messages.join('|')).toBe('route|waitForNavigation|evaluate'); }); - it('should await navigating specified target', async({page, server, httpsServer}) => { + it.fail(CHROMIUM)('assigning location twice should await navigation', async({page, server}) => { const messages = []; - server.setRoute('/empty.html', async (req, res) => { - messages.push('route'); - res.end('done'); - }); + server.setRoute('/empty.html?cancel', async (req, res) => { messages.push('routecancel'); res.end('done'); }); + server.setRoute('/empty.html?override', async (req, res) => { messages.push('routeoverride'); res.end('done'); }); + await Promise.all([ + page.evaluate(` + window.location.href = "${server.EMPTY_PAGE}?cancel"; + window.location.href = "${server.EMPTY_PAGE}?override"; + `).then(() => messages.push('evaluate')), + ]); + expect(messages.join('|')).toBe('routeoverride|evaluate'); + }); + it('evaluating reload should await navigation', async({page, server}) => { + const messages = []; + await page.goto(server.EMPTY_PAGE); + server.setRoute('/empty.html', async (req, res) => { messages.push('route'); res.end('done'); }); + + await Promise.all([ + page.evaluate(`window.location.reload()`).then(() => messages.push('evaluate')), + page.waitForNavigation({ waitUntil: [] }).then(() => messages.push('waitForNavigation')), + ]); + expect(messages.join('|')).toBe('route|waitForNavigation|evaluate'); + }); + it('should await navigating specified target', async({page, server}) => { + const messages = []; + server.setRoute('/empty.html', async (req, res) => { messages.push('route'); res.end('done'); }); await page.setContent(` empty.html @@ -902,25 +922,39 @@ module.exports.describe = function({testRunner, expect, playwright, MAC, WIN, FF }); }); - describe('Page.automaticWaiting should not hang', () => { - it('should not throw when clicking on links which do not commit navigation', async({page, server, httpsServer}) => { + describe('Page.automaticWaiting should not hang when', () => { + it('clicking on links which do not commit navigation', async({page, server, httpsServer}) => { await page.goto(server.EMPTY_PAGE); await page.setContent(`foobar`); await page.click('a'); }); - it('should not throw when clicking on download link', async({page, server, httpsServer}) => { + it('clicking on download link', async({page, server, httpsServer}) => { await page.setContent(`table2.wasm`); await page.click('a'); }); - it('should not hang on window.stop', async({page, server, httpsServer}) => { + it('window.stop async', async({page, server, httpsServer}) => { server.setRoute('/empty.html', async (req, res) => {}); - await Promise.all([ - page.waitForNavigation().catch(e => {}), - page.evaluate((url) => { + await page.evaluate((url) => { window.location.href = url; setTimeout(() => window.stop(), 100); - }, server.EMPTY_PAGE) - ]); + }, server.EMPTY_PAGE); + }); + it.fail(WEBKIT)('window.stop sync', async({page, server, httpsServer}) => { + server.setRoute('/empty.html', async (req, res) => {}); + await page.evaluate((url) => { + window.location.href = url; + window.stop(); + }, server.EMPTY_PAGE); + }); + it('assigning location to about:blank', async({page, server}) => { + await page.goto(server.EMPTY_PAGE); + await page.evaluate(`window.location.href = "about:blank";`); + }); + it('assigning location to about:blank after non-about:blank', async({page, server}) => { + server.setRoute('/empty.html', async (req, res) => {}); + await page.evaluate(` + window.location.href = "${server.EMPTY_PAGE}"; + window.location.href = "about:blank";`); }); });