refactor: move navigation management to FrameManager (#3266)

This patch:
- moves implementation of page.goto and page.waitForNavigation
  into FrameManager. The defaultNavigationTimeout gets moved to
  FrameManager as well.
- moves NavigatorWatcher into FrameManager to avoid circular dependency

References #2918

lib/FrameManager.js

@@ -19,6 +19,8 @@ const EventEmitter = require('events');
 const {helper, assert} = require('./helper');
 const {ExecutionContext} = require('./ExecutionContext');
 const {TimeoutError} = require('./Errors');
+const {NetworkManager} = require('./NetworkManager');
+const {Connection} = require('./Connection');
 
 const readFileAsync = helper.promisify(fs.readFile);
 
@@ -27,11 +29,14 @@ class FrameManager extends EventEmitter {
    * @param {!Puppeteer.CDPSession} client
    * @param {!Protocol.Page.FrameTree} frameTree
    * @param {!Puppeteer.Page} page
+   * @param {!Puppeteer.NetworkManager} networkManager
    */
-  constructor(client, frameTree, page) {
+  constructor(client, frameTree, page, networkManager) {
     super();
     this._client = client;
     this._page = page;
+    this._networkManager = networkManager;
+    this._defaultNavigationTimeout = 30000;
     /** @type {!Map<string, !Frame>} */
     this._frames = new Map();
     /** @type {!Map<number, !ExecutionContext>} */
@@ -50,6 +55,76 @@ class FrameManager extends EventEmitter {
     this._handleFrameTree(frameTree);
   }
 
+  /**
+   * @param {number} timeout
+   */
+  setDefaultNavigationTimeout(timeout) {
+    this._defaultNavigationTimeout = timeout;
+  }
+
+  /**
+   * @param {!Puppeteer.Frame} frame
+   * @param {string} url
+   * @param {!Object=} options
+   * @return {!Promise<?Puppeteer.Response>}
+   */
+  async navigateFrame(frame, url, options = {}) {
+    const referrer = typeof options.referer === 'string' ? options.referer : this._networkManager.extraHTTPHeaders()['referer'];
+
+    const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout;
+    const watcher = new NavigatorWatcher(this._client, this, this._networkManager, frame, timeout, options);
+    let ensureNewDocumentNavigation = false;
+    let error = await Promise.race([
+      navigate(this._client, url, referrer),
+      watcher.timeoutOrTerminationPromise(),
+    ]);
+    if (!error) {
+      error = await Promise.race([
+        watcher.timeoutOrTerminationPromise(),
+        ensureNewDocumentNavigation ? watcher.newDocumentNavigationPromise() : watcher.sameDocumentNavigationPromise(),
+      ]);
+    }
+    watcher.dispose();
+    if (error)
+      throw error;
+    return watcher.navigationResponse();
+
+    /**
+     * @param {!Puppeteer.CDPSession} client
+     * @param {string} url
+     * @param {string} referrer
+     * @return {!Promise<?Error>}
+     */
+    async function navigate(client, url, referrer) {
+      try {
+        const response = await client.send('Page.navigate', {url, referrer});
+        ensureNewDocumentNavigation = !!response.loaderId;
+        return response.errorText ? new Error(`${response.errorText} at ${url}`) : null;
+      } catch (error) {
+        return error;
+      }
+    }
+  }
+
+  /**
+   * @param {!Puppeteer.Frame} frame
+   * @param {!Object=} options
+   * @return {!Promise<?Puppeteer.Response>}
+   */
+  async waitForFrameNavigation(frame, options) {
+    const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout;
+    const watcher = new NavigatorWatcher(this._client, this, this._networkManager, frame, timeout, options);
+    const error = await Promise.race([
+      watcher.timeoutOrTerminationPromise(),
+      watcher.sameDocumentNavigationPromise(),
+      watcher.newDocumentNavigationPromise()
+    ]);
+    watcher.dispose();
+    if (error)
+      throw error;
+    return watcher.navigationResponse();
+  }
+
   /**
    * @param {!Protocol.Page.lifecycleEventPayload} event
    */
@@ -1017,4 +1092,165 @@ async function waitForPredicatePageFunction(predicateBody, polling, timeout, ...
   }
 }
 
+class NavigatorWatcher {
+  /**
+   * @param {!Puppeteer.CDPSession} client
+   * @param {!FrameManager} frameManager
+   * @param {!NetworkManager} networkManager
+   * @param {!Puppeteer.Frame} frame
+   * @param {number} timeout
+   * @param {!Object=} 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'];
+    if (Array.isArray(options.waitUntil))
+      waitUntil = options.waitUntil.slice();
+    else if (typeof options.waitUntil === 'string')
+      waitUntil = [options.waitUntil];
+    this._expectedLifecycle = waitUntil.map(value => {
+      const protocolEvent = puppeteerToProtocolLifecycle[value];
+      assert(protocolEvent, 'Unknown value for options.waitUntil: ' + value);
+      return protocolEvent;
+    });
+
+    this._frameManager = frameManager;
+    this._networkManager = networkManager;
+    this._frame = frame;
+    this._initialLoaderId = frame._loaderId;
+    this._timeout = timeout;
+    /** @type {?Puppeteer.Request} */
+    this._navigationRequest = null;
+    this._hasSameDocumentNavigation = false;
+    this._eventListeners = [
+      helper.addEventListener(Connection.fromSession(client), Connection.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._checkLifecycleComplete.bind(this)),
+      helper.addEventListener(this._networkManager, NetworkManager.Events.Request, this._onRequest.bind(this)),
+    ];
+
+    this._sameDocumentNavigationPromise = new Promise(fulfill => {
+      this._sameDocumentNavigationCompleteCallback = fulfill;
+    });
+
+    this._newDocumentNavigationPromise = new Promise(fulfill => {
+      this._newDocumentNavigationCompleteCallback = fulfill;
+    });
+
+    this._timeoutPromise = this._createTimeoutPromise();
+    this._terminationPromise = new Promise(fulfill => {
+      this._terminationCallback = fulfill;
+    });
+  }
+
+  /**
+   * @param {!Puppeteer.Request} request
+   */
+  _onRequest(request) {
+    if (request.frame() !== this._frame || !request.isNavigationRequest())
+      return;
+    this._navigationRequest = request;
+  }
+
+  /**
+   * @return {?Puppeteer.Response}
+   */
+  navigationResponse() {
+    return this._navigationRequest ? this._navigationRequest.response() : null;
+  }
+
+  /**
+   * @param {!Error} error
+   */
+  _terminate(error) {
+    this._terminationCallback.call(null, error);
+  }
+
+  /**
+   * @return {!Promise<?Error>}
+   */
+  sameDocumentNavigationPromise() {
+    return this._sameDocumentNavigationPromise;
+  }
+
+  /**
+   * @return {!Promise<?Error>}
+   */
+  newDocumentNavigationPromise() {
+    return this._newDocumentNavigationPromise;
+  }
+
+  /**
+   * @return {!Promise<?Error>}
+   */
+  timeoutOrTerminationPromise() {
+    return Promise.race([this._timeoutPromise, this._terminationPromise]);
+  }
+
+  /**
+   * @return {!Promise<?Error>}
+   */
+  _createTimeoutPromise() {
+    if (!this._timeout)
+      return new Promise(() => {});
+    const errorMessage = 'Navigation Timeout Exceeded: ' + this._timeout + 'ms exceeded';
+    return new Promise(fulfill => this._maximumTimer = setTimeout(fulfill, this._timeout))
+        .then(() => new TimeoutError(errorMessage));
+  }
+
+  /**
+   * @param {!Puppeteer.Frame} frame
+   */
+  _navigatedWithinDocument(frame) {
+    if (frame !== this._frame)
+      return;
+    this._hasSameDocumentNavigation = true;
+    this._checkLifecycleComplete();
+  }
+
+  _checkLifecycleComplete() {
+    // We expect navigation to commit.
+    if (this._frame._loaderId === this._initialLoaderId && !this._hasSameDocumentNavigation)
+      return;
+    if (!checkLifecycle(this._frame, this._expectedLifecycle))
+      return;
+    if (this._hasSameDocumentNavigation)
+      this._sameDocumentNavigationCompleteCallback();
+    if (this._frame._loaderId !== this._initialLoaderId)
+      this._newDocumentNavigationCompleteCallback();
+
+    /**
+     * @param {!Puppeteer.Frame} frame
+     * @param {!Array<string>} expectedLifecycle
+     * @return {boolean}
+     */
+    function checkLifecycle(frame, expectedLifecycle) {
+      for (const event of expectedLifecycle) {
+        if (!frame._lifecycleEvents.has(event))
+          return false;
+      }
+      for (const child of frame.childFrames()) {
+        if (!checkLifecycle(child, expectedLifecycle))
+          return false;
+      }
+      return true;
+    }
+  }
+
+  dispose() {
+    helper.removeEventListeners(this._eventListeners);
+    clearTimeout(this._maximumTimer);
+  }
+}
+
+const puppeteerToProtocolLifecycle = {
+  'load': 'load',
+  'domcontentloaded': 'DOMContentLoaded',
+  'networkidle0': 'networkIdle',
+  'networkidle2': 'networkAlmostIdle',
+};
+
 module.exports = {FrameManager, Frame};

lib/NavigatorWatcher.js

@@ -1,184 +0,0 @@
-/**
- * Copyright 2017 Google Inc. All rights reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-const {helper, assert} = require('./helper');
-const {FrameManager} = require('./FrameManager');
-const {NetworkManager} = require('./NetworkManager');
-const {TimeoutError} = require('./Errors');
-const {Connection} = require('./Connection');
-
-class NavigatorWatcher {
-  /**
-   * @param {!Puppeteer.CDPSession} client
-   * @param {!FrameManager} frameManager
-   * @param {!NetworkManager} networkManager
-   * @param {!Puppeteer.Frame} frame
-   * @param {number} timeout
-   * @param {!Object=} 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'];
-    if (Array.isArray(options.waitUntil))
-      waitUntil = options.waitUntil.slice();
-    else if (typeof options.waitUntil === 'string')
-      waitUntil = [options.waitUntil];
-    this._expectedLifecycle = waitUntil.map(value => {
-      const protocolEvent = puppeteerToProtocolLifecycle[value];
-      assert(protocolEvent, 'Unknown value for options.waitUntil: ' + value);
-      return protocolEvent;
-    });
-
-    this._frameManager = frameManager;
-    this._networkManager = networkManager;
-    this._frame = frame;
-    this._initialLoaderId = frame._loaderId;
-    this._timeout = timeout;
-    /** @type {?Puppeteer.Request} */
-    this._navigationRequest = null;
-    this._hasSameDocumentNavigation = false;
-    this._eventListeners = [
-      helper.addEventListener(Connection.fromSession(client), Connection.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._checkLifecycleComplete.bind(this)),
-      helper.addEventListener(this._networkManager, NetworkManager.Events.Request, this._onRequest.bind(this)),
-    ];
-
-    this._sameDocumentNavigationPromise = new Promise(fulfill => {
-      this._sameDocumentNavigationCompleteCallback = fulfill;
-    });
-
-    this._newDocumentNavigationPromise = new Promise(fulfill => {
-      this._newDocumentNavigationCompleteCallback = fulfill;
-    });
-
-    this._timeoutPromise = this._createTimeoutPromise();
-    this._terminationPromise = new Promise(fulfill => {
-      this._terminationCallback = fulfill;
-    });
-  }
-
-  /**
-   * @param {!Puppeteer.Request} request
-   */
-  _onRequest(request) {
-    if (request.frame() !== this._frame || !request.isNavigationRequest())
-      return;
-    this._navigationRequest = request;
-  }
-
-  /**
-   * @return {?Puppeteer.Response}
-   */
-  navigationResponse() {
-    return this._navigationRequest ? this._navigationRequest.response() : null;
-  }
-
-  /**
-   * @param {!Error} error
-   */
-  _terminate(error) {
-    this._terminationCallback.call(null, error);
-  }
-
-  /**
-   * @return {!Promise<?Error>}
-   */
-  sameDocumentNavigationPromise() {
-    return this._sameDocumentNavigationPromise;
-  }
-
-  /**
-   * @return {!Promise<?Error>}
-   */
-  newDocumentNavigationPromise() {
-    return this._newDocumentNavigationPromise;
-  }
-
-  /**
-   * @return {!Promise<?Error>}
-   */
-  timeoutOrTerminationPromise() {
-    return Promise.race([this._timeoutPromise, this._terminationPromise]);
-  }
-
-  /**
-   * @return {!Promise<?Error>}
-   */
-  _createTimeoutPromise() {
-    if (!this._timeout)
-      return new Promise(() => {});
-    const errorMessage = 'Navigation Timeout Exceeded: ' + this._timeout + 'ms exceeded';
-    return new Promise(fulfill => this._maximumTimer = setTimeout(fulfill, this._timeout))
-        .then(() => new TimeoutError(errorMessage));
-  }
-
-  /**
-   * @param {!Puppeteer.Frame} frame
-   */
-  _navigatedWithinDocument(frame) {
-    if (frame !== this._frame)
-      return;
-    this._hasSameDocumentNavigation = true;
-    this._checkLifecycleComplete();
-  }
-
-  _checkLifecycleComplete() {
-    // We expect navigation to commit.
-    if (this._frame._loaderId === this._initialLoaderId && !this._hasSameDocumentNavigation)
-      return;
-    if (!checkLifecycle(this._frame, this._expectedLifecycle))
-      return;
-    if (this._hasSameDocumentNavigation)
-      this._sameDocumentNavigationCompleteCallback();
-    if (this._frame._loaderId !== this._initialLoaderId)
-      this._newDocumentNavigationCompleteCallback();
-
-    /**
-     * @param {!Puppeteer.Frame} frame
-     * @param {!Array<string>} expectedLifecycle
-     * @return {boolean}
-     */
-    function checkLifecycle(frame, expectedLifecycle) {
-      for (const event of expectedLifecycle) {
-        if (!frame._lifecycleEvents.has(event))
-          return false;
-      }
-      for (const child of frame.childFrames()) {
-        if (!checkLifecycle(child, expectedLifecycle))
-          return false;
-      }
-      return true;
-    }
-  }
-
-  dispose() {
-    helper.removeEventListeners(this._eventListeners);
-    clearTimeout(this._maximumTimer);
-  }
-}
-
-const puppeteerToProtocolLifecycle = {
-  'load': 'load',
-  'domcontentloaded': 'DOMContentLoaded',
-  'networkidle0': 'networkIdle',
-  'networkidle2': 'networkAlmostIdle',
-};
-
-module.exports = {NavigatorWatcher};

lib/NetworkManager.js

@@ -20,12 +20,11 @@ const Multimap = require('./Multimap');
 class NetworkManager extends EventEmitter {
   /**
    * @param {!Puppeteer.CDPSession} client
-   * @param {!Puppeteer.FrameManager} frameManager
    */
-  constructor(client, frameManager) {
+  constructor(client) {
     super();
     this._client = client;
-    this._frameManager = frameManager;
+    this._frameManager = null;
     /** @type {!Map<string, !Request>} */
     this._requestIdToRequest = new Map();
     /** @type {!Map<string, !Protocol.Network.requestWillBeSentPayload>} */
@@ -54,6 +53,13 @@ class NetworkManager extends EventEmitter {
     this._client.on('Network.loadingFailed', this._onLoadingFailed.bind(this));
   }
 
+  /**
+   * @param {!Puppeteer.FrameManager} frameManager
+   */
+  setFrameManager(frameManager) {
+    this._frameManager = frameManager;
+  }
+
   /**
    * @param {?{username: string, password: string}} credentials
    */
@@ -196,7 +202,7 @@ class NetworkManager extends EventEmitter {
         redirectChain = request._redirectChain;
       }
     }
-    const frame = event.frameId ? this._frameManager.frame(event.frameId) : null;
+    const frame = event.frameId && this._frameManager ? this._frameManager.frame(event.frameId) : null;
     const request = new Request(this._client, frame, interceptionId, this._userRequestInterceptionEnabled, event, redirectChain);
     this._requestIdToRequest.set(event.requestId, request);
     this.emit(NetworkManager.Events.Request, request);

lib/Page.js

@@ -18,7 +18,6 @@ const fs = require('fs');
 const EventEmitter = require('events');
 const mime = require('mime');
 const {NetworkManager} = require('./NetworkManager');
-const {NavigatorWatcher} = require('./NavigatorWatcher');
 const {Dialog} = require('./Dialog');
 const {EmulationManager} = require('./EmulationManager');
 const {FrameManager} = require('./FrameManager');
@@ -79,16 +78,16 @@ class Page extends EventEmitter {
     this._keyboard = new Keyboard(client);
     this._mouse = new Mouse(client, this._keyboard);
     this._touchscreen = new Touchscreen(client, this._keyboard);
+    this._networkManager = new NetworkManager(client);
     /** @type {!FrameManager} */
-    this._frameManager = new FrameManager(client, frameTree, this);
+    this._frameManager = new FrameManager(client, frameTree, this, this._networkManager);
-    this._networkManager = new NetworkManager(client, this._frameManager);
+    this._networkManager.setFrameManager(this._frameManager);
     this._emulationManager = new EmulationManager(client);
     this._tracing = new Tracing(client);
     /** @type {!Map<string, Function>} */
     this._pageBindings = new Map();
     this._ignoreHTTPSErrors = ignoreHTTPSErrors;
     this._coverage = new Coverage(client);
-    this._defaultNavigationTimeout = 30000;
     this._javascriptEnabled = true;
     /** @type {?Puppeteer.Viewport} */
     this._viewport = null;
@@ -254,7 +253,7 @@ class Page extends EventEmitter {
    * @param {number} timeout
    */
   setDefaultNavigationTimeout(timeout) {
-    this._defaultNavigationTimeout = timeout;
+    this._frameManager.setDefaultNavigationTimeout(timeout);
   }
 
   /**
@@ -577,42 +576,8 @@ class Page extends EventEmitter {
    * @return {!Promise<?Puppeteer.Response>}
    */
   async goto(url, options = {}) {
-    const referrer = typeof options.referer === 'string' ? options.referer : this._networkManager.extraHTTPHeaders()['referer'];
-
     const mainFrame = this._frameManager.mainFrame();
-    const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout;
+    return await this._frameManager.navigateFrame(mainFrame, url, options);
-    const watcher = new NavigatorWatcher(this._client, this._frameManager, this._networkManager, mainFrame, timeout, options);
-    let ensureNewDocumentNavigation = false;
-    let error = await Promise.race([
-      navigate(this._client, url, referrer),
-      watcher.timeoutOrTerminationPromise(),
-    ]);
-    if (!error) {
-      error = await Promise.race([
-        watcher.timeoutOrTerminationPromise(),
-        ensureNewDocumentNavigation ? watcher.newDocumentNavigationPromise() : watcher.sameDocumentNavigationPromise(),
-      ]);
-    }
-    watcher.dispose();
-    if (error)
-      throw error;
-    return watcher.navigationResponse();
-
-    /**
-     * @param {!Puppeteer.CDPSession} client
-     * @param {string} url
-     * @param {string} referrer
-     * @return {!Promise<?Error>}
-     */
-    async function navigate(client, url, referrer) {
-      try {
-        const response = await client.send('Page.navigate', {url, referrer});
-        ensureNewDocumentNavigation = !!response.loaderId;
-        return response.errorText ? new Error(`${response.errorText} at ${url}`) : null;
-      } catch (error) {
-        return error;
-      }
-    }
   }
 
   /**
@@ -633,17 +598,7 @@ class Page extends EventEmitter {
    */
   async waitForNavigation(options = {}) {
     const mainFrame = this._frameManager.mainFrame();
-    const timeout = typeof options.timeout === 'number' ? options.timeout : this._defaultNavigationTimeout;
+    return await this._frameManager.waitForFrameNavigation(mainFrame, options);
-    const watcher = new NavigatorWatcher(this._client, this._frameManager, this._networkManager, mainFrame, timeout, options);
-    const error = await Promise.race([
-      watcher.timeoutOrTerminationPromise(),
-      watcher.sameDocumentNavigationPromise(),
-      watcher.newDocumentNavigationPromise()
-    ]);
-    watcher.dispose();
-    if (error)
-      throw error;
-    return watcher.navigationResponse();
   }
 
   /**