Bug 1368101 - getCurrentUrl can retrieve the URL via the chrome process. r=ato
authorHenrik Skupin <mail@hskupin.info>
Fri, 26 May 2017 20:37:36 +0200
changeset 409221 4a025d46da1014ade337547bcbbf6b3280bd1ce5
parent 409220 e91c5252226ae59cd6d34df3623b751bba0745a8
child 409222 e54757a91c349e8ba921d218489b53965e7a0161
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1368101
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368101 - getCurrentUrl can retrieve the URL via the chrome process. r=ato There is no reason for the command to call into the content framescript. It's only adding overhead and can currently cause hangs if the framescript gets reloaded. Instead use the content browser which is always aware of the current URL. MozReview-Commit-ID: 9Ui7qClFEWJ
testing/marionette/driver.js
testing/marionette/harness/marionette_harness/tests/unit/test_get_current_url_chrome.py
testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
testing/marionette/harness/marionette_harness/tests/unit/unit-tests.ini
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -1017,17 +1017,22 @@ GeckoDriver.prototype.getCurrentUrl = fu
   const win = assert.window(this.getCurrentWindow());
   assert.noUserPrompt(this.dialog);
 
   switch (this.context) {
     case Context.CHROME:
       return win.location.href;
 
     case Context.CONTENT:
-      return this.listener.getCurrentUrl();
+      if (this.curBrowser.contentBrowser) {
+        return this.curBrowser.contentBrowser.currentURI.spec;
+      } else {
+        throw new NoSuchWindowError(
+          "Not a browser window, or no tab currently selected");
+      }
   }
 };
 
 /**
  * Gets the current title of the window.
  *
  * @return {string}
  *     Document title of the top-level browsing context.
@@ -1107,17 +1112,17 @@ GeckoDriver.prototype.goBack = function*
     // Navigation does not work for non-browser windows
     return;
   }
 
   if (!this.curBrowser.contentBrowser.webNavigation.canGoBack) {
     return;
   }
 
-  let currentURL = yield this.listener.getCurrentUrl();
+  let currentURL = this.getCurrentUrl();
   let goBack = this.listener.goBack({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
@@ -1154,17 +1159,17 @@ GeckoDriver.prototype.goForward = functi
     // Navigation does not work for non-browser windows
     return;
   }
 
   if (!this.curBrowser.contentBrowser.webNavigation.canGoForward) {
     return;
   }
 
-  let currentURL = yield this.listener.getCurrentUrl();
+  let currentURL = this.getCurrentUrl();
   let goForward = this.listener.goForward({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
new file mode 100644
--- /dev/null
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_get_current_url_chrome.py
@@ -0,0 +1,49 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from marionette_driver.errors import NoSuchWindowException
+
+from marionette_harness import MarionetteTestCase, WindowManagerMixin, skip_if_mobile
+
+
+class TestGetCurrentUrlChrome(WindowManagerMixin, MarionetteTestCase):
+
+    def setUp(self):
+        super(TestGetCurrentUrlChrome, self).setUp()
+        self.marionette.set_context("chrome")
+
+    def tearDown(self):
+        self.close_all_windows()
+        super(TestGetCurrentUrlChrome, self).tearDown()
+
+    def test_browser_window(self):
+        url = self.marionette.absolute_url("test.html")
+
+        with self.marionette.using_context("content"):
+            self.marionette.navigate(url)
+            self.assertEqual(self.marionette.get_url(), url)
+
+        chrome_url = self.marionette.execute_script("return window.location.href;")
+        self.assertEqual(self.marionette.get_url(), chrome_url)
+
+    @skip_if_mobile("Fennec doesn't support other chrome windows")
+    def test_no_browser_window(self):
+
+        def open_window_with_js():
+            with self.marionette.using_context("chrome"):
+                self.marionette.execute_script("""
+                  window.open('chrome://marionette/content/test.xul',
+                              'foo', 'chrome,centerscreen');
+                """)
+
+        win = self.open_window(trigger=open_window_with_js)
+        self.marionette.switch_to_window(win)
+
+        chrome_url = self.marionette.execute_script("return window.location.href;")
+        self.assertEqual(self.marionette.get_url(), chrome_url)
+
+        # With no tabbrowser available an exception will be thrown
+        with self.assertRaises(NoSuchWindowException):
+            with self.marionette.using_context("content"):
+                self.marionette.get_url()
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py
@@ -212,17 +212,16 @@ class TestNavigate(BaseNavigationTestCas
     def test_no_hang_when_navigating_after_closing_original_tab(self):
         # Close the start tab
         self.marionette.switch_to_window(self.start_tab)
         self.marionette.close()
 
         self.marionette.switch_to_window(self.new_tab)
         self.marionette.navigate(self.test_page_remote)
 
-    @skip("Bug 1334137 - Intermittent: Process killed because of hang in getCurrentUrl()")
     @skip_if_mobile("Interacting with chrome elements not available for Fennec")
     def test_type_to_non_remote_tab(self):
         self.marionette.navigate(self.test_page_not_remote)
         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")
--- a/testing/marionette/harness/marionette_harness/tests/unit/unit-tests.ini
+++ b/testing/marionette/harness/marionette_harness/tests/unit/unit-tests.ini
@@ -34,16 +34,17 @@ skip-if = true # "Bug 896046"
 
 [test_execute_async_script.py]
 [test_execute_script.py]
 [test_simpletest_fail.js]
 [test_element_retrieval.py]
 [test_findelement_chrome.py]
 skip-if = appname == 'fennec'
 
+[test_get_current_url_chrome.py]
 [test_navigation.py]
 
 [test_timeouts.py]
 
 [test_single_finger_desktop.py]
 skip-if = appname == 'fennec' || os == "win" # Bug 1025040
 
 [test_simpletest_pass.js]
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -464,17 +464,16 @@ var getTitleFn = dispatch(getTitle);
 var getPageSourceFn = dispatch(getPageSource);
 var getActiveElementFn = dispatch(getActiveElement);
 var getElementAttributeFn = dispatch(getElementAttribute);
 var getElementPropertyFn = dispatch(getElementProperty);
 var getElementTextFn = dispatch(getElementText);
 var getElementTagNameFn = dispatch(getElementTagName);
 var getElementRectFn = dispatch(getElementRect);
 var isElementEnabledFn = dispatch(isElementEnabled);
-var getCurrentUrlFn = dispatch(getCurrentUrl);
 var findElementContentFn = dispatch(findElementContent);
 var findElementsContentFn = dispatch(findElementsContent);
 var isElementSelectedFn = dispatch(isElementSelected);
 var clearElementFn = dispatch(clearElement);
 var isElementDisplayedFn = dispatch(isElementDisplayed);
 var getElementValueOfCssPropertyFn = dispatch(getElementValueOfCssProperty);
 var switchToShadowRootFn = dispatch(switchToShadowRoot);
 var getCookiesFn = dispatch(getCookies);
@@ -503,17 +502,16 @@ function startListeners() {
   addMessageListenerId("Marionette:singleTap", singleTapFn);
   addMessageListenerId("Marionette:performActions", performActionsFn);
   addMessageListenerId("Marionette:releaseActions", releaseActionsFn);
   addMessageListenerId("Marionette:actionChain", actionChainFn);
   addMessageListenerId("Marionette:multiAction", multiActionFn);
   addMessageListenerId("Marionette:get", get);
   addMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded);
   addMessageListenerId("Marionette:cancelRequest", cancelRequest);
-  addMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
   addMessageListenerId("Marionette:getTitle", getTitleFn);
   addMessageListenerId("Marionette:getPageSource", getPageSourceFn);
   addMessageListenerId("Marionette:goBack", goBack);
   addMessageListenerId("Marionette:goForward", goForward);
   addMessageListenerId("Marionette:refresh", refresh);
   addMessageListenerId("Marionette:findElementContent", findElementContentFn);
   addMessageListenerId("Marionette:findElementsContent", findElementsContentFn);
   addMessageListenerId("Marionette:getActiveElement", getActiveElementFn);
@@ -582,17 +580,16 @@ function deleteSession(msg) {
   removeMessageListenerId("Marionette:releaseActions", releaseActionsFn);
   removeMessageListenerId("Marionette:actionChain", actionChainFn);
   removeMessageListenerId("Marionette:multiAction", multiActionFn);
   removeMessageListenerId("Marionette:get", get);
   removeMessageListenerId("Marionette:waitForPageLoaded", waitForPageLoaded);
   removeMessageListenerId("Marionette:cancelRequest", cancelRequest);
   removeMessageListenerId("Marionette:getTitle", getTitleFn);
   removeMessageListenerId("Marionette:getPageSource", getPageSourceFn);
-  removeMessageListenerId("Marionette:getCurrentUrl", getCurrentUrlFn);
   removeMessageListenerId("Marionette:goBack", goBack);
   removeMessageListenerId("Marionette:goForward", goForward);
   removeMessageListenerId("Marionette:refresh", refresh);
   removeMessageListenerId("Marionette:findElementContent", findElementContentFn);
   removeMessageListenerId("Marionette:findElementsContent", findElementsContentFn);
   removeMessageListenerId("Marionette:getActiveElement", getActiveElementFn);
   removeMessageListenerId("Marionette:clickElement", clickElement);
   removeMessageListenerId("Marionette:getElementAttribute", getElementAttributeFn);
@@ -1218,23 +1215,16 @@ function refresh(msg) {
     }, command_id, pageTimeout);
 
   } catch (e) {
     sendError(e, command_id);
   }
 }
 
 /**
- * Get URL of the top-level browsing context.
- */
-function getCurrentUrl() {
-  return content.location.href;
-}
-
-/**
  * Get the title of the current browsing context.
  */
 function getTitle() {
   return curContainer.frame.top.document.title;
 }
 
 /**
  * Get source of the current browsing context's DOM.