diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index ca53f8399b..5da8b1c9f1 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -435,7 +435,6 @@ class FrameSession { eventsHelper.addEventListener(this._client, 'Page.frameDetached', event => this._onFrameDetached(event.frameId, event.reason)), eventsHelper.addEventListener(this._client, 'Page.frameNavigated', event => this._onFrameNavigated(event.frame, false)), eventsHelper.addEventListener(this._client, 'Page.frameRequestedNavigation', event => this._onFrameRequestedNavigation(event)), - eventsHelper.addEventListener(this._client, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)), eventsHelper.addEventListener(this._client, 'Page.javascriptDialogOpening', event => this._onDialog(event)), eventsHelper.addEventListener(this._client, 'Page.navigatedWithinDocument', event => this._onFrameNavigatedWithinDocument(event.frameId, event.url)), eventsHelper.addEventListener(this._client, 'Runtime.bindingCalled', event => this._onBindingCalled(event)), @@ -601,12 +600,6 @@ class FrameSession { this._page._frameManager.frameLifecycleEvent(event.frameId, 'domcontentloaded'); } - _onFrameStoppedLoading(frameId: string) { - if (this._eventBelongsToStaleFrame(frameId)) - return; - this._page._frameManager.frameStoppedLoading(frameId); - } - _handleFrameTree(frameTree: Protocol.Page.FrameTree) { this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId || null); this._onFrameNavigated(frameTree.frame, true); diff --git a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts index 4f69b42d38..a1f984c312 100644 --- a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts @@ -48,7 +48,7 @@ export class FrameDispatcher extends Dispatcher { diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index cf2a8afd83..be8c5ca224 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -62,6 +62,8 @@ export type GotoResult = { type ConsoleTagHandler = () => void; +type RegularLifecycleEvent = Exclude; + export type FunctionWithSource = (source: { context: BrowserContext, page: Page, frame: Frame}, ...args: any) => any; export type NavigationEvent = { @@ -279,17 +281,11 @@ export class FrameManager { const frame = this._frames.get(frameId); if (frame) { this._removeFramesRecursively(frame); - // Recalculate subtree lifecycle for the whole tree - it should not be that big. - this._page.mainFrame()._recalculateLifecycle(); + this._page.mainFrame()._recalculateNetworkIdle(); } } - frameStoppedLoading(frameId: string) { - this.frameLifecycleEvent(frameId, 'domcontentloaded'); - this.frameLifecycleEvent(frameId, 'load'); - } - - frameLifecycleEvent(frameId: string, event: types.LifecycleEvent) { + frameLifecycleEvent(frameId: string, event: RegularLifecycleEvent) { const frame = this._frames.get(frameId); if (frame) frame._onLifecycleEvent(event); @@ -478,8 +474,8 @@ export class Frame extends SdkObject { }; _id: string; - private _firedLifecycleEvents = new Set(); - _subtreeLifecycleEvents = new Set(); + _firedLifecycleEvents = new Set(); + private _firedNetworkIdleSelf = false; _currentDocument: DocumentInfo; private _pendingDocument: DocumentInfo | undefined; readonly _page: Page; @@ -516,7 +512,6 @@ export class Frame extends SdkObject { this._parentFrame._childFrames.add(this); this._firedLifecycleEvents.add('commit'); - this._subtreeLifecycleEvents.add('commit'); if (id !== kDummyFrameId) this._startNetworkIdleTimer(); } @@ -525,23 +520,26 @@ export class Frame extends SdkObject { return this._detached; } - _onLifecycleEvent(event: types.LifecycleEvent) { + _onLifecycleEvent(event: RegularLifecycleEvent) { if (this._firedLifecycleEvents.has(event)) return; this._firedLifecycleEvents.add(event); - // Recalculate subtree lifecycle for the whole tree - it should not be that big. - this._page.mainFrame()._recalculateLifecycle(); + this.emit(Frame.Events.AddLifecycle, event); + if (this === this._page.mainFrame() && this._url !== 'about:blank') + debugLogger.log('api', ` "${event}" event fired`); + this._page.mainFrame()._recalculateNetworkIdle(); } _onClearLifecycle() { + for (const event of this._firedLifecycleEvents) + this.emit(Frame.Events.RemoveLifecycle, event); this._firedLifecycleEvents.clear(); - // Recalculate subtree lifecycle for the whole tree - it should not be that big. - this._page.mainFrame()._recalculateLifecycle(this); // Keep the current navigation request if any. this._inflightRequests = new Set(Array.from(this._inflightRequests).filter(request => request === this._currentDocument.request)); this._stopNetworkIdleTimer(); if (this._inflightRequests.size === 0) this._startNetworkIdleTimer(); + this._page.mainFrame()._recalculateNetworkIdle(this); this._onLifecycleEvent('commit'); } @@ -599,37 +597,26 @@ export class Frame extends SdkObject { }); } - _recalculateLifecycle(frameThatAllowsRemovingLifecycleEvents?: Frame) { - const events = new Set(this._firedLifecycleEvents); + _recalculateNetworkIdle(frameThatAllowsRemovingNetworkIdle?: Frame) { + let isNetworkIdle = this._firedNetworkIdleSelf; for (const child of this._childFrames) { - child._recalculateLifecycle(frameThatAllowsRemovingLifecycleEvents); - // We require a particular lifecycle event to be fired in the whole - // frame subtree, and then consider it done. - for (const event of events) { - if (!child._subtreeLifecycleEvents.has(event)) - events.delete(event); - } + child._recalculateNetworkIdle(frameThatAllowsRemovingNetworkIdle); + // We require networkidle event to be fired in the whole frame subtree, and then consider it done. + if (!child._firedLifecycleEvents.has('networkidle')) + isNetworkIdle = false; } - if (frameThatAllowsRemovingLifecycleEvents !== this) { - // Usually, lifecycle events are fired once and not removed after that, so we keep existing ones. + if (isNetworkIdle && !this._firedLifecycleEvents.has('networkidle')) { + this._firedLifecycleEvents.add('networkidle'); + this.emit(Frame.Events.AddLifecycle, 'networkidle'); + if (this === this._page.mainFrame() && this._url !== 'about:blank') + debugLogger.log('api', ` "networkidle" event fired`); + } + if (frameThatAllowsRemovingNetworkIdle !== this && this._firedLifecycleEvents.has('networkidle') && !isNetworkIdle) { + // Usually, networkidle is fired once and not removed after that. // However, when we clear them right before a new commit, this is allowed for a particular frame. - for (const event of this._subtreeLifecycleEvents) - events.add(event); + this._firedLifecycleEvents.delete('networkidle'); + this.emit(Frame.Events.RemoveLifecycle, 'networkidle'); } - const mainFrame = this._page.mainFrame(); - for (const event of events) { - // Checking whether we have already notified about this event. - if (!this._subtreeLifecycleEvents.has(event)) { - this.emit(Frame.Events.AddLifecycle, event); - if (this === mainFrame && this._url !== 'about:blank') - debugLogger.log('api', ` "${event}" event fired`); - } - } - for (const event of this._subtreeLifecycleEvents) { - if (!events.has(event)) - this.emit(Frame.Events.RemoveLifecycle, event); - } - this._subtreeLifecycleEvents = events; } async raceNavigationAction(progress: Progress, options: types.GotoOptions, action: () => Promise): Promise { @@ -706,7 +693,7 @@ export class Frame extends SdkObject { event = await sameDocument.promise; } - if (!this._subtreeLifecycleEvents.has(waitUntil)) + if (!this._firedLifecycleEvents.has(waitUntil)) await helper.waitForEvent(progress, this, Frame.Events.AddLifecycle, (e: types.LifecycleEvent) => e === waitUntil).promise; const request = event.newDocument ? event.newDocument.request : undefined; @@ -731,7 +718,7 @@ export class Frame extends SdkObject { if (navigationEvent.error) throw navigationEvent.error; - if (!this._subtreeLifecycleEvents.has(waitUntil)) + if (!this._firedLifecycleEvents.has(waitUntil)) await helper.waitForEvent(progress, this, Frame.Events.AddLifecycle, (e: types.LifecycleEvent) => e === waitUntil).promise; const request = navigationEvent.newDocument ? navigationEvent.newDocument.request : undefined; @@ -740,7 +727,7 @@ export class Frame extends SdkObject { async _waitForLoadState(progress: Progress, state: types.LifecycleEvent): Promise { const waitUntil = verifyLifecycle('state', state); - if (!this._subtreeLifecycleEvents.has(waitUntil)) + if (!this._firedLifecycleEvents.has(waitUntil)) await helper.waitForEvent(progress, this, Frame.Events.AddLifecycle, (e: types.LifecycleEvent) => e === waitUntil).promise; } @@ -1629,7 +1616,10 @@ export class Frame extends SdkObject { // after the frame was detached - probably a race in the Firefox itself. if (this._firedLifecycleEvents.has('networkidle') || this._detached) return; - this._networkIdleTimer = setTimeout(() => this._onLifecycleEvent('networkidle'), 500); + this._networkIdleTimer = setTimeout(() => { + this._firedNetworkIdleSelf = true; + this._page.mainFrame()._recalculateNetworkIdle(); + }, 500); } _stopNetworkIdleTimer() { diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 044d92c39c..4c7c54a66b 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -380,9 +380,8 @@ export class WKPage implements PageDelegate { eventsHelper.addEventListener(this._session, 'Page.willCheckNavigationPolicy', event => this._onWillCheckNavigationPolicy(event.frameId)), eventsHelper.addEventListener(this._session, 'Page.didCheckNavigationPolicy', event => this._onDidCheckNavigationPolicy(event.frameId, event.cancel)), eventsHelper.addEventListener(this._session, 'Page.frameScheduledNavigation', event => this._onFrameScheduledNavigation(event.frameId)), - eventsHelper.addEventListener(this._session, 'Page.frameStoppedLoading', event => this._onFrameStoppedLoading(event.frameId)), - eventsHelper.addEventListener(this._session, 'Page.loadEventFired', event => this._onLifecycleEvent(event.frameId, 'load')), - eventsHelper.addEventListener(this._session, 'Page.domContentEventFired', event => this._onLifecycleEvent(event.frameId, 'domcontentloaded')), + eventsHelper.addEventListener(this._session, 'Page.loadEventFired', event => this._page._frameManager.frameLifecycleEvent(event.frameId, 'load')), + eventsHelper.addEventListener(this._session, 'Page.domContentEventFired', event => this._page._frameManager.frameLifecycleEvent(event.frameId, 'domcontentloaded')), eventsHelper.addEventListener(this._session, 'Runtime.executionContextCreated', event => this._onExecutionContextCreated(event.context)), eventsHelper.addEventListener(this._session, 'Runtime.bindingCalled', event => this._onBindingCalled(event.contextId, event.argument)), eventsHelper.addEventListener(this._session, 'Console.messageAdded', event => this._onConsoleMessage(event)), @@ -450,14 +449,6 @@ export class WKPage implements PageDelegate { this._page._frameManager.frameRequestedNavigation(frameId); } - private _onFrameStoppedLoading(frameId: string) { - this._page._frameManager.frameStoppedLoading(frameId); - } - - private _onLifecycleEvent(frameId: string, event: types.LifecycleEvent) { - this._page._frameManager.frameLifecycleEvent(frameId, event); - } - private _handleFrameTree(frameTree: Protocol.Page.FrameResourceTree) { this._onFrameAttached(frameTree.frame.id, frameTree.frame.parentId || null); this._onFrameNavigated(frameTree.frame, true); diff --git a/tests/library/ignorehttpserrors.spec.ts b/tests/library/ignorehttpserrors.spec.ts index 02ccf3af77..5f52b1c940 100644 --- a/tests/library/ignorehttpserrors.spec.ts +++ b/tests/library/ignorehttpserrors.spec.ts @@ -53,7 +53,7 @@ it('should work with mixed content', async ({ browser, server, httpsServer }) => }); const context = await browser.newContext({ ignoreHTTPSErrors: true }); const page = await context.newPage(); - await page.goto(httpsServer.PREFIX + '/mixedcontent.html', { waitUntil: 'domcontentloaded' }); + await page.goto(httpsServer.PREFIX + '/mixedcontent.html', { waitUntil: 'load' }); expect(page.frames().length).toBe(2); // Make sure blocked iframe has functional execution context // @see https://github.com/GoogleChrome/puppeteer/issues/2709 diff --git a/tests/page/page-goto.spec.ts b/tests/page/page-goto.spec.ts index 3dd5e08bd6..e31d8bc82b 100644 --- a/tests/page/page-goto.spec.ts +++ b/tests/page/page-goto.spec.ts @@ -629,9 +629,20 @@ it('should properly wait for load', async ({ page, server, browserName }) => { ]); }); -it('should properly report window.stop()', async ({ page, server }) => { - server.setRoute('/module.js', async (req, res) => void 0); - await page.goto(server.PREFIX + '/window-stop.html'); +it('should not resolve goto upon window.stop()', async ({ browserName, page, server }) => { + let response; + server.setRoute('/module.js', (req, res) => { + res.writeHead(200, { 'Content-Type': 'text/javascript' }); + response = res; + }); + let done = false; + page.goto(server.PREFIX + '/window-stop.html').then(() => done = true).catch(() => {}); + await server.waitForRequest('/module.js'); + expect(done).toBe(false); + await page.waitForTimeout(1000); // give it some time to erroneously resolve + response.end(''); + await page.waitForTimeout(1000); // give it more time to erroneously resolve + expect(done).toBe(browserName === 'firefox'); // Firefox fires DOMContentLoaded and load events in this case. }); it('should return from goto if new navigation is started', async ({ page, server, browserName, isAndroid }) => { diff --git a/tests/page/page-wait-for-navigation.spec.ts b/tests/page/page-wait-for-navigation.spec.ts index 8dad7dabea..92cb01f326 100644 --- a/tests/page/page-wait-for-navigation.spec.ts +++ b/tests/page/page-wait-for-navigation.spec.ts @@ -155,18 +155,18 @@ it('should work with DOM history.back()/history.forward()', async ({ page, serve expect(page.url()).toBe(server.PREFIX + '/second.html'); }); -it('should work when subframe issues window.stop()', async ({ page, server }) => { +it('should work when subframe issues window.stop()', async ({ browserName, page, server }) => { server.setRoute('/frames/style.css', (req, res) => {}); - const navigationPromise = page.goto(server.PREFIX + '/frames/one-frame.html'); + let done = false; + page.goto(server.PREFIX + '/frames/one-frame.html').then(() => done = true).catch(() => {}); const frame = await new Promise(f => page.once('frameattached', f)); await new Promise(fulfill => page.on('framenavigated', f => { if (f === frame) fulfill(); })); - await Promise.all([ - frame.evaluate(() => window.stop()), - navigationPromise - ]); + await frame.evaluate(() => window.stop()); + await page.waitForTimeout(2000); // give it some time to erroneously resolve + expect(done).toBe(browserName !== 'webkit'); // Chromium and Firefox issue load event in this case. }); it('should work with url match', async ({ page, server }) => {