Bug 1121705 - Look at window handles decrement when closing tab. r?whimboo draft
authorAndreas Tolfsen <ato@sny.no>
Wed, 04 Jul 2018 16:36:05 +0100
changeset 814592 d337c0e29514d0860cfff7b106359e65f836c758
parent 814584 e8b7331398910233e3adaaed01cad6b75bb562a2
push id115273
push userbmo:ato@sny.no
push dateThu, 05 Jul 2018 18:36:16 +0000
reviewerswhimboo
bugs1121705
milestone63.0a1
Bug 1121705 - Look at window handles decrement when closing tab. r?whimboo When Puppeteer opens a new tab using various strategies it relies on the list of window handles increasing. When performing the reverse operation of closing a tab, it looks at the length of <tab> chrome elements in the UI. This changes the close operation to use the same mechanism as opening a new tab to determine if the tab has been closed. This seems to be as reliable as looking at the number of <tab> elements. As part of a forthcoming window tracking refactoring of Marionette (https://bugzilla.mozilla.org/show_bug.cgi?id=marionette-window-tracking), the list of window handles will be made even more reliable: a content browser will not appear in the window handle list until both the tab and it's linked content browser has been created and properly initialised. MozReview-Commit-ID: FY5vGBpn64R
testing/firefox-ui/tests/puppeteer/test_tabbar.py
testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py
--- a/testing/firefox-ui/tests/puppeteer/test_tabbar.py
+++ b/testing/firefox-ui/tests/puppeteer/test_tabbar.py
@@ -151,21 +151,23 @@ class TestTab(PuppeteerMixin, Marionette
         # Close a tab by each trigger method
         close_strategies = ('button',
                             'menu',
                             'shortcut',
                             lambda tab: tab.close_button.click())
         for trigger in close_strategies:
             new_tab = tabbar.open_tab()
             self.assertEqual(len(tabbar.tabs), 2)
+            self.assertEqual(len(self.marionette.window_handles), 2)
             self.assertEqual(new_tab.handle, self.marionette.current_window_handle)
             self.assertEqual(new_tab.handle, tabbar.tabs[1].handle)
 
             new_tab.close(trigger=trigger)
             self.assertEqual(len(tabbar.tabs), 1)
+            self.assertEqual(len(self.marionette.window_handles), 1)
             self.assertEqual(tabbar.tabs[0].handle, self.marionette.current_window_handle)
             self.assertNotEqual(new_tab.handle, tabbar.tabs[0].handle)
 
     def test_location(self):
         url = self.marionette.absolute_url('layout/mozilla.html')
         with self.marionette.using_context(self.marionette.CONTEXT_CONTENT):
             self.marionette.navigate(url)
         self.assertEqual(self.browser.tabbar.tabs[0].location, url)
--- a/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py
+++ b/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py
@@ -112,44 +112,51 @@ class TabBar(UIBaseLib):
 
     def open_tab(self, trigger='menu'):
         """Opens a new tab in the current browser window.
 
         If the tab opens in the foreground, a call to :func:`switch_to` will
         automatically be performed. But if it opens in the background, the current
         tab will keep its focus.
 
+        It will first verify that a new `<tab>` element has been
+        introduced in the tabbar, and then that a new content
+        browser has been created.
+
         :param trigger: Optional, method to open the new tab. This can
          be a string with one of `menu`, `button` or `shortcut`, or a callback
          which gets triggered with the current :class:`Tab` as parameter.
          Defaults to `menu`.
 
         :returns: :class:`Tab` instance for the opened tab.
         """
         start_handles = self.marionette.window_handles
+        start_tabs = self.window.tabbar.tabs
 
         # Prepare action which triggers the opening of the browser window
         if callable(trigger):
             trigger(self.selected_tab)
         elif trigger == 'button':
             self.window.tabbar.newtab_button.click()
         elif trigger == 'menu':
-            self.window.menubar.select_by_id('file-menu',
-                                             'menu_newNavigatorTab')
+            self.window.menubar.select_by_id('file-menu', 'menu_newNavigatorTab')
         elif trigger == 'shortcut':
-            self.window.send_shortcut(self.window.localize_entity('tabCmd.commandkey'),
-                                      accel=True)
+            self.window.send_shortcut(
+                self.window.localize_entity('tabCmd.commandkey'),
+                accel=True)
         # elif - need to add other cases
         else:
             raise ValueError('Unknown opening method: "%s"' % trigger)
 
-        # TODO: Needs to be replaced with event handling code (bug 1121705)
+        Wait(self.marionette).until(
+            lambda _: len(self.window.tabbar.tabs) == len(start_tabs) + 1,
+            message='No new tab present in tabbar')
         Wait(self.marionette).until(
             lambda mn: len(mn.window_handles) == len(start_handles) + 1,
-            message='No new tab has been opened.')
+            message='No new content browser created')
 
         handles = self.marionette.window_handles
         [new_handle] = list(set(handles) - set(start_handles))
         [new_tab] = [tab for tab in self.tabs if tab.handle == new_handle]
 
         # if the new tab is the currently selected tab, switch to it
         if new_tab == self.selected_tab:
             new_tab.switch_to()
@@ -286,28 +293,33 @@ class Tab(UIBaseLib):
     # Methods for helpers when working with tabs #
 
     def __eq__(self, other):
         return self.handle == other.handle
 
     def close(self, trigger='menu', force=False):
         """Closes the tab by using the specified trigger.
 
+        To ensure the tab was closed, it will first ensure the
+        `<tab>` element is removed from the tabbar, and then confirm
+        that the content browser was discarded.
+
         When the tab is closed a :func:`switch_to` call is automatically performed, so that
         the new selected tab becomes active.
 
         :param trigger: Optional, method in how to close the tab. This can
          be a string with one of `button`, `menu` or `shortcut`, or a callback which
          gets triggered with the current :class:`Tab` as parameter. Defaults to `menu`.
 
         :param force: Optional, forces the closing of the window by using the Gecko API.
          Defaults to `False`.
         """
         handle = self.handle
         start_handles = self.marionette.window_handles
+        start_tabs = self.window.tabbar.tabs
 
         self.switch_to()
 
         if force:
             self.marionette.close()
         elif callable(trigger):
             trigger(self)
         elif trigger == 'button':
@@ -316,18 +328,21 @@ class Tab(UIBaseLib):
             self.window.menubar.select_by_id('file-menu', 'menu_close')
         elif trigger == 'shortcut':
             self.window.send_shortcut(self.window.localize_entity('closeCmd.key'),
                                       accel=True)
         else:
             raise ValueError('Unknown closing method: "%s"' % trigger)
 
         Wait(self.marionette).until(
-            lambda _: len(self.window.tabbar.tabs) == len(start_handles) - 1,
-            message='Tab with handle "%s" has not been closed.' % handle)
+            lambda _: len(self.window.tabbar.tabs) == len(start_tabs) - 1,
+            message='Tab"%s" has not been closed' % handle)
+        Wait(self.marionette).until(
+            lambda mn: len(mn.window_handles) == len(start_handles) - 1,
+            message='Content browser "%s" has not been closed' % handle)
 
         # Ensure to switch to the window handle which represents the new selected tab
         self.window.tabbar.selected_tab.switch_to()
 
     def select(self):
         """Selects the tab and sets the focus to it."""
         self.tab_element.click()
         self.switch_to()