From 7fdf800a39231caf3b2792c5aa95f8e4fe6a5167 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pfeldman@chromium.org>
Date: Fri, 28 Jul 2017 16:52:38 -0700
Subject: [PATCH] Simplify frame manager, do not fetch resources upfront (#159)

This patch:
- gets rid of Page.getResourceTree() protocol method
- rolls chromium to 490379

Fixes #127
---
 lib/FrameManager.js | 84 ++++++++++++++++-----------------------------
 lib/Page.js         |  5 ++-
 package.json        |  2 +-
 3 files changed, 35 insertions(+), 56 deletions(-)

diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index 9ef74a23..2e2830cb 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -20,28 +20,17 @@ let EventEmitter = require('events');
 let helper = require('./helper');
 
 class FrameManager extends EventEmitter {
-  /**
-   * @param {!Connection} client
-   * @param {!Mouse} mouse
-   * @return {!Promise<!FrameManager>}
-   */
-  static async create(client, mouse) {
-    let mainFramePayload = await client.send('Page.getResourceTree');
-    return new FrameManager(client, mainFramePayload.frameTree, mouse);
-  }
-
   /**
    * @param {!Connection} client
    * @param {!Object} frameTree
    * @param {!Mouse} mouse
    */
-  constructor(client, frameTree, mouse) {
+  constructor(client, mouse) {
     super();
     this._client = client;
     this._mouse = mouse;
     /** @type {!Map<string, !Frame>} */
     this._frames = new Map();
-    this._mainFrame = this._addFramesRecursively(null, frameTree);
 
     this._client.on('Page.frameAttached', event => this._onFrameAttached(event.frameId, event.parentFrameId));
     this._client.on('Page.frameNavigated', event => this._onFrameNavigated(event.frame));
@@ -82,13 +71,34 @@ class FrameManager extends EventEmitter {
    * @param {!Object} framePayload
    */
   _onFrameNavigated(framePayload) {
-    let frame = this._frames.get(framePayload.id);
-    if (!frame) {
-      // Simulate missed "frameAttached" for a main frame navigation to the new backend process.
-      console.assert(!framePayload.parentId, 'Main frame shouldn\'t have parent frame id.');
-      frame = this._mainFrame;
+    const isMainFrame = !framePayload.parentId;
+    let frame = isMainFrame ? this._mainFrame : this._frames.get(framePayload.id);
+    console.assert(isMainFrame || frame, 'We either navigate top level or have old version of the navigated frame');
+
+    // Detach all child frames first.
+    if (frame) {
+      for (let child of frame.childFrames())
+        this._removeFramesRecursively(child);
     }
-    this._navigateFrame(frame, framePayload);
+
+    // Update or create main frame.
+    if (isMainFrame) {
+      if (frame) {
+        // Update frame id to retain frame identity on cross-process navigation.
+        this._frames.delete(frame._id, frame);
+        frame._id = framePayload.id;
+      } else {
+        // Initial main frame navigation.
+        frame = new Frame(this._client, this._mouse, null, framePayload.id);
+      }
+      this._frames.set(framePayload.id, frame);
+      this._mainFrame = frame;
+    }
+
+    // Update frame payload.
+    frame._navigated(framePayload);
+
+    this.emit(FrameManager.Events.FrameNavigated, frame);
   }
 
   /**
@@ -106,40 +116,8 @@ class FrameManager extends EventEmitter {
     if (!frame)
       return;
     frame._defaultContextId = context.id;
-  }
-
-  /**
-   * @param {!Frame} frame
-   * @param {?Object} newFramePayload
-   */
-  _navigateFrame(frame, newFramePayload) {
-    // Detach all child frames first.
-    for (let child of frame.childFrames())
-      this._removeFramesRecursively(child);
-    this._frames.delete(frame._id);
-
-    // Update frame id to retain frame identity.
-    frame._id = newFramePayload.id;
-
-    frame._navigated(newFramePayload);
-    this._frames.set(newFramePayload.id, frame);
-    this.emit(FrameManager.Events.FrameNavigated, frame);
-  }
-
-  /**
-   * @param {?Frame} parentFrame
-   * @param {!Object} frameTreePayload
-   * @return {!Frame}
-   */
-  _addFramesRecursively(parentFrame, frameTreePayload) {
-    let framePayload = frameTreePayload.frame;
-    let frame = new Frame(this._client, this._mouse, parentFrame, framePayload.id);
-    frame._navigated(framePayload);
-    this._frames.set(frame._id, frame);
-
-    for (let i = 0; frameTreePayload.childFrames && i < frameTreePayload.childFrames.length; ++i)
-      this._addFramesRecursively(frame, frameTreePayload.childFrames[i]);
-    return frame;
+    for (let waitTask of frame._waitTasks)
+      waitTask.rerun();
   }
 
   /**
@@ -442,8 +420,6 @@ class Frame {
   _navigated(framePayload) {
     this._name = framePayload.name;
     this._url = framePayload.url;
-    for (let waitTask of this._waitTasks)
-      waitTask.rerun();
   }
 
   _detach() {
diff --git a/lib/Page.js b/lib/Page.js
index fe5f3b1d..708df3f1 100644
--- a/lib/Page.js
+++ b/lib/Page.js
@@ -40,11 +40,12 @@ class Page extends EventEmitter {
     ]);
     const keyboard = new Keyboard(client);
     const mouse = new Mouse(client, keyboard);
-    const frameManager = await FrameManager.create(client, mouse);
+    const frameManager = new FrameManager(client, mouse);
     const networkManager = new NetworkManager(client);
     const page = new Page(client, frameManager, networkManager, screenshotTaskQueue, mouse, keyboard);
     // Initialize default page size.
     await page.setViewport({width: 400, height: 300});
+    await page.navigate('about:blank');
     return page;
   }
 
@@ -273,6 +274,8 @@ class Page extends EventEmitter {
    * @return {!Promise<?Response>}
    */
   async reload(options) {
+    if (!this.mainFrame())
+      return;
     this._client.send('Page.reload');
     return this.waitForNavigation(options);
   }
diff --git a/package.json b/package.json
index 1fc5b2d8..cc1ff17b 100644
--- a/package.json
+++ b/package.json
@@ -29,7 +29,7 @@
     "ws": "^3.0.0"
   },
   "puppeteer": {
-    "chromium_revision": "488994"
+    "chromium_revision": "490379"
   },
   "devDependencies": {
     "commonmark": "^0.27.0",