Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle. r=ato
authorHenrik Skupin <mail@hskupin.info>
Sun, 05 Feb 2017 15:55:10 +0100
changeset 341006 f08c78b5f19b35aff049f5a3d652649ac8251172
parent 341005 9908b674ae2c30814cefdbd0f1d6373be1794caa
child 341007 f5f206ed675b42e520c03c2754a4d56437790390
push id86615
push userkwierso@gmail.com
push dateTue, 07 Feb 2017 01:52:08 +0000
treeherdermozilla-inbound@f0453084d86e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1336445
milestone54.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 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle. r=ato Using switch_to_window() with a chrome window handle should not change the selected tab within a browser window. It means we first have to check if it is a chrome handle, and only if it's not, we should care about content window handles. MozReview-Commit-ID: IcmCcmVy26T
testing/marionette/driver.js
testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -1195,68 +1195,72 @@ GeckoDriver.prototype.setWindowPosition 
  *      A boolean value which determines whether to focus
  *      the window. Defaults to true.
  */
 GeckoDriver.prototype.switchToWindow = function* (cmd, resp) {
   let switchTo = cmd.parameters.name;
   let focus = (cmd.parameters.focus !== undefined) ? cmd.parameters.focus : true;
   let found;
 
-  let byNameOrId = function (name, outerId, contentWindowId) {
-    return switchTo == name ||
-        switchTo == contentWindowId ||
-        switchTo == outerId;
+  let byNameOrId = function (name, windowId) {
+    return switchTo === name || switchTo === windowId;
   };
 
   let winEn = Services.wm.getEnumerator(null);
   while (winEn.hasMoreElements()) {
     let win = winEn.getNext();
     let outerId = getOuterWindowId(win);
-    let tabbrowser = browser.getTabBrowser(win);
+    let tabBrowser = browser.getTabBrowser(win);
 
-    if (tabbrowser) {
-      for (let i = 0; i < tabbrowser.tabs.length; ++i) {
-        let contentBrowser = browser.getBrowserForTab(tabbrowser.tabs[i]);
+    if (byNameOrId(win.name, outerId)) {
+      // In case the wanted window is a chrome window, we are done.
+      found = {win: win, outerId: outerId, hasTabBrowser: !!tabBrowser};
+      break;
+
+    } else if (tabBrowser) {
+      // Otherwise check if the chrome window has a tab browser, and that it
+      // contains a tab with the wanted window handle.
+      for (let i = 0; i < tabBrowser.tabs.length; ++i) {
+        let contentBrowser = browser.getBrowserForTab(tabBrowser.tabs[i]);
         let contentWindowId = this.getIdForBrowser(contentBrowser);
 
-        if (byNameOrId(win.name, contentWindowId, outerId)) {
+        if (byNameOrId(win.name, contentWindowId)) {
           found = {
             win: win,
             outerId: outerId,
+            hasTabBrowser: true,
             tabIndex: i,
           };
           break;
         }
       }
-    } else {
-      if (byNameOrId(win.name, outerId)) {
-        found = {win: win, outerId: outerId};
-        break;
-      }
     }
   }
 
   if (found) {
-    // Initialise Marionette if browser has not been seen before,
-    // otherwise switch to known browser and activate the tab if it's a
-    // content browser.
     if (!(found.outerId in this.browsers)) {
+      // Initialise Marionette if the current chrome window has not been seen
+      // before. Also register the initial tab, if one exists.
       let registerBrowsers, browserListening;
-      if ("tabIndex" in found) {
+
+      if (found.hasTabBrowser) {
         registerBrowsers = this.registerPromise();
         browserListening = this.listeningPromise();
       }
 
       this.startBrowser(found.win, false /* isNewSession */);
 
       if (registerBrowsers && browserListening) {
         yield registerBrowsers;
         yield browserListening;
       }
+
     } else {
+      // Otherwise switch to the known chrome window, and activate the tab
+      // if it's a content browser.
       this.curBrowser = this.browsers[found.outerId];
 
       if ("tabIndex" in found) {
         this.curBrowser.switchToTab(found.tabIndex, found.win, focus);
       }
     }
   } else {
     throw new NoSuchWindowError(`Unable to locate window: ${switchTo}`);
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py
@@ -1,16 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla ublic
 # 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 import Actions, By
+from marionette_driver import Actions, By, Wait
 from marionette_driver.keys import Keys
 
-from marionette_harness import MarionetteTestCase, WindowManagerMixin
+from marionette_harness import MarionetteTestCase, skip_if_mobile, WindowManagerMixin
 
 
 class TestSwitchToWindowContent(WindowManagerMixin, MarionetteTestCase):
 
     def setUp(self):
         super(TestSwitchToWindowContent, self).setUp()
 
         if self.marionette.session_capabilities["platformName"] == "darwin":
@@ -86,17 +86,19 @@ class TestSwitchToWindowContent(WindowMa
         with self.marionette.using_context("content"):
             self.assertEqual(self.marionette.get_url(), self.test_page)
 
         self.marionette.switch_to_window(new_tab)
         self.assertEqual(self.marionette.current_window_handle, new_tab)
         self.assertNotEqual(self.get_selected_tab_index(), self.selected_tab_index)
 
         with self.marionette.using_context("content"):
-            self.assertEqual(self.marionette.get_url(), self.empty_page)
+            Wait(self.marionette).until(
+                lambda _: self.marionette.get_url() == self.empty_page,
+                message="{} has been loaded in the newly opened tab.".format(self.empty_page))
 
         self.marionette.switch_to_window(self.start_tab, focus=True)
         self.assertEqual(self.marionette.current_window_handle, self.start_tab)
         self.assertEqual(self.get_selected_tab_index(), self.selected_tab_index)
         with self.marionette.using_context("content"):
             self.assertEqual(self.marionette.get_url(), self.test_page)
 
         self.marionette.switch_to_window(new_tab)
@@ -129,8 +131,41 @@ class TestSwitchToWindowContent(WindowMa
         self.marionette.switch_to_window(new_tab)
         self.marionette.close()
 
         self.marionette.switch_to_window(self.start_tab)
         self.assertEqual(self.marionette.current_window_handle, self.start_tab)
         self.assertEqual(self.get_selected_tab_index(), self.selected_tab_index)
         with self.marionette.using_context("content"):
             self.assertEqual(self.marionette.get_url(), self.test_page)
+
+    def test_switch_from_content_to_chrome_window_should_not_change_selected_tab(self):
+        new_tab = self.open_tab(self.open_tab_in_foreground)
+
+        self.marionette.switch_to_window(new_tab)
+        self.assertEqual(self.marionette.current_window_handle, new_tab)
+        new_tab_index = self.get_selected_tab_index()
+
+        self.marionette.switch_to_window(self.start_window)
+        self.assertEqual(self.marionette.current_window_handle, new_tab)
+        self.assertEqual(self.get_selected_tab_index(), new_tab_index)
+
+    @skip_if_mobile("New windows not supported in Fennec")
+    def test_switch_to_new_private_browsing_window_has_to_register_browsers(self):
+        # Test that tabs (browsers) are correctly registered for a newly opened
+        # private browsing window. This has to also happen without explicitely
+        # switching to the tab itself before using any commands in content scope.
+        #
+        # Note: Not sure why this only affects private browsing windows only.
+
+        def open_private_browsing_window():
+            with self.marionette.using_context("content"):
+                self.marionette.navigate("about:privatebrowsing")
+                button = self.marionette.find_element(By.ID, "startPrivateBrowsing")
+                button.click()
+
+        new_window = self.open_window(open_private_browsing_window)
+        self.marionette.switch_to_window(new_window)
+        self.assertEqual(self.marionette.current_chrome_window_handle, new_window)
+        self.assertNotEqual(self.marionette.current_window_handle, self.start_tab)
+
+        with self.marionette.using_context("content"):
+            self.marionette.execute_script(" return true; ")