api: make BrowserContext.pages() synchronous (#1369)

Returns all pages which have been initialized already.

References #1348.

docs/api.md

@@ -452,10 +452,7 @@ const crypto = require('crypto');
 Creates a new page in the browser context.
 
 #### browserContext.pages()
-- returns: <[Promise]<[Array]<[Page]>>> Promise which resolves to an array of all open pages. Non visible pages, such as `"background_page"`, will not be listed here. You can find them using
-[chromiumBrowserContext.backgroundPages()](#chromiumbrowsercontextbackgroundpages).
-
-An array of all pages inside the browser context.
+- returns: <[Array]<[Page]>> All open pages in the context. Non visible pages, such as `"background_page"`, will not be listed here. You can find them using [chromiumBrowserContext.backgroundPages()](#chromiumbrowsercontextbackgroundpages).
 
 #### browserContext.route(url, handler)
 - `url` <[string]|[RegExp]|[function]\([string]\):[boolean]> A glob pattern, regex pattern or predicate receiving [URL] to match while routing.
@@ -3804,7 +3801,7 @@ Emitted when new background page is created in the context.
 Emitted when new service worker is created in the context.
 
 #### chromiumBrowserContext.backgroundPages()
-- returns: <[Promise]<[Array]<[Page]>>> Promise which resolves to an array of all existing background pages in the context.
+- returns: <[Array]<[Page]>> All existing background pages in the context.
 
 #### chromiumBrowserContext.newCDPSession(page)
 - `page` <[Page]> Page to create new session for.
@@ -4011,8 +4008,7 @@ const { chromium } = require('playwright');
       `--load-extension=${pathToExtension}`
     ]
   });
-  const backgroundPages = await browserContext.backgroundPages();
-  const backgroundPage = backgroundPages[0];
+  const backgroundPage = browserContext.backgroundPages()[0];
   // Test the background page as you would any other page.
   await browserContext.close();
 })();

src/browserContext.ts

@@ -41,7 +41,7 @@ export type BrowserContextOptions = {
 export interface BrowserContext {
   setDefaultNavigationTimeout(timeout: number): void;
   setDefaultTimeout(timeout: number): void;
-  pages(): Promise<Page[]>;
+  pages(): Page[];
   newPage(): Promise<Page>;
   cookies(urls?: string | string[]): Promise<network.NetworkCookie[]>;
   addCookies(cookies: network.SetNetworkCookieParam[]): Promise<void>;
@@ -74,10 +74,8 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement
     this._closePromise = new Promise(fulfill => this._closePromiseFulfill = fulfill);
   }
 
-  abstract _existingPages(): Page[];
-
   _browserClosed() {
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       page._didClose();
     this._didCloseInternal();
   }
@@ -89,7 +87,7 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement
   }
 
   // BrowserContext methods.
-  abstract pages(): Promise<Page[]>;
+  abstract pages(): Page[];
   abstract newPage(): Promise<Page>;
   abstract cookies(...urls: string[]): Promise<network.NetworkCookie[]>;
   abstract addCookies(cookies: network.SetNetworkCookieParam[]): Promise<void>;
@@ -126,8 +124,7 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement
 }
 
 export function assertBrowserContextIsNotOwned(context: BrowserContextBase) {
-  const pages = context._existingPages();
-  for (const page of pages) {
+  for (const page of context.pages()) {
     if (page._ownedContext)
       throw new Error('Please use browser.newContext() for multi-page scripts that share the context.');
   }

src/chromium/crBrowser.ts

@@ -38,6 +38,8 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
   readonly _defaultContext: CRBrowserContext;
   readonly _contexts = new Map<string, CRBrowserContext>();
   _targets = new Map<string, CRTarget>();
+  readonly _firstPagePromise: Promise<void>;
+  private _firstPageCallback = () => {};
 
   private _tracingRecording = false;
   private _tracingPath: string | null = '';
@@ -83,6 +85,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
     this._session.on('Target.targetDestroyed', this._targetDestroyed.bind(this));
     this._session.on('Target.targetInfoChanged', this._targetInfoChanged.bind(this));
     this._session.on('Target.attachedToTarget', this._onAttachedToTarget.bind(this));
+    this._firstPagePromise = new Promise(f => this._firstPageCallback = f);
   }
 
   async newContext(options: BrowserContextOptions = {}): Promise<BrowserContext> {
@@ -102,7 +105,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
     return createPageInNewContext(this, options);
   }
 
-  async _onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
+  _onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
     const session = this._connection.session(sessionId)!;
     if (!CRTarget.isPageType(targetInfo.type)) {
       assert(targetInfo.type === 'service_worker' || targetInfo.type === 'browser' || targetInfo.type === 'other');
@@ -115,29 +118,24 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
       return;
     }
     const { context, target } = this._createTarget(targetInfo, session);
-    try {
-      switch (targetInfo.type) {
-        case 'page': {
-          const event = new PageEvent(context, target.pageOrError());
-          context.emit(CommonEvents.BrowserContext.Page, event);
-          const opener = target.opener();
-          if (!opener)
-            break;
-          const openerPage = await opener.pageOrError();
-          if (openerPage instanceof Page && !openerPage.isClosed())
-            openerPage.emit(CommonEvents.Page.Popup, new PageEvent(context, target.pageOrError()));
-          break;
-        }
-        case 'background_page': {
-          const event = new PageEvent(context, target.pageOrError());
-          context.emit(Events.CRBrowserContext.BackgroundPage, event);
-          break;
-        }
+
+    if (!CRTarget.isPageType(targetInfo.type))
+      return;
+    const pageEvent = new PageEvent(context, target.pageOrError());
+    target.pageOrError().then(async () => {
+      if (targetInfo.type === 'page') {
+        this._firstPageCallback();
+        context.emit(CommonEvents.BrowserContext.Page, pageEvent);
+        const opener = target.opener();
+        if (!opener)
+          return;
+        const openerPage = await opener.pageOrError();
+        if (openerPage instanceof Page && !openerPage.isClosed())
+          openerPage.emit(CommonEvents.Page.Popup, pageEvent);
+      } else if (targetInfo.type === 'background_page') {
+        context.emit(Events.CRBrowserContext.BackgroundPage, pageEvent);
       }
-    } catch (e) {
-      // Do not dispatch the event if initialization failed.
-      debugError(e);
-    }
+    });
   }
 
   async _targetCreated({targetInfo}: Protocol.Target.targetCreatedPayload) {
@@ -176,10 +174,6 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
     await this._session.send('Target.closeTarget', { targetId: CRTarget.fromPage(page)._targetId });
   }
 
-  _allTargets(): CRTarget[] {
-    return Array.from(this._targets.values());
-  }
-
   async close() {
     const disconnected = new Promise(f => this._connection.once(ConnectionEvents.Disconnected, f));
     await Promise.all(this.contexts().map(context => context.close()));
@@ -268,19 +262,12 @@ export class CRBrowserContext extends BrowserContextBase {
       await this.setHTTPCredentials(this._options.httpCredentials);
   }
 
-  _existingPages(): Page[] {
-    const pages: Page[] = [];
-    for (const target of this._browser._allTargets()) {
-      if (target.context() === this && target._crPage)
-        pages.push(target._crPage.page());
-    }
-    return pages;
+  _targets(): CRTarget[] {
+    return Array.from(this._browser._targets.values()).filter(target => target.context() === this);
   }
 
-  async pages(): Promise<Page[]> {
-    const targets = this._browser._allTargets().filter(target => target.context() === this && target.type() === 'page');
-    const pages = await Promise.all(targets.map(target => target.pageOrError()));
-    return pages.filter(page => (page instanceof Page) && !page.isClosed()) as Page[];
+  pages(): Page[] {
+    return this._targets().filter(target => target.type() === 'page').map(target => target._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
   async newPage(): Promise<Page> {
@@ -351,51 +338,51 @@ export class CRBrowserContext extends BrowserContextBase {
     if (geolocation)
       geolocation = verifyGeolocation(geolocation);
     this._options.geolocation = geolocation || undefined;
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage)._client.send('Emulation.setGeolocationOverride', geolocation || {});
   }
 
   async setExtraHTTPHeaders(headers: network.Headers): Promise<void> {
     this._options.extraHTTPHeaders = network.verifyHeaders(headers);
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage).updateExtraHTTPHeaders();
   }
 
   async setOffline(offline: boolean): Promise<void> {
     this._options.offline = offline;
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage)._networkManager.setOffline(offline);
   }
 
   async setHTTPCredentials(httpCredentials: types.Credentials | null): Promise<void> {
     this._options.httpCredentials = httpCredentials || undefined;
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage)._networkManager.authenticate(httpCredentials);
   }
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) {
     const source = await helper.evaluationScript(script, args);
     this._evaluateOnNewDocumentSources.push(source);
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage).evaluateOnNewDocument(source);
   }
 
   async exposeFunction(name: string, playwrightFunction: Function): Promise<void> {
-    for (const page of this._existingPages()) {
+    for (const page of this.pages()) {
       if (page._pageBindings.has(name))
         throw new Error(`Function "${name}" has been already registered in one of the pages`);
     }
     if (this._pageBindings.has(name))
       throw new Error(`Function "${name}" has been already registered`);
     const binding = new PageBinding(name, playwrightFunction);
     this._pageBindings.set(name, binding);
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage).exposeBinding(binding);
   }
 
   async route(url: types.URLMatch, handler: network.RouteHandler): Promise<void> {
     this._routes.push({ url, handler });
-    for (const page of this._existingPages())
+    for (const page of this.pages())
       await (page._delegate as CRPage).updateRequestInterception();
   }
 
@@ -413,10 +400,8 @@ export class CRBrowserContext extends BrowserContextBase {
     this._didCloseInternal();
   }
 
-  async backgroundPages(): Promise<Page[]> {
-    const targets = this._browser._allTargets().filter(target => target.context() === this && target.type() === 'background_page');
-    const pages = await Promise.all(targets.map(target => target.pageOrError()));
-    return pages.filter(page => (page instanceof Page) && !page.isClosed()) as Page[];
+  backgroundPages(): Page[] {
+    return this._targets().filter(target => target.type() === 'background_page').map(target => target._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
   async newCDPSession(page: Page): Promise<CRSession> {

src/chromium/crPage.ts

@@ -43,6 +43,7 @@ const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 
 export class CRPage implements PageDelegate {
   _client: CRSession;
+  _initialized = false;
   private readonly _page: Page;
   readonly _networkManager: CRNetworkManager;
   private _contextIdToContext = new Map<number, dom.FrameExecutionContext>();
@@ -143,6 +144,7 @@ export class CRPage implements PageDelegate {
       promises.push(this.evaluateOnNewDocument(source));
     promises.push(this._client.send('Runtime.runIfWaitingForDebugger'));
     await Promise.all(promises);
+    this._initialized = true;
   }
 
   didClose() {

src/chromium/crTarget.ts

@@ -70,6 +70,12 @@ export class CRTarget {
       this._crPage.didClose();
   }
 
+  _initializedPage(): Page | null {
+    if (this._crPage && this._crPage._initialized)
+      return this._crPage.page();
+    return null;
+  }
+
   async pageOrError(): Promise<Page | Error> {
     if (CRTarget.isPageType(this.type()))
       return this._pagePromise!;

src/dom.ts

@@ -160,8 +160,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const frameId = await this._page._delegate.getOwnerFrame(this);
     if (!frameId)
       return null;
-    const pages = this._page._browserContext._existingPages();
-    for (const page of pages) {
+    for (const page of this._page._browserContext.pages()) {
       const frame = page._frameManager.frame(frameId);
       if (frame)
         return frame;

src/firefox/ffBrowser.ts

@@ -119,7 +119,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
     ffPage.didClose();
   }
 
-  async _onAttachedToTarget(payload: Protocol.Browser.attachedToTargetPayload) {
+  _onAttachedToTarget(payload: Protocol.Browser.attachedToTargetPayload) {
     const {targetId, browserContextId, openerId, type} = payload.targetInfo;
     assert(type === 'page');
     const context = browserContextId ? this._contexts.get(browserContextId)! : this._defaultContext;
@@ -129,15 +129,15 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
     this._ffPages.set(targetId, ffPage);
 
     const pageEvent = new PageEvent(context, ffPage.pageOrError());
-    context.emit(Events.BrowserContext.Page, pageEvent);
-
-    ffPage.pageOrError().then(() => this._firstPageCallback());
-
-    if (!opener)
-      return;
-    const openerPage = await opener.pageOrError();
-    if (openerPage instanceof Page && !openerPage.isClosed())
-      openerPage.emit(Events.Page.Popup, pageEvent);
+    ffPage.pageOrError().then(async () => {
+      this._firstPageCallback();
+      context.emit(Events.BrowserContext.Page, pageEvent);
+      if (!opener)
+        return;
+      const openerPage = await opener.pageOrError();
+      if (openerPage instanceof Page && !openerPage.isClosed())
+        openerPage.emit(Events.Page.Popup, pageEvent);
+    });
   }
 
   async close() {
@@ -182,10 +182,6 @@ export class FFBrowserContext extends BrowserContextBase {
     return Array.from(this._browser._ffPages.values()).filter(ffPage => ffPage._browserContext === this);
   }
 
-  _existingPages(): Page[] {
-    return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
-  }
-
   setDefaultNavigationTimeout(timeout: number) {
     this._timeoutSettings.setDefaultNavigationTimeout(timeout);
   }
@@ -194,9 +190,8 @@ export class FFBrowserContext extends BrowserContextBase {
     this._timeoutSettings.setDefaultTimeout(timeout);
   }
 
-  async pages(): Promise<Page[]> {
-    const pagesOrErrors = await Promise.all(this._ffPages().map(ffPage => ffPage.pageOrError()));
-    return pagesOrErrors.filter(pageOrError => pageOrError instanceof Page && !pageOrError.isClosed()) as Page[];
+  pages(): Page[] {
+    return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[];
   }
 
   async newPage(): Promise<Page> {
@@ -279,7 +274,7 @@ export class FFBrowserContext extends BrowserContextBase {
   }
 
   async exposeFunction(name: string, playwrightFunction: Function): Promise<void> {
-    for (const page of this._existingPages()) {
+    for (const page of this.pages()) {
       if (page._pageBindings.has(name))
         throw new Error(`Function "${name}" has been already registered in one of the pages`);
     }

src/server/chromium.ts

@@ -66,15 +66,8 @@ export class Chromium implements BrowserType {
     const { timeout = 30000 } = options || {};
     const { transport } = await this._launchServer(options, 'persistent', userDataDir);
     const browser = await CRBrowser.connect(transport!, true);
-    const browserContext = browser._defaultContext;
-
-    function targets() {
-      return browser._allTargets().filter(target => target.context() === browserContext && target.type() === 'page');
-    }
-    const firstTarget = targets().length ? Promise.resolve() : new Promise(f => browserContext.once('page', f));
-    const firstPage = firstTarget.then(() => targets()[0].pageOrError());
-    await helper.waitWithTimeout(firstPage, 'first page', timeout);
-    return browserContext;
+    await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout);
+    return browser._defaultContext;
   }
 
   private async _launchServer(options: LaunchOptions = {}, launchType: LaunchType, userDataDir?: string, port?: number): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport }> {