From 808ec675bee27d5275e6ed8e804deb5e45daf3a2 Mon Sep 17 00:00:00 2001 From: Sebastian Hengst Date: Wed, 29 Mar 2017 00:30:56 +0200 Subject: [PATCH] Backed out changeset cf7d24fe5cec (bug 1291320) --- testing/marionette/driver.js | 12 +- .../tests/unit/test_navigation.py | 154 +++-- testing/marionette/listener.js | 563 +++++++++++------- testing/marionette/navigate.js | 106 +++- testing/marionette/test_navigate.js | 52 +- 5 files changed, 552 insertions(+), 335 deletions(-) diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index 13d0220bd362..f79ef41b5679 100644 --- a/testing/marionette/driver.js +++ b/testing/marionette/driver.js @@ -961,7 +961,7 @@ GeckoDriver.prototype.executeJSScript = function* (cmd, resp) { * @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()); @@ -980,7 +980,7 @@ GeckoDriver.prototype.get = function* (cmd, resp) { startTime: new Date().getTime(), }; this.mm.broadcastAsyncMessage( - "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId, + "Marionette:pollForReadyState" + this.curBrowser.curFrameId, parameters); }); @@ -1081,7 +1081,9 @@ GeckoDriver.prototype.goBack = function* (cmd, resp) { 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); }); @@ -1121,7 +1123,9 @@ GeckoDriver.prototype.goForward = function* (cmd, resp) { 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); }); diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py index c896962155a6..f5356253badf 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py @@ -31,7 +31,7 @@ class BaseNavigationTestCase(WindowManagerMixin, MarionetteTestCase): 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 @@ -92,15 +92,56 @@ class BaseNavigationTestCase(WindowManagerMixin, MarionetteTestCase): 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): @@ -162,13 +203,13 @@ class TestNavigate(BaseNavigationTestCase): 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) @@ -179,7 +220,7 @@ class TestNavigate(BaseNavigationTestCase): 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")) @@ -204,7 +245,7 @@ class TestNavigate(BaseNavigationTestCase): 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") @@ -232,10 +273,10 @@ class TestNavigate(BaseNavigationTestCase): 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", @@ -248,9 +289,9 @@ class TestNavigate(BaseNavigationTestCase): 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( @@ -265,7 +306,7 @@ class TestNavigate(BaseNavigationTestCase): 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) @@ -276,51 +317,6 @@ class TestNavigate(BaseNavigationTestCase): 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() @@ -397,18 +393,18 @@ class TestBackForwardNavigation(BaseNavigationTestCase): 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) @@ -440,9 +436,9 @@ class TestBackForwardNavigation(BaseNavigationTestCase): 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 @@ -455,29 +451,21 @@ class TestBackForwardNavigation(BaseNavigationTestCase): 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 = [ diff --git a/testing/marionette/listener.js b/testing/marionette/listener.js index 64836bc487ad..79ee4dade45e 100644 --- a/testing/marionette/listener.js +++ b/testing/marionette/listener.js @@ -78,6 +78,9 @@ var originalOnError; 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 @@ -109,186 +112,6 @@ var modalHandler = function() { 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), command_id); - return; - } - } - - if (loadEventExpected) { - let startTime = new Date().getTime(); - this.start(command_id, timeout, startTime, true); - } - - try { - trigger(); - } catch (e) { - if (loadEventExpected) { - this.stop(); - } - sendError(new UnknownCommandError(e.message), command_id); - return; - } - - if (!loadEventExpected) { - sendOk(command_id); - } - }, -} - /** * Called when listener is first started up. * The listener sends its unique window ID and its current URI to the actor. @@ -360,7 +183,7 @@ function dispatch(fn) { 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 { @@ -442,7 +265,7 @@ function startListeners() { 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); @@ -547,7 +370,7 @@ function deleteSession(msg) { 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); @@ -624,8 +447,8 @@ function sendToServer(uuid, data = undefined) { * @param {UUID} uuid * Unique identifier of the request. */ -function sendResponse(obj, uuid) { - sendToServer(uuid, obj); +function sendResponse(obj, id) { + sendToServer(id, obj); } /** @@ -1062,31 +885,84 @@ function multiAction(args, maxLen) { 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; - loadListener.waitForLoadAfterRemotenessChange(command_id, pageTimeout, startTime); + 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); + + // 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(); } /** @@ -1096,49 +972,163 @@ function waitForPageLoaded(msg) { * 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); + } } /** @@ -1162,6 +1152,117 @@ 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 */ diff --git a/testing/marionette/navigate.js b/testing/marionette/navigate.js index a24440b4a261..edbfa552a28a 100644 --- a/testing/marionette/navigate.js +++ b/testing/marionette/navigate.js @@ -14,30 +14,106 @@ 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) { - // assume we will go somewhere exciting - if (typeof url == "undefined") { - throw TypeError("Expected destination URL"); +navigate.isLoadEventExpected = function (current, future = undefined) { + if (typeof current == "undefined") { + throw TypeError("Expected at least one URL"); } - switch (new URL(url).protocol) { - // assume javascript: 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; + // assume we will go somewhere exciting + if (typeof future == "undefined") { + return true; + } + + let cur = new navigate.IdempotentURL(current); + let fut = new navigate.IdempotentURL(future); + + // assume javascript: 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, + }; +}; diff --git a/testing/marionette/test_navigate.js b/testing/marionette/test_navigate.js index bee79df640cc..6fe29ec653bd 100644 --- a/testing/marionette/test_navigate.js +++ b/testing/marionette/test_navigate.js @@ -10,10 +10,58 @@ Cu.import("chrome://marionette/content/navigate.js"); add_test(function test_isLoadEventExpected() { Assert.throws(() => navigate.isLoadEventExpected(undefined), - /Expected destination URL/); + /Expected at least one URL/); equal(true, navigate.isLoadEventExpected("http://a/")); - equal(false, navigate.isLoadEventExpected("javascript:whatever")); + equal(true, navigate.isLoadEventExpected("http://a/", "http://a/")); + equal(true, navigate.isLoadEventExpected("http://a/", "http://a/b")); + equal(true, navigate.isLoadEventExpected("http://a/", "http://b")); + equal(true, navigate.isLoadEventExpected("http://a/", "data:text/html;charset=utf-8,foo")); + equal(true, navigate.isLoadEventExpected("about:blank", "http://a/")); + equal(true, navigate.isLoadEventExpected("http://a/", "about:blank")); + equal(true, navigate.isLoadEventExpected("http://a/", "https://a/")); + + equal(false, navigate.isLoadEventExpected("http://a/", "javascript:whatever")); + equal(false, navigate.isLoadEventExpected("http://a/", "http://a/#")); + equal(false, navigate.isLoadEventExpected("http://a/", "http://a/#b")); + equal(false, navigate.isLoadEventExpected("http://a/#b", "http://a/#b")); + equal(false, navigate.isLoadEventExpected("http://a/#b", "http://a/#c")); + equal(false, navigate.isLoadEventExpected("data:text/html;charset=utf-8,foo", "data:text/html;charset=utf-8,foo#bar")); + equal(false, navigate.isLoadEventExpected("data:text/html;charset=utf-8,foo", "data:text/html;charset=utf-8,foo#")); + + run_next_test(); +}); + +add_test(function test_IdempotentURL() { + Assert.throws(() => new navigate.IdempotentURL(undefined)); + Assert.throws(() => new navigate.IdempotentURL(true)); + Assert.throws(() => new navigate.IdempotentURL({})); + Assert.throws(() => new navigate.IdempotentURL(42)); + + // propagated URL properties + let u1 = new URL("http://a/b"); + let u2 = new navigate.IdempotentURL(u1); + equal(u1.host, u2.host); + equal(u1.hostname, u2.hostname); + equal(u1.href, u2.href); + equal(u1.origin, u2.origin); + equal(u1.password, u2.password); + equal(u1.port, u2.port); + equal(u1.protocol, u2.protocol); + equal(u1.search, u2.search); + equal(u1.username, u2.username); + + // specialisations + equal("#b", new navigate.IdempotentURL("http://a/#b").hash); + equal("#", new navigate.IdempotentURL("http://a/#").hash); + equal("", new navigate.IdempotentURL("http://a/").hash); + equal("#bar", new navigate.IdempotentURL("data:text/html;charset=utf-8,foo#bar").hash); + equal("#", new navigate.IdempotentURL("data:text/html;charset=utf-8,foo#").hash); + equal("", new navigate.IdempotentURL("data:text/html;charset=utf-8,foo").hash); + + equal("/", new navigate.IdempotentURL("http://a/").pathname); + equal("/", new navigate.IdempotentURL("http://a/#b").pathname); + equal("text/html;charset=utf-8,foo", new navigate.IdempotentURL("data:text/html;charset=utf-8,foo#bar").pathname); run_next_test(); });