[Sessions] Remove Group and Window entries when the last tab is restored

Pass the SessionId of the window or group to remove when they become
empty after a restore.

This prevents a bug where the recent tabs menu could show entries with
0 restorable tabs. See bug for screenshot.

Change-Id: I44c4364121d14061079df06cebd86db450531eaf
Bug: 405426765
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6382275
Reviewed-by: Eshwar Stalin <estalin@chromium.org>
Commit-Queue: Darryl James <dljames@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1436327}

chrome/browser/sessions/tab_restore_browsertest.cc

@@ -54,6 +54,7 @@
 #include "components/saved_tab_groups/public/saved_tab_group_tab.h"
 #include "components/saved_tab_groups/public/types.h"
 #include "components/sessions/content/content_test_helper.h"
+#include "components/sessions/core/session_id.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/sessions/core/tab_restore_service_impl.h"
 #include "components/sessions/core/tab_restore_service_observer.h"
@@ -623,6 +624,72 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreGroup) {
             gfx::Range(2, 4));
 }
 
+IN_PROC_BROWSER_TEST_F(TabRestoreTest,
+                       PRE_RestoringAllTabsInWindowRemovesEntryFromService) {
+  AddFileSchemeTabs(browser(), 1);
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+}
+
+IN_PROC_BROWSER_TEST_F(TabRestoreTest,
+                       RestoringAllTabsInWindowRemovesEntryFromService) {
+  sessions::TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(browser()->profile());
+  ASSERT_TRUE(service);
+  EXPECT_EQ(1u, service->entries().size());
+
+  sessions::tab_restore::Entry* entry = service->entries().front().get();
+  ASSERT_EQ(sessions::tab_restore::WINDOW, entry->type);
+
+  auto* window = static_cast<sessions::tab_restore::Window*>(entry);
+  ASSERT_EQ(2u, window->tabs.size());
+
+  SessionID tab_1_id = window->tabs[0]->id;
+  SessionID tab_2_id = window->tabs[1]->id;
+
+  // Restoring the first tab from the window should keep the window entry.
+  service->RestoreEntryById(browser()->live_tab_context(), tab_1_id,
+                            WindowOpenDisposition::NEW_WINDOW);
+  EXPECT_EQ(1u, service->entries().size());
+
+  // Restoring the last tab from the window should remove the window entry.
+  service->RestoreEntryById(browser()->live_tab_context(), tab_2_id,
+                            WindowOpenDisposition::NEW_WINDOW);
+  EXPECT_EQ(0u, service->entries().size());
+}
+
+IN_PROC_BROWSER_TEST_F(TabRestoreTest,
+                       RestoringAllTabsInGroupRemovesEntryFromService) {
+  AddFileSchemeTabs(browser(), 3);
+
+  tab_groups::TabGroupId group =
+      browser()->tab_strip_model()->AddToNewGroup({1, 2});
+  CloseGroup(group);
+
+  sessions::TabRestoreService* service =
+      TabRestoreServiceFactory::GetForProfile(browser()->profile());
+  ASSERT_TRUE(service);
+  EXPECT_EQ(1u, service->entries().size());
+
+  sessions::tab_restore::Entry* entry = service->entries().front().get();
+  ASSERT_EQ(sessions::tab_restore::GROUP, entry->type);
+
+  auto* tab_group = static_cast<sessions::tab_restore::Group*>(entry);
+  ASSERT_EQ(2u, tab_group->tabs.size());
+
+  SessionID tab_1_id = tab_group->tabs[0]->id;
+  SessionID tab_2_id = tab_group->tabs[1]->id;
+
+  // Restoring the first tab from the group should keep the group entry.
+  service->RestoreEntryById(browser()->live_tab_context(), tab_1_id,
+                            WindowOpenDisposition::CURRENT_TAB);
+  EXPECT_EQ(1u, service->entries().size());
+
+  // Restoring the last tab from the group should remove the group entry.
+  service->RestoreEntryById(browser()->live_tab_context(), tab_2_id,
+                            WindowOpenDisposition::CURRENT_TAB);
+  EXPECT_EQ(0u, service->entries().size());
+}
+
 // Verifies that restoring a grouped tab in a browser that does not support tab
 // groups, does restore the tab but does not recreate the group.
 IN_PROC_BROWSER_TEST_F(TabRestoreTest,

components/sessions/core/tab_restore_service_helper.cc

@@ -695,7 +695,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
           // The entries_ may by changed after the tabs restored and the
           // entry_iterator may be no longer valid. So call RemoveEntryById here
           // instead of entries_.erase(entry_iterator).
-          RemoveEntryById(id);
+          RemoveEntryById(window.id);
         }
       }
 
@@ -751,7 +751,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
               // The entries_ may by changed after the tabs restored and the
               // entry_iterator may be no longer valid. So call RemoveEntryById
               // here instead of entries_.erase(entry_iterator).
-              RemoveEntryById(id);
+              RemoveEntryById(group.id);
             }
 
             break;