Restore groups from session windows to TabRestoreService

Both TabRestoreService and SessionService support tab groups, but
restoring a window after a browser restart did not restore
groups. This was because the code translating SessionService entries
to TabRestoreService entries did not copy over group data.

Adds the necessary code to restore groups correctly. This allows, for
example, restoring a recent window with groups after a browser crash.

Fixed: 1168751
Change-Id: I6510ab036caa1450f904ea7e1209885acf08682d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2641334Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845702}

chrome/browser/sessions/tab_restore_service_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -35,6 +36,8 @@
 #include "components/sessions/core/serialized_navigation_entry_test_helper.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sessions/core/tab_restore_service_observer.h"
+#include "components/tab_groups/tab_group_id.h"
+#include "components/tab_groups/tab_group_visual_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -150,9 +153,15 @@ class TabRestoreServiceImplTest : public ChromeRenderViewHostTestHarness {
     SynchronousLoadTabsFromLastSession();
   }
 
-  // Adds a window with one tab and url to the profile's session service.
-  // If |pinned| is true, the tab is marked as pinned in the session service.
-  void AddWindowWithOneTabToSessionService(bool pinned) {
+  // Adds a window with one tab and url to the profile's session
+  // service. If |pinned| is true, the tab is marked as pinned in the
+  // session service. If |group| is present, sets the tab's group ID. If
+  // |group_visual_data| is also present, sets |group|'s visual data.
+  void AddWindowWithOneTabToSessionService(
+      bool pinned,
+      base::Optional<tab_groups::TabGroupId> group = base::nullopt,
+      base::Optional<tab_groups::TabGroupVisualData> group_visual_data =
+          base::nullopt) {
     // Create new window / tab IDs so that these remain distinct.
     window_id_ = SessionID::NewUnique();
     tab_id_ = SessionID::NewUnique();
@@ -165,6 +174,12 @@ class TabRestoreServiceImplTest : public ChromeRenderViewHostTestHarness {
     session_service->SetSelectedTabInWindow(window_id(), 0);
     if (pinned)
       session_service->SetPinnedState(window_id(), tab_id(), true);
+    if (group)
+      session_service->SetTabGroup(window_id(), tab_id(), group);
+    if (group && group_visual_data)
+      session_service->SetTabGroupMetadata(window_id(), *group,
+                                           &*group_visual_data);
+
     session_service->UpdateTabNavigation(
         window_id(), tab_id(),
         ContentTestHelper::CreateNavigation(url1_.spec(), "title"));
@@ -977,3 +992,26 @@ TEST_F(TabRestoreServiceImplTest, GoToLoadedWhenHaveMaxEntries) {
   SynchronousLoadTabsFromLastSession();
   EXPECT_TRUE(service_->IsLoaded());
 }
+
+// Ensures tab group data is restored from previous session.
+TEST_F(TabRestoreServiceImplTest, TabGroupsRestoredFromSessionData) {
+  CreateSessionServiceWithOneWindow(false);
+
+  auto group = tab_groups::TabGroupId::GenerateNew();
+  auto group_visual_data = tab_groups::TabGroupVisualData(
+      base::ASCIIToUTF16("Foo"), tab_groups::TabGroupColorId::kBlue);
+  AddWindowWithOneTabToSessionService(false, group, group_visual_data);
+
+  SessionServiceFactory::GetForProfile(profile())
+      ->MoveCurrentSessionToLastSession();
+  EXPECT_FALSE(service_->IsLoaded());
+  SynchronousLoadTabsFromLastSession();
+
+  ASSERT_EQ(2u, service_->entries().size());
+  Entry* entry = service_->entries().back().get();
+  ASSERT_EQ(sessions::TabRestoreService::WINDOW, entry->type);
+  auto* window = static_cast<sessions::TabRestoreService::Window*>(entry);
+  ASSERT_EQ(1u, window->tabs.size());
+  EXPECT_EQ(group, window->tabs[0]->group);
+  EXPECT_EQ(group_visual_data, window->tab_groups[group]);
+}

components/sessions/core/tab_restore_service_impl.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <string.h>
+#include <map>
 #include <utility>
 #include <vector>
 
@@ -1137,10 +1138,28 @@ void TabRestoreServiceImpl::PersistenceDelegate::OnGotPreviousSession(
 bool TabRestoreServiceImpl::PersistenceDelegate::ConvertSessionWindowToWindow(
     SessionWindow* session_window,
     Window* window) {
+  // The group visual datas must be stored in both |window| and each
+  // grouped tab.
+  std::map<tab_groups::TabGroupId, tab_groups::TabGroupVisualData>
+      group_visual_datas;
+  for (size_t i = 0; i < session_window->tab_groups.size(); ++i) {
+    auto group_id = session_window->tab_groups[i]->id;
+    group_visual_datas[group_id] = session_window->tab_groups[i]->visual_data;
+  }
+
   for (size_t i = 0; i < session_window->tabs.size(); ++i) {
-    if (!session_window->tabs[i]->navigations.empty()) {
+    if (session_window->tabs[i]->navigations.empty())
+      continue;
+
     window->tabs.push_back(std::make_unique<Tab>());
     Tab& tab = *window->tabs.back();
+
+    auto group_id = session_window->tabs[i]->group;
+    if (group_id.has_value()) {
+      tab.group = group_id;
+      tab.group_visual_data = group_visual_datas[group_id.value()];
+    }
+
     tab.pinned = session_window->tabs[i]->pinned;
     tab.navigations.swap(session_window->tabs[i]->navigations);
     tab.current_navigation_index =
@@ -1148,10 +1167,11 @@ bool TabRestoreServiceImpl::PersistenceDelegate::ConvertSessionWindowToWindow(
     tab.extension_app_id = session_window->tabs[i]->extension_app_id;
     tab.timestamp = base::Time();
   }
-  }
+
   if (window->tabs.empty())
     return false;
 
+  window->tab_groups = std::move(group_visual_datas);
   window->selected_tab_index =
       std::min(session_window->selected_tab_index,
                static_cast<int>(window->tabs.size() - 1));