Bug 1291320 - Make refresh command synchronous. r=ato,automatedtester
authorHenrik Skupin <mail@hskupin.info>
Mon, 27 Mar 2017 16:16:36 +0200
changeset 351044 c6eb25a8dc1cfe765b9e2ec11ce7fa1504383783
parent 351043 c7d68974f5a8fe73c25002a6f9ee725eb064996e
child 351045 c55face618541f04ac4ceed8e4a3531e5811337b
push id31598
push usercbook@mozilla.com
push dateTue, 04 Apr 2017 10:33:38 +0000
treeherdermozilla-central@916a4ee676a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato, automatedtester
bugs1291320
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1291320 - Make refresh command synchronous. r=ato,automatedtester Update the refresh command to make it synchronous, and let it return once the target page has been loaded. This can be accomplished by using the loadListener object in listener.js. MozReview-Commit-ID: Lc8QoGFeQrY
testing/marionette/driver.js
testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -1133,22 +1133,24 @@ GeckoDriver.prototype.goForward = functi
     this.mm.broadcastAsyncMessage(
         "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId,
         parameters);
   });
 
   yield goForward;
 };
 
-/** Refresh the page. */
-GeckoDriver.prototype.refresh = function*(cmd, resp) {
+/**
+ * Causes the browser to reload the page in in current top-level browsing context.
+ */
+GeckoDriver.prototype.refresh = function* (cmd, resp) {
   assert.content(this.context);
   assert.window(this.getCurrentWindow());
 
-  yield this.listener.refresh();
+  yield this.listener.refresh({pageTimeout: this.timeouts.pageLoad});
 };
 
 /**
  * Forces an update for the given browser's id.
  */
 GeckoDriver.prototype.updateIdForBrowser = function (browser, newId) {
   this._browserIds.set(browser.permanentKey, newId);
 };
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
@@ -1,14 +1,13 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import contextlib
-import time
 import urllib
 
 from marionette_driver import By, errors, expected, Wait
 from marionette_driver.keys import Keys
 from marionette_harness import (
     MarionetteTestCase,
     run_if_e10s,
     run_if_manage_instance,
@@ -22,16 +21,17 @@ def inline(doc):
     return "data:text/html;charset=utf-8,%s" % urllib.quote(doc)
 
 
 class BaseNavigationTestCase(WindowManagerMixin, MarionetteTestCase):
 
     def setUp(self):
         super(BaseNavigationTestCase, self).setUp()
 
+        self.test_page_frameset = self.marionette.absolute_url("frameset.html")
         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":
             self.mod_key = Keys.META
         else:
             self.mod_key = Keys.CONTROL
@@ -121,41 +121,24 @@ class TestNavigate(BaseNavigationTestCas
         self.assertEqual(page_iframe, self.marionette.get_url())
 
     def test_get_current_url(self):
         self.marionette.navigate(self.test_page_remote)
         self.assertEqual(self.test_page_remote, self.marionette.get_url())
         self.marionette.navigate("about:blank")
         self.assertEqual("about:blank", self.marionette.get_url())
 
-    def test_refresh(self):
-        self.marionette.navigate(self.test_page_remote)
-        self.assertEqual("Marionette Test", self.marionette.title)
-        self.assertTrue(self.marionette.execute_script(
-            """var elem = window.document.createElement('div'); elem.id = 'someDiv';
-            window.document.body.appendChild(elem); return true;"""))
-        self.assertFalse(self.marionette.execute_script(
-            "return window.document.getElementById('someDiv') == undefined"))
-        self.marionette.refresh()
-        # TODO(ato): Bug 1291320
-        time.sleep(0.2)
-        self.assertEqual("Marionette Test", self.marionette.title)
-        self.assertTrue(self.marionette.execute_script(
-            "return window.document.getElementById('someDiv') == undefined"))
-
     def test_navigate_in_child_frame_changes_to_top(self):
-        page_frameset = self.marionette.absolute_url("frameset.html")
-
-        self.marionette.navigate(page_frameset)
+        self.marionette.navigate(self.test_page_frameset)
         frame = self.marionette.find_element(By.NAME, "third")
         self.marionette.switch_to_frame(frame)
         self.assertRaises(errors.NoSuchElementException,
                           self.marionette.find_element, By.NAME, "third")
 
-        self.marionette.navigate(page_frameset)
+        self.marionette.navigate(self.test_page_frameset)
         self.marionette.find_element(By.NAME, "third")
 
     def test_invalid_url(self):
         with self.assertRaises(errors.MarionetteException):
             self.marionette.navigate("foo")
         with self.assertRaises(errors.MarionetteException):
             self.marionette.navigate("thisprotocoldoesnotexist://")
 
@@ -485,16 +468,75 @@ class TestBackForwardNavigation(BaseNavi
              "error": errors.InsecureCertificateException},
             {"url": self.test_page_remote},
             {"url": self.test_page_insecure,
              "error": errors.InsecureCertificateException},
         ]
         self.run_bfcache_test(test_pages)
 
 
+class TestRefresh(BaseNavigationTestCase):
+
+    def test_basic(self):
+        self.marionette.navigate(self.test_page_remote)
+        self.assertEqual(self.test_page_remote, self.marionette.get_url())
+
+        self.marionette.execute_script("""
+          let elem = window.document.createElement('div');
+          elem.id = 'someDiv';
+          window.document.body.appendChild(elem);
+        """)
+        self.marionette.find_element(By.ID, "someDiv")
+
+        self.marionette.refresh()
+        self.assertEqual(self.test_page_remote, self.marionette.get_url())
+        with self.assertRaises(errors.NoSuchElementException):
+            self.marionette.find_element(By.ID, "someDiv")
+
+    def test_refresh_in_child_frame_navigates_to_top(self):
+        self.marionette.navigate(self.test_page_frameset)
+        self.assertEqual(self.test_page_frameset, self.marionette.get_url())
+
+        frame = self.marionette.find_element(By.NAME, "third")
+        self.marionette.switch_to_frame(frame)
+        self.assertRaises(errors.NoSuchElementException,
+                          self.marionette.find_element, By.NAME, "third")
+
+        self.marionette.refresh()
+        self.marionette.find_element(By.NAME, "third")
+
+    def test_image(self):
+        image = self.marionette.absolute_url('black.png')
+
+        self.marionette.navigate(image)
+        self.assertEqual(image, self.marionette.get_url())
+
+        self.marionette.refresh()
+        self.assertEqual(image, self.marionette.get_url())
+
+    def test_timeout_error(self):
+        slow_page = self.marionette.absolute_url("slow?delay=3")
+
+        self.marionette.navigate(slow_page)
+        self.assertEqual(slow_page, self.marionette.get_url())
+
+        self.marionette.timeout.page_load = 0.5
+        with self.assertRaises(errors.TimeoutException):
+            self.marionette.refresh()
+        self.assertEqual(slow_page, self.marionette.get_url())
+
+    def test_insecure_error(self):
+        with self.assertRaises(errors.InsecureCertificateException):
+            self.marionette.navigate(self.test_page_insecure)
+        self.assertEqual(self.test_page_insecure, self.marionette.get_url())
+
+        with self.assertRaises(errors.InsecureCertificateException):
+            self.marionette.refresh()
+
+
 class TestTLSNavigation(MarionetteTestCase):
     insecure_tls = {"acceptInsecureCerts": True}
     secure_tls = {"acceptInsecureCerts": False}
 
     def setUp(self):
         super(TestTLSNavigation, self).setUp()
 
         self.test_page_insecure = self.fixtures.where_is("test.html", on="https")
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -1137,16 +1137,36 @@ function goForward(msg) {
   let {command_id, pageTimeout} = msg.json;
 
   loadListener.navigate(() => {
     curContainer.frame.history.forward();
   }, command_id, pageTimeout);
 }
 
 /**
+ * Causes the browser to reload the page in in current top-level 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 refresh(msg) {
+  let {command_id, pageTimeout} = msg.json;
+
+  // We need to move to the top frame before navigating
+  sendSyncMessage("Marionette:switchedToFrame", {frameValue: null});
+  curContainer.frame = content;
+
+  loadListener.navigate(() => {
+    curContainer.frame.location.reload(true);
+  }, command_id, pageTimeout);
+}
+
+/**
  * Get URL of the top-level browsing context.
  */
 function getCurrentUrl() {
   return content.location.href;
 }
 
 /**
  * Get the title of the current browsing context.
@@ -1158,29 +1178,16 @@ function getTitle() {
 /**
  * Get source of the current browsing context's DOM.
  */
 function getPageSource() {
   return curContainer.frame.document.documentElement.outerHTML;
 }
 
 /**
- * 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);
-  };
-  addEventListener("DOMContentLoaded", listen, false);
-}
-
-/**
  * Find an element in the current browsing context's document using the
  * given search strategy.
  */
 function* findElementContent(strategy, selector, opts = {}) {
   if (!SUPPORTED_STRATEGIES.has(strategy)) {
     throw new InvalidSelectorError("Strategy not supported: " + strategy);
   }