feat(api): wait for popups and downloads when performing actions (#1744)

src/chromium/crBrowser.ts

@@ -134,6 +134,10 @@ export class CRBrowser extends BrowserBase {
       const opener = targetInfo.openerId ? this._crPages.get(targetInfo.openerId) || null : null;
       const crPage = new CRPage(session, targetInfo.targetId, context, opener);
       this._crPages.set(targetInfo.targetId, crPage);
+      if (opener && opener._initializedPage) {
+        for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers)
+          signalBarrier.addPopup(crPage.pageOrError());
+      }
       crPage.pageOrError().then(() => {
         this._firstPageCallback();
         context.emit(CommonEvents.BrowserContext.Page, crPage._page);

src/chromium/crPage.ts

@@ -315,15 +315,22 @@ class FrameSession {
   private _eventListeners: RegisteredListener[] = [];
   readonly _targetId: string;
   private _firstNonInitialNavigationCommittedPromise: Promise<void>;
-  private _firstNonInitialNavigationCommittedCallback = () => {};
+  private _firstNonInitialNavigationCommittedFulfill = () => {};
+  private _firstNonInitialNavigationCommittedReject = (e: Error) => {};
 
   constructor(crPage: CRPage, client: CRSession, targetId: string) {
     this._client = client;
     this._crPage = crPage;
     this._page = crPage._page;
     this._targetId = targetId;
     this._networkManager = new CRNetworkManager(client, this._page);
-    this._firstNonInitialNavigationCommittedPromise = new Promise(f => this._firstNonInitialNavigationCommittedCallback = f);
+    this._firstNonInitialNavigationCommittedPromise = new Promise((f, r) => {
+      this._firstNonInitialNavigationCommittedFulfill = f;
+      this._firstNonInitialNavigationCommittedReject = r;
+    });
+    client.once(CRSessionEvents.Disconnected, () => {
+      this._firstNonInitialNavigationCommittedReject(new Error('Page closed'));
+    });
   }
 
   private _isMainFrame(): boolean {
@@ -386,7 +393,7 @@ class FrameSession {
             this._eventListeners.push(helper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event)));
           });
         } else {
-          this._firstNonInitialNavigationCommittedCallback();
+          this._firstNonInitialNavigationCommittedFulfill();
           this._eventListeners.push(helper.addEventListener(this._client, 'Page.lifecycleEvent', event => this._onLifecycleEvent(event)));
         }
       }),
@@ -479,7 +486,7 @@ class FrameSession {
   _onFrameNavigated(framePayload: Protocol.Page.Frame, initial: boolean) {
     this._page._frameManager.frameCommittedNewDocumentNavigation(framePayload.id, framePayload.url, framePayload.name || '', framePayload.loaderId, initial);
     if (!initial)
-      this._firstNonInitialNavigationCommittedCallback();
+      this._firstNonInitialNavigationCommittedFulfill();
   }
 
   _onFrameRequestedNavigation(payload: Protocol.Page.frameRequestedNavigationPayload) {

src/dom.ts

@@ -55,7 +55,7 @@ export class FrameExecutionContext extends js.ExecutionContext {
   }
 
   async _doEvaluateInternal(returnByValue: boolean, waitForNavigations: boolean, pageFunction: string | Function, ...args: any[]): Promise<any> {
-    return await this.frame._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    return await this.frame._page._frameManager.waitForSignalsCreatedBy(async () => {
       return this._delegate.evaluate(this, returnByValue, pageFunction, ...args);
     }, Number.MAX_SAFE_INTEGER, waitForNavigations ? undefined : { waitUntil: 'nowait' });
   }
@@ -227,7 +227,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     if (!force)
       await this._waitForHitTargetAt(point, deadline);
 
-    await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
       let restoreModifiers: input.Modifier[] | undefined;
       if (options && options.modifiers)
         restoreModifiers = await this._page.keyboard._ensureModifiers(options.modifiers);
@@ -269,15 +269,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
       if (option.index !== undefined)
         assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
     }
-    return await this._page._frameManager.waitForNavigationsCreatedBy<string[]>(async () => {
+    return await this._page._frameManager.waitForSignalsCreatedBy<string[]>(async () => {
       return this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions);
     }, deadline, options);
   }
 
   async fill(value: string, options?: types.NavigatingActionWaitOptions): Promise<void> {
     assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
     const deadline = this._page._timeoutSettings.computeDeadline(options);
-    await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
       const errorOrNeedsInput = await this._evaluateInUtility(({ injected, node }, value) => injected.fill(node, value), value);
       if (typeof errorOrNeedsInput === 'string')
         throw new Error(errorOrNeedsInput);
@@ -323,7 +323,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
         filePayloads.push(item);
       }
     }
-    await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
       await this._page._delegate.setInputFiles(this as any as ElementHandle<HTMLInputElement>, filePayloads);
     }, deadline, options);
   }
@@ -341,15 +341,15 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
 
   async type(text: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
     const deadline = this._page._timeoutSettings.computeDeadline(options);
-    await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
       await this.focus();
       await this._page.keyboard.type(text, options);
     }, deadline, options, true);
   }
 
   async press(key: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
     const deadline = this._page._timeoutSettings.computeDeadline(options);
-    await this._page._frameManager.waitForNavigationsCreatedBy(async () => {
+    await this._page._frameManager.waitForSignalsCreatedBy(async () => {
       await this.focus();
       await this._page.keyboard.press(key, options);
     }, deadline, options, true);

src/download.ts

@@ -39,6 +39,8 @@ export class Download {
     this._url = url;
     this._finishedCallback = () => {};
     this._finishedPromise = new Promise(f => this._finishedCallback = f);
+    for (const barrier of this._page._frameManager._signalBarriers)
+      barrier.addDownload();
     this._page.emit(Events.Page.Download, this);
     page._browserContext._downloads.add(this);
     this._acceptDownloads = !!this._page._browserContext._options.acceptDownloads;

src/firefox/ffBrowser.ts

@@ -129,6 +129,10 @@ export class FFBrowser extends BrowserBase {
     const ffPage = new FFPage(session, context, opener);
     this._ffPages.set(targetId, ffPage);
 
+    if (opener && opener._initializedPage) {
+      for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers)
+        signalBarrier.addPopup(ffPage.pageOrError());
+    }
     ffPage.pageOrError().then(async () => {
       this._firstPageCallback();
       const page = ffPage._page;
@@ -200,7 +204,7 @@ export class FFBrowserContext extends BrowserContextBase {
   }
 
   pages(): Page[] {
-    return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
+    return this._ffPages().map(ffPage => ffPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
   async newPage(): Promise<Page> {

src/firefox/ffPage.ts

@@ -43,7 +43,7 @@ export class FFPage implements PageDelegate {
   readonly _browserContext: FFBrowserContext;
   private _pagePromise: Promise<Page | Error>;
   private _pageCallback: (pageOrError: Page | Error) => void = () => {};
-  private _initialized = false;
+  _initializedPage: Page | null = null;
   private readonly _opener: FFPage | null;
   private readonly _contextIdToContext: Map<string, dom.FrameExecutionContext>;
   private _eventListeners: RegisteredListener[];
@@ -70,6 +70,7 @@ export class FFPage implements PageDelegate {
       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.willOpenNewWindowAsynchronously', this._onWillOpenNewWindowAsynchronously.bind(this)),
       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)),
@@ -84,17 +85,13 @@ export class FFPage implements PageDelegate {
     session.once(FFSessionEvents.Disconnected, () => this._page._didDisconnect());
     this._session.once('Page.ready', () => {
       this._pageCallback(this._page);
-      this._initialized = true;
+      this._initializedPage = this._page;
     });
     // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy.
     // Therefore, we can end up with an initialized page without utility world, although very unlikely.
     this._session.send('Page.addScriptToEvaluateOnNewDocument', { script: '', worldName: UTILITY_WORLD_NAME }).catch(this._pageCallback);
   }
 
-  _initializedPage(): Page | null {
-    return this._initialized ? this._page : null;
-  }
-
   async pageOrError(): Promise<Page | Error> {
     return this._pagePromise;
   }
@@ -136,12 +133,21 @@ export class FFPage implements PageDelegate {
       this._page._frameManager.frameDidPotentiallyRequestNavigation();
   }
 
+  _onWillOpenNewWindowAsynchronously() {
+    for (const barrier of this._page._frameManager._signalBarriers)
+      barrier.expectPopup();
+  }
+
   _onNavigationStarted(params: Protocol.Page.navigationStartedPayload) {
     this._page._frameManager.frameRequestedNavigation(params.frameId, params.navigationId);
   }
 
   _onNavigationAborted(params: Protocol.Page.navigationAbortedPayload) {
     const frame = this._page._frameManager.frame(params.frameId)!;
+    if (params.errorText === 'Will download to file') {
+      for (const barrier of this._page._frameManager._signalBarriers)
+        barrier.expectDownload();
+    }
     for (const task of frame._frameTasks)
       task.onNewDocument(params.navigationId, new Error(params.errorText));
   }

src/frames.ts

@@ -51,7 +51,7 @@ export class FrameManager {
   private _frames = new Map<string, Frame>();
   private _mainFrame: Frame;
   readonly _consoleMessageTags = new Map<string, ConsoleTagHandler>();
-  private _pendingNavigationBarriers = new Set<PendingNavigationBarrier>();
+  readonly _signalBarriers = new Set<SignalBarrier>();
 
   constructor(page: Page) {
     this._page = page;
@@ -100,11 +100,11 @@ export class FrameManager {
     }
   }
 
-  async waitForNavigationsCreatedBy<T>(action: () => Promise<T>, deadline: number, options: types.NavigatingActionWaitOptions = {}, input?: boolean): Promise<T> {
+  async waitForSignalsCreatedBy<T>(action: () => Promise<T>, deadline: number, options: types.NavigatingActionWaitOptions = {}, input?: boolean): Promise<T> {
     if (options.waitUntil === 'nowait')
       return action();
-    const barrier = new PendingNavigationBarrier({ waitUntil: 'domcontentloaded', ...options }, deadline);
-    this._pendingNavigationBarriers.add(barrier);
+    const barrier = new SignalBarrier({ waitUntil: 'domcontentloaded', ...options }, deadline);
+    this._signalBarriers.add(barrier);
     try {
       const result = await action();
       if (input)
@@ -114,26 +114,26 @@ export class FrameManager {
       await new Promise(helper.makeWaitForNextTask());
       return result;
     } finally {
-      this._pendingNavigationBarriers.delete(barrier);
+      this._signalBarriers.delete(barrier);
     }
   }
 
   frameWillPotentiallyRequestNavigation() {
-    for (const barrier of this._pendingNavigationBarriers)
+    for (const barrier of this._signalBarriers)
       barrier.retain();
   }
 
   frameDidPotentiallyRequestNavigation() {
-    for (const barrier of this._pendingNavigationBarriers)
+    for (const barrier of this._signalBarriers)
       barrier.release();
   }
 
   frameRequestedNavigation(frameId: string, documentId: string) {
     const frame = this._frames.get(frameId);
     if (!frame)
       return;
-    for (const barrier of this._pendingNavigationBarriers)
-      barrier.addFrame(frame);
+    for (const barrier of this._signalBarriers)
+      barrier.addFrameNavigation(frame);
     frame._pendingDocumentId = documentId;
   }
 
@@ -939,10 +939,12 @@ function selectorToString(selector: string, waitFor: 'attached' | 'detached' | '
   return `${label}${selector}`;
 }
 
-class PendingNavigationBarrier {
+export class SignalBarrier {
   private _frameIds = new Map<string, number>();
   private _options: types.NavigatingActionWaitOptions;
   private _protectCount = 0;
+  private _expectedPopups = 0;
+  private _expectedDownloads = 0;
   private _promise: Promise<void>;
   private _promiseCallback = () => {};
   private _deadline: number;
@@ -959,14 +961,41 @@ class PendingNavigationBarrier {
     return this._promise;
   }
 
-  async addFrame(frame: Frame) {
+  async addFrameNavigation(frame: Frame) {
     this.retain();
     const timeout = helper.timeUntilDeadline(this._deadline);
     const options = { ...this._options, timeout } as types.NavigateOptions;
     await frame.waitForNavigation(options).catch(e => {});
     this.release();
   }
 
+  async expectPopup() {
+    ++this._expectedPopups;
+  }
+
+  async unexpectPopup() {
+    --this._expectedPopups;
+    this._maybeResolve();
+  }
+
+  async addPopup(pageOrError: Promise<Page | Error>) {
+    if (this._expectedPopups)
+      --this._expectedPopups;
+    this.retain();
+    await pageOrError;
+    this.release();
+  }
+
+  async expectDownload() {
+    ++this._expectedDownloads;
+  }
+
+  async addDownload() {
+    if (this._expectedDownloads)
+      --this._expectedDownloads;
+    this._maybeResolve();
+  }
+
   retain() {
     ++this._protectCount;
   }
@@ -977,7 +1006,7 @@ class PendingNavigationBarrier {
   }
 
   private async _maybeResolve() {
-    if (!this._protectCount && !this._frameIds.size)
+    if (!this._protectCount && !this._expectedPopups && !this._expectedDownloads && !this._frameIds.size)
       this._promiseCallback();
   }
 }

src/webkit/wkBrowser.ts

@@ -128,6 +128,10 @@ export class WKBrowser extends BrowserBase {
     const wkPage = new WKPage(context, pageProxySession, opener || null);
     this._wkPages.set(pageProxyId, wkPage);
 
+    if (opener && opener._initializedPage) {
+      for (const signalBarrier of opener._initializedPage._frameManager._signalBarriers)
+        signalBarrier.addPopup(wkPage.pageOrError());
+    }
     wkPage.pageOrError().then(async () => {
       this._firstPageCallback();
       const page = wkPage._page;
@@ -216,7 +220,7 @@ export class WKBrowserContext extends BrowserContextBase {
   }
 
   pages(): Page[] {
-    return this._wkPages().map(wkPage => wkPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
+    return this._wkPages().map(wkPage => wkPage._initializedPage).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
   async newPage(): Promise<Page> {