Bug 1340842 - Allow BrowserTestUtils.removeTab to pass options along to tabbrowser's removeTab method. r?Mossop draft
authorMike Conley <mconley@mozilla.com>
Fri, 17 Mar 2017 09:59:38 -0400
changeset 503029 bb328ee7a303958925769e705202a33935f9f814
parent 503028 e7ad044f1b6cfec1df15136f211db2c1e98735b5
child 503030 46a40c4f8f407947a5ae659ec8b100b15d1772d8
push id50459
push usermconley@mozilla.com
push dateWed, 22 Mar 2017 17:36:55 +0000
reviewersMossop
bugs1340842
milestone55.0a1
Bug 1340842 - Allow BrowserTestUtils.removeTab to pass options along to tabbrowser's removeTab method. r?Mossop There were options already being passed to BrowserTestUtils.removeTab, with only a single property being observed, "dontRemove". This caused BrowserTestUtils.removeTab to return a Promise once a tab is removed, but didn't actually remove the tab (as the calling test would be responsible for that themselves). This patch removes that option, and adds a method to BrowserTestUtils called tabRemoved to use for that case instead. The options being passed to removeTab are now forwarded along directly to tabbrowser's removeTab method. MozReview-Commit-ID: JzfZuoZmlJ0
browser/base/content/test/captivePortal/browser_captivePortal_certErrorUI.js
browser/base/content/test/general/browser_bug455852.js
browser/base/content/test/general/browser_tabs_close_beforeunload.js
browser/base/content/test/tabcrashed/browser_withoutDump.js
browser/base/content/test/urlbar/browser_tabMatchesInAwesomebar_perwindowpb.js
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
--- a/browser/base/content/test/captivePortal/browser_captivePortal_certErrorUI.js
+++ b/browser/base/content/test/captivePortal/browser_captivePortal_certErrorUI.js
@@ -59,17 +59,17 @@ add_task(function* checkCaptivePortalCer
   yield ContentTask.spawn(browser, null, () => {
     info("Clicking the Open Login Page button.");
     content.document.getElementById("openPortalLoginPageButton").click();
   });
 
   let portalTab2 = yield portalTabPromise;
   is(portalTab2, portalTab, "The existing portal tab should be focused.");
 
-  let portalTabRemoved = BrowserTestUtils.removeTab(portalTab, {dontRemove: true});
+  let portalTabRemoved = BrowserTestUtils.tabRemoved(portalTab);
   let errorTabReloaded = BrowserTestUtils.waitForErrorPage(browser);
 
   Services.obs.notifyObservers(null, "captive-portal-login-success", null);
   yield portalTabRemoved;
 
   info("Waiting for error tab to be reloaded after the captive portal was freed.");
   yield errorTabReloaded;
   yield ContentTask.spawn(browser, null, () => {
--- a/browser/base/content/test/general/browser_bug455852.js
+++ b/browser/base/content/test/general/browser_bug455852.js
@@ -2,17 +2,17 @@ add_task(function*() {
   is(gBrowser.tabs.length, 1, "one tab is open");
 
   gBrowser.selectedBrowser.focus();
   isnot(document.activeElement, gURLBar.inputField, "location bar is not focused");
 
   var tab = gBrowser.selectedTab;
   gPrefService.setBoolPref("browser.tabs.closeWindowWithLastTab", false);
 
-  let tabClosedPromise = BrowserTestUtils.removeTab(tab, {dontRemove: true});
+  let tabClosedPromise = BrowserTestUtils.tabRemoved(tab);
   EventUtils.synthesizeKey("w", { accelKey: true });
   yield tabClosedPromise;
 
   is(tab.parentNode, null, "ctrl+w removes the tab");
   is(gBrowser.tabs.length, 1, "a new tab has been opened");
   is(document.activeElement, gURLBar.inputField, "location bar is focused for the new tab");
 
   if (gPrefService.prefHasUserValue("browser.tabs.closeWindowWithLastTab"))
--- a/browser/base/content/test/general/browser_tabs_close_beforeunload.js
+++ b/browser/base/content/test/general/browser_tabs_close_beforeunload.js
@@ -27,17 +27,17 @@ add_task(function*() {
     content.document.getElementsByTagName("a")[0].click();
   });
   info("Waiting for the second tab to be opened");
   yield tabOpened;
   info("Waiting for the load in that tab to finish");
   yield secondTabLoadedPromise;
 
   let closeBtn = document.getAnonymousElementByAttribute(secondTab, "anonid", "close-button");
-  let closePromise = BrowserTestUtils.removeTab(secondTab, {dontRemove: true});
+  let closePromise = BrowserTestUtils.tabRemoved(secondTab);
   info("closing second tab (which will self-close in beforeunload)");
   closeBtn.click();
   ok(secondTab.closing, "Second tab should be marked as closing synchronously.");
   yield closePromise;
   ok(secondTab.closing, "Second tab should still be marked as closing");
   ok(!secondTab.linkedBrowser, "Second tab's browser should be dead");
   ok(!firstTab.closing, "First tab should not be closing");
   ok(firstTab.linkedBrowser, "First tab's browser should be alive");
--- a/browser/base/content/test/tabcrashed/browser_withoutDump.js
+++ b/browser/base/content/test/tabcrashed/browser_withoutDump.js
@@ -20,17 +20,17 @@ add_task(function* setup() {
 add_task(function* test_without_dump() {
   return BrowserTestUtils.withNewTab({
     gBrowser,
     url: PAGE,
   }, function*(browser) {
     let tab = gBrowser.getTabForBrowser(browser);
     yield BrowserTestUtils.crashBrowser(browser);
 
-    let tabRemovedPromise = BrowserTestUtils.removeTab(tab, { dontRemove: true });
+    let tabRemovedPromise = BrowserTestUtils.tabRemoved(tab);
 
     yield ContentTask.spawn(browser, null, function*() {
       let doc = content.document;
       Assert.ok(!doc.documentElement.classList.contains("crashDumpAvailable"),
          "doesn't have crash dump");
 
       let options = doc.getElementById("options");
       Assert.ok(options, "has crash report options");
--- a/browser/base/content/test/urlbar/browser_tabMatchesInAwesomebar_perwindowpb.js
+++ b/browser/base/content/test/urlbar/browser_tabMatchesInAwesomebar_perwindowpb.js
@@ -61,17 +61,17 @@ function* runTest(aSourceWindow, aDestWi
   let {controller, popup} = aDestWindow.gURLBar;
   while (popup.selectedIndex < controller.matchCount - 1) {
     info("handling key navigation for DOM_VK_DOWN key");
     controller.handleKeyNavigation(KeyEvent.DOM_VK_DOWN);
   }
 
   let awaitTabSwitch;
   if (aExpectSwitch) {
-    awaitTabSwitch = BrowserTestUtils.removeTab(testTab, {dontRemove: true})
+    awaitTabSwitch = BrowserTestUtils.tabRemoved(testTab)
   }
 
   // Execute the selected action.
   controller.handleEnter(true);
   info("sent Enter command to the controller");
 
   if (aExpectSwitch) {
     // If we expect a tab switch then the current tab
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -817,32 +817,49 @@ this.BrowserTestUtils = {
   synthesizeMouseAtPoint(offsetX, offsetY, event, browser)
   {
     return BrowserTestUtils.synthesizeMouse(null, offsetX, offsetY, event, browser);
   },
 
   /**
    * Removes the given tab from its parent tabbrowser and
    * waits until its final message has reached the parent.
+   *
+   * @param (tab) tab
+   *        The tab to remove.
+   * @param (Object) options
+   *        Extra options to pass to tabbrowser's removeTab method.
+   * @returns (Promise)
+   * @resolves When the tab is removed. Does not get passed a value.
    */
   removeTab(tab, options = {}) {
-    let dontRemove = options && options.dontRemove;
+    let tabRemoved = BrowserTestUtils.tabRemoved(tab);
+    if (!tab.closing) {
+      tab.ownerGlobal.gBrowser.removeTab(tab, options);
+    }
+    return tabRemoved;
+  },
 
+  /**
+   * Returns a Promise that resolves once a tab has been removed.
+   *
+   * @param (tab) tab
+   *        The tab that will be removed.
+   * @returns (Promise)
+   * @resolves When the tab is removed. Does not get passed a value.
+   */
+  tabRemoved(tab) {
     return new Promise(resolve => {
       let {messageManager: mm, frameLoader} = tab.linkedBrowser;
       mm.addMessageListener("SessionStore:update", function onMessage(msg) {
         if (msg.targetFrameLoader == frameLoader && msg.data.isFinal) {
           mm.removeMessageListener("SessionStore:update", onMessage);
           resolve();
         }
       }, true);
-
-      if (!dontRemove && !tab.closing) {
-        tab.ownerGlobal.gBrowser.removeTab(tab);
-      }
     });
   },
 
   /**
    * Crashes a remote browser tab and cleans up the generated minidumps.
    * Resolves with the data from the .extra file (the crash annotations).
    *
    * @param (Browser) browser