From 89b5d2f7be1aeae5b55d6cfddc2018a6025613f0 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 27 Jan 2020 16:51:52 -0800 Subject: [PATCH] fix(setContent): manually reset lifecycyle for all browsers at the right moment (#679) --- src/chromium/crPage.ts | 10 +------ src/firefox/ffPage.ts | 4 --- src/frames.ts | 59 ++++++++++++++++++++++++++---------------- src/page.ts | 10 +++---- src/webkit/wkPage.ts | 14 +++++----- test/page.spec.js | 3 +-- 6 files changed, 51 insertions(+), 49 deletions(-) diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index a1b8c6e2c6..578ce07966 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -130,16 +130,8 @@ export class CRPage implements PageDelegate { return { newDocumentId: response.loaderId, isSameDocument: !response.loaderId }; } - needsLifecycleResetOnSetContent(): boolean { - // We rely upon the fact that document.open() will reset frame lifecycle with "init" - // lifecycle event. @see https://crrev.com/608658 - return false; - } - _onLifecycleEvent(event: Protocol.Page.lifecycleEventPayload) { - if (event.name === 'init') - this._page._frameManager.frameLifecycleEvent(event.frameId, 'clear'); - else if (event.name === 'load') + if (event.name === 'load') this._page._frameManager.frameLifecycleEvent(event.frameId, 'load'); else if (event.name === 'DOMContentLoaded') this._page._frameManager.frameLifecycleEvent(event.frameId, 'domcontentloaded'); diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 726acc930a..7b53ee3797 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -260,10 +260,6 @@ export class FFPage implements PageDelegate { return { newDocumentId: response.navigationId || undefined, isSameDocument: !response.navigationId }; } - needsLifecycleResetOnSetContent(): boolean { - return true; - } - async setExtraHTTPHeaders(headers: network.Headers): Promise { const array = []; for (const [name, value] of Object.entries(headers)) diff --git a/src/frames.ts b/src/frames.ts index fee0864ff1..6bbe0f6b3b 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -54,6 +54,7 @@ export type LifecycleEvent = 'load' | 'domcontentloaded' | 'networkidle0' | 'net const kLifecycleEvents: Set = new Set(['load', 'domcontentloaded', 'networkidle0', 'networkidle2']); export type WaitForOptions = types.TimeoutOptions & { waitFor?: types.Visibility | 'nowait' }; +type ConsoleTagHandler = () => void; export class FrameManager { private _page: Page; @@ -61,6 +62,7 @@ export class FrameManager { private _webSockets = new Map(); private _mainFrame: Frame; readonly _lifecycleWatchers = new Set(); + readonly _consoleMessageTags = new Map(); constructor(page: Page) { this._page = page; @@ -116,8 +118,7 @@ export class FrameManager { frame._url = url; frame._name = name; frame._lastDocumentId = documentId; - this.frameLifecycleEvent(frameId, 'clear'); - this.clearInflightRequests(frame); + this.clearFrameLifecycle(frame); this.clearWebSockets(frame); if (!initial) { for (const watcher of this._lifecycleWatchers) @@ -158,24 +159,21 @@ export class FrameManager { this._page.emit(Events.Page.Load); } - frameLifecycleEvent(frameId: string, event: LifecycleEvent | 'clear') { + frameLifecycleEvent(frameId: string, event: LifecycleEvent) { const frame = this._frames.get(frameId); if (!frame) return; - if (event === 'clear') { - frame._firedLifecycleEvents.clear(); - } else { - frame._firedLifecycleEvents.add(event); - for (const watcher of this._lifecycleWatchers) - watcher._onLifecycleEvent(frame); - } + frame._firedLifecycleEvents.add(event); + for (const watcher of this._lifecycleWatchers) + watcher._onLifecycleEvent(frame); if (frame === this._mainFrame && event === 'load') this._page.emit(Events.Page.Load); if (frame === this._mainFrame && event === 'domcontentloaded') this._page.emit(Events.Page.DOMContentLoaded); } - clearInflightRequests(frame: Frame) { + clearFrameLifecycle(frame: Frame) { + frame._firedLifecycleEvents.clear(); // Keep the current navigation request if any. frame._inflightRequests = new Set(Array.from(frame._inflightRequests).filter(request => request._documentId === frame._lastDocumentId)); this._stopNetworkIdleTimer(frame, 'networkidle0'); @@ -330,6 +328,18 @@ export class FrameManager { clearTimeout(timeoutId); frame._networkIdleTimers.delete(event); } + + interceptConsoleMessage(message: ConsoleMessage): boolean { + if (message.type() !== 'debug') + return false; + const tag = message.text(); + const handler = this._consoleMessageTags.get(tag); + if (!handler) + return false; + this._consoleMessageTags.delete(tag); + handler(); + return true; + } } export class Frame { @@ -345,6 +355,7 @@ export class Frame { _name = ''; _inflightRequests = new Set(); readonly _networkIdleTimers = new Map(); + private _setContentCounter = 0; constructor(page: Page, id: string, parentFrame: Frame | null) { this._id = id; @@ -510,23 +521,27 @@ export class Frame { } async setContent(html: string, options?: NavigateOptions): Promise { + const tag = `--playwright--set--content--${this._id}--${++this._setContentCounter}--`; const context = await this._utilityContext(); - if (this._page._delegate.needsLifecycleResetOnSetContent()) { - this._page._frameManager.frameLifecycleEvent(this._id, 'clear'); - this._page._frameManager.clearInflightRequests(this); - } - await context.evaluate(html => { + let watcher: LifecycleWatcher; + this._page._frameManager._consoleMessageTags.set(tag, () => { + // Clear lifecycle right after document.open() - see 'tag' below. + this._page._frameManager.clearFrameLifecycle(this); + watcher = new LifecycleWatcher(this, options, false /* supportUrlMatch */); + }); + await context.evaluate((html, tag) => { window.stop(); document.open(); + console.debug(tag); // eslint-disable-line no-console document.write(html); document.close(); - }, html); - const watcher = new LifecycleWatcher(this, options, false /* supportUrlMatch */); + }, html, tag); + assert(watcher!, 'Was not able to clear lifecycle in setContent'); const error = await Promise.race([ - watcher.timeoutOrTerminationPromise, - watcher.lifecyclePromise, + watcher!.timeoutOrTerminationPromise, + watcher!.lifecyclePromise, ]); - watcher.dispose(); + watcher!.dispose(); if (error) throw error; } @@ -1092,4 +1107,4 @@ function selectorToString(selector: string, visibility: types.Visibility): strin label = ''; break; } return `${label}${selector}`; -} \ No newline at end of file +} diff --git a/src/page.ts b/src/page.ts index 84a3a551e6..8838e87178 100644 --- a/src/page.ts +++ b/src/page.ts @@ -43,7 +43,6 @@ export interface PageDelegate { closePage(runBeforeUnload: boolean): Promise; navigateFrame(frame: frames.Frame, url: string, referrer: string | undefined): Promise; - needsLifecycleResetOnSetContent(): boolean; setExtraHTTPHeaders(extraHTTPHeaders: network.Headers): Promise; setViewport(viewport: types.Viewport): Promise; @@ -296,11 +295,12 @@ export class Page extends platform.EventEmitter { } _addConsoleMessage(type: string, args: js.JSHandle[], location: ConsoleMessageLocation, text?: string) { - if (!this.listenerCount(Events.Page.Console)) { + const message = new ConsoleMessage(type, text, args, location); + const intercepted = this._frameManager.interceptConsoleMessage(message); + if (intercepted || !this.listenerCount(Events.Page.Console)) args.forEach(arg => arg.dispose()); - return; - } - this.emit(Events.Page.Console, new ConsoleMessage(type, text, args, location)); + else + this.emit(Events.Page.Console, message); } url(): string { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 730ff3301a..be190ff457 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -48,6 +48,7 @@ export class WKPage implements PageDelegate { private readonly _requestIdToRequest = new Map(); private readonly _workers: WKWorkers; private readonly _contextIdToContext: Map; + private _mainFrameContextId?: number; private _sessionListeners: RegisteredListener[] = []; private readonly _bootstrapScripts: string[] = []; @@ -287,6 +288,8 @@ export class WKPage implements PageDelegate { frame._contextCreated('main', context); else if (contextPayload.name === UTILITY_WORLD_NAME) frame._contextCreated('utility', context); + if (contextPayload.isPageContext && frame === this._page.mainFrame()) + this._mainFrameContextId = contextPayload.id; this._contextIdToContext.set(contextPayload.id, context); } @@ -298,11 +301,9 @@ export class WKPage implements PageDelegate { return { newDocumentId: result.loaderId, isSameDocument: !result.loaderId }; } - needsLifecycleResetOnSetContent(): boolean { - return true; - } - - private async _onConsoleMessage(event: Protocol.Console.messageAddedPayload) { + private _onConsoleMessage(event: Protocol.Console.messageAddedPayload) { + // Note: do no introduce await in this function, otherwise we lose the ordering. + // For example, frame.setContent relies on this. const { type, level, text, parameters, url, line: lineNumber, column: columnNumber, source } = event.message; if (level === 'debug' && parameters && parameters[0].value === BINDING_CALL_MESSAGE) { const parsedObjectId = JSON.parse(parameters[1].objectId!); @@ -323,14 +324,13 @@ export class WKPage implements PageDelegate { else if (type === 'timing') derivedType = 'timeEnd'; - const mainFrameContext = await this._page.mainFrame()._mainContext(); const handles = (parameters || []).map(p => { let context: dom.FrameExecutionContext | null = null; if (p.objectId) { const objectId = JSON.parse(p.objectId); context = this._contextIdToContext.get(objectId.injectedScriptId)!; } else { - context = mainFrameContext; + context = this._contextIdToContext.get(this._mainFrameContextId!)!; } return context._createHandle(p); }); diff --git a/test/page.spec.js b/test/page.spec.js index 5c0bf4513d..f5597bdb19 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -534,8 +534,7 @@ module.exports.describe = function({testRunner, expect, headless, playwright, FF const result = await page.content(); expect(result).toBe(expectedOutput); }); - it.skip(FFOX || WEBKIT)('should not confuse with previous navigation', async({page, server}) => { - // TODO: ffox and webkit lack 'init' lifecycle event. + it('should not confuse with previous navigation', async({page, server}) => { const imgPath = '/img.png'; let imgResponse = null; server.setRoute(imgPath, (req, res) => imgResponse = res);