Backed out changeset e084deb550c2 (bug 1335778)

testing/marionette/driver.js

@@ -2026,25 +2026,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):
@@ -269,46 +277,3 @@ class TestClick(TestLegacyClick):
                                      "does not have pointer events enabled"):
             button.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 import errors
 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):

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/interaction.js

@@ -178,8 +178,8 @@ function* webdriverClickElement (el, a11y) {
   yield interaction.flushEventLoop(win);
 
   // step 10
-  // if the click causes navigation, the post-navigation checks are
+  // TODO(ato): if the click causes navigation,
-  // handled by the load listener in listener.js
+  // run post-navigation checks
 }
 
 function* chromeClick (el, a11y) {
@@ -291,19 +291,20 @@ interaction.selectOption = function (el) {
  *     |win| has closed or been unloaded before the queue can be flushed.
  */
 interaction.flushEventLoop = function* (win) {
-  return new Promise(resolve => {
+  let unloadEv;
-    let handleEvent = event => {
-      win.removeEventListener("beforeunload", this);
-      resolve();
-    };
 
+  return new Promise((resolve, reject) => {
     if (win.closed) {
-      resolve();
+      reject();
       return;
     }
 
-    win.addEventListener("beforeunload", handleEvent);
+    unloadEv = reject;
-    win.requestAnimationFrame(handleEvent);
+    win.addEventListener("unload", unloadEv, {once: true});
+
+    win.requestAnimationFrame(resolve);
+  }).then(() => {
+    win.removeEventListener("unload", unloadEv);
   });
 };
 

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,81 +136,48 @@ 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("hashchange", this, false);
       addEventListener("pagehide", this, false);
-
-      // The event can only be received if the listener gets added to the
-      // currently selected frame.
-      curContainer.frame.addEventListener("beforeunload", this, false);
-
-      Services.obs.addObserver(this, "outer-window-destroyed");
     } 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("hashchange", this);
     removeEventListener("pagehide", this);
     removeEventListener("DOMContentLoaded", this);
     removeEventListener("pageshow", this);
-
-    // If the original content window, where the navigation was triggered,
-    // doesn't exist anymore, exceptions can be silently ignored.
-    try {
-      curContainer.frame.removeEventListener("beforeunload", this);
-    } catch (e if e.name == "TypeError") {}
-
-    // In the case when the observer was added before a remoteness change,
-    // it will no longer be available. Exceptions can be silently ignored.
-    try {
-      Services.obs.removeObserver(this, "outer-window-destroyed");
-    } catch (e) {}
   },
 
   /**
    * Callback for registered DOM events.
    */
   handleEvent: function (event) {
-    logger.debug(`Received DOM event "${event.type}" for "${event.originalTarget.baseURI}"`);
-
     switch (event.type) {
-      case "beforeunload":
-        this.seenUnload = true;
-        break;
-
       case "pagehide":
         if (event.originalTarget === curContainer.frame.document) {
-          this.seenUnload = true;
-
           removeEventListener("hashchange", this);
           removeEventListener("pagehide", this);
 
@@ -258,43 +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) {
-          logger.debug("Canceled page load listener because no navigation " +
-              "has been detected");
-          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;
-    }
-  },
-
-  observe: function (subject, topic, data) {
-    const winID = subject.QueryInterface(Components.interfaces.nsISupportsPRUint64).data;
-    const curWinID = curContainer.frame.QueryInterface(Ci.nsIInterfaceRequestor)
-        .getInterface(Ci.nsIDOMWindowUtils).outerWindowID;
-
-    logger.debug(`Received observer notification "${topic}" for "${winID}"`);
-
-    switch (topic) {
-      // In the case when the currently selected frame is about to close,
-      // there will be no further load events. Stop listening immediately.
-      case "outer-window-destroyed":
-        if (curWinID === winID) {
-          this.stop();
-          sendOk(this.command_id);
-        }
-        break;
-    }
   },
 
   /**
@@ -320,15 +251,23 @@ var loadListener = {
    * @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 finished loading.
+   *     Timeout in milliseconds the method has to wait for the page being finished loading.
-   * @param {boolean=} loadEventExpected
-   *     Optional flag, which indicates that navigate has to wait for the page
-   *     finished loading.
    * @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);
@@ -338,14 +277,8 @@ var loadListener = {
       yield trigger();
 
     }).then(val => {
-     if (!loadEventExpected) {
+      if (!loadEventExpected) {
         sendOk(command_id);
-        return;
-      }
-
-      // If requested setup a timer to detect a possible page load
-      if (useUnloadTimer) {
-        this.timerPageUnload.initWithCallback(this, 200, Ci.nsITimer.TYPE_ONE_SHOT);
       }
 
     }).catch(err => {
@@ -353,6 +286,7 @@ var loadListener = {
         this.stop();
       }
 
+      // Check why we do not raise an error if err is of type Event
       sendError(err, command_id);
       return;
     });
@@ -468,6 +402,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);
@@ -522,7 +457,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);
@@ -627,7 +562,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);
@@ -1166,26 +1101,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);
@@ -1331,36 +1255,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) {