Bug 1361983 - Extend page unload timer if flushEventLoop returns after beforeunload event.
In cases when the navigation trigger function returns too late and the
beforeunload event has already been fired, the page load currently gets
canceled. This happens because the page unload timer hasn't been set at
this time and the unbeforeunload handler doesn't extend its time.
With this patch a flag is used which indicates if an beforeunload
event already occurred, so when the pageunload timer notifies the page
listener the unload timer can be extended to 5s as expected.
MozReview-Commit-ID: FKK0oGEWijU
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
@@ -332,16 +332,20 @@ class TestClickNavigation(MarionetteTest
self.assertEqual(self.marionette.get_url(), self.test_page)
finally:
self.close_notification()
def test_click_no_link(self):
self.marionette.find_element(By.ID, "showbutton").click()
self.assertEqual(self.marionette.get_url(), self.test_page)
+ def test_click_option_navigate(self):
+ self.marionette.find_element(By.ID, "option").click()
+ self.marionette.find_element(By.ID, "delay")
+
@run_if_e10s("Requires e10s mode enabled")
def test_click_remoteness_change(self):
self.marionette.navigate("about:robots")
self.marionette.navigate(self.test_page)
self.marionette.find_element(By.ID, "anchor")
self.marionette.navigate("about:robots")
with self.assertRaises(errors.NoSuchElementException):
--- a/testing/marionette/harness/marionette_harness/www/clicks.html
+++ b/testing/marionette/harness/marionette_harness/www/clicks.html
@@ -33,10 +33,13 @@
<div id="addbuttonlistener" onclick="var el = document.getElementById('showbutton');el.addEventListener('mousedown', function (evt) { document.getElementById('showbutton').innerHTML = evt.button; }, false);">
click to add an event listener
</div>
<div id="showbutton">
this will show the button clicked
</div>
<a href="xhtmlTest.html">333333</a>
<p><a href="xhtmlTest.html" id="embeddedBlock"><span style="display: block;">I have a span</span><div>And a div</div><span>And another span</span></a></p>
+<select id="option" onclick="window.location = '/slow?delay=1'">
+ <option>Click to navigate</option>
+</select>
</body>
</html>
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -116,17 +116,18 @@ 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,
- seenUnload: null,
+ seenBeforeUnload: false,
+ seenUnload: false,
timeout: null,
timerPageLoad: null,
timerPageUnload: null,
/**
* Start listening for page unload/load events.
*
* @param {number} command_id
@@ -137,16 +138,17 @@ var loadListener = {
* 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;
+ this.seenBeforeUnload = false;
this.seenUnload = false;
this.timerPageLoad = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
this.timerPageUnload = null;
// In case of a remoteness change, only wait the remaining time
timeout = startTime + timeout - new Date().getTime();
@@ -208,27 +210,17 @@ var loadListener = {
* Callback for registered DOM events.
*/
handleEvent: function (event) {
let location = event.target.baseURI || event.target.location.href;
logger.debug(`Received DOM event "${event.type}" for "${location}"`);
switch (event.type) {
case "beforeunload":
- if (this.timerPageUnload) {
- // In the case when a document has a beforeunload handler registered,
- // the currently active command will return immediately due to the
- // modal dialog observer in proxy.js.
- // Otherwise the timeout waiting for the document to start navigating
- // is increased by 5000 ms to ensure a possible load event is not missed.
- // In the common case such an event should occur pretty soon after
- // beforeunload, and we optimise for this.
- this.timerPageUnload.cancel();
- this.timerPageUnload.initWithCallback(this, 5000, Ci.nsITimer.TYPE_ONE_SHOT)
- }
+ this.seenBeforeUnload = true;
break;
case "unload":
this.seenUnload = true;
break;
case "pagehide":
if (event.target === curContainer.frame.document) {
@@ -277,20 +269,31 @@ var loadListener = {
}
},
/**
* Callback for navigation timeout timer.
*/
notify: function (timer) {
switch (timer) {
- // If the page unload timer is raised, ensure to properly stop the load
- // listener, and return from the currently active command.
case this.timerPageUnload:
- if (!this.seenUnload) {
+ // In the case when a document has a beforeunload handler registered,
+ // the currently active command will return immediately due to the
+ // modal dialog observer in proxy.js.
+ // Otherwise the timeout waiting for the document to start navigating
+ // is increased by 5000 ms to ensure a possible load event is not missed.
+ // In the common case such an event should occur pretty soon after
+ // beforeunload, and we optimise for this.
+ if (this.seenBeforeUnload) {
+ this.seenBeforeUnload = null;
+ this.timerPageUnload.initWithCallback(this, 5000, Ci.nsITimer.TYPE_ONE_SHOT)
+
+ // If no page unload has been detected, ensure to properly stop the load
+ // listener, and return from the currently active command.
+ } else 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: