Bug 1330348 - Make pollForReadyState a shared method for navigation commands. r=ato, a=test-only
authorHenrik Skupin <mail@hskupin.info>
Thu, 09 Mar 2017 11:21:30 +0100
changeset 355363 b3a10f8056464027893eaf65b4a4c6f28747ea1c
parent 355362 f2415416bddf9829d2cb789862de93c96893f061
child 355364 867ab104af2ffde878a1390008dad61565a82215
push id6946
push userryanvm@gmail.com
push dateThu, 16 Mar 2017 19:46:03 +0000
treeherdermozilla-esr52@055d03b56e14 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato, test-only
bugs1330348
milestone52.0.1
Bug 1330348 - Make pollForReadyState a shared method for navigation commands. r=ato, a=test-only All navigation commands including get, goBack, goForward, and maybe others in the future should rely on the same method for fetching the readyState of a document. As such prepare `pollForReadyState` and `get` for the upcoming usage. MozReview-Commit-ID: 5Y4U9dgM7uj
testing/marionette/driver.js
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -939,28 +939,30 @@ GeckoDriver.prototype.executeJSScript = 
  *     URL to navigate to.
  */
 GeckoDriver.prototype.get = function*(cmd, resp) {
   assert.content(this.context);
 
   let url = cmd.parameters.url;
 
   let get = this.listener.get({url: url, pageTimeout: this.timeouts.pageLoad});
-  // TODO(ato): Bug 1242595
-  let id = this.listener.activeMessageId;
 
   // If a remoteness update interrupts our page load, this will never return
   // We need to re-issue this request to correctly poll for readyState and
   // send errors.
   this.curBrowser.pendingCommands.push(() => {
-    cmd.parameters.command_id = id;
-    cmd.parameters.pageTimeout = this.timeouts.pageLoad;
+    let parameters = {
+      // TODO(ato): Bug 1242595
+      command_id: this.listener.activeMessageId,
+      pageTimeout: this.timeouts.pageLoad,
+      startTime: new Date().getTime(),
+    };
     this.mm.broadcastAsyncMessage(
         "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
-        cmd.parameters);
+        parameters);
   });
 
   yield get;
   browser.getBrowserForTab(this.curBrowser.tab).focus();
 };
 
 /**
  * Get a string representing the current URL.
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -881,81 +881,97 @@ function multiAction(args, maxLen) {
 
   // now concurrent event is made of sets where each set contain a list of actions that need to be fired.
   // note: each action belongs to a different finger
   // pendingTouches keeps track of current touches that's on the screen
   let pendingTouches = [];
   setDispatch(concurrentEvent, pendingTouches);
 }
 
-/*
+/**
  * This implements the latter part of a get request (for the case we need to resume one
  * when a remoteness update happens in the middle of a navigate request). This is most of
  * of the work of a navigate request, but doesn't assume DOMContentLoaded is yet to fire.
+ *
+ * @param {function=} cleanupCallback
+ *     Callback to execute when registered event handlers or observer notifications
+ *     have to be cleaned-up.
+ * @param {number} command_id
+ *     ID of the currently handled message between the driver and listener.
+ * @param {number} pageTimeout
+ *     Timeout in seconds the method has to wait for the page being finished loading.
+ * @param {number} startTime
+ *     Unix timestap when the navitation request got triggred.
  */
-function pollForReadyState(msg, start = undefined, callback = undefined) {
-  let {pageTimeout, url, command_id} = msg.json;
-  if (!start) {
-    start = new Date().getTime();
+function pollForReadyState(msg) {
+  let {cleanupCallback, command_id, pageTimeout, startTime} = msg.json;
+
+  if (typeof startTime == "undefined") {
+    startTime = new Date().getTime();
   }
-  if (!callback) {
-    callback = () => {};
+
+  if (typeof cleanupCallback == "undefined") {
+    cleanupCallback = () => {};
   }
 
+  let endTime = startTime + pageTimeout;
+
   let checkLoad = function() {
     navTimer.cancel();
 
     let doc = curContainer.frame.document;
-    let now = new Date().getTime();
-    if (pageTimeout == null || (now - start) <= pageTimeout) {
+
+    if (pageTimeout === null || new Date().getTime() <= endTime) {
       // document fully loaded
-      if (doc.readyState == "complete") {
-        callback();
+      if (doc.readyState === "complete") {
+        cleanupCallback();
         sendOk(command_id);
 
       // document with an insecure cert
-      } else if (doc.readyState == "interactive" &&
+      } else if (doc.readyState === "interactive" &&
           doc.baseURI.startsWith("about:certerror")) {
-        callback();
+        cleanupCallback();
         sendError(new InsecureCertificateError(), command_id);
 
       // we have reached an error url without requesting it
-      } else if (doc.readyState == "interactive" &&
+      } else if (doc.readyState === "interactive" &&
           /about:.+(error)\?/.exec(doc.baseURI)) {
-        callback();
+        cleanupCallback();
         sendError(new UnknownError("Reached error page: " + doc.baseURI), command_id);
 
       // return early for about: urls
-      } else if (doc.readyState == "interactive" && doc.baseURI.startsWith("about:")) {
-        callback();
+      } else if (doc.readyState === "interactive" && doc.baseURI.startsWith("about:")) {
+        cleanupCallback();
         sendOk(command_id);
 
       // document not fully loaded
       } else {
         navTimer.initWithCallback(checkLoad, 100, Ci.nsITimer.TYPE_ONE_SHOT);
       }
 
     } else {
-      callback();
+      cleanupCallback();
       sendError(new TimeoutError("Error loading page, timed out (checkLoad)"), command_id);
     }
   };
+
   checkLoad();
 }
 
 /**
  * Navigate to the given URL.  The operation will be performed on the
  * current browsing context, which means it handles the case where we
  * navigate within an iframe.  All other navigation is handled by the
  * driver (in chrome space).
  */
 function get(msg) {
-  let start = new Date().getTime();
   let {pageTimeout, url, command_id} = msg.json;
 
+  let startTime = new Date().getTime();
+
   // We need to move to the top frame before navigating
   sendSyncMessage("Marionette:switchedToFrame", {frameValue: null});
   curContainer.frame = content;
 
   let docShell = curContainer.frame
       .document
       .defaultView
       .QueryInterface(Ci.nsIInterfaceRequestor)
@@ -1013,19 +1029,25 @@ function get(msg) {
         sawLoad = true;
       }
 
       // This indicates network stop or last request stop outside of
       // loading the document.  We hit this when DOMContentLoaded is
       // not triggered, which is the case for image documents.
       else if (state & Ci.nsIWebProgressListener.STATE_STOP &&
           content.document instanceof content.ImageDocument) {
-        pollForReadyState(msg, start, () => {
-          removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
-        });
+        pollForReadyState({json: {
+          command_id: command_id,
+          pageTimeout: pageTimeout,
+          startTime: startTime,
+          cleanupCallback: () => {
+            webProgress.removeProgressListener(loadListener);
+            removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+          }
+        }});
       }
     },
 
     onLocationChange() {},
     onProgressChange() {},
     onStatusChange() {},
     onSecurityChange() {},
   };
@@ -1052,20 +1074,25 @@ function get(msg) {
 
     // We also need to make sure that if the requested URL is not about:blank
     // the DOMContentLoaded we saw isn't for the initial about:blank of a newly
     // created docShell.
     let loadedRequestedURI = (requestedURL == "about:blank") ||
         docShell.hasLoadedNonBlankURI;
 
     if (correctFrame && sawLoad && loadedRequestedURI) {
-      webProgress.removeProgressListener(loadListener);
-      pollForReadyState(msg, start, () => {
-        removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
-      });
+      pollForReadyState({json: {
+        command_id: command_id,
+        pageTimeout: pageTimeout,
+        startTime: startTime,
+        cleanupCallback: () => {
+          webProgress.removeProgressListener(loadListener);
+          removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+        }
+      }});
     }
   };
 
   if (typeof pageTimeout != "undefined") {
     let onTimeout = function() {
       if (loadEventExpected) {
         removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
       }