Bug 1722567 - Save group of closed tabs to restore the all group. r=kashav
authorAntonin LOUBIERE <pyjacpp@laposte.net>
Sun, 12 Sep 2021 17:01:17 +0000
changeset 591716 06634a58a08a2a262083753da474e4e7f65eade2
parent 591715 003b669ee6c62ed3b70379dcde71b26bff45bbec
child 591717 5588dd03bcfb58a81c97e2e6100550810dd3c310
push id149584
push userkmadan@mozilla.com
push dateSun, 12 Sep 2021 17:03:45 +0000
treeherderautoland@5588dd03bcfb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskashav
bugs1722567
milestone94.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 1722567 - Save group of closed tabs to restore the all group. r=kashav When a group of tabs is closed, save the it in session data so tabs could be restored together. Differential Revision: https://phabricator.services.mozilla.com/D121110
browser/base/content/browser.js
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser_undo_close_tabs.js
browser/components/sessionstore/SessionStore.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -8268,17 +8268,16 @@ function undoCloseTab(aIndex) {
     if (SessionStore.getClosedTabCount(window) > index) {
       tab = SessionStore.undoCloseTab(window, index);
 
       if (blankTabToRemove) {
         gBrowser.removeTab(blankTabToRemove);
       }
     }
   }
-  SessionStore.setLastClosedTabCount(window, 1);
 
   return tab;
 }
 
 /**
  * Re-open a closed window.
  * @param aIndex
  *        The index of the window (via SessionStore.getClosedWindowData)
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2578,17 +2578,16 @@
         UserInteraction.start("browser.tabs.opening", "initting", window);
       }
 
       // Don't use document.l10n.setAttributes because the FTL file is loaded
       // lazily and we won't be able to resolve the string.
       document
         .getElementById("History:UndoCloseTab")
         .setAttribute("data-l10n-args", JSON.stringify({ tabCount: 1 }));
-      SessionStore.setLastClosedTabCount(window, 1);
 
       // if we're adding tabs, we're past interrupt mode, ditch the owner
       if (this.selectedTab.owner) {
         this.selectedTab.owner = null;
       }
 
       // Find the tab that opened this one, if any. This is used for
       // determining positioning, and inherited attributes such as the
@@ -3420,28 +3419,29 @@
         window.closeWindow(
           true,
           suppressWarnAboutClosingWindow ? null : window.warnAboutClosingWindow,
           "close-last-tab"
         );
         return;
       }
 
-      let initialTabCount = tabs.length;
+      SessionStore.resetLastClosedTabCount(window);
       this._clearMultiSelectionLocked = true;
 
       // Guarantee that _clearMultiSelectionLocked lock gets released.
       try {
         let tabsWithBeforeUnloadPrompt = [];
         let tabsWithoutBeforeUnload = [];
         let beforeUnloadPromises = [];
         let lastToClose;
         let aParams = { animate, prewarmed: true };
 
         for (let tab of tabs) {
+          tab._closedInGroup = true;
           if (tab.selected) {
             lastToClose = tab;
             let toBlurTo = this._findTabToBlurTo(lastToClose, tabs);
             if (toBlurTo) {
               this._getSwitcher().warmupTab(toBlurTo);
             }
           } else if (this._hasBeforeUnload(tab)) {
             TelemetryStopwatch.start("FX_TAB_CLOSE_PERMIT_UNLOAD_TIME_MS", tab);
@@ -3511,40 +3511,41 @@
         );
         if (!done) {
           return;
         }
 
         // Now run again sequentially the beforeunload listeners that will result in a prompt.
         for (let tab of tabsWithBeforeUnloadPrompt) {
           this.removeTab(tab, aParams);
+          if (!tab.closing) {
+            // If we abort the closing of the tab.
+            tab._closedInGroup = false;
+          }
         }
 
         // Avoid changing the selected browser several times by removing it,
         // if appropriate, lastly.
         if (lastToClose) {
           this.removeTab(lastToClose, aParams);
         }
       } catch (e) {
         Cu.reportError(e);
       }
 
       this._clearMultiSelectionLocked = false;
       this.avoidSingleSelectedTab();
-      let closedTabsCount =
-        initialTabCount - tabs.filter(t => t.isConnected && !t.closing).length;
       // Don't use document.l10n.setAttributes because the FTL file is loaded
       // lazily and we won't be able to resolve the string.
-      document
-        .getElementById("History:UndoCloseTab")
-        .setAttribute(
-          "data-l10n-args",
-          JSON.stringify({ tabCount: closedTabsCount })
-        );
-      SessionStore.setLastClosedTabCount(window, closedTabsCount);
+      document.getElementById("History:UndoCloseTab").setAttribute(
+        "data-l10n-args",
+        JSON.stringify({
+          tabCount: SessionStore.getLastClosedTabCount(window),
+        })
+      );
     },
 
     removeCurrentTab(aParams) {
       this.removeTab(this.selectedTab, aParams);
     },
 
     removeTab(
       aTab,
--- a/browser/base/content/test/tabs/browser_undo_close_tabs.js
+++ b/browser/base/content/test/tabs/browser_undo_close_tabs.js
@@ -21,34 +21,41 @@ add_task(async function withMultiSelecte
   gBrowser.selectedTab = tab2;
   await triggerClickOn(tab4, { shiftKey: true });
 
   ok(!initialTab.multiselected, "InitialTab is not multiselected");
   ok(!tab1.multiselected, "Tab1 is not multiselected");
   ok(tab2.multiselected, "Tab2 is multiselected");
   ok(tab3.multiselected, "Tab3 is multiselected");
   ok(tab4.multiselected, "Tab4 is multiselected");
-  is(gBrowser.multiSelectedTabsCount, 3, "Two multiselected tabs");
+  is(gBrowser.multiSelectedTabsCount, 3, "Three multiselected tabs");
 
   gBrowser.removeMultiSelectedTabs();
   await TestUtils.waitForCondition(
-    () => gBrowser.tabs.length == 2,
-    "wait for the multiselected tabs to close"
+    () => SessionStore.getLastClosedTabCount(window) == 3,
+    "wait for the multi selected tabs to close in SessionStore"
   );
   is(
     SessionStore.getLastClosedTabCount(window),
     3,
     "SessionStore should know how many tabs were just closed"
   );
 
   undoCloseTab();
   await TestUtils.waitForCondition(
     () => gBrowser.tabs.length == 5,
     "wait for the tabs to reopen"
   );
+
+  is(
+    SessionStore.getLastClosedTabCount(window),
+    SessionStore.getClosedTabCount(window) ? 1 : 0,
+    "LastClosedTabCount should be reset"
+  );
+
   info("waiting for the browsers to finish loading");
   // Check that the tabs are restored in the correct order
   for (let tabId of [2, 3, 4]) {
     let browser = gBrowser.tabs[tabId].linkedBrowser;
     await ContentTask.spawn(browser, tabId, async aTabId => {
       await ContentTaskUtils.waitForCondition(() => {
         return (
           content?.document?.readyState == "complete" &&
@@ -56,16 +63,77 @@ add_task(async function withMultiSelecte
         );
       }, "waiting for tab " + aTabId + " to load");
     });
   }
 
   gBrowser.removeAllTabsBut(initialTab);
 });
 
+add_task(async function withBothGroupsAndTab() {
+  let initialTab = gBrowser.selectedTab;
+  let tab1 = await addTab("https://example.com/1");
+  let tab2 = await addTab("https://example.com/2");
+  let tab3 = await addTab("https://example.com/3");
+
+  gBrowser.selectedTab = tab2;
+  await triggerClickOn(tab3, { shiftKey: true });
+
+  ok(!initialTab.multiselected, "InitialTab is not multiselected");
+  ok(!tab1.multiselected, "Tab1 is not multiselected");
+  ok(tab2.multiselected, "Tab2 is multiselected");
+  ok(tab3.multiselected, "Tab3 is multiselected");
+  is(gBrowser.multiSelectedTabsCount, 2, "Two multiselected tabs");
+
+  gBrowser.removeMultiSelectedTabs();
+  await TestUtils.waitForCondition(
+    () => gBrowser.tabs.length == 2,
+    "wait for the multiselected tabs to close"
+  );
+
+  is(
+    SessionStore.getLastClosedTabCount(window),
+    2,
+    "SessionStore should know how many tabs were just closed"
+  );
+
+  let tab4 = await addTab("http://example.com/4");
+
+  is(
+    SessionStore.getLastClosedTabCount(window),
+    2,
+    "LastClosedTabCount should be the same"
+  );
+
+  gBrowser.removeTab(tab4);
+
+  await TestUtils.waitForCondition(
+    () => SessionStore.getLastClosedTabCount(window) == 1,
+    "wait for the tab to close in SessionStore"
+  );
+
+  let count = 3;
+  for (let i = 0; i < 3; i++) {
+    is(
+      SessionStore.getLastClosedTabCount(window),
+      1,
+      "LastClosedTabCount should be one"
+    );
+    undoCloseTab();
+
+    await TestUtils.waitForCondition(
+      () => gBrowser.tabs.length == count,
+      "wait for the tabs to reopen"
+    );
+    count++;
+  }
+
+  gBrowser.removeAllTabsBut(initialTab);
+});
+
 add_task(async function withCloseTabsToTheRight() {
   let initialTab = gBrowser.selectedTab;
   let tab1 = await addTab("https://example.com/1");
   await addTab("https://example.com/2");
   await addTab("https://example.com/3");
   await addTab("https://example.com/4");
 
   gBrowser.removeTabsToTheEndFrom(tab1);
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -304,18 +304,19 @@ var SessionStore = {
       aRestoreImmediately,
       aOptions
     );
   },
 
   getLastClosedTabCount(aWindow) {
     return SessionStoreInternal.getLastClosedTabCount(aWindow);
   },
-  setLastClosedTabCount(aWindow, aNumber) {
-    return SessionStoreInternal.setLastClosedTabCount(aWindow, aNumber);
+
+  resetLastClosedTabCount(aWindow) {
+    SessionStoreInternal.resetLastClosedTabCount(aWindow);
   },
 
   getClosedTabCount: function ss_getClosedTabCount(aWindow) {
     return SessionStoreInternal.getClosedTabCount(aWindow);
   },
 
   getClosedTabData: function ss_getClosedTabData(aWindow, aAsString = true) {
     return SessionStoreInternal.getClosedTabData(aWindow, aAsString);
@@ -758,17 +759,16 @@ var SessionStoreInternal = {
 
     TelemetryTimestamps.add("sessionRestoreInitialized");
     OBSERVING.forEach(function(aTopic) {
       Services.obs.addObserver(this, aTopic, true);
     }, this);
 
     this._initPrefs();
     this._initialized = true;
-    this._closedTabCache = new WeakMap();
 
     Services.telemetry
       .getHistogramById("FX_SESSION_RESTORE_PRIVACY_LEVEL")
       .add(Services.prefs.getIntPref("browser.sessionstore.privacy_level"));
   },
 
   /**
    * Initialize the session using the state provided by SessionStartup
@@ -1193,39 +1193,39 @@ var SessionStoreInternal = {
 
   onFinalTabStateUpdateComplete(browser) {
     let permanentKey = browser.permanentKey;
 
     if (
       this._closedTabs.has(permanentKey) &&
       !this._crashedBrowsers.has(permanentKey)
     ) {
-      let { closedTabs, tabData } = this._closedTabs.get(permanentKey);
+      let { winData, closedTabs, tabData } = this._closedTabs.get(permanentKey);
 
       // We expect no further updates.
       this._closedTabs.delete(permanentKey);
 
       // The tab state no longer needs this reference.
       delete tabData.permanentKey;
 
       // Determine whether the tab state is worth saving.
       let shouldSave = this._shouldSaveTabState(tabData.state);
       let index = closedTabs.indexOf(tabData);
 
       if (shouldSave && index == -1) {
         // If the tab state is worth saving and we didn't push it onto
         // the list of closed tabs when it was closed (because we deemed
         // the state not worth saving) then add it to the window's list
         // of closed tabs now.
-        this.saveClosedTabData(closedTabs, tabData);
+        this.saveClosedTabData(winData, closedTabs, tabData);
       } else if (!shouldSave && index > -1) {
         // Remove from the list of closed tabs. The update messages sent
         // after the tab was closed changed enough state so that we no
         // longer consider its data interesting enough to keep around.
-        this.removeClosedTabData(closedTabs, index);
+        this.removeClosedTabData(winData, closedTabs, index);
       }
     }
 
     // If this the final message we need to resolve all pending flush
     // requests for the given browser as they might have been sent too
     // late and will never respond. If they have been sent shortly after
     // switching a browser's remoteness there isn't too much data to skip.
     TabStateFlusher.resolveAll(browser);
@@ -1472,16 +1472,17 @@ var SessionStoreInternal = {
       );
     }
 
     // and create its data object
     this._windows[aWindow.__SSi] = {
       tabs: [],
       selected: 0,
       _closedTabs: [],
+      _lastClosedTabGroupCount: -1,
       busy: false,
     };
 
     if (PrivateBrowsingUtils.isWindowPrivate(aWindow)) {
       this._windows[aWindow.__SSi].isPrivate = true;
     }
     if (!this._isWindowLoaded(aWindow)) {
       this._windows[aWindow.__SSi]._restoring = true;
@@ -2489,35 +2490,37 @@ var SessionStoreInternal = {
 
     let tabData = {
       permanentKey,
       state: tabState,
       title: aTab.label,
       image: aWindow.gBrowser.getIcon(aTab),
       pos: aTab._tPos,
       closedAt: Date.now(),
+      closedInGroup: aTab._closedInGroup,
     };
 
-    let closedTabs = this._windows[aWindow.__SSi]._closedTabs;
+    let winData = this._windows[aWindow.__SSi];
+    let closedTabs = winData._closedTabs;
 
     // Determine whether the tab contains any information worth saving. Note
     // that there might be pending state changes queued in the child that
     // didn't reach the parent yet. If a tab is emptied before closing then we
     // might still remove it from the list of closed tabs later.
     if (this._shouldSaveTabState(tabState)) {
       // Save the tab state, for now. We might push a valid tab out
       // of the list but those cases should be extremely rare and
       // do probably never occur when using the browser normally.
       // (Tests or add-ons might do weird things though.)
-      this.saveClosedTabData(closedTabs, tabData);
+      this.saveClosedTabData(winData, closedTabs, tabData);
     }
 
     // Remember the closed tab to properly handle any last updates included in
     // the final "update" message sent by the frame script's unload handler.
-    this._closedTabs.set(permanentKey, { closedTabs, tabData });
+    this._closedTabs.set(permanentKey, { winData, closedTabs, tabData });
   },
 
   /**
    * Remove listeners which were added when browser was inserted and reset restoring state.
    * Also re-instate lazy data and basically revert tab to its lazy browser state.
    * @param aTab
    *        Tab reference
    */
@@ -2621,22 +2624,24 @@ var SessionStoreInternal = {
   },
 
   /**
    * Insert a given |tabData| object into the list of |closedTabs|. We will
    * determine the right insertion point based on the .closedAt properties of
    * all tabs already in the list. The list will be truncated to contain a
    * maximum of |this._max_tabs_undo| entries.
    *
+   * @param winData (object)
+   *        The data of the window.
+   * @param tabData (object)
+   *        The tabData to be inserted.
    * @param closedTabs (array)
    *        The list of closed tabs for a window.
-   * @param tabData (object)
-   *        The tabData to be inserted.
    */
-  saveClosedTabData(closedTabs, tabData) {
+  saveClosedTabData(winData, closedTabs, tabData) {
     // Find the index of the first tab in the list
     // of closed tabs that was closed before our tab.
     let index = closedTabs.findIndex(tab => {
       return tab.closedAt < tabData.closedAt;
     });
 
     // If we found no tab closed before our
     // tab then just append it to the list.
@@ -2646,37 +2651,57 @@ var SessionStoreInternal = {
 
     // About to save the closed tab, add a unique ID.
     tabData.closedId = this._nextClosedId++;
 
     // Insert tabData at the right position.
     closedTabs.splice(index, 0, tabData);
     this._closedObjectsChanged = true;
 
+    if (tabData.closedInGroup) {
+      if (winData._lastClosedTabGroupCount < this._max_tabs_undo) {
+        if (winData._lastClosedTabGroupCount < 0) {
+          winData._lastClosedTabGroupCount = 1;
+        } else {
+          winData._lastClosedTabGroupCount++;
+        }
+      }
+    } else {
+      winData._lastClosedTabGroupCount = -1;
+    }
+
     // Truncate the list of closed tabs, if needed.
     if (closedTabs.length > this._max_tabs_undo) {
       closedTabs.splice(this._max_tabs_undo, closedTabs.length);
     }
   },
 
   /**
    * Remove the closed tab data at |index| from the list of |closedTabs|. If
    * the tab's final message is still pending we will simply discard it when
    * it arrives so that the tab doesn't reappear in the list.
    *
+   * @param winData (object)
+   *        The data of the window.
+   * @param index (uint)
+   *        The index of the tab to remove.
    * @param closedTabs (array)
    *        The list of closed tabs for a window.
-   * @param index (uint)
-   *        The index of the tab to remove.
    */
-  removeClosedTabData(closedTabs, index) {
+  removeClosedTabData(winData, closedTabs, index) {
     // Remove the given index from the list.
     let [closedTab] = closedTabs.splice(index, 1);
     this._closedObjectsChanged = true;
 
+    // If the tab is part of the last closed group,
+    // we need to deduct the tab from the count.
+    if (index < winData._lastClosedTabGroupCount) {
+      winData._lastClosedTabGroupCount--;
+    }
+
     // If the closed tab's state still has a .permanentKey property then we
     // haven't seen its final update message yet. Remove it from the map of
     // closed tabs so that we will simply discard its last messages and will
     // not add it back to the list of closed tabs again.
     if (closedTab.permanentKey) {
       this._closedTabs.delete(closedTab.permanentKey);
       this._closedWindowTabs.delete(closedTab.permanentKey);
       delete closedTab.permanentKey;
@@ -3095,34 +3120,31 @@ var SessionStoreInternal = {
       });
     });
 
     return newTab;
   },
 
   getLastClosedTabCount(aWindow) {
     if ("__SSi" in aWindow) {
-      // Blank tabs cannot be undo-closed, so the number returned by
-      // the ClosedTabCache can be greater than the return value of
-      // getClosedTabCount. We won't restore blank tabs, so we return
-      // the minimum of these two values.
       return Math.min(
-        this._closedTabCache.get(aWindow) || 1,
+        Math.max(this._windows[aWindow.__SSi]._lastClosedTabGroupCount, 1),
         this.getClosedTabCount(aWindow)
       );
     }
 
     throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
   },
-  setLastClosedTabCount(aWindow, aNumber) {
+
+  resetLastClosedTabCount(aWindow) {
     if ("__SSi" in aWindow) {
-      return this._closedTabCache.set(aWindow, aNumber);
-    }
-
-    throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
+      this._windows[aWindow.__SSi]._lastClosedTabGroupCount = -1;
+    } else {
+      throw (Components.returnCode = Cr.NS_ERROR_INVALID_ARG);
+    }
   },
 
   getClosedTabCount: function ssi_getClosedTabCount(aWindow) {
     if ("__SSi" in aWindow) {
       return this._windows[aWindow.__SSi]._closedTabs.length;
     }
 
     if (!DyingWindowCache.has(aWindow)) {
@@ -3158,29 +3180,33 @@ var SessionStoreInternal = {
   undoCloseTab: function ssi_undoCloseTab(aWindow, aIndex) {
     if (!aWindow.__SSi) {
       throw Components.Exception(
         "Window is not tracked",
         Cr.NS_ERROR_INVALID_ARG
       );
     }
 
-    var closedTabs = this._windows[aWindow.__SSi]._closedTabs;
+    let winData = this._windows[aWindow.__SSi];
 
     // default to the most-recently closed tab
     aIndex = aIndex || 0;
-    if (!(aIndex in closedTabs)) {
+    if (!(aIndex in winData._closedTabs)) {
       throw Components.Exception(
         "Invalid index: not in the closed tabs",
         Cr.NS_ERROR_INVALID_ARG
       );
     }
 
     // fetch the data of closed tab, while removing it from the array
-    let { state, pos } = this.removeClosedTabData(closedTabs, aIndex);
+    let { state, pos } = this.removeClosedTabData(
+      winData,
+      winData._closedTabs,
+      aIndex
+    );
 
     // create a new tab
     let tabbrowser = aWindow.gBrowser;
     let tab = (tabbrowser.selectedTab = tabbrowser.addTrustedTab(null, {
       index: pos,
       pinned: state.pinned,
       userContextId: state.userContextId,
     }));
@@ -3197,29 +3223,29 @@ var SessionStoreInternal = {
   forgetClosedTab: function ssi_forgetClosedTab(aWindow, aIndex) {
     if (!aWindow.__SSi) {
       throw Components.Exception(
         "Window is not tracked",
         Cr.NS_ERROR_INVALID_ARG
       );
     }
 
-    var closedTabs = this._windows[aWindow.__SSi]._closedTabs;
+    let winData = this._windows[aWindow.__SSi];
 
     // default to the most-recently closed tab
     aIndex = aIndex || 0;
-    if (!(aIndex in closedTabs)) {
+    if (!(aIndex in winData._closedTabs)) {
       throw Components.Exception(
         "Invalid index: not in the closed tabs",
         Cr.NS_ERROR_INVALID_ARG
       );
     }
 
     // remove closed tab from the array
-    this.removeClosedTabData(closedTabs, aIndex);
+    this.removeClosedTabData(winData, winData._closedTabs, aIndex);
 
     // Notify of changes to closed objects.
     this._notifyOfClosedObjectsChange();
   },
 
   getClosedWindowCount: function ssi_getClosedWindowCount() {
     return this._closedWindows.length;
   },
@@ -3494,17 +3520,17 @@ var SessionStoreInternal = {
         let indexes = [];
         windowState._closedTabs.forEach((closedTab, index) => {
           if (closedTab.state.userContextId == userContextId) {
             indexes.push(index);
           }
         });
 
         for (let index of indexes.reverse()) {
-          this.removeClosedTabData(windowState._closedTabs, index);
+          this.removeClosedTabData(windowState, windowState._closedTabs, index);
         }
       }
     }
 
     // Notify of changes to closed objects.
     this._notifyOfClosedObjectsChange();
   },