Bug 1493343 - Make adoptTab remove the new tab if the original tab couldn't be adopted. r=mixedpuppy
authorDão Gottwald <dao@mozilla.com>
Tue, 09 Oct 2018 16:38:02 +0000
changeset 496021 190cbcbf2dd268cb48de9dd0a970edc5f36d85f4
parent 496020 8027a01f2d708dff9aeb453af35b69224b50cf1a
child 496022 299b6e20c942209d2a1c5ffd7330ff28ad03abd4
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1493343
milestone64.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 1493343 - Make adoptTab remove the new tab if the original tab couldn't be adopted. r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D8077
browser/base/content/tabbrowser.js
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2756,17 +2756,20 @@ window._gBrowser = {
     var isLastTab = (this.tabs.length - this._removingTabs.length == 1);
     let windowUtils = window.windowUtils;
     // We have to sample the tab width now, since _beginRemoveTab might
     // end up modifying the DOM in such a way that aTab gets a new
     // frame created for it (for example, by updating the visually selected
     // state).
     let tabWidth = windowUtils.getBoundsWithoutFlushing(aTab).width;
 
-    if (!this._beginRemoveTab(aTab, null, null, true, skipPermitUnload)) {
+    if (!this._beginRemoveTab(aTab, {
+          closeWindowFastpath: true,
+          skipPermitUnload,
+        })) {
       TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_ANIM_MS", aTab);
       TelemetryStopwatch.cancel("FX_TAB_CLOSE_TIME_NO_ANIM_MS", aTab);
       return;
     }
 
     if (!aTab.pinned && !aTab.hidden && aTab._fullyOpen && byMouse)
       this.tabContainer._lockTabSizing(aTab, tabWidth);
     else
@@ -2804,23 +2807,29 @@ window._gBrowser = {
 
   _hasBeforeUnload(aTab) {
     let browser = aTab.linkedBrowser;
     return browser.isRemoteBrowser && browser.frameLoader &&
            browser.frameLoader.tabParent &&
            browser.frameLoader.tabParent.hasBeforeUnload;
   },
 
-  _beginRemoveTab(aTab, aAdoptedByTab, aCloseWindowWithLastTab, aCloseWindowFastpath, aSkipPermitUnload) {
+  _beginRemoveTab(aTab, {
+    adoptedByTab,
+    closeWindowWithLastTab,
+    closeWindowFastpath,
+    skipPermitUnload,
+  } = {}) {
     if (aTab.closing ||
-      this._windowIsClosing)
+        this._windowIsClosing) {
       return false;
+    }
 
     var browser = this.getBrowserForTab(aTab);
-    if (!aSkipPermitUnload && !aAdoptedByTab &&
+    if (!skipPermitUnload && !adoptedByTab &&
         aTab.linkedPanel && !aTab._pendingPermitUnload &&
         (!browser.isRemoteBrowser || this._hasBeforeUnload(aTab))) {
       TelemetryStopwatch.start("FX_TAB_CLOSE_PERMIT_UNLOAD_TIME_MS", aTab);
 
       // We need to block while calling permitUnload() because it
       // processes the event queue and may lead to another removeTab()
       // call before permitUnload() returns.
       aTab._pendingPermitUnload = true;
@@ -2844,44 +2853,44 @@ window._gBrowser = {
       this._tabLayerCache.splice(tabCacheIndex, 1);
     }
 
     this._blurTab(aTab);
 
     var closeWindow = false;
     var newTab = false;
     if (this.tabs.length - this._removingTabs.length == 1) {
-      closeWindow = aCloseWindowWithLastTab != null ? aCloseWindowWithLastTab :
+      closeWindow = closeWindowWithLastTab != null ? closeWindowWithLastTab :
         !window.toolbar.visible ||
         Services.prefs.getBoolPref("browser.tabs.closeWindowWithLastTab");
 
       if (closeWindow) {
         // We've already called beforeunload on all the relevant tabs if we get here,
         // so avoid calling it again:
         window.skipNextCanClose = true;
       }
 
       // Closing the tab and replacing it with a blank one is notably slower
       // than closing the window right away. If the caller opts in, take
       // the fast path.
       if (closeWindow &&
-          aCloseWindowFastpath &&
+          closeWindowFastpath &&
           this._removingTabs.length == 0) {
         // This call actually closes the window, unless the user
         // cancels the operation.  We are finished here in both cases.
         this._windowIsClosing = window.closeWindow(true, window.warnAboutClosingWindow);
         return false;
       }
 
       newTab = true;
     }
     aTab._endRemoveArgs = [closeWindow, newTab];
 
     // swapBrowsersAndCloseOther will take care of closing the window without animation.
-    if (closeWindow && aAdoptedByTab) {
+    if (closeWindow && adoptedByTab) {
       // Remove the tab's filter and progress listener to avoid leaking.
       if (aTab.linkedPanel) {
         const filter = this._tabFilters.get(aTab);
         browser.webProgress.removeProgressListener(filter);
         const listener = this._tabListeners.get(aTab);
         filter.removeProgressListener(listener);
         listener.destroy();
         this._tabListeners.delete(aTab);
@@ -2894,17 +2903,17 @@ window._gBrowser = {
       // If the opening tab animation hasn't finished before we start closing the
       // tab, decrement the animation count since _handleNewTab will not get called.
       this.tabAnimationsInProgress--;
     }
 
     this.tabAnimationsInProgress++;
 
     // Mute audio immediately to improve perceived speed of tab closure.
-    if (!aAdoptedByTab && aTab.hasAttribute("soundplaying")) {
+    if (!adoptedByTab && aTab.hasAttribute("soundplaying")) {
       // Don't persist the muted state as this wasn't a user action.
       // This lets undo-close-tab return it to an unmuted state.
       aTab.linkedBrowser.mute(true);
     }
 
     aTab.closing = true;
     this._removingTabs.push(aTab);
     this._visibleTabs = null; // invalidate cache
@@ -2919,43 +2928,43 @@ window._gBrowser = {
       });
     else
       TabBarVisibility.update();
 
     // We're committed to closing the tab now.
     // Dispatch a notification.
     // We dispatch it before any teardown so that event listeners can
     // inspect the tab that's about to close.
-    var evt = new CustomEvent("TabClose", { bubbles: true, detail: { adoptedBy: aAdoptedByTab } });
+    let evt = new CustomEvent("TabClose", { bubbles: true, detail: { adoptedBy: adoptedByTab } });
     aTab.dispatchEvent(evt);
 
     if (this.tabs.length == 2) {
       // We're closing one of our two open tabs, inform the other tab that its
       // sibling is going away.
       window.messageManager
             .broadcastAsyncMessage("Browser:HasSiblings", false);
     }
 
     if (aTab.linkedPanel) {
-      if (!aAdoptedByTab && !gMultiProcessBrowser) {
+      if (!adoptedByTab && !gMultiProcessBrowser) {
         // Prevent this tab from showing further dialogs, since we're closing it
         browser.contentWindow.windowUtils.disableDialogs();
       }
 
       // Remove the tab's filter and progress listener.
       const filter = this._tabFilters.get(aTab);
 
       browser.webProgress.removeProgressListener(filter);
 
       const listener = this._tabListeners.get(aTab);
       filter.removeProgressListener(listener);
       listener.destroy();
     }
 
-    if (browser.registeredOpenURI && !aAdoptedByTab) {
+    if (browser.registeredOpenURI && !adoptedByTab) {
       let userContextId = browser.getAttribute("usercontextid") || 0;
       this.UrlbarProviderOpenTabs.unregisterOpenTab(browser.registeredOpenURI.spec,
                                                     userContextId);
       delete browser.registeredOpenURI;
     }
 
     // We are no longer the primary content area.
     browser.removeAttribute("primary");
@@ -3131,29 +3140,35 @@ window._gBrowser = {
 
     return tab;
   },
 
   _blurTab(aTab) {
     this.selectedTab = this._findTabToBlurTo(aTab);
   },
 
+  /**
+   * @returns {boolean}
+   *   False if swapping isn't permitted, true otherwise.
+   */
   swapBrowsersAndCloseOther(aOurTab, aOtherTab) {
     // Do not allow transfering a private tab to a non-private window
     // and vice versa.
     if (PrivateBrowsingUtils.isWindowPrivate(window) !=
-      PrivateBrowsingUtils.isWindowPrivate(aOtherTab.ownerGlobal))
-      return;
+        PrivateBrowsingUtils.isWindowPrivate(aOtherTab.ownerGlobal)) {
+      return false;
+    }
 
     let ourBrowser = this.getBrowserForTab(aOurTab);
     let otherBrowser = aOtherTab.linkedBrowser;
 
     // Can't swap between chrome and content processes.
-    if (ourBrowser.isRemoteBrowser != otherBrowser.isRemoteBrowser)
-      return;
+    if (ourBrowser.isRemoteBrowser != otherBrowser.isRemoteBrowser) {
+      return false;
+    }
 
     // Keep the userContextId if set on other browser
     if (otherBrowser.hasAttribute("usercontextid")) {
       ourBrowser.setAttribute("usercontextid", otherBrowser.getAttribute("usercontextid"));
     }
 
     // That's gBrowser for the other window, not the tab's browser!
     var remoteBrowser = aOtherTab.ownerGlobal.gBrowser;
@@ -3168,18 +3183,22 @@ window._gBrowser = {
       aOtherTab._soundPlayingAttrRemovalTimer = 0;
       aOtherTab.removeAttribute("soundplaying");
       remoteBrowser._tabAttrModified(aOtherTab, ["soundplaying"]);
     }
 
     // First, start teardown of the other browser.  Make sure to not
     // fire the beforeunload event in the process.  Close the other
     // window if this was its last tab.
-    if (!remoteBrowser._beginRemoveTab(aOtherTab, aOurTab, true))
-      return;
+    if (!remoteBrowser._beginRemoveTab(aOtherTab, {
+          adoptedByTab: aOurTab,
+          closeWindowWithLastTab: true,
+        })) {
+      return false;
+    }
 
     // If this is the last tab of the window, hide the window
     // immediately without animation before the docshell swap, to avoid
     // about:blank being painted.
     let [closeWindow] = aOtherTab._endRemoveArgs;
     if (closeWindow) {
       let win = aOtherTab.ownerGlobal;
       win.windowUtils.suppressAnimation(true);
@@ -3274,16 +3293,18 @@ window._gBrowser = {
     // If the tab was already selected (this happpens in the scenario
     // of replaceTabWithWindow), notify onLocationChange, etc.
     if (aOurTab.selected)
       this.updateCurrentBrowser(true);
 
     if (modifiedAttrs.length) {
       this._tabAttrModified(aOurTab, modifiedAttrs);
     }
+
+    return true;
   },
 
   swapBrowsers(aOurTab, aOtherTab, aFlags) {
     let otherBrowser = aOtherTab.linkedBrowser;
     let otherTabBrowser = otherBrowser.getTabBrowser();
 
     // We aren't closing the other tab so, we also need to swap its tablisteners.
     let filter = otherTabBrowser._tabFilters.get(aOtherTab);
@@ -3669,16 +3690,19 @@ window._gBrowser = {
     if (nextTab)
       this.moveTabTo(this.selectedTab, nextTab._tPos);
     else if (this.arrowKeysShouldWrap)
       this.moveTabToStart();
   },
 
   /**
    * Adopts a tab from another browser window, and inserts it at aIndex
+   *
+   * @returns {object}
+   *    The new tab in the current window, null if the tab couldn't be adopted.
    */
   adoptTab(aTab, aIndex, aSelectTab) {
     // Swap the dropped tab with a new one we create and then close
     // it in the other window (making it seem to have moved between
     // windows). We also ensure that the tab we create to swap into has
     // the same remote type and process as the one we're swapping in.
     // This makes sure we don't get a short-lived process for the new tab.
     let linkedBrowser = aTab.linkedBrowser;
@@ -3697,39 +3721,33 @@ window._gBrowser = {
 
     if (aTab.hasAttribute("usercontextid")) {
       // new tab must have the same usercontextid as the old one
       params.userContextId = aTab.getAttribute("usercontextid");
     }
     let newTab = this.addWebTab("about:blank", params);
     let newBrowser = this.getBrowserForTab(newTab);
 
+    aTab.parentNode._finishAnimateTabMove();
+
     // Stop the about:blank load.
     newBrowser.stop();
     // Make sure it has a docshell.
     newBrowser.docShell;
 
-    // We need to select the tab before calling swapBrowsersAndCloseOther
-    // so that window.content in chrome windows points to the right tab
-    // when pagehide/show events are fired. This is no longer necessary
-    // for any exiting browser code, but it may be necessary for add-on
-    // compatibility.
+    if (!this.swapBrowsersAndCloseOther(newTab, aTab)) {
+      // Swapping wasn't permitted. Bail out.
+      this.removeTab(newTab);
+      return null;
+    }
+
     if (aSelectTab) {
       this.selectedTab = newTab;
     }
 
-    aTab.parentNode._finishAnimateTabMove();
-    this.swapBrowsersAndCloseOther(newTab, aTab);
-
-    if (aSelectTab) {
-      // Call updateCurrentBrowser to make sure the URL bar is up to date
-      // for our new tab after we've done swapBrowsersAndCloseOther.
-      this.updateCurrentBrowser(true);
-    }
-
     return newTab;
   },
 
   moveTabBackward() {
     let previousTab = this.selectedTab.previousElementSibling;
     while (previousTab && previousTab.hidden)
       previousTab = previousTab.previousElementSibling;