Bug 1939599: Tab groups can be restored by Ctrl+Shift+T r=dao,sthompson,sessionstore-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D233102

browser/base/content/browser.js

@@ -5977,6 +5977,10 @@ function restoreLastClosedTabOrWindowOrSession() {
         undoCloseTab();
         break;
       }
+      case SessionStore.LAST_ACTION_CLOSED_TAB_GROUP: {
+        undoCloseTabGroup(lastActionTaken.closedId);
+        break;
+      }
       case SessionStore.LAST_ACTION_CLOSED_WINDOW: {
         undoCloseWindow();
         break;
@@ -6051,6 +6055,38 @@ function undoCloseTab(aIndex, sourceWindowSSId) {
   return tab;
 }
 
+/**
+ * Re-open a closed tab group.
+ * @param {string} tabGroupId
+ *        The tab group's unique ID.
+ * @returns {MozTabbrowserTabGroup} the reopened group.
+ */
+function undoCloseTabGroup(tabGroupId) {
+  let targetWindow = window;
+
+  // wallpaper patch to prevent an unnecessary blank tab (bug 343895)
+  let blankTabToRemove = null;
+  if (
+    targetWindow.gBrowser.visibleTabs.length == 1 &&
+    targetWindow.gBrowser.selectedTab.isEmpty
+  ) {
+    blankTabToRemove = targetWindow.gBrowser.selectedTab;
+  }
+
+  let group;
+  try {
+    group = SessionStore.undoCloseTabGroup(window, tabGroupId, targetWindow);
+  } catch (err) {
+    group = SessionStore.openSavedTabGroup(tabGroupId, targetWindow);
+  }
+
+  if (group && blankTabToRemove) {
+    targetWindow.gBrowser.removeTab(blankTabToRemove);
+  }
+
+  return group;
+}
+
 /**
  * Re-open a closed window.
  * @param aIndex

browser/base/content/browser.js.globals

@@ -90,6 +90,7 @@
   "AddKeywordForSearchField",
   "restoreLastClosedTabOrWindowOrSession",
   "undoCloseTab",
+  "undoCloseTabGroup",
   "undoCloseWindow",
   "ReportFalseDeceptiveSite",
   "ReportSiteIssue",

browser/components/sessionstore/SessionStore.sys.mjs

@@ -237,6 +237,10 @@ export var SessionStore = {
     return SessionStoreInternal._LAST_ACTION_CLOSED_TAB;
   },
 
+  get LAST_ACTION_CLOSED_TAB_GROUP() {
+    return SessionStoreInternal._LAST_ACTION_CLOSED_TAB_GROUP;
+  },
+
   get LAST_ACTION_CLOSED_WINDOW() {
     return SessionStoreInternal._LAST_ACTION_CLOSED_WINDOW;
   },
@@ -1047,6 +1051,8 @@ var SessionStoreInternal = {
 
   _LAST_ACTION_CLOSED_TAB: "tab",
 
+  _LAST_ACTION_CLOSED_TAB_GROUP: "tab-group",
+
   _LAST_ACTION_CLOSED_WINDOW: "window",
 
   _log: null,
@@ -3046,6 +3052,15 @@ var SessionStoreInternal = {
     win,
     tabGroup
   ) {
+    // don't update our internal state if we don't have to
+    if (this._max_tabs_undo == 0) {
+      return;
+    }
+
+    // Record the closed action right away to make sure it's also being
+    // recorded for groups that are saved and closed
+    this._addClosedAction(this._LAST_ACTION_CLOSED_TAB_GROUP, tabGroup.id);
+
     if (this.getSavedTabGroup(tabGroup.id)) {
       // If a tab group is being removed from the tab strip but it's already
       // saved, then this is a "save and close" action; the saved tab group
@@ -3054,24 +3069,17 @@ var SessionStoreInternal = {
       return;
     }
 
-    // don't update our internal state if we don't have to
-    if (this._max_tabs_undo == 0) {
-      return;
-    }
-
     let closedGroups = this._windows[win.__SSi].closedGroups;
-
     let tabGroupState = lazy.TabGroupState.closed(tabGroup, win.__SSi);
     tabGroupState.tabs = this._collectClosedTabsForTabGroup(tabGroup.tabs, win);
 
     // TODO(jswinarton) it's unclear if updating lastClosedTabGroupCount is
     // necessary when restoring tab groups — it largely depends on how we
-    // decide to do the restore. If we add a _LAST_ACTION_CLOSED_TAB_GROUP
+    // decide to do the restore.
-    // item to the closed actions list, this may not be necessary.
     // To address in bug1915174
     this._windows[win.__SSi]._lastClosedTabGroupCount =
       tabGroupState.tabs.length;
-    closedGroups.push(tabGroupState);
+    closedGroups.unshift(tabGroupState);
     this._closedObjectsChanged = true;
   },
 
@@ -3312,8 +3320,16 @@ var SessionStoreInternal = {
    *        The tabData to be inserted.
    * @param closedTabs (array)
    *        The list of closed tabs for a window.
+   * @param saveAction (boolean|null)
+   *        Whether or not to add an action to the closed actions stack on save.
+   *        True by default if the tab was not closed as part of a tab group; false otherwise.
    */
-  saveClosedTabData(winData, closedTabs, tabData, saveAction = true) {
+  saveClosedTabData(winData, closedTabs, tabData, saveAction = null) {
+    if (saveAction === null) {
+      // By default, a closed action for this tab, but only if it wasn't closed as part of a tab group.
+      saveAction = closedTabs === winData._closedTabs;
+    }
+
     // Find the index of the first tab in the list
     // of closed tabs that was closed before our tab.
     let index = closedTabs.findIndex(tab => {
@@ -4128,7 +4144,7 @@ var SessionStoreInternal = {
 
         if (
           groupIdx < closedGroups.length &&
-          (tabIdx >= closedTabs.length || group?.closedAt < tab?.closedAt)
+          (tabIdx >= closedTabs.length || group?.closedAt > tab?.closedAt)
         ) {
           group.tabs.forEach((groupTab, idx) => {
             groupTab._originalStateIndex = idx;

browser/components/sessionstore/test/browser_restoreLastClosedTabOrWindowOrSession.js

@@ -173,6 +173,74 @@ add_task(async function test_undo_last_action() {
     "Window still has two open tabs"
   );
 
+  // create a tab group, delete it, and re-open it
+  const groupedTab = BrowserTestUtils.addTab(
+    window.gBrowser,
+    "https://example.com"
+  );
+  await BrowserTestUtils.browserLoaded(groupedTab.linkedBrowser);
+  const tabGroup = window.gBrowser.addTabGroup([groupedTab]);
+  Assert.equal(
+    window.gBrowser.tabs.length,
+    3,
+    "Window has three tabs after creating tab group"
+  );
+
+  await TabStateFlusher.flushWindow(window);
+  let removePromise = BrowserTestUtils.waitForEvent(
+    tabGroup,
+    "TabGroupRemoved"
+  );
+  window.gBrowser.removeTabGroup(tabGroup);
+  await removePromise;
+
+  restoreLastClosedTabOrWindowOrSession();
+  Assert.equal(
+    window.gBrowser.tabs.length,
+    3,
+    "Window has three tabs after restoring tab group"
+  );
+  Assert.equal(
+    window.gBrowser.tabGroups.length,
+    1,
+    "Tab group exists on the tab strip after restore"
+  );
+
+  // Save and close the tab group, and then re-open it
+  const groupedTabToSave = BrowserTestUtils.addTab(
+    window.gBrowser,
+    "https://example.com"
+  );
+  await BrowserTestUtils.browserLoaded(groupedTabToSave.linkedBrowser);
+  const tabGroupToSave = window.gBrowser.addTabGroup([groupedTabToSave]);
+  Assert.equal(
+    window.gBrowser.tabs.length,
+    4,
+    "Window has four tabs after creating tab group"
+  );
+
+  await TabStateFlusher.flushWindow(window);
+  tabGroupToSave.save();
+  removePromise = BrowserTestUtils.waitForEvent(
+    tabGroupToSave,
+    "TabGroupRemoved"
+  );
+  window.gBrowser.removeTabGroup(tabGroupToSave);
+  await removePromise;
+
+  await TabStateFlusher.flushWindow(window);
+  restoreLastClosedTabOrWindowOrSession();
+  Assert.equal(
+    window.gBrowser.tabs.length,
+    4,
+    "Window has four tabs after restoring tab group"
+  );
+  Assert.equal(
+    window.gBrowser.tabGroups.length,
+    2,
+    "Tab group exists on the tab strip after restore"
+  );
+
   gBrowser.removeAllTabsBut(gBrowser.tabs[0]);
 });