Backed out changeset 938d57f025ea (bug 1335778)

MozReview-Commit-ID: 1PODrjHutgJ

testing/marionette/driver.js

@@ -1897,25 +1897,7 @@ GeckoDriver.prototype.clickElement = function*(cmd, resp) {
       // listen for it and then just send an error back. The person making the
       // call should be aware something isnt right and handle accordingly
       this.addFrameCloseListener("click");
-
+      yield this.listener.clickElement(id);
-      let click = this.listener.clickElement({id: id, pageTimeout: this.timeouts.pageLoad});
-
-      // 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(() => {
-        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,
-            parameters);
-      });
-
-      yield click;
       break;
   }
 };

testing/marionette/harness/marionette_harness/tests/unit/test_click.py

@@ -8,7 +8,7 @@ from marionette_driver.by import By
 from marionette_driver import errors
 from marionette_driver.wait import Wait
 
-from marionette_harness import MarionetteTestCase, run_if_e10s
+from marionette_harness import MarionetteTestCase
 
 
 def inline(doc):
@@ -97,7 +97,11 @@ class TestLegacyClick(MarionetteTestCase):
         test_html = self.marionette.absolute_url("clicks.html")
         self.marionette.navigate(test_html)
         self.marionette.find_element(By.LINK_TEXT, "333333").click()
-        self.marionette.find_element(By.ID, "username")
+        # Bug 1335778 - Missing implicit wait for page being loaded
+        Wait(self.marionette, timeout=self.marionette.timeout.page_load,
+             ignored_exceptions=errors.NoSuchElementException).until(
+            lambda m: m.find_element(By.ID, "username"),
+            message="Username field hasn't been found")
         self.assertEqual(self.marionette.title, "XHTML Test Page")
 
     def test_clicking_an_element_that_is_not_displayed_raises(self):
@@ -111,7 +115,11 @@ class TestLegacyClick(MarionetteTestCase):
         test_html = self.marionette.absolute_url("clicks.html")
         self.marionette.navigate(test_html)
         self.marionette.find_element(By.ID, "overflowLink").click()
-        self.marionette.find_element(By.ID, "username")
+        # Bug 1335778 - Missing implicit wait for page being loaded
+        Wait(self.marionette, timeout=self.marionette.timeout.page_load,
+             ignored_exceptions=errors.NoSuchElementException).until(
+            lambda m: m.find_element(By.ID, "username"),
+            message="Username field hasn't been found")
         self.assertEqual(self.marionette.title, "XHTML Test Page")
 
     def test_scroll_into_view_near_end(self):
@@ -252,45 +260,3 @@ class TestClick(TestLegacyClick):
         with self.assertRaises(errors.ElementClickInterceptedException):
             obscured.click()
         self.assertFalse(self.marionette.execute_script("return window.clicked", sandbox=None))
-
-
-class TestClickNavigation(MarionetteTestCase):
-
-    def setUp(self):
-        super(TestClickNavigation, self).setUp()
-
-    def test_click_link_page_load(self):
-        test_html = self.marionette.absolute_url("clicks.html")
-        self.marionette.navigate(test_html)
-        self.marionette.find_element(By.LINK_TEXT, "333333").click()
-        self.assertNotEqual(self.marionette.get_url(), test_html)
-        self.assertEqual(self.marionette.title, "XHTML Test Page")
-
-    def test_click_link_anchor(self):
-        test_html = self.marionette.absolute_url("clicks.html")
-        self.marionette.navigate(test_html)
-        self.marionette.find_element(By.ID, "anchor").click()
-        self.assertEqual(self.marionette.get_url(), "{}#".format(test_html))
-
-    def test_click_no_link(self):
-        test_html = self.marionette.absolute_url("clicks.html")
-        self.marionette.navigate(test_html)
-        self.marionette.find_element(By.ID, "showbutton").click()
-        self.assertEqual(self.marionette.get_url(), test_html)
-
-    @run_if_e10s("Requires e10s mode enabled")
-    def test_click_remoteness_change(self):
-        test_html = self.marionette.absolute_url("clicks.html")
-
-        self.marionette.navigate("about:robots")
-        self.marionette.navigate(test_html)
-        self.marionette.find_element(By.ID, "anchor")
-        self.marionette.navigate("about:robots")
-        with self.assertRaises(errors.NoSuchElementException):
-            self.marionette.find_element(By.ID, "anchor")
-
-        self.marionette.go_back()
-        self.marionette.find_element(By.ID, "anchor")
-        self.marionette.find_element(By.ID, "history-back").click()
-        with self.assertRaises(errors.NoSuchElementException):
-            self.marionette.find_element(By.ID, "anchor")

testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py

@@ -8,7 +8,7 @@ from marionette_driver.expected import element_present
 from marionette_driver.marionette import Alert
 from marionette_driver.wait import Wait
 
-from marionette_harness import MarionetteTestCase, WindowManagerMixin
+from marionette_harness import MarionetteTestCase, skip_if_e10s, WindowManagerMixin
 
 
 class BaseAlertTestCase(WindowManagerMixin, MarionetteTestCase):
@@ -194,6 +194,7 @@ class TestTabModalAlerts(BaseAlertTestCase):
         alert.accept()
         self.wait_for_condition(lambda mn: mn.get_url() == "about:blank")
 
+    @skip_if_e10s("Bug 1325044")
     def test_unrelated_command_when_alert_present(self):
         click_handler = self.marionette.find_element(By.ID, "click-handler")
         text = self.marionette.find_element(By.ID, "click-result").text

testing/marionette/harness/marionette_harness/www/clicks.html

@@ -17,8 +17,6 @@
 </iframe>
 
 <a href="xhtmlTest.html" id="normal">I'm a normal link</a>
-<a href="#" onclick="history.back();" id="history-back">Back in browser history</a>
-<a href="#" onclick="history.forward();" id="history-forward">Forward in browser history</a>
 <a href="#" id="anchor">I go to an anchor</a>
 <a href="javascript:window.open('xhtmlTest.html', '_blank')" id="new-window">I open a window with javascript</a>
 <a href="xhtmlTest.html" id="twoClientRects"><span></span><span>Click me</span></a>

testing/marionette/listener.js

@@ -117,10 +117,8 @@ var sandboxName = "default";
  */
 var loadListener = {
   command_id: null,
-  seenUnload: null,
   timeout: null,
-  timerPageLoad: null,
+  timer: null,
-  timerPageUnload: null,
 
   /**
    * Start listening for page unload/load events.
@@ -138,44 +136,35 @@ var loadListener = {
     this.command_id = command_id;
     this.timeout = timeout;
 
-    this.seenUnload = false;
-
-    this.timerPageLoad = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    this.timerPageUnload = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-
     // In case of a remoteness change, only wait the remaining time
     timeout = startTime + timeout - new Date().getTime();
 
     if (timeout <= 0) {
-      this.notify(this.timerPageLoad);
+      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("beforeunload", this, false);
       addEventListener("hashchange", this, false);
       addEventListener("pagehide", this, false);
     } else {
       addEventListener("DOMContentLoaded", loadListener, false);
       addEventListener("pageshow", loadListener, false);
     }
-
-    this.timerPageLoad.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT);
   },
 
   /**
    * Stop listening for page unload/load events.
    */
   stop: function () {
-    if (this.timerPageLoad) {
+    if (this.timer) {
-      this.timerPageLoad.cancel();
+      this.timer.cancel();
+      this.timer = null;
     }
 
-    if (this.timerPageUnload) {
-      this.timerPageUnload.cancel();
-    }
-
-    removeEventListener("beforeunload", this);
     removeEventListener("hashchange", this);
     removeEventListener("pagehide", this);
     removeEventListener("DOMContentLoaded", this);
@@ -187,13 +176,8 @@ var loadListener = {
    */
   handleEvent: function (event) {
     switch (event.type) {
-      case "beforeunload":
-        this.seenUnload = true;
-        break;
-
       case "pagehide":
         if (event.originalTarget === curContainer.frame.document) {
-          removeEventListener("beforeunload", this);
           removeEventListener("hashchange", this);
           removeEventListener("pagehide", this);
 
@@ -239,22 +223,9 @@ var loadListener = {
    * Callback for navigation timeout timer.
    */
   notify: function (timer) {
-    switch (timer) {
+    this.stop();
-      // If the page unload timer is raised, ensure to properly stop the load
+    sendError(new TimeoutError("Timeout loading page after " + this.timeout + "ms"),
-      // listener, and return from the currently active command.
+              this.command_id);
-      case this.timerPageUnload:
-        if (!this.seenUnload) {
-          this.stop();
-          sendOk(this.command_id);
-        }
-        break;
-
-    case this.timerPageLoad:
-      this.stop();
-      sendError(new TimeoutError(`Timeout loading page after ${this.timeout}ms`),
-          this.command_id);
-      break;
-    }
   },
 
   /**
@@ -281,13 +252,22 @@ var loadListener = {
    *     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 {boolean=} loadEventExpected
-   *     TODO
    * @param {string=} url
    *     Optional URL, which is used to check if a page load is expected.
    */
-  navigate: function (trigger, command_id, timeout, loadEventExpected = true,
+  navigate: function (trigger, command_id, timeout, url = undefined) {
-      useUnloadTimer = false) {
+    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), command_id);
+        return;
+      }
+    }
+
     if (loadEventExpected) {
       let startTime = new Date().getTime();
       this.start(command_id, timeout, startTime, true);
@@ -297,13 +277,7 @@ var loadListener = {
       yield trigger();
 
     }).then(val => {
-      if (loadEventExpected) {
+      if (!loadEventExpected) {
-        // Setup timer to detect a possible page load
-        // TODO: Make it optional to wait + time with multiplier
-        if (useUnloadTimer) {
-          this.timerPageUnload.initWithCallback(this, "200", Ci.nsITimer.TYPE_ONE_SHOT);
-        }
-      } else {
         sendOk(command_id);
       }
 
@@ -427,6 +401,7 @@ function removeMessageListenerId(messageName, handler) {
 var getTitleFn = dispatch(getTitle);
 var getPageSourceFn = dispatch(getPageSource);
 var getActiveElementFn = dispatch(getActiveElement);
+var clickElementFn = dispatch(clickElement);
 var getElementAttributeFn = dispatch(getElementAttribute);
 var getElementPropertyFn = dispatch(getElementProperty);
 var getElementTextFn = dispatch(getElementText);
@@ -481,7 +456,7 @@ function startListeners() {
   addMessageListenerId("Marionette:findElementContent", findElementContentFn);
   addMessageListenerId("Marionette:findElementsContent", findElementsContentFn);
   addMessageListenerId("Marionette:getActiveElement", getActiveElementFn);
-  addMessageListenerId("Marionette:clickElement", clickElement);
+  addMessageListenerId("Marionette:clickElement", clickElementFn);
   addMessageListenerId("Marionette:getElementAttribute", getElementAttributeFn);
   addMessageListenerId("Marionette:getElementProperty", getElementPropertyFn);
   addMessageListenerId("Marionette:getElementText", getElementTextFn);
@@ -586,7 +561,7 @@ function deleteSession(msg) {
   removeMessageListenerId("Marionette:findElementContent", findElementContentFn);
   removeMessageListenerId("Marionette:findElementsContent", findElementsContentFn);
   removeMessageListenerId("Marionette:getActiveElement", getActiveElementFn);
-  removeMessageListenerId("Marionette:clickElement", clickElement);
+  removeMessageListenerId("Marionette:clickElement", clickElementFn);
   removeMessageListenerId("Marionette:getElementAttribute", getElementAttributeFn);
   removeMessageListenerId("Marionette:getElementProperty", getElementPropertyFn);
   removeMessageListenerId("Marionette:getElementText", getElementTextFn);
@@ -1125,26 +1100,15 @@ function waitForPageLoaded(msg) {
  */
 function get(msg) {
   let {command_id, pageTimeout, url} = msg.json;
-  let loadEventExpected = true;
 
   try {
-    if (typeof url == "string") {
-      try {
-        let requestedURL = new URL(url).toString();
-        loadEventExpected = navigate.isLoadEventExpected(requestedURL);
-      } catch (e) {
-        sendError(new InvalidArgumentError("Malformed URL: " + e.message), command_id);
-        return;
-      }
-    }
-
     // 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, loadEventExpected);
+    }, command_id, pageTimeout, url);
 
   } catch (e) {
     sendError(e, command_id);
@@ -1290,36 +1254,15 @@ function getActiveElement() {
 /**
  * Send click event to element.
  *
- * @param {number} command_id
- *     ID of the currently handled message between the driver and listener.
  * @param {WebElement} id
  *     Reference to the web element to click.
- * @param {number} pageTimeout
- *     Timeout in milliseconds the method has to wait for the page being finished loading.
  */
-function clickElement(msg) {
+function clickElement(id) {
-  let {command_id, id, pageTimeout} = msg.json;
+  let el = seenEls.get(id, curContainer);
-
+  return interaction.clickElement(
-  try {
+      el,
-    let loadEventExpected = true;
+      capabilities.get("moz:accessibilityChecks"),
-
+      capabilities.get("specificationLevel") >= 1);
-    let target = getElementAttribute(id, "target");
-
-    if (target === "_blank") {
-      loadEventExpected = false;
-    }
-
-    loadListener.navigate(() => {
-      return interaction.clickElement(
-        seenEls.get(id, curContainer),
-        capabilities.get("moz:accessibilityChecks"),
-        capabilities.get("specificationLevel") >= 1
-      );
-    }, command_id, pageTimeout, loadEventExpected, true);
-
-  } catch (e) {
-    sendError(e, command_id);
-  }
 }
 
 function getElementAttribute(id, name) {