From 51a98f5c1b41398e70b2fcaf67a306294deaef8a Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Tue, 28 Mar 2017 21:41:38 +0200 Subject: [PATCH] Bug 1291320 - Refactor page load algorithm for listener framescript. r=ato,automatedtester This refactoring allows us to re-use the same load algorithm for each command which could trigger a page load. It also takes remoteness changes into account, and waits until the load has been done. With this change we no longer check for readyState only, but observe the necessary DOM events as fired for page unloads and loads. This will help us to implement the page loading strategy later. By observing the DOM events, I also expect a small increase of performance for any kind of page load, given that we now return immediately and do not have a delay of 100ms at maximum. MozReview-Commit-ID: IVtO6KgJFES --HG-- extra : rebase_source : 1216be48a23f8841fe1b12873e6154480ace125a --- testing/marionette/driver.js | 12 +- .../tests/unit/test_navigation.py | 154 ++--- testing/marionette/listener.js | 563 +++++++----------- testing/marionette/navigate.js | 104 +--- testing/marionette/test_navigate.js | 52 +- 5 files changed, 334 insertions(+), 551 deletions(-) diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index f79ef41b5679..13d0220bd362 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:pollForReadyState" + this.curBrowser.curFrameId, + "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId, parameters); }); @@ -1081,9 +1081,7 @@ GeckoDriver.prototype.goBack = function* (cmd, resp) { startTime: new Date().getTime(), }; this.mm.broadcastAsyncMessage( - // TODO: combine with - // "Marionette:pollForReadyState" + this.curBrowser.curFrameId, - "Marionette:pollForReadyState" + this.curBrowser.curFrameId, + "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId, parameters); }); @@ -1123,9 +1121,7 @@ GeckoDriver.prototype.goForward = function* (cmd, resp) { startTime: new Date().getTime(), }; this.mm.broadcastAsyncMessage( - // TODO: combine with - // "Marionette:pollForReadyState" + this.curBrowser.curFrameId, - "Marionette:pollForReadyState" + this.curBrowser.curFrameId, + "Marionette:waitForPageLoaded" + 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 f5356253badf..c896962155a6 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,56 +92,15 @@ 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 = '%s'" % self.test_page_remote) + "window.location.href = '{}'".format(self.test_page_remote), + sandbox=None) Wait(self.marionette).until( - lambda _: self.test_page_remote == self.marionette.get_url()) + lambda mn: self.test_page_remote == mn.get_url()) self.assertEqual("Marionette Test", self.marionette.title) def test_navigate_chrome_unsupported_error(self): @@ -203,13 +162,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") + "return window.document.readyState", sandbox=None) 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.1 + self.marionette.timeout.page_load = 0.5 with self.assertRaises(errors.TimeoutException): self.marionette.navigate(self.marionette.absolute_url("slow")) self.assertEqual(self.is_remote_tab, is_remote_before_timeout) @@ -220,7 +179,7 @@ class TestNavigate(BaseNavigationTestCase): self.marionette.navigate("about:robots") self.assertFalse(self.is_remote_tab) - self.marionette.timeout.page_load = 0.1 + self.marionette.timeout.page_load = 0.5 with self.assertRaises(errors.TimeoutException): self.marionette.navigate(self.marionette.absolute_url("slow")) @@ -245,7 +204,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") @@ -273,10 +232,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", @@ -289,9 +248,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( @@ -306,7 +265,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) @@ -317,6 +276,51 @@ 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() @@ -393,18 +397,18 @@ class TestBackForwardNavigation(BaseNavigationTestCase): self.marionette.find_element(*test_element_locator) self.assertEqual(self.marionette.get_url(), page) - def test_image_document_to_html(self): + def test_image_to_html_to_image(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_document_to_image_document(self): + def test_image_to_image(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) @@ -436,9 +440,9 @@ class TestBackForwardNavigation(BaseNavigationTestCase): def test_timeout_error(self): urls = [ - self.marionette.absolute_url('slow'), + self.marionette.absolute_url("slow?delay=3"), self.test_page_remote, - self.marionette.absolute_url('slow'), + self.marionette.absolute_url("slow?delay=4"), ] # First, load all pages completely to get them added to the cache @@ -451,21 +455,29 @@ class TestBackForwardNavigation(BaseNavigationTestCase): self.assertEqual(urls[1], self.marionette.get_url()) # Force triggering a timeout error - self.marionette.timeout.page_load = 0.1 + self.marionette.timeout.page_load = 0.5 with self.assertRaises(errors.TimeoutException): self.marionette.go_back() - self.assertEqual(urls[0], self.marionette.get_url()) - self.marionette.timeout.page_load = 300000 + 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.marionette.go_forward() self.assertEqual(urls[1], self.marionette.get_url()) # Force triggering a timeout error - self.marionette.timeout.page_load = 0.1 + self.marionette.timeout.page_load = 0.5 with self.assertRaises(errors.TimeoutException): self.marionette.go_forward() - self.assertEqual(urls[2], self.marionette.get_url()) - self.marionette.timeout.page_load = 300000 + 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") def test_certificate_error(self): test_pages = [ diff --git a/testing/marionette/listener.js b/testing/marionette/listener.js index 79ee4dade45e..64836bc487ad 100644 --- a/testing/marionette/listener.js +++ b/testing/marionette/listener.js @@ -78,9 +78,6 @@ 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 @@ -112,6 +109,186 @@ 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. @@ -183,7 +360,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 { @@ -265,7 +442,7 @@ function startListeners() { addMessageListenerId("Marionette:actionChain", actionChainFn); addMessageListenerId("Marionette:multiAction", multiActionFn); addMessageListenerId("Marionette:get", get); - addMessageListenerId("Marionette:pollForReadyState", pollForReadyState); + addMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded); addMessageListenerId("Marionette:cancelRequest", cancelRequest); addMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn); addMessageListenerId("Marionette:getTitle", getTitleFn); @@ -370,7 +547,7 @@ function deleteSession(msg) { removeMessageListenerId("Marionette:actionChain", actionChainFn); removeMessageListenerId("Marionette:multiAction", multiActionFn); removeMessageListenerId("Marionette:get", get); - removeMessageListenerId("Marionette:pollForReadyState", pollForReadyState); + removeMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded); removeMessageListenerId("Marionette:cancelRequest", cancelRequest); removeMessageListenerId("Marionette:getTitle", getTitleFn); removeMessageListenerId("Marionette:getPageSource", getPageSourceFn); @@ -447,8 +624,8 @@ function sendToServer(uuid, data = undefined) { * @param {UUID} uuid * Unique identifier of the request. */ -function sendResponse(obj, id) { - sendToServer(id, obj); +function sendResponse(obj, uuid) { + sendToServer(uuid, obj); } /** @@ -885,84 +1062,31 @@ 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 pollForReadyState(msg) { - let {cleanupCallback, command_id, lastSeenURL, pageTimeout, startTime} = msg.json; +function waitForPageLoaded(msg) { + let {command_id, pageTimeout, startTime} = msg.json; - 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(); + loadListener.waitForLoadAfterRemotenessChange(command_id, pageTimeout, startTime); } /** @@ -972,163 +1096,49 @@ function pollForReadyState(msg) { * driver (in chrome space). */ function get(msg) { - let {pageTimeout, url, command_id} = msg.json; - - let startTime = new Date().getTime(); + let {command_id, pageTimeout, url} = msg.json; // We need to move to the top frame before navigating sendSyncMessage("Marionette:switchedToFrame", {frameValue: null}); curContainer.frame = content; - 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); - } + loadListener.navigate(() => { + curContainer.frame.location = url; + }, command_id, pageTimeout, url); } /** - * 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. + * 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 cancelRequest() { - navTimer.cancel(); - if (onDOMContentLoaded) { - removeEventListener("DOMContentLoaded", onDOMContentLoaded, false); - } +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); } /** @@ -1152,117 +1162,6 @@ 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 edbfa552a28a..a24440b4a261 100644 --- a/testing/marionette/navigate.js +++ b/testing/marionette/navigate.js @@ -14,106 +14,30 @@ this.navigate = {}; /** * Determines if we expect to get a DOM load event (DOMContentLoaded) - * on navigating to the |future| URL. + * on navigating to the |url|. * - * @param {string} current - * URL the browser is currently visiting. - * @param {string=} future - * Destination URL, if known. + * @param {string} url + * Destination URL * * @return {boolean} - * Full page load would be expected if future is followed. + * Full page load would be expected if url gets loaded. * * @throws TypeError - * If |current| is not defined, or any of |current| or |future| - * are invalid URLs. + * If |url| is an invalid URL. */ -navigate.isLoadEventExpected = function (current, future = undefined) { - if (typeof current == "undefined") { - throw TypeError("Expected at least one URL"); - } - +navigate.isLoadEventExpected = function (url) { // assume we will go somewhere exciting - if (typeof future == "undefined") { - return true; + if (typeof url == "undefined") { + throw TypeError("Expected destination URL"); } - 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; + 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; } 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 6fe29ec653bd..bee79df640cc 100644 --- a/testing/marionette/test_navigate.js +++ b/testing/marionette/test_navigate.js @@ -10,58 +10,10 @@ Cu.import("chrome://marionette/content/navigate.js"); add_test(function test_isLoadEventExpected() { Assert.throws(() => navigate.isLoadEventExpected(undefined), - /Expected at least one URL/); + /Expected destination URL/); equal(true, navigate.isLoadEventExpected("http://a/")); - 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); + equal(false, navigate.isLoadEventExpected("javascript:whatever")); run_next_test(); });