api(waitForLoadState): move waitUntil to be a first parameter (#1490)

Author: dgozman

docs/api.md

@@ -325,7 +325,7 @@ const [page] = await Promise.all([
 console.log(await page.evaluate('location.href'));
 ```
 
-> **NOTE** Use [Page.waitForLoadState](#pagewaitforloadstateoptions) to wait until the page gets to a particular state (you should not need it in most cases).
+> **NOTE** Use [`page.waitForLoadState([state[, options]])`](#pagewaitforloadstatestate-options) to wait until the page gets to a particular state (you should not need it in most cases).
 
 #### browserContext.addCookies(cookies)
 - `cookies` <[Array]<[Object]>>
@@ -692,7 +692,7 @@ page.removeListener('request', logRequest);
 - [page.waitFor(selectorOrFunctionOrTimeout[, options[, arg]])](#pagewaitforselectororfunctionortimeout-options-arg)
 - [page.waitForEvent(event[, optionsOrPredicate])](#pagewaitforeventevent-optionsorpredicate)
 - [page.waitForFunction(pageFunction, arg[, options])](#pagewaitforfunctionpagefunction-arg-options)
-- [page.waitForLoadState([options])](#pagewaitforloadstateoptions)
+- [page.waitForLoadState([state[, options]])](#pagewaitforloadstatestate-options)
 - [page.waitForNavigation([options])](#pagewaitfornavigationoptions)
 - [page.waitForRequest(urlOrPredicate[, options])](#pagewaitforrequesturlorpredicate-options)
 - [page.waitForResponse(urlOrPredicate[, options])](#pagewaitforresponseurlorpredicate-options)
@@ -780,7 +780,8 @@ const [popup] = await Promise.all([
 ]);
 console.log(await popup.evaluate('location.href'));
 ```
-> **NOTE** Use [Page.waitForLoadState](#pagewaitforloadstateoptions) to wait until the page gets to a particular state (you should not need it in most cases).
+
+> **NOTE** Use [`page.waitForLoadState([state[, options]])`](#pagewaitforloadstatestate-options) to wait until the page gets to a particular state (you should not need it in most cases).
 
 #### event: 'request'
 - <[Request]>
@@ -1705,25 +1706,33 @@ await page.waitForFunction(selector => !!document.querySelector(selector), selec
 
 Shortcut for [page.mainFrame().waitForFunction(pageFunction, arg, options]])](#framewaitforfunctionpagefunction-arg-options).
 
-#### page.waitForLoadState([options])
-- `options` <[Object]> Navigation parameters which might have the following properties:
-  - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods.
-  - `waitUntil` <"load"|"domcontentloaded"|"networkidle0"|"networkidle2">  When to consider navigation succeeded, defaults to `load`. Events can be either:
-    - `'load'` - consider navigation to be finished when the `load` event is fired.
-    - `'domcontentloaded'` - consider navigation to be finished when the `DOMContentLoaded` event is fired.
-    - `'networkidle0'` - consider navigation to be finished when there are no more than 0 network connections for at least `500` ms.
-    - `'networkidle2'` - consider navigation to be finished when there are no more than 2 network connections for at least `500` ms.
-- returns: <[Promise]> Promise which resolves when the load state has been achieved.
+#### page.waitForLoadState([state[, options]])
+- `state` <"load"|"domcontentloaded"|"networkidle0"|"networkidle2"> Load state to wait for, defaults to `load`. If the state has been already reached while loading current document, the method resolves immediately.
+  - `'load'` - wait for the `load` event to be fired.
+  - `'domcontentloaded'` - wait for the `DOMContentLoaded` event to be fired.
+  - `'networkidle0'` - wait until there are no more than 0 network connections for at least `500` ms.
+  - `'networkidle2'` - wait until there are no more than 2 network connections for at least `500` ms.
+- `options` <[Object]>
+  - `timeout` <[number]> Maximum waiting time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods.
+- returns: <[Promise]> Promise which resolves when the required load state has been reached.
 
-This resolves when the page reaches a required load state, `load` by default. The navigation can be in progress when it is called.
-If navigation is already at a required state, resolves immediately.
+This resolves when the page reaches a required load state, `load` by default. The navigation must have been committed when this method is called. If current document has already reached the required state, resolves immediately.
 
 ```js
 await page.click('button'); // Click triggers navigation.
-await page.waitForLoadState(); // The promise resolves after navigation has finished.
+await page.waitForLoadState(); // The promise resolves after 'load' event.
+```
+
+```js
+const [popup] = await Promise.all([
+  page.waitForEvent('popup'),
+  page.click('button'), // Click triggers a popup.
+])
+await popup.waitForLoadState('domcontentloaded'); // The promise resolves after 'domcontentloaded' event.
+console.log(await popup.title()); // Popup is ready to use.
 ```
 
-Shortcut for [page.mainFrame().waitForLoadState([options])](#framewaitforloadstateoptions).
+Shortcut for [page.mainFrame().waitForLoadState([options])](#framewaitforloadstatestate-options).
 
 #### page.waitForNavigation([options])
 - `options` <[Object]> Navigation parameters which might have the following properties:
@@ -1881,7 +1890,7 @@ An example of getting text from an iframe element:
 - [frame.url()](#frameurl)
 - [frame.waitFor(selectorOrFunctionOrTimeout[, options[, arg]])](#framewaitforselectororfunctionortimeout-options-arg)
 - [frame.waitForFunction(pageFunction, arg[, options])](#framewaitforfunctionpagefunction-arg-options)
-- [frame.waitForLoadState([options])](#framewaitforloadstateoptions)
+- [frame.waitForLoadState([state[, options]])](#framewaitforloadstatestate-options)
 - [frame.waitForNavigation([options])](#framewaitfornavigationoptions)
 - [frame.waitForSelector(selector[, options])](#framewaitforselectorselector-options)
 <!-- GEN:stop -->
@@ -2369,22 +2378,21 @@ const selector = '.foo';
 await frame.waitForFunction(selector => !!document.querySelector(selector), selector);
 ```
 
-#### frame.waitForLoadState([options])
-- `options` <[Object]> Navigation parameters which might have the following properties:
-  - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods.
-  - `waitUntil` <"load"|"domcontentloaded"|"networkidle0"|"networkidle2">  When to consider navigation succeeded, defaults to `load`. Events can be either:
-    - `'load'` - consider navigation to be finished when the `load` event is fired.
-    - `'domcontentloaded'` - consider navigation to be finished when the `DOMContentLoaded` event is fired.
-    - `'networkidle0'` - consider navigation to be finished when there are no more than 0 network connections for at least `500` ms.
-    - `'networkidle2'` - consider navigation to be finished when there are no more than 2 network connections for at least `500` ms.
-- returns: <[Promise]> Promise which resolves when the load state has been achieved.
+#### frame.waitForLoadState([state[, options]])
+- `state` <"load"|"domcontentloaded"|"networkidle0"|"networkidle2"> Load state to wait for, defaults to `load`. If the state has been already reached while loading current document, the method resolves immediately.
+  - `'load'` - wait for the `load` event to be fired.
+  - `'domcontentloaded'` - wait for the `DOMContentLoaded` event to be fired.
+  - `'networkidle0'` - wait until there are no more than 0 network connections for at least `500` ms.
+  - `'networkidle2'` - wait until there are no more than 2 network connections for at least `500` ms.
+- `options` <[Object]>
+  - `timeout` <[number]> Maximum waiting time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultNavigationTimeout(timeout)](#browsercontextsetdefaultnavigationtimeouttimeout), [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout), [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods.
+- returns: <[Promise]> Promise which resolves when the required load state has been reached.
 
-This resolves when the page reaches a required load state, `load` by default. The navigation can be in progress when it is called.
-If navigation is already at a required state, resolves immediately.
+This resolves when the frame reaches a required load state, `load` by default. The navigation must have been committed when this method is called. If current document has already reached the required state, resolves immediately.
 
 ```js
 await frame.click('button'); // Click triggers navigation.
-await frame.waitForLoadState(); // The promise resolves after navigation has finished.
+await frame.waitForLoadState(); // The promise resolves after 'load' event.
 ```
 
 #### frame.waitForNavigation([options])

src/frames.ts

@@ -370,7 +370,7 @@ export class Frame {
       await sameDocumentPromise;
     }
     const request = (navigateResult && navigateResult.newDocumentId) ? frameTask.request(navigateResult.newDocumentId) : null;
-    await frameTask.waitForLifecycle(options.waitUntil);
+    await frameTask.waitForLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil);
     frameTask.done();
     return request ? request._finalRequest().response() : null;
   }
@@ -383,14 +383,14 @@ export class Frame {
       frameTask.waitForSameDocumentNavigation(options.url),
     ]);
     const request = documentId ? frameTask.request(documentId) : null;
-    await frameTask.waitForLifecycle(options.waitUntil);
+    await frameTask.waitForLifecycle(options.waitUntil === undefined ? 'load' : options.waitUntil);
     frameTask.done();
     return request ? request._finalRequest().response() : null;
   }
 
-  async waitForLoadState(options: types.NavigateOptions = {}): Promise<void> {
+  async waitForLoadState(state: types.LifecycleEvent = 'load', options: types.TimeoutOptions = {}): Promise<void> {
     const frameTask = new FrameTask(this, options);
-    await frameTask.waitForLifecycle(options.waitUntil);
+    await frameTask.waitForLifecycle(state);
     frameTask.done();
   }
 
@@ -507,7 +507,7 @@ export class Frame {
       this._page._frameManager._consoleMessageTags.set(tag, () => {
         // Clear lifecycle right after document.open() - see 'tag' below.
         this._page._frameManager.clearFrameLifecycle(this);
-        this.waitForLoadState(options).then(resolve).catch(reject);
+        this.waitForLoadState(options ? options.waitUntil : 'load', options).then(resolve).catch(reject);
       });
     });
     const contentPromise = context.evaluateInternal(({ html, tag }) => {
@@ -1078,7 +1078,7 @@ export class FrameTask {
     }));
   }
 
-  waitForLifecycle(waitUntil: types.LifecycleEvent = 'load'): Promise<void> {
+  waitForLifecycle(waitUntil: types.LifecycleEvent): Promise<void> {
     if (!types.kLifecycleEvents.has(waitUntil))
       throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`);
     return this.raceAgainstFailures(new Promise((resolve, reject) => {

src/page.ts

@@ -295,8 +295,8 @@ export class Page extends platform.EventEmitter {
     return waitPromise;
   }
 
-  async waitForLoadState(options?: types.NavigateOptions): Promise<void> {
-    return this.mainFrame().waitForLoadState(options);
+  async waitForLoadState(state?: types.LifecycleEvent, options?: types.TimeoutOptions): Promise<void> {
+    return this.mainFrame().waitForLoadState(state, options);
   }
 
   async waitForNavigation(options?: types.WaitForNavigationOptions): Promise<network.Response | null> {

test/browsercontext.spec.js

@@ -497,7 +497,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
         context.waitForEvent('page'),
         page.evaluate(url => window.open(url), server.EMPTY_PAGE)
       ]);
-      await otherPage.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await otherPage.waitForLoadState('domcontentloaded');
       expect(otherPage.url()).toBe(server.EMPTY_PAGE);
       await context.close();
     });
@@ -508,7 +508,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
         context.waitForEvent('page'),
         page.evaluate(url => window.open(url), 'about:blank')
       ]);
-      await otherPage.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await otherPage.waitForLoadState('domcontentloaded');
       expect(otherPage.url()).toBe('about:blank');
       await context.close();
     });
@@ -519,7 +519,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
         context.waitForEvent('page'),
         page.evaluate(() => window.open())
       ]);
-      await otherPage.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await otherPage.waitForLoadState('domcontentloaded');
       expect(otherPage.url()).toBe('about:blank');
       await context.close();
     });
@@ -577,7 +577,7 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FF
       // Issue a redirect.
       serverResponse.writeHead(302, { location: '/injectedstyle.css' });
       serverResponse.end();
-      await newPage.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await newPage.waitForLoadState('domcontentloaded');
       expect(newPage.url()).toBe(server.PREFIX + '/one-style.html');
       // Cleanup.
       await context.close();

test/elementhandle.spec.js

@@ -218,7 +218,7 @@ module.exports.describe = function({testRunner, expect, FFOX, CHROMIUM, WEBKIT})
         return div;
       });
       expect(await divHandle.ownerFrame()).toBe(page.mainFrame());
-      await popup.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await popup.waitForLoadState('domcontentloaded');
       await page.evaluate(() => {
         const div = document.querySelector('div');
         window.__popup.document.body.appendChild(div);

test/emulation.spec.js

@@ -351,7 +351,7 @@ module.exports.describe = function({testRunner, expect, playwright, headless, FF
         page.waitForEvent('popup'),
         page.evaluate(url => window._popup = window.open(url), server.PREFIX + '/formatted-number.html'),
       ]);
-      await popup.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await popup.waitForLoadState('domcontentloaded');
       const result = await popup.evaluate(() => window.result);
       expect(result).toBe('1 000 000,5');
       await context.close();
@@ -364,7 +364,7 @@ module.exports.describe = function({testRunner, expect, playwright, headless, FF
         page.waitForEvent('popup'),
         page.evaluate(url => window._popup = window.open(url), server.PREFIX + '/formatted-number.html'),
       ]);
-      await popup.waitForLoadState({ waitUntil: 'domcontentloaded' });
+      await popup.waitForLoadState('domcontentloaded');
       const result = await popup.evaluate(() => window.initialNavigatorLanguage);
       expect(result).toBe('fr-CH');
       await context.close();