fix(setContent): manually reset lifecycyle for all browsers at the right moment (#679)

Author: dgozman

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');

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<void> {
     const array = [];
     for (const [name, value] of Object.entries(headers))

src/frames.ts

@@ -54,13 +54,15 @@ export type LifecycleEvent = 'load' | 'domcontentloaded' | 'networkidle0' | 'net
 const kLifecycleEvents: Set<LifecycleEvent> = new Set(['load', 'domcontentloaded', 'networkidle0', 'networkidle2']);
 
 export type WaitForOptions = types.TimeoutOptions & { waitFor?: types.Visibility | 'nowait' };
+type ConsoleTagHandler = () => void;
 
 export class FrameManager {
   private _page: Page;
   private _frames = new Map<string, Frame>();
   private _webSockets = new Map<string, network.WebSocket>();
   private _mainFrame: Frame;
   readonly _lifecycleWatchers = new Set<LifecycleWatcher>();
+  readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
 
   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<network.Request>();
   readonly _networkIdleTimers = new Map<LifecycleEvent, NodeJS.Timer>();
+  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<void> {
+    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}`;
-}
+}

src/page.ts

@@ -43,7 +43,6 @@ export interface PageDelegate {
   closePage(runBeforeUnload: boolean): Promise<void>;
 
   navigateFrame(frame: frames.Frame, url: string, referrer: string | undefined): Promise<frames.GotoResult>;
-  needsLifecycleResetOnSetContent(): boolean;
 
   setExtraHTTPHeaders(extraHTTPHeaders: network.Headers): Promise<void>;
   setViewport(viewport: types.Viewport): Promise<void>;
@@ -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 {

src/webkit/wkPage.ts

@@ -48,6 +48,7 @@ export class WKPage implements PageDelegate {
   private readonly _requestIdToRequest = new Map<string, WKInterceptableRequest>();
   private readonly _workers: WKWorkers;
   private readonly _contextIdToContext: Map<number, dom.FrameExecutionContext>;
+  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);
     });

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);