From 4adf11427af540b271913435a10d4378527c9281 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pfeldman@chromium.org>
Date: Tue, 25 Jul 2017 14:58:10 -0700
Subject: [PATCH] Unconditionally update frame payload on navigation only.
 (#130)

---
 lib/FrameManager.js | 47 +++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/lib/FrameManager.js b/lib/FrameManager.js
index bc277c307e9..7cdc7c2290b 100644
--- a/lib/FrameManager.js
+++ b/lib/FrameManager.js
@@ -72,7 +72,7 @@ class FrameManager extends EventEmitter {
       return;
     console.assert(parentFrameId);
     let parentFrame = this._frames.get(parentFrameId);
-    let frame = new Frame(this._client, this._mouse, parentFrame, frameId, null);
+    let frame = new Frame(this._client, this._mouse, parentFrame, frameId);
     this._frames.set(frame._id, frame);
     this.emit(FrameManager.Events.FrameAttached, frame);
   }
@@ -87,7 +87,7 @@ class FrameManager extends EventEmitter {
       console.assert(!framePayload.parentId, 'Main frame shouldn\'t have parent frame id.');
       frame = this._mainFrame;
     }
-    this._navigateFrame(frame, framePayload.id, framePayload);
+    this._navigateFrame(frame, framePayload);
   }
 
   /**
@@ -109,17 +109,19 @@ class FrameManager extends EventEmitter {
 
   /**
    * @param {!Frame} frame
-   * @param {string} newFrameId
    * @param {?Object} newFramePayload
    */
-  _navigateFrame(frame, newFrameId, newFramePayload) {
+  _navigateFrame(frame, newFramePayload) {
     // Detach all child frames first.
     for (let child of frame.childFrames())
       this._removeFramesRecursively(child);
     this._frames.delete(frame._id, frame);
-    frame._id = newFrameId;
-    frame._adoptPayload(newFramePayload);
-    this._frames.set(newFrameId, frame);
+
+    // 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);
   }
 
@@ -130,7 +132,8 @@ class FrameManager extends EventEmitter {
    */
   _addFramesRecursively(parentFrame, frameTreePayload) {
     let framePayload = frameTreePayload.frame;
-    let frame = new Frame(this._client, this._mouse, parentFrame, framePayload.id, framePayload);
+    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)
@@ -166,9 +169,8 @@ class Frame {
    * @param {!Mouse} mouse
    * @param {?Frame} parentFrame
    * @param {string} frameId
-   * @param {?Object} payload
    */
-  constructor(client, mouse, parentFrame, frameId, payload) {
+  constructor(client, mouse, parentFrame, frameId) {
     this._client = client;
     this._mouse = mouse;
     this._parentFrame = parentFrame;
@@ -178,8 +180,6 @@ class Frame {
     /** @type {!Set<!WaitTask>} */
     this._waitTasks = new Set();
 
-    this._adoptPayload(payload);
-
     /** @type {!Set<!Frame>} */
     this._childFrames = new Set();
     if (this._parentFrame)
@@ -301,12 +301,7 @@ class Frame {
     const timeout = options.timeout || 30000;
     const waitForVisible = !!options.visible;
     const pageScript = helper.evaluationString(waitForSelectorPageFunction, selector, waitForVisible, timeout);
-    const waitTask = new WaitTask(this, pageScript, timeout);
-
-    this._waitTasks.add(waitTask);
-    let cleanup = () => this._waitTasks.delete(waitTask);
-    waitTask.promise.then(cleanup, cleanup);
-    return waitTask.promise;
+    return new WaitTask(this, pageScript, timeout).promise;
   }
 
   /**
@@ -398,17 +393,13 @@ class Frame {
   }
 
   /**
-   * @param {?Object} framePayload
+   * @param {!Object} framePayload
    */
-  _adoptPayload(framePayload) {
-    framePayload = framePayload || {
-      name: '',
-      url: '',
-    };
+  _navigated(framePayload) {
     this._name = framePayload.name;
     this._url = framePayload.url;
     for (let waitTask of this._waitTasks)
-      waitTask.run();
+      waitTask.rerun();
   }
 
   _detach() {
@@ -432,6 +423,7 @@ class WaitTask {
     this._frame = frame;
     this._pageScript = pageScript;
     this._runningTask = null;
+    frame._waitTasks.add(this);
     this.promise = new Promise((resolve, reject) => {
       this._resolve = resolve;
       this._reject = reject;
@@ -439,7 +431,7 @@ class WaitTask {
     // Since page navigation requires us to re-install the pageScript, we should track
     // timeout on our end.
     this._timeoutTimer = setTimeout(() => this.terminate(new Error(`waiting failed: timeout ${timeout}ms exceeded`)), timeout);
-    this.run();
+    this.rerun();
   }
 
   /**
@@ -450,7 +442,7 @@ class WaitTask {
     this._cleanup();
   }
 
-  run() {
+  rerun() {
     let runningTask = this._frame._evaluateExpression(this._pageScript, true).then(finish.bind(this), finish.bind(this, false));
     this._runningTask = runningTask;
 
@@ -474,6 +466,7 @@ class WaitTask {
 
   _cleanup() {
     clearTimeout(this._timeoutTimer);
+    this._frame._waitTasks.delete(this);
     this._runningTask = null;
   }
 }