Backed out changeset 7c314416a41c (bug 1291320)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 23 Mar 2017 21:48:36 +0100
changeset 504122 0e16d2a2ce164e7c58577fd70e6053420effb5ef
parent 504121 c62bb4dbe6046ad1e58039447ae30ee8d7531aff
child 504123 b1f8a4a80d1a75d7446262626b74149e0dbe65f6
push id50739
push userbmo:emilio+bugs@crisal.io
push dateThu, 23 Mar 2017 23:47:46 +0000
bugs1291320
milestone55.0a1
backs out7c314416a41c2c76a16281771f1b3a66cea7d63c
Backed out changeset 7c314416a41c (bug 1291320)
testing/marionette/driver.js
testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
testing/marionette/listener.js
testing/marionette/navigate.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -958,17 +958,17 @@ GeckoDriver.prototype.executeJSScript = 
  *
  * In chrome context it will change the current window's location to
  * the supplied URL and wait until document.readyState equals "complete"
  * or the page timeout duration has elapsed.
  *
  * @param {string} url
  *     URL to navigate to.
  */
-GeckoDriver.prototype.get = function* (cmd, resp) {
+GeckoDriver.prototype.get = function*(cmd, resp) {
   assert.content(this.context);
   assert.window(this.getCurrentWindow());
 
   let url = cmd.parameters.url;
 
   let get = this.listener.get({url: url, pageTimeout: this.timeouts.pageLoad});
 
   // If a remoteness update interrupts our page load, this will never return
@@ -977,17 +977,17 @@ GeckoDriver.prototype.get = function* (c
   this.curBrowser.pendingCommands.push(() => {
     let parameters = {
       // TODO(ato): Bug 1242595
       command_id: this.listener.activeMessageId,
       pageTimeout: this.timeouts.pageLoad,
       startTime: new Date().getTime(),
     };
     this.mm.broadcastAsyncMessage(
-        "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId,
+        "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
         parameters);
   });
 
   yield get;
   browser.getBrowserForTab(this.curBrowser.tab).focus();
 };
 
 /**
@@ -1078,17 +1078,19 @@ GeckoDriver.prototype.goBack = function*
     let parameters = {
       // TODO(ato): Bug 1242595
       command_id: this.listener.activeMessageId,
       lastSeenURL: currentURL,
       pageTimeout: this.timeouts.pageLoad,
       startTime: new Date().getTime(),
     };
     this.mm.broadcastAsyncMessage(
-        "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId,
+        // TODO: combine with
+        // "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
+        "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
         parameters);
   });
 
   yield goBack;
 };
 
 /**
  * Cause the browser to traverse one step forward in the joint history
@@ -1118,17 +1120,19 @@ GeckoDriver.prototype.goForward = functi
     let parameters = {
       // TODO(ato): Bug 1242595
       command_id: this.listener.activeMessageId,
       lastSeenURL: currentURL,
       pageTimeout: this.timeouts.pageLoad,
       startTime: new Date().getTime(),
     };
     this.mm.broadcastAsyncMessage(
-        "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId,
+        // TODO: combine with
+        // "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
+        "Marionette:pollForReadyState" + this.curBrowser.curFrameId,
         parameters);
   });
 
   yield goForward;
 };
 
 /** Refresh the page. */
 GeckoDriver.prototype.refresh = function*(cmd, resp) {
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
@@ -26,17 +26,17 @@ class BaseNavigationTestCase(WindowManag
 
     def setUp(self):
         super(BaseNavigationTestCase, self).setUp()
 
         self.test_page_insecure = self.fixtures.where_is("test.html", on="https")
         self.test_page_not_remote = "about:robots"
         self.test_page_remote = self.marionette.absolute_url("test.html")
 
-        if self.marionette.session_capabilities["platformName"] == "darwin":
+        if self.marionette.session_capabilities['platformName'] == 'darwin':
             self.mod_key = Keys.META
         else:
             self.mod_key = Keys.CONTROL
 
         def open_with_link():
             link = self.marionette.find_element(By.ID, "new-blank-tab")
             link.click()
 
@@ -87,25 +87,66 @@ class BaseNavigationTestCase(WindowManag
 
               } else {
                 return null;
               }
 
               return tabBrowser.isRemoteBrowser;
             """)
 
+    def run_bfcache_test(self, test_pages):
+        # Helper method to run simple back and forward testcases.
+        for index, page in enumerate(test_pages):
+            if "error" in page:
+                with self.assertRaises(page["error"]):
+                    self.marionette.navigate(page["url"])
+            else:
+                self.marionette.navigate(page["url"])
+            self.assertEqual(page["url"], self.marionette.get_url())
+            self.assertEqual(self.history_length, index + 1)
+
+            if "is_remote" in page:
+                self.assertEqual(page["is_remote"], self.is_remote_tab,
+                                 "'{}' doesn't match expected remoteness state: {}".format(
+                                     page["url"], page["is_remote"]))
+
+        for page in test_pages[-2::-1]:
+            if "error" in page:
+                with self.assertRaises(page["error"]):
+                    self.marionette.go_back()
+            else:
+                self.marionette.go_back()
+            self.assertEqual(page["url"], self.marionette.get_url())
+
+        if "is_remote" in page:
+            self.assertEqual(page["is_remote"], self.is_remote_tab,
+                             "'{}' doesn't match expected remoteness state: {}".format(
+                                 page["url"], page["is_remote"]))
+
+        for page in test_pages[1::]:
+            if "error" in page:
+                with self.assertRaises(page["error"]):
+                    self.marionette.go_forward()
+            else:
+                self.marionette.go_forward()
+            self.assertEqual(page["url"], self.marionette.get_url())
+
+        if "is_remote" in page:
+            self.assertEqual(page["is_remote"], self.is_remote_tab,
+                             "'{}' doesn't match expected remoteness state: {}".format(
+                                 page["url"], page["is_remote"]))
+
 
 class TestNavigate(BaseNavigationTestCase):
 
     def test_set_location_through_execute_script(self):
         self.marionette.execute_script(
-            "window.location.href = '{}'".format(self.test_page_remote),
-            sandbox=None)
+            "window.location.href = '%s'" % self.test_page_remote)
         Wait(self.marionette).until(
-            lambda mn: self.test_page_remote == mn.get_url())
+            lambda _: self.test_page_remote == self.marionette.get_url())
         self.assertEqual("Marionette Test", self.marionette.title)
 
     def test_navigate_chrome_unsupported_error(self):
         with self.marionette.using_context("chrome"):
             self.assertRaises(errors.UnsupportedOperationException,
                               self.marionette.navigate, "about:blank")
             self.assertRaises(errors.UnsupportedOperationException, self.marionette.go_back)
             self.assertRaises(errors.UnsupportedOperationException, self.marionette.go_forward)
@@ -156,34 +197,34 @@ class TestNavigate(BaseNavigationTestCas
     @skip_if_mobile("Bug 1323755 - Socket timeout")
     def test_invalid_protocol(self):
         with self.assertRaises(errors.MarionetteException):
             self.marionette.navigate("thisprotocoldoesnotexist://")
 
     def test_find_element_state_complete(self):
         self.marionette.navigate(self.test_page_remote)
         state = self.marionette.execute_script(
-            "return window.document.readyState", sandbox=None)
+            "return window.document.readyState")
         self.assertEqual("complete", state)
         self.assertTrue(self.marionette.find_element(By.ID, "mozLink"))
 
     def test_navigate_timeout_error_no_remoteness_change(self):
         is_remote_before_timeout = self.is_remote_tab
-        self.marionette.timeout.page_load = 0.5
+        self.marionette.timeout.page_load = 0.1
         with self.assertRaises(errors.TimeoutException):
             self.marionette.navigate(self.marionette.absolute_url("slow"))
         self.assertEqual(self.is_remote_tab, is_remote_before_timeout)
 
     @run_if_e10s("Requires e10s mode enabled")
     def test_navigate_timeout_error_remoteness_change(self):
         self.assertTrue(self.is_remote_tab)
         self.marionette.navigate("about:robots")
         self.assertFalse(self.is_remote_tab)
 
-        self.marionette.timeout.page_load = 0.5
+        self.marionette.timeout.page_load = 0.1
         with self.assertRaises(errors.TimeoutException):
             self.marionette.navigate(self.marionette.absolute_url("slow"))
 
         # Even with the page not finished loading the browser is remote
         self.assertTrue(self.is_remote_tab)
 
     def test_navigate_to_same_image_document_twice(self):
         self.marionette.navigate(self.fixtures.where_is("black.png"))
@@ -198,17 +239,17 @@ class TestNavigate(BaseNavigationTestCas
         self.marionette.navigate("{}#foo".format(doc))
         self.assertTrue(self.marionette.execute_script(
             "return window.visited", sandbox=None))
 
     @skip_if_mobile("Bug 1334095 - Timeout: No new tab has been opened")
     def test_about_blank_for_new_docshell(self):
         self.assertEqual(self.marionette.get_url(), "about:blank")
 
-        self.marionette.navigate("about:blank")
+        self.marionette.navigate('about:blank')
 
     @run_if_manage_instance("Only runnable if Marionette manages the instance")
     @skip_if_mobile("Bug 1322993 - Missing temporary folder")
     def test_focus_after_navigation(self):
         self.marionette.quit()
         self.marionette.start_session()
 
         self.marionette.navigate(inline("<input autofocus>"))
@@ -226,105 +267,60 @@ class TestNavigate(BaseNavigationTestCas
 
     @skip("Bug 1334137 - Intermittent: Process killed because of hang in getCurrentUrl()")
     @skip_if_mobile("Interacting with chrome elements not available for Fennec")
     def test_type_to_non_remote_tab(self):
         self.marionette.navigate(self.test_page_not_remote)
         self.assertFalse(self.is_remote_tab)
 
         with self.marionette.using_context("chrome"):
-            urlbar = self.marionette.find_element(By.ID, "urlbar")
-            urlbar.send_keys(self.mod_key + "a")
-            urlbar.send_keys(self.mod_key + "x")
-            urlbar.send_keys("about:support" + Keys.ENTER)
+            urlbar = self.marionette.find_element(By.ID, 'urlbar')
+            urlbar.send_keys(self.mod_key + 'a')
+            urlbar.send_keys(self.mod_key + 'x')
+            urlbar.send_keys('about:support' + Keys.ENTER)
 
         Wait(self.marionette).until(
             lambda mn: mn.get_url() == "about:support",
             message="'about:support' hasn't been loaded")
         self.assertFalse(self.is_remote_tab)
 
     @skip_if_mobile("Interacting with chrome elements not available for Fennec")
     @run_if_e10s("Requires e10s mode enabled")
     def test_type_to_remote_tab(self):
         self.assertTrue(self.is_remote_tab)
 
         with self.marionette.using_context("chrome"):
-            urlbar = self.marionette.find_element(By.ID, "urlbar")
-            urlbar.send_keys(self.mod_key + "a")
-            urlbar.send_keys(self.mod_key + "x")
+            urlbar = self.marionette.find_element(By.ID, 'urlbar')
+            urlbar.send_keys(self.mod_key + 'a')
+            urlbar.send_keys(self.mod_key + 'x')
             urlbar.send_keys(self.test_page_remote + Keys.ENTER)
 
         Wait(self.marionette).until(
             lambda mn: mn.get_url() == self.test_page_remote,
             message="'{}' hasn't been loaded".format(self.test_page_remote))
         self.assertTrue(self.is_remote_tab)
 
     @skip_if_mobile("On Android no shortcuts are available")
     def test_navigate_shortcut_key(self):
 
         def open_with_shortcut():
             self.marionette.navigate(self.test_page_remote)
             with self.marionette.using_context("chrome"):
                 main_win = self.marionette.find_element(By.ID, "main-window")
-                main_win.send_keys(self.mod_key, Keys.SHIFT, "a")
+                main_win.send_keys(self.mod_key, Keys.SHIFT, 'a')
 
         new_tab = self.open_tab(trigger=open_with_shortcut)
         self.marionette.switch_to_window(new_tab)
 
         Wait(self.marionette).until(lambda mn: mn.get_url() == "about:addons",
                                     message="'about:addons' hasn't been loaded")
 
 
 class TestBackForwardNavigation(BaseNavigationTestCase):
 
-    def run_bfcache_test(self, test_pages):
-        # Helper method to run simple back and forward testcases.
-        for index, page in enumerate(test_pages):
-            if "error" in page:
-                with self.assertRaises(page["error"]):
-                    self.marionette.navigate(page["url"])
-            else:
-                self.marionette.navigate(page["url"])
-            self.assertEqual(page["url"], self.marionette.get_url())
-            self.assertEqual(self.history_length, index + 1)
-
-            if "is_remote" in page:
-                self.assertEqual(page["is_remote"], self.is_remote_tab,
-                                 "'{}' doesn't match expected remoteness state: {}".format(
-                                     page["url"], page["is_remote"]))
-
-        # Now going back in history for all test pages by backward iterating
-        # through the list (-1) and skipping the first entry at the end (-2).
-        for page in test_pages[-2::-1]:
-            if "error" in page:
-                with self.assertRaises(page["error"]):
-                    self.marionette.go_back()
-            else:
-                self.marionette.go_back()
-            self.assertEqual(page["url"], self.marionette.get_url())
-
-        if "is_remote" in page:
-            self.assertEqual(page["is_remote"], self.is_remote_tab,
-                             "'{}' doesn't match expected remoteness state: {}".format(
-                                 page["url"], page["is_remote"]))
-
-        # Now going forward in history by skipping the first entry.
-        for page in test_pages[1::]:
-            if "error" in page:
-                with self.assertRaises(page["error"]):
-                    self.marionette.go_forward()
-            else:
-                self.marionette.go_forward()
-            self.assertEqual(page["url"], self.marionette.get_url())
-
-        if "is_remote" in page:
-            self.assertEqual(page["is_remote"], self.is_remote_tab,
-                             "'{}' doesn't match expected remoteness state: {}".format(
-                                 page["url"], page["is_remote"]))
-
     def test_no_history_items(self):
         # Both methods should not raise a failure if no navigation is possible
         self.marionette.go_back()
         self.marionette.go_forward()
 
     def test_data_urls(self):
         test_pages = [
             {"url": inline("<p>foobar</p>")},
@@ -391,28 +387,28 @@ class TestBackForwardNavigation(BaseNavi
         # Go forward to the frame the click navigated to
         # TODO: See above for automatic browser context switches. Hard to do here
         frame = self.marionette.find_element(By.ID, "fifth")
         self.marionette.switch_to_frame(frame)
         self.marionette.go_forward()
         self.marionette.find_element(*test_element_locator)
         self.assertEqual(self.marionette.get_url(), page)
 
-    def test_image_to_html_to_image(self):
+    def test_image_document_to_html(self):
         test_pages = [
-            {"url": self.marionette.absolute_url("black.png")},
+            {"url": self.marionette.absolute_url('black.png')},
             {"url": self.test_page_remote},
-            {"url": self.marionette.absolute_url("white.png")},
+            {"url": self.marionette.absolute_url('white.png')},
         ]
         self.run_bfcache_test(test_pages)
 
-    def test_image_to_image(self):
+    def test_image_document_to_image_document(self):
         test_pages = [
-            {"url": self.marionette.absolute_url("black.png")},
-            {"url": self.marionette.absolute_url("white.png")},
+            {"url": self.marionette.absolute_url('black.png')},
+            {"url": self.marionette.absolute_url('white.png')},
         ]
         self.run_bfcache_test(test_pages)
 
     @run_if_e10s("Requires e10s mode enabled")
     def test_remoteness_change(self):
         test_pages = [
             {"url": "about:robots", "is_remote": False},
             {"url": self.test_page_remote, "is_remote": True},
@@ -434,54 +430,46 @@ class TestBackForwardNavigation(BaseNavi
             {"url": "about:neterror"},
             {"url": self.test_page_remote},
             {"url": "about:blocked"},
         ]
         self.run_bfcache_test(test_pages)
 
     def test_timeout_error(self):
         urls = [
-            self.marionette.absolute_url("slow?delay=3"),
+            self.marionette.absolute_url('slow'),
             self.test_page_remote,
-            self.marionette.absolute_url("slow?delay=4"),
+            self.marionette.absolute_url('slow'),
         ]
 
         # First, load all pages completely to get them added to the cache
         for index, url in enumerate(urls):
             self.marionette.navigate(url)
             self.assertEqual(url, self.marionette.get_url())
             self.assertEqual(self.history_length, index + 1)
 
         self.marionette.go_back()
         self.assertEqual(urls[1], self.marionette.get_url())
 
         # Force triggering a timeout error
-        self.marionette.timeout.page_load = 0.5
+        self.marionette.timeout.page_load = 0.1
         with self.assertRaises(errors.TimeoutException):
             self.marionette.go_back()
-        self.marionette.timeout.reset()
-
-        Wait(self.marionette, self.marionette.timeout.page_load).until(
-            lambda mn: urls[0] == mn.get_url(),
-            message="Slow loading page has been successfully loaded after going back")
-        self.assertEqual(self.marionette.find_element(By.ID, "delay").text, "3")
+        self.assertEqual(urls[0], self.marionette.get_url())
+        self.marionette.timeout.page_load = 300000
 
         self.marionette.go_forward()
         self.assertEqual(urls[1], self.marionette.get_url())
 
         # Force triggering a timeout error
-        self.marionette.timeout.page_load = 0.5
+        self.marionette.timeout.page_load = 0.1
         with self.assertRaises(errors.TimeoutException):
             self.marionette.go_forward()
-        self.marionette.timeout.reset()
-
-        Wait(self.marionette, self.marionette.timeout.page_load).until(
-            lambda mn: urls[2] == mn.get_url(),
-            message="Slow loading page has been successfully loaded after going forward")
-        self.assertEqual(self.marionette.find_element(By.ID, "delay").text, "4")
+        self.assertEqual(urls[2], self.marionette.get_url())
+        self.marionette.timeout.page_load = 300000
 
     def test_certificate_error(self):
         test_pages = [
             {"url": self.test_page_insecure,
              "error": errors.InsecureCertificateException},
             {"url": self.test_page_remote},
             {"url": self.test_page_insecure,
              "error": errors.InsecureCertificateException},
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -73,16 +73,19 @@ var asyncTestTimeoutId;
 
 var inactivityTimeoutId = null;
 
 var originalOnError;
 //timer for doc changes
 var checkTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
 //timer for readystate
 var readyStateTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+// timer for navigation commands.
+var navTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+var onDOMContentLoaded;
 // Send move events about this often
 var EVENT_INTERVAL = 30; // milliseconds
 // last touch for each fingerId
 var multiLast = {};
 var asyncChrome = proxy.toChromeAsync({
   addMessageListener: addMessageListenerId.bind(this),
   removeMessageListener: removeMessageListenerId.bind(this),
   sendAsyncMessage: sendAsyncMessage.bind(this),
@@ -105,188 +108,16 @@ var modalHandler = function() {
   curContainer = {frame: content, shadowRoot: null};
 };
 
 // sandbox storage and name of the current sandbox
 var sandboxes = new Sandboxes(() => curContainer.frame);
 var sandboxName = "default";
 
 /**
- * The load listener singleton helps to keep track of active page load activities,
- * and can be used by any command which might cause a navigation to happen. In the
- * specific case of remoteness changes it allows to continue observing the current
- * page load.
- */
-var loadListener = {
-  command_id: null,
-  timeout: null,
-  timer: null,
-
-  /**
-   * Start listening for page unload/load events.
-   *
-   * @param {number} command_id
-   *     ID of the currently handled message between the driver and listener.
-   * @param {number} timeout
-   *     Timeout in seconds the method has to wait for the page being finished loading.
-   * @param {number} startTime
-   *     Unix timestap when the navitation request got triggered.
-   * @param {boolean=} waitForUnloaded
-   *     If `true` wait for page unload events, otherwise only for page load events.
-   */
-  start: function (command_id, timeout, startTime, waitForUnloaded = true) {
-    this.command_id = command_id;
-    this.timeout = timeout;
-
-    // In case of a remoteness change, only wait the remaining time
-    timeout = startTime + timeout - new Date().getTime();
-
-    if (timeout <= 0) {
-      this.notify();
-      return;
-    }
-
-    this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    this.timer.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
-
-    if (waitForUnloaded) {
-      addEventListener("hashchange", this, false);
-      addEventListener("pagehide", this, false);
-    } else {
-      addEventListener("DOMContentLoaded", loadListener, false);
-      addEventListener("pageshow", loadListener, false);
-    }
-  },
-
-  /**
-   * Stop listening for page unload/load events.
-   */
-  stop: function () {
-    if (this.timer) {
-      this.timer.cancel();
-      this.timer = null;
-    }
-
-    removeEventListener("hashchange", this);
-    removeEventListener("pagehide", this);
-    removeEventListener("DOMContentLoaded", this);
-    removeEventListener("pageshow", this);
-  },
-
-  /**
-   * Callback for registered DOM events.
-   */
-  handleEvent: function (event) {
-    switch (event.type) {
-      case "pagehide":
-        if (event.originalTarget === curContainer.frame.document) {
-          removeEventListener("hashchange", this);
-          removeEventListener("pagehide", this);
-
-          // Now wait until the target page has been loaded
-          addEventListener("DOMContentLoaded", this, false);
-          addEventListener("pageshow", this, false);
-        }
-        break;
-
-      case "hashchange":
-        this.stop();
-        sendOk(this.command_id);
-        break;
-
-      case "DOMContentLoaded":
-        if (event.originalTarget.baseURI.startsWith("about:certerror")) {
-          this.stop();
-          sendError(new InsecureCertificateError(), this.command_id);
-
-        } else if (/about:.*(error)\?/.exec(event.originalTarget.baseURI)) {
-          this.stop();
-          sendError(new UnknownError("Reached error page: " +
-              event.originalTarget.baseURI), this.command_id);
-
-        // Special-case about:blocked pages which should be treated as non-error
-        // pages but do not raise a pageshow event.
-        } else if (/about:blocked\?/.exec(event.originalTarget.baseURI)) {
-          this.stop();
-          sendOk(this.command_id);
-        }
-        break;
-
-      case "pageshow":
-        if (event.originalTarget === curContainer.frame.document) {
-          this.stop();
-          sendOk(this.command_id);
-        }
-        break;
-    }
-  },
-
-  /**
-   * Callback for navigation timeout timer.
-   */
-  notify: function (timer) {
-    this.stop();
-    sendError(new TimeoutError("Timeout loading page after " + this.timeout + "ms"),
-              this.command_id);
-  },
-
-  /**
-   * Continue to listen for page load events after a remoteness change happened.
-   *
-   * @param {number} command_id
-   *     ID of the currently handled message between the driver and listener.
-   * @param {number} timeout
-   *     Timeout in milliseconds the method has to wait for the page being finished loading.
-   * @param {number} startTime
-   *     Unix timestap when the navitation request got triggered.
-   */
-  waitForLoadAfterRemotenessChange: function (command_id, timeout, startTime) {
-    this.start(command_id, timeout, startTime, false);
-  },
-
-  /**
-   * Use a trigger callback to initiate a page load, and attach listeners if
-   * a page load is expected.
-   *
-   * @param {function} trigger
-   *     Callback that triggers the page load.
-   * @param {number} command_id
-   *     ID of the currently handled message between the driver and listener.
-   * @param {number} pageTimeout
-   *     Timeout in milliseconds the method has to wait for the page being finished loading.
-   * @param {string=} url
-   *     Optional URL, which is used to check if a page load is expected.
-   */
-  navigate: function (trigger, command_id, timeout, url = undefined) {
-    let loadEventExpected = true;
-
-    if (typeof url == "string") {
-      try {
-        let requestedURL = new URL(url).toString();
-        loadEventExpected = navigate.isLoadEventExpected(requestedURL);
-      } catch (e) {
-        sendError(new InvalidArgumentError("Malformed URL: " + e.message), this.command_id);
-        return;
-      }
-    }
-
-    if (loadEventExpected) {
-      let startTime = new Date().getTime();
-      this.start(command_id, timeout, startTime, true);
-    }
-
-    trigger();
-
-    if (!loadEventExpected) {
-      sendOk(this.command_id);
-    }
-  },
-}
-
-/**
  * Called when listener is first started up.
  * The listener sends its unique window ID and its current URI to the actor.
  * If the actor returns an ID, we start the listeners. Otherwise, nothing happens.
  */
 function registerSelf() {
   let msg = {value: winUtil.outerWindowID};
   // register will have the ID and a boolean describing if this is the main process or not
   let register = sendSyncMessage("Marionette:register", msg);
@@ -347,17 +178,17 @@ function emitTouchEventForIFrame(message
 function dispatch(fn) {
   if (typeof fn != "function") {
     throw new TypeError("Provided dispatch handler is not a function");
   }
 
   return function (msg) {
     let id = msg.json.command_id;
 
-    let req = Task.spawn(function* () {
+    let req = Task.spawn(function*() {
       if (typeof msg.json == "undefined" || msg.json instanceof Array) {
         return yield fn.apply(null, msg.json);
       } else {
         return yield fn(msg.json);
       }
     });
 
     let okOrValueResponse = rv => {
@@ -429,17 +260,17 @@ function startListeners() {
   addMessageListenerId("Marionette:executeInSandbox", executeInSandboxFn);
   addMessageListenerId("Marionette:executeSimpleTest", executeSimpleTestFn);
   addMessageListenerId("Marionette:singleTap", singleTapFn);
   addMessageListenerId("Marionette:performActions", performActionsFn);
   addMessageListenerId("Marionette:releaseActions", releaseActionsFn);
   addMessageListenerId("Marionette:actionChain", actionChainFn);
   addMessageListenerId("Marionette:multiAction", multiActionFn);
   addMessageListenerId("Marionette:get", get);
-  addMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded);
+  addMessageListenerId("Marionette:pollForReadyState", pollForReadyState);
   addMessageListenerId("Marionette:cancelRequest", cancelRequest);
   addMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
   addMessageListenerId("Marionette:getTitle", getTitleFn);
   addMessageListenerId("Marionette:getPageSource", getPageSourceFn);
   addMessageListenerId("Marionette:goBack", goBack);
   addMessageListenerId("Marionette:goForward", goForward);
   addMessageListenerId("Marionette:refresh", refresh);
   addMessageListenerId("Marionette:findElementContent", findElementContentFn);
@@ -534,17 +365,17 @@ function deleteSession(msg) {
   removeMessageListenerId("Marionette:executeInSandbox", executeInSandboxFn);
   removeMessageListenerId("Marionette:executeSimpleTest", executeSimpleTestFn);
   removeMessageListenerId("Marionette:singleTap", singleTapFn);
   removeMessageListenerId("Marionette:performActions", performActionsFn);
   removeMessageListenerId("Marionette:releaseActions", releaseActionsFn);
   removeMessageListenerId("Marionette:actionChain", actionChainFn);
   removeMessageListenerId("Marionette:multiAction", multiActionFn);
   removeMessageListenerId("Marionette:get", get);
-  removeMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded);
+  removeMessageListenerId("Marionette:pollForReadyState", pollForReadyState);
   removeMessageListenerId("Marionette:cancelRequest", cancelRequest);
   removeMessageListenerId("Marionette:getTitle", getTitleFn);
   removeMessageListenerId("Marionette:getPageSource", getPageSourceFn);
   removeMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
   removeMessageListenerId("Marionette:goBack", goBack);
   removeMessageListenerId("Marionette:goForward", goForward);
   removeMessageListenerId("Marionette:refresh", refresh);
   removeMessageListenerId("Marionette:findElementContent", findElementContentFn);
@@ -611,18 +442,18 @@ function sendToServer(uuid, data = undef
 /**
  * Send asynchronous reply with value to chrome.
  *
  * @param {?} obj
  *     JSON serialisable object of arbitrary type and complexity.
  * @param {UUID} uuid
  *     Unique identifier of the request.
  */
-function sendResponse(obj, uuid) {
-  sendToServer(uuid, obj);
+function sendResponse(obj, id) {
+  sendToServer(id, obj);
 }
 
 /**
  * Send asynchronous reply to chrome.
  *
  * @param {UUID} uuid
  *     Unique identifier of the request.
  */
@@ -1050,92 +881,259 @@ 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);
 }
 
 /**
- * Cancel the polling and remove the event listener associated with a
- * current navigation request in case we're interupted by an onbeforeunload
- * handler and navigation doesn't complete.
- */
-function cancelRequest() {
-  loadListener.stop();
-}
-
-/**
  * 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 {string=} lastSeenURL
+ *     Last URL as seen before the navigation request got triggered.
  * @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 waitForPageLoaded(msg) {
-  let {command_id, pageTimeout, startTime} = msg.json;
+function pollForReadyState(msg) {
+  let {cleanupCallback, command_id, lastSeenURL, pageTimeout, startTime} = msg.json;
+
+  if (typeof startTime == "undefined") {
+    startTime = new Date().getTime();
+  }
+
+  if (typeof cleanupCallback == "undefined") {
+    cleanupCallback = () => {};
+  }
+
+  let endTime = startTime + pageTimeout;
+
+  let checkLoad = () => {
+    navTimer.cancel();
+
+    let doc = curContainer.frame.document;
+
+    if (pageTimeout === null || new Date().getTime() <= endTime) {
+      // Under some conditions (eg. for error pages) the pagehide event is fired
+      // even with a readyState complete for the formerly loaded page.
+      // To prevent race conditition for goBack and goForward we have to wait
+      // until the last seen page has been fully unloaded.
+      // TODO: Bug 1333458 has to improve this.
+      if (!doc.location || lastSeenURL && doc.location.href === lastSeenURL) {
+        navTimer.initWithCallback(checkLoad, 100, Ci.nsITimer.TYPE_ONE_SHOT);
 
-  loadListener.waitForLoadAfterRemotenessChange(command_id, pageTimeout, startTime);
+      // document fully loaded
+      } else if (doc.readyState === "complete") {
+        cleanupCallback();
+        sendOk(command_id);
+
+      // document with an insecure cert
+      } else if (doc.readyState === "interactive" &&
+          doc.baseURI.startsWith("about:certerror")) {
+        cleanupCallback();
+        sendError(new InsecureCertificateError(), command_id);
+
+      // we have reached an error url without requesting it
+      } else if (doc.readyState === "interactive" &&
+          /about:.+(error)\?/.exec(doc.baseURI)) {
+        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:")) {
+        cleanupCallback();
+        sendOk(command_id);
+
+      // document not fully loaded
+      } else {
+        navTimer.initWithCallback(checkLoad, 100, Ci.nsITimer.TYPE_ONE_SHOT);
+      }
+
+    } else {
+      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 {command_id, pageTimeout, url} = msg.json;
+  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;
 
-  loadListener.navigate(() => {
-    curContainer.frame.location = url;
-  }, command_id, pageTimeout, url);
+  let docShell = curContainer.frame
+      .document
+      .defaultView
+      .QueryInterface(Ci.nsIInterfaceRequestor)
+      .getInterface(Ci.nsIWebNavigation)
+      .QueryInterface(Ci.nsIDocShell);
+  let webProgress = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+      .getInterface(Ci.nsIWebProgress);
+  let sawLoad = false;
+
+  let requestedURL;
+  let loadEventExpected = false;
+  try {
+    requestedURL = new URL(url).toString();
+    let curURL = curContainer.frame.location;
+    loadEventExpected = navigate.isLoadEventExpected(curURL, requestedURL);
+  } catch (e) {
+    sendError(new InvalidArgumentError("Malformed URL: " + e.message), command_id);
+    return;
+  }
+
+  // It's possible that a site we're being sent to will end up redirecting
+  // us before we end up on a page that fires DOMContentLoaded. We can ensure
+  // This loadListener ensures that we don't send a success signal back to
+  // the caller until we've seen the load of the requested URL attempted
+  // on this frame.
+  let loadListener = {
+    QueryInterface: XPCOMUtils.generateQI(
+        [Ci.nsIWebProgressListener, Ci.nsISupportsWeakReference]),
+
+    onStateChange(webProgress, request, state, status) {
+      if (!(request instanceof Ci.nsIChannel)) {
+        return;
+      }
+
+      const isDocument = state & Ci.nsIWebProgressListener.STATE_IS_DOCUMENT;
+      const loadedURL = request.URI.spec;
+
+      // We have to look at the originalURL because of about: pages,
+      // the loadedURL is what the about: page resolves to, and is
+      // not the one that was requested.
+      const originalURL = request.originalURI.spec;
+      const isRequestedURL = loadedURL == requestedURL ||
+          originalURL == requestedURL;
+
+      if (!isDocument || !isRequestedURL) {
+        return;
+      }
+
+      // We started loading the requested document. This document
+      // might not be the one that ends up firing DOMContentLoaded
+      // (if it, for example, redirects), but because we've started
+      // loading this URL, we know that any future DOMContentLoaded's
+      // are fair game to tell the Marionette client about.
+      if (state & Ci.nsIWebProgressListener.STATE_START) {
+        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({json: {
+          command_id: command_id,
+          pageTimeout: pageTimeout,
+          startTime: startTime,
+          cleanupCallback: () => {
+            webProgress.removeProgressListener(loadListener);
+            removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+          }
+        }});
+      }
+    },
+
+    onLocationChange() {},
+    onProgressChange() {},
+    onStatusChange() {},
+    onSecurityChange() {},
+  };
+
+  webProgress.addProgressListener(
+      loadListener, Ci.nsIWebProgress.NOTIFY_STATE_DOCUMENT);
+
+  // Prevent DOMContentLoaded events from frames from invoking this
+  // code, unless the event is coming from the frame associated with
+  // the current window (i.e. someone has used switch_to_frame).
+  onDOMContentLoaded = ev => {
+    let frameEl = ev.originalTarget.defaultView.frameElement;
+    let correctFrame = !frameEl || frameEl == curContainer.frame.frameElement;
+
+    // If the page we're at fired DOMContentLoaded and appears to
+    // be the one we asked to load, then we definitely saw the load
+    // occur. We need this because for error pages, like about:neterror
+    // for unsupported protocols, we don't end up opening a channel that
+    // our WebProgressListener can monitor.
+    if (curContainer.frame.location == requestedURL) {
+      sawLoad = true;
+    }
+
+    // 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) {
+      pollForReadyState({json: {
+        command_id: command_id,
+        pageTimeout: pageTimeout,
+        startTime: startTime,
+        cleanupCallback: () => {
+          webProgress.removeProgressListener(loadListener);
+          removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+        }
+      }});
+    }
+  };
+
+  if (typeof pageTimeout != "undefined") {
+    let onTimeout = () => {
+      if (loadEventExpected) {
+        removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+      }
+      webProgress.removeProgressListener(loadListener);
+      sendError(new TimeoutError("Error loading page, timed out (onDOMContentLoaded)"), command_id);
+    };
+    navTimer.initWithCallback(onTimeout, pageTimeout, Ci.nsITimer.TYPE_ONE_SHOT);
+  }
+
+  if (loadEventExpected) {
+    addEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+  }
+  curContainer.frame.location = requestedURL;
+  if (!loadEventExpected) {
+    sendOk(command_id);
+  }
 }
 
 /**
- * Cause the browser to traverse one step backward in the joint history
- * of the current browsing context.
- *
- * @param {number} command_id
- *     ID of the currently handled message between the driver and listener.
- * @param {number} pageTimeout
- *     Timeout in milliseconds the method has to wait for the page being finished loading.
+ * Cancel the polling and remove the event listener associated with a
+ * current navigation request in case we're interupted by an onbeforeunload
+ * handler and navigation doesn't complete.
  */
-function goBack(msg) {
-  let {command_id, pageTimeout} = msg.json;
-
-  loadListener.navigate(() => {
-    curContainer.frame.history.back();
-  }, command_id, pageTimeout);
-}
-
-/**
- * Cause the browser to traverse one step forward in the joint history
- * of the current browsing context.
- *
- * @param {number} command_id
- *     ID of the currently handled message between the driver and listener.
- * @param {number} pageTimeout
- *     Timeout in milliseconds the method has to wait for the page being finished loading.
- */
-function goForward(msg) {
-  let {command_id, pageTimeout} = msg.json;
-
-  loadListener.navigate(() => {
-    curContainer.frame.history.forward();
-  }, command_id, pageTimeout);
+function cancelRequest() {
+  navTimer.cancel();
+  if (onDOMContentLoaded) {
+    removeEventListener("DOMContentLoaded", onDOMContentLoaded, false);
+  }
 }
 
 /**
  * Get URL of the top-level browsing context.
  */
 function getCurrentUrl() {
   return content.location.href;
 }
@@ -1150,16 +1148,127 @@ function getTitle() {
 /**
  * Get source of the current browsing context's DOM.
  */
 function getPageSource() {
   return curContainer.frame.document.documentElement.outerHTML;
 }
 
 /**
+ * Wait for the current page to be unloaded after a navigation got triggered.
+ *
+ * @param {function} trigger
+ *     Callback to execute which triggers a page navigation.
+ * @param {function} doneCallback
+ *     Callback to execute when the current page has been unloaded.
+ *
+ *     It receives a dictionary with the following items as argument:
+ *         loading - Flag if a page load will follow.
+ *         lastSeenURL - Last seen URL before the navigation request.
+ *         startTime - Time when the navigation request has been triggered.
+ */
+function waitForPageUnloaded(trigger, doneCallback) {
+  let currentURL = curContainer.frame.location.href;
+  let start = new Date().getTime();
+
+  function handleEvent(event) {
+    // In case of a remoteness change it can happen that we are no longer able
+    // to access the document's location. In those cases ignore the event,
+    // but keep the code waiting, and assume in the driver that waiting for the
+    // page load is necessary. Bug 1333458 should improve things.
+    if (typeof event.originalTarget.location == "undefined") {
+      return;
+    }
+
+    switch (event.type) {
+      case "hashchange":
+        removeEventListener("hashchange", handleEvent);
+        removeEventListener("pagehide", handleEvent);
+        removeEventListener("unload", handleEvent);
+
+        doneCallback({loading: false, lastSeenURL: currentURL});
+        break;
+
+      case "pagehide":
+      case "unload":
+        if (event.originalTarget === curContainer.frame.document) {
+          removeEventListener("hashchange", handleEvent);
+          removeEventListener("pagehide", handleEvent);
+          removeEventListener("unload", handleEvent);
+
+          doneCallback({loading: true, lastSeenURL: currentURL, startTime: start});
+        }
+        break;
+    }
+  }
+
+  addEventListener("hashchange", handleEvent, false);
+  addEventListener("pagehide", handleEvent, false);
+  addEventListener("unload", handleEvent, false);
+
+  trigger();
+}
+
+/**
+ * Cause the browser to traverse one step backward in the joint history
+ * of the current browsing context.
+ *
+ * @param {number} command_id
+ *     ID of the currently handled message between the driver and listener.
+ * @param {number} pageTimeout
+ *     Timeout in milliseconds the method has to wait for the page being finished loading.
+ */
+function goBack(msg) {
+  let {command_id, pageTimeout} = msg.json;
+
+  waitForPageUnloaded(() => {
+      curContainer.frame.history.back();
+    }, pageLoadStatus => {
+    if (pageLoadStatus.loading) {
+      pollForReadyState({json: {
+        command_id: command_id,
+        lastSeenURL: pageLoadStatus.lastSeenURL,
+        pageTimeout: pageTimeout,
+        startTime: pageLoadStatus.startTime,
+      }});
+    } else {
+      sendOk(command_id);
+    }
+  });
+}
+
+/**
+ * Cause the browser to traverse one step forward in the joint history
+ * of the current browsing context.
+ *
+ * @param {number} command_id
+ *     ID of the currently handled message between the driver and listener.
+ * @param {number} pageTimeout
+ *     Timeout in milliseconds the method has to wait for the page being finished loading.
+ */
+function goForward(msg) {
+  let {command_id, pageTimeout} = msg.json;
+
+  waitForPageUnloaded(() => {
+    curContainer.frame.history.forward();
+  }, pageLoadStatus => {
+    if (pageLoadStatus.loading) {
+      pollForReadyState({json: {
+        command_id: command_id,
+        lastSeenURL: pageLoadStatus.lastSeenURL,
+        pageTimeout: pageTimeout,
+        startTime: pageLoadStatus.startTime,
+      }});
+    } else {
+      sendOk(command_id);
+    }
+  });
+}
+
+/**
  * Refresh the page
  */
 function refresh(msg) {
   let command_id = msg.json.command_id;
   curContainer.frame.location.reload(true);
   let listen = function() {
     removeEventListener("DOMContentLoaded", listen, false);
     sendOk(command_id);
--- a/testing/marionette/navigate.js
+++ b/testing/marionette/navigate.js
@@ -9,35 +9,111 @@ const {classes: Cc, interfaces: Ci, util
 Cu.importGlobalProperties(["URL"]);
 
 this.EXPORTED_SYMBOLS = ["navigate"];
 
 this.navigate = {};
 
 /**
  * Determines if we expect to get a DOM load event (DOMContentLoaded)
- * on navigating to the |url|.
+ * on navigating to the |future| URL.
  *
- * @param {string} url
- *     Destination URL
+ * @param {string} current
+ *     URL the browser is currently visiting.
+ * @param {string=} future
+ *     Destination URL, if known.
  *
  * @return {boolean}
- *     Full page load would be expected if url gets loaded.
+ *     Full page load would be expected if future is followed.
  *
  * @throws TypeError
- *     If |url| is an invalid URL.
+ *     If |current| is not defined, or any of |current| or |future|
+ *     are invalid URLs.
  */
-navigate.isLoadEventExpected = function (url) {
+navigate.isLoadEventExpected = function (current, future = undefined) {
+  if (typeof current == "undefined") {
+    throw TypeError("Expected at least one URL");
+  }
+
   // assume we will go somewhere exciting
-  if (typeof url == "undefined") {
-    throw TypeError("Expected destination URL");
+  if (typeof future == "undefined") {
+    return true;
   }
 
-  switch (new URL(url).protocol) {
-    // assume javascript:<whatever> will modify current document
-    // but this is not an entirely safe assumption to make,
-    // considering it could be used to set window.location
-    case "javascript:":
-      return false;
+  let cur = new navigate.IdempotentURL(current);
+  let fut = new navigate.IdempotentURL(future);
+
+  // assume javascript:<whatever> will modify current document
+  // but this is not an entirely safe assumption to make,
+  // considering it could be used to set window.location
+  if (fut.protocol == "javascript:") {
+    return false;
+  }
+
+  // navigating to same url, but with any hash
+  if (cur.origin == fut.origin &&
+      cur.pathname == fut.pathname &&
+      fut.hash != "") {
+    return false;
   }
 
   return true;
 };
+
+/**
+ * Sane URL implementation that normalises URL fragments (hashes) and
+ * path names for "data:" URLs, and makes them idempotent.
+ *
+ * At the time of writing this, the web is approximately 10 000 days (or
+ * ~27.39 years) old.  One should think that by this point we would have
+ * solved URLs.  The following code is prudent example that we have not.
+ *
+ * When a URL with a fragment identifier but no explicit name for the
+ * fragment is given, i.e. "#", the {@code hash} property a {@code URL}
+ * object computes is an empty string.  This is incidentally the same as
+ * the default value of URLs without fragments, causing a lot of confusion.
+ *
+ * This means that the URL "http://a/#b" produces a hash of "#b", but that
+ * "http://a/#" produces "".  This implementation rectifies this behaviour
+ * by returning the actual full fragment, which is "#".
+ *
+ * "data:" URLs that contain fragments, which if they have the same origin
+ * and path name are not meant to cause a page reload on navigation,
+ * confusingly adds the fragment to the {@code pathname} property.
+ * This implementation remedies this behaviour by trimming it off.
+ *
+ * The practical result of this is that while {@code URL} objects are
+ * not idempotent, the returned URL elements from this implementation
+ * guarantees that |url.hash == url.hash|.
+ *
+ * @param {string|URL} o
+ *     Object to make an URL of.
+ *
+ * @return {navigate.IdempotentURL}
+ *     Considered by some to be a somewhat saner URL.
+ *
+ * @throws TypeError
+ *     If |o| is not a valid type or if is a string that cannot be parsed
+ *     as a URL.
+ */
+navigate.IdempotentURL = function (o) {
+  let url = new URL(o);
+
+  let hash = url.hash;
+  if (hash == "" && url.href[url.href.length - 1] == "#") {
+    hash = "#";
+  }
+
+  return {
+    hash: hash,
+    host: url.host,
+    hostname: url.hostname,
+    href: url.href,
+    origin: url.origin,
+    password: url.password,
+    pathname: url.pathname,
+    port: url.port,
+    protocol: url.protocol,
+    search: url.search,
+    searchParams: url.searchParams,
+    username: url.username,
+  };
+};