feat(page): support waitUntil option for page.setContent (#3557)

This patch teaches `page.setContent` to await resources in
the new document.

**NOTE**: This patch changes behavior: currently, `page.setContent`
awaits the `"domcontentloaded"` event; with this patch, we can now await
other lifecycle events, and switched default to the `"load"` event.

The change is justified since current behavior made `page.setContent`
unusable for its main designated usecases, pushing our client
to use [dataURL workaround](#728 (comment)).

Fixes #728

docs/api.md

@@ -126,7 +126,7 @@
   * [page.select(selector, ...values)](#pageselectselector-values)
   * [page.setBypassCSP(enabled)](#pagesetbypasscspenabled)
   * [page.setCacheEnabled([enabled])](#pagesetcacheenabledenabled)
-  * [page.setContent(html)](#pagesetcontenthtml)
+  * [page.setContent(html[, options])](#pagesetcontenthtml-options)
   * [page.setCookie(...cookies)](#pagesetcookiecookies)
   * [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout)
   * [page.setExtraHTTPHeaders(headers)](#pagesetextrahttpheadersheaders)
@@ -206,7 +206,7 @@
   * [frame.name()](#framename)
   * [frame.parentFrame()](#frameparentframe)
   * [frame.select(selector, ...values)](#frameselectselector-values)
-  * [frame.setContent(html)](#framesetcontenthtml)
+  * [frame.setContent(html[, options])](#framesetcontenthtml-options)
   * [frame.tap(selector)](#frametapselector)
   * [frame.title()](#frametitle)
   * [frame.type(selector, text[, options])](#frametypeselector-text-options)
@@ -1606,8 +1606,15 @@ that `page.setBypassCSP` should be called before navigating to the domain.
 
 Toggles ignoring cache for each request based on the enabled state. By default, caching is enabled.
 
-#### page.setContent(html)
+#### page.setContent(html[, options])
 - `html` <[string]> HTML markup to assign to the page.
+- `options` <[Object]> Parameters which might have the following properties:
+  - `timeout` <[number]> Maximum time in milliseconds for resources to load, defaults to 30 seconds, pass `0` to disable timeout.
+  - `waitUntil` <[string]|[Array]<[string]>> When to consider setting markup succeeded, defaults to `load`. Given an array of event strings, setting content is considered to be successful after all events have been fired. Events can be either:
+    - `load` - consider setting content to be finished when the `load` event is fired.
+    - `domcontentloaded` - consider setting content to be finished when the `DOMContentLoaded` event is fired.
+    - `networkidle0` - consider setting content to be finished when there are no more than 0 network connections for at least `500` ms.
+    - `networkidle2` - consider setting content to be finished when there are no more than 2 network connections for at least `500` ms.
 - returns: <[Promise]>
 
 #### page.setCookie(...cookies)
@@ -2552,8 +2559,15 @@ frame.select('select#colors', 'blue'); // single selection
 frame.select('select#colors', 'red', 'green', 'blue'); // multiple selections
 ```
 
-#### frame.setContent(html)
+#### frame.setContent(html[, options])
 - `html` <[string]> HTML markup to assign to the page.
+- `options` <[Object]> Parameters which might have the following properties:
+  - `timeout` <[number]> Maximum time in milliseconds for resources to load, defaults to 30 seconds, pass `0` to disable timeout.
+  - `waitUntil` <[string]|[Array]<[string]>> When to consider setting markup succeeded, defaults to `load`. Given an array of event strings, setting content is considered to be successful after all events have been fired. Events can be either:
+    - `load` - consider setting content to be finished when the `load` event is fired.
+    - `domcontentloaded` - consider setting content to be finished when the `DOMContentLoaded` event is fired.
+    - `networkidle0` - consider setting content to be finished when there are no more than 0 network connections for at least `500` ms.
+    - `networkidle2` - consider setting content to be finished when there are no more than 2 network connections for at least `500` ms.
 - returns: <[Promise]>
 
 #### frame.tap(selector)
@@ -3533,4 +3547,4 @@ TimeoutError is emitted whenever certain operations are terminated due to timeou
 [Worker]: #class-worker "Worker"
 [Accessibility]: #class-accessibility "Accessibility"
 [AXNode]: #accessibilitysnapshotoptions "AXNode"
-[ConnectionTransport]: ../lib/WebSocketTransport.js "ConnectionTransport"
+[ConnectionTransport]: ../lib/WebSocketTransport.js "ConnectionTransport"

lib/FrameManager.js

@@ -69,12 +69,14 @@ class FrameManager extends EventEmitter {
    * @return {!Promise<?Puppeteer.Response>}
    */
   async navigateFrame(frame, url, options = {}) {
+    assertNoLegacyNavigationOptions(options);
     const {
       referer = this._networkManager.extraHTTPHeaders()['referer'],
+      waitUntil = ['load'],
       timeout = this._defaultNavigationTimeout,
     } = options;
 
-    const watcher = new NavigatorWatcher(this._client, this, this._networkManager, frame, timeout, options);
+    const watcher = new LifecycleWatcher(this, frame, waitUntil, timeout);
     let ensureNewDocumentNavigation = false;
     let error = await Promise.race([
       navigate(this._client, url, referer, frame._id),
@@ -115,10 +117,12 @@ class FrameManager extends EventEmitter {
    * @return {!Promise<?Puppeteer.Response>}
    */
   async waitForFrameNavigation(frame, options = {}) {
+    assertNoLegacyNavigationOptions(options);
     const {
-      timeout = this._defaultNavigationTimeout
+      waitUntil = ['load'],
+      timeout = this._defaultNavigationTimeout,
     } = options;
-    const watcher = new NavigatorWatcher(this._client, this, this._networkManager, frame, timeout, options);
+    const watcher = new LifecycleWatcher(this, frame, waitUntil, timeout);
     const error = await Promise.race([
       watcher.timeoutOrTerminationPromise(),
       watcher.sameDocumentNavigationPromise(),
@@ -525,13 +529,28 @@ class Frame {
 
   /**
    * @param {string} html
+   * @param {!{timeout?: number, waitUntil?: string|!Array<string>}=} options
    */
-  async setContent(html) {
+  async setContent(html, options = {}) {
+    const {
+      waitUntil = ['load'],
+      timeout = 30000,
+    } = options;
+    // We rely upon the fact that document.open() will reset frame lifecycle with "init"
+    // lifecycle event. @see https://crrev.com/608658
     await this.evaluate(html => {
       document.open();
       document.write(html);
       document.close();
     }, html);
+    const watcher = new LifecycleWatcher(this._frameManager, this, waitUntil, timeout);
+    const error = await Promise.race([
+      watcher.timeoutOrTerminationPromise(),
+      watcher.lifecyclePromise(),
+    ]);
+    watcher.dispose();
+    if (error)
+      throw error;
   }
 
   /**
@@ -1127,22 +1146,20 @@ async function waitForPredicatePageFunction(predicateBody, polling, timeout, ...
   }
 }
 
-class NavigatorWatcher {
+function assertNoLegacyNavigationOptions(options) {
+  assert(options['networkIdleTimeout'] === undefined, 'ERROR: networkIdleTimeout option is no longer supported.');
+  assert(options['networkIdleInflight'] === undefined, 'ERROR: networkIdleInflight option is no longer supported.');
+  assert(options.waitUntil !== 'networkidle', 'ERROR: "networkidle" option is no longer supported. Use "networkidle2" instead');
+}
+
+class LifecycleWatcher {
   /**
-   * @param {!Puppeteer.CDPSession} client
    * @param {!FrameManager} frameManager
-   * @param {!NetworkManager} networkManager
    * @param {!Puppeteer.Frame} frame
+   * @param {string|!Array<string>} waitUntil
    * @param {number} timeout
-   * @param {!{waitUntil?: string|!Array<string>}} options
-   */
-  constructor(client, frameManager, networkManager, frame, timeout, options = {}) {
-    assert(options['networkIdleTimeout'] === undefined, 'ERROR: networkIdleTimeout option is no longer supported.');
-    assert(options['networkIdleInflight'] === undefined, 'ERROR: networkIdleInflight option is no longer supported.');
-    assert(options.waitUntil !== 'networkidle', 'ERROR: "networkidle" option is no longer supported. Use "networkidle2" instead');
-    let {
-      waitUntil = ['load']
-    } = options;
+   */
+  constructor(frameManager, frame, waitUntil, timeout) {
     if (Array.isArray(waitUntil))
       waitUntil = waitUntil.slice();
     else if (typeof waitUntil === 'string')
@@ -1154,15 +1171,14 @@ class NavigatorWatcher {
     });
 
     this._frameManager = frameManager;
-    this._networkManager = networkManager;
+    this._networkManager = frameManager._networkManager;
     this._frame = frame;
     this._initialLoaderId = frame._loaderId;
     this._timeout = timeout;
     /** @type {?Puppeteer.Request} */
     this._navigationRequest = null;
-    this._hasSameDocumentNavigation = false;
     this._eventListeners = [
-      helper.addEventListener(client, CDPSession.Events.Disconnected, () => this._terminate(new Error('Navigation failed because browser has disconnected!'))),
+      helper.addEventListener(frameManager._client, CDPSession.Events.Disconnected, () => this._terminate(new Error('Navigation failed because browser has disconnected!'))),
       helper.addEventListener(this._frameManager, FrameManager.Events.LifecycleEvent, this._checkLifecycleComplete.bind(this)),
       helper.addEventListener(this._frameManager, FrameManager.Events.FrameNavigatedWithinDocument, this._navigatedWithinDocument.bind(this)),
       helper.addEventListener(this._frameManager, FrameManager.Events.FrameDetached, this._onFrameDetached.bind(this)),
@@ -1173,6 +1189,10 @@ class NavigatorWatcher {
       this._sameDocumentNavigationCompleteCallback = fulfill;
     });
 
+    this._lifecyclePromise = new Promise(fulfill => {
+      this._lifecycleCallback = fulfill;
+    });
+
     this._newDocumentNavigationPromise = new Promise(fulfill => {
       this._newDocumentNavigationCompleteCallback = fulfill;
     });
@@ -1231,6 +1251,13 @@ class NavigatorWatcher {
     return this._newDocumentNavigationPromise;
   }
 
+  /**
+   * @return {!Promise}
+   */
+  lifecyclePromise() {
+    return this._lifecyclePromise;
+  }
+
   /**
    * @return {!Promise<?Error>}
    */
@@ -1261,10 +1288,11 @@ class NavigatorWatcher {
 
   _checkLifecycleComplete() {
     // We expect navigation to commit.
-    if (this._frame._loaderId === this._initialLoaderId && !this._hasSameDocumentNavigation)
-      return;
     if (!checkLifecycle(this._frame, this._expectedLifecycle))
       return;
+    this._lifecycleCallback();
+    if (this._frame._loaderId === this._initialLoaderId && !this._hasSameDocumentNavigation)
+      return;
     if (this._hasSameDocumentNavigation)
       this._sameDocumentNavigationCompleteCallback();
     if (this._frame._loaderId !== this._initialLoaderId)

lib/Page.js

@@ -609,9 +609,10 @@ class Page extends EventEmitter {
 
   /**
    * @param {string} html
+   * @param {!{timeout?: number, waitUntil?: string|!Array<string>}=} options
    */
-  async setContent(html) {
-    await this._frameManager.mainFrame().setContent(html);
+  async setContent(html, options) {
+    await this._frameManager.mainFrame().setContent(html, options);
   }
 
   /**

test/page.spec.js

@@ -1284,6 +1284,17 @@ module.exports.addTests = function({testRunner, expect, headless}) {
       const result = await page.content();
       expect(result).toBe(`${doctype}${expectedOutput}`);
     });
+    it('should await resources to load', async({page, server}) => {
+      const imgPath = '/img.png';
+      let imgResponse = null;
+      server.setRoute(imgPath, (req, res) => imgResponse = res);
+      let loaded = false;
+      const contentPromise = page.setContent(`<img src="${server.PREFIX + imgPath}"></img>`).then(() => loaded = true);
+      await server.waitForRequest(imgPath);
+      expect(loaded).toBe(false);
+      imgResponse.end();
+      await contentPromise;
+    });
   });
 
   describe('Page.setBypassCSP', function() {

utils/doclint/check_public_api/index.js

@@ -30,7 +30,7 @@ const EXCLUDE_CLASSES = new Set([
   'Helper',
   'Launcher',
   'Multimap',
-  'NavigatorWatcher',
+  'LifecycleWatcher',
   'NetworkManager',
   'PipeTransport',
   'TaskQueue',