Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle. r=ato a=test-only
authorHenrik Skupin <mail@hskupin.info>
Sun, 05 Feb 2017 15:55:10 +0100
changeset 375958 4116e3e124601c356950880ad01f015626b21345
parent 375957 2a4e9d28de7198d08e1553d1c242116e55734fc8
child 375959 cc8652056af98ec6331d1dfc8143fcb9984fb49e
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato, test-only
bugs1336445
milestone53.0a2
Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle. r=ato a=test-only 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; ")