[Tab Groups] Restore tab groups when reopening a recently closed tab.

The tab metadata (color and title) is only restored if the tab is the
only one in the group or else the current metadata will be unchanged.

Bug: 979274
Change-Id: I0c03a1d6fb6286ffda7144caea559a6cc37ebc04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873503Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Connie Wan <connily@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Commit-Queue: Charlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723662}

chrome/browser/sessions/session_service.cc

@@ -88,7 +88,7 @@ SessionService::SessionService(Profile* profile)
 }
 
 SessionService::SessionService(const base::FilePath& save_path)
-    : profile_(NULL),
+    : profile_(nullptr),
       should_use_delayed_save_(false),
       base_session_service_(new sessions::BaseSessionService(
           sessions::BaseSessionService::SESSION_RESTORE,
@@ -131,7 +131,7 @@ bool SessionService::ShouldNewWindowStartSession() {
 }
 
 bool SessionService::RestoreIfNecessary(const std::vector<GURL>& urls_to_open) {
-  return RestoreIfNecessary(urls_to_open, NULL);
+  return RestoreIfNecessary(urls_to_open, nullptr);
 }
 
 void SessionService::ResetFromCurrentBrowsers() {
@@ -482,9 +482,8 @@ void SessionService::TabRestored(WebContents* tab, bool pinned) {
   if (!ShouldTrackChangesToWindow(session_tab_helper->window_id()))
     return;
 
-  // TODO(crbug.com/930991): handle tab groups here.
   BuildCommandsForTab(session_tab_helper->window_id(), tab, -1, base::nullopt,
-                      pinned, NULL);
+                      pinned, nullptr);
   base_session_service_->StartSaveTimer();
 }
 

chrome/browser/sessions/tab_restore_browsertest.cc

@@ -904,24 +904,69 @@ class TabRestoreTestWithTabGroupsEnabled : public TabRestoreTest {
   base::test::ScopedFeatureList feature_list_;
 };
 
-IN_PROC_BROWSER_TEST_F(TabRestoreTestWithTabGroupsEnabled, RestoreGroupedTab) {
+// Closing the last tab in a group then restoring will place the group back with
+// its metadata.
+IN_PROC_BROWSER_TEST_F(TabRestoreTestWithTabGroupsEnabled,
+                       RestoreSingleGroupedTab) {
   const int tab_count = AddSomeTabs(browser(), 1);
   ASSERT_LE(2, tab_count);
 
   const int grouped_tab_index = tab_count - 1;
-  browser()->tab_strip_model()->AddToNewGroup({grouped_tab_index});
+  TabGroupId group_id =
+      browser()->tab_strip_model()->AddToNewGroup({grouped_tab_index});
+  const TabGroupVisualData visual_data(base::ASCIIToUTF16("Foo"), SK_ColorCYAN);
+
+  TabGroup* group =
+      browser()->tab_strip_model()->group_model()->GetTabGroup(group_id);
+
+  group->SetVisualData(visual_data);
   CloseTab(grouped_tab_index);
 
   ASSERT_NO_FATAL_FAILURE(RestoreTab(0, grouped_tab_index));
   ASSERT_EQ(tab_count, browser()->tab_strip_model()->count());
 
-  // Make sure the tab is *not* grouped when restored. We have not yet decided
-  // how to handle groups with the same group ID in different browser
-  // windows. Until we figure this out, we don't have a good way to handle
-  // restoring individual grouped tabs. TODO(crbug.com/930991): resolve this and
-  // change this expectation.
-  EXPECT_EQ(base::nullopt,
-            browser()->tab_strip_model()->GetTabGroupForTab(grouped_tab_index));
+  group = browser()->tab_strip_model()->group_model()->GetTabGroup(group_id);
+
+  EXPECT_EQ(group_id, browser()
+                          ->tab_strip_model()
+                          ->GetTabGroupForTab(grouped_tab_index)
+                          .value());
+  const TabGroupVisualData* data = group->visual_data();
+  EXPECT_EQ(data->title(), visual_data.title());
+  EXPECT_EQ(data->color(), visual_data.color());
+}
+
+// Closing a tab in a group then updating the metadata before restoring will
+// place the group back without updating the metadata.
+IN_PROC_BROWSER_TEST_F(TabRestoreTestWithTabGroupsEnabled,
+                       RestoreTabIntoGroup) {
+  const int tab_count = AddSomeTabs(browser(), 2);
+  ASSERT_LE(3, tab_count);
+
+  const int closed_tab_index = 1;
+
+  TabGroupId group_id = browser()->tab_strip_model()->AddToNewGroup({0, 1});
+  const TabGroupVisualData visual_data_1(base::ASCIIToUTF16("Foo1"),
+                                         SK_ColorCYAN);
+  const TabGroupVisualData visual_data_2(base::ASCIIToUTF16("Foo2"),
+                                         SK_ColorCYAN);
+  TabGroup* group =
+      browser()->tab_strip_model()->group_model()->GetTabGroup(group_id);
+
+  group->SetVisualData(visual_data_1);
+  CloseTab(closed_tab_index);
+  group->SetVisualData(visual_data_2);
+
+  ASSERT_NO_FATAL_FAILURE(RestoreTab(0, closed_tab_index));
+  ASSERT_EQ(tab_count, browser()->tab_strip_model()->count());
+
+  EXPECT_EQ(group_id, browser()
+                          ->tab_strip_model()
+                          ->GetTabGroupForTab(closed_tab_index)
+                          .value());
+  const TabGroupVisualData* data = group->visual_data();
+  EXPECT_EQ(data->title(), visual_data_2.title());
+  EXPECT_EQ(data->color(), visual_data_2.color());
 }
 
 IN_PROC_BROWSER_TEST_F(TabRestoreTestWithTabGroupsEnabled,

chrome/browser/ui/android/tab_model/android_live_tab_context.cc

@@ -98,6 +98,7 @@ sessions::LiveTab* AndroidLiveTabContext::AddRestoredTab(
     int selected_navigation,
     const std::string& extension_app_id,
     base::Optional<base::Token> group,
+    const TabGroupMetadata* group_metadata,
     bool select,
     bool pin,
     bool from_last_session,

chrome/browser/ui/android/tab_model/android_live_tab_context.h

@@ -44,6 +44,7 @@ class AndroidLiveTabContext : public sessions::LiveTabContext {
       int selected_navigation,
       const std::string& extension_app_id,
       base::Optional<base::Token> group,
+      const TabGroupMetadata* group_metadata,
       bool select,
       bool pin,
       bool from_last_session,

chrome/browser/ui/browser_commands.cc

@@ -750,17 +750,17 @@ WebContents* DuplicateTabAt(Browser* browser, int index) {
   if (browser->CanSupportWindowFeature(Browser::FEATURE_TABSTRIP)) {
     // If this is a tabbed browser, just create a duplicate tab inside the same
     // window next to the tab being duplicated.
-    const int index =
-        browser->tab_strip_model()->GetIndexOfWebContents(contents);
-    pinned = browser->tab_strip_model()->IsTabPinned(index);
+    TabStripModel* tab_strip_model = browser->tab_strip_model();
+    const int index = tab_strip_model->GetIndexOfWebContents(contents);
+    pinned = tab_strip_model->IsTabPinned(index);
     int add_types = TabStripModel::ADD_ACTIVE |
                     TabStripModel::ADD_INHERIT_OPENER |
                     (pinned ? TabStripModel::ADD_PINNED : 0);
-    const auto old_group = browser->tab_strip_model()->GetTabGroupForTab(index);
-    browser->tab_strip_model()->InsertWebContentsAt(
-        index + 1, std::move(contents_dupe), add_types, old_group);
+    const auto old_group = tab_strip_model->GetTabGroupForTab(index);
+    tab_strip_model->InsertWebContentsAt(index + 1, std::move(contents_dupe),
+                                         add_types, old_group);
   } else {
-    Browser* new_browser = NULL;
+    Browser* new_browser = nullptr;
     if (browser->deprecated_is_app()) {
       new_browser = new Browser(Browser::CreateParams::CreateForApp(
           browser->app_name(), browser->is_trusted_source(), gfx::Rect(),
@@ -911,7 +911,7 @@ void BookmarkCurrentTabAllowingExtensionOverrides(Browser* browser) {
   DCHECK(!chrome::ShouldRemoveBookmarkThisTabUI(browser->profile()));
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-  const extensions::Extension* extension = NULL;
+  const extensions::Extension* extension = nullptr;
   extensions::Command command;
   if (GetBookmarkOverrideCommand(browser->profile(), &extension, &command)) {
     switch (command.type()) {
@@ -1303,7 +1303,7 @@ void ToggleDistilledView(Browser* browser) {
 
 bool CanRequestTabletSite(WebContents* current_tab) {
   return current_tab &&
-         current_tab->GetController().GetLastCommittedEntry() != NULL;
+         current_tab->GetController().GetLastCommittedEntry() != nullptr;
 }
 
 bool IsRequestingTabletSite(Browser* browser) {

chrome/browser/ui/browser_live_tab_context.cc

@@ -129,6 +129,7 @@ sessions::LiveTab* BrowserLiveTabContext::AddRestoredTab(
     int selected_navigation,
     const std::string& extension_app_id,
     base::Optional<base::Token> group,
+    const TabGroupMetadata* group_metadata,
     bool select,
     bool pin,
     bool from_last_session,
@@ -141,11 +142,29 @@ sessions::LiveTab* BrowserLiveTabContext::AddRestoredTab(
                 ->session_storage_namespace()
           : nullptr;
 
+  TabGroupModel* group_model = browser_->tab_strip_model()->group_model();
+  const base::Optional<TabGroupId> group_id =
+      group.has_value()
+          ? base::Optional<TabGroupId>{TabGroupId::FromRawToken(group.value())}
+          : base::nullopt;
+  const bool first_tab_in_group =
+      group.has_value() ? !group_model->ContainsTabGroup(group_id.value())
+                        : false;
+
   WebContents* web_contents = chrome::AddRestoredTab(
       browser_, navigations, tab_index, selected_navigation, extension_app_id,
       group, select, pin, from_last_session, base::TimeTicks(),
       storage_namespace, user_agent_override, false /* from_session_restore */);
 
+  // Only update the metadata if the group doesn't already exist since the
+  // existing group has the latest metadata, which may have changed from the
+  // time the tab was closed.
+  if (first_tab_in_group) {
+    TabGroupVisualData visual_data(group_metadata->title,
+                                   group_metadata->color);
+    group_model->GetTabGroup(group_id.value())->SetVisualData(visual_data);
+  }
+
 #if BUILDFLAG(ENABLE_SESSION_SERVICE)
   // The focused tab will be loaded by Browser, and TabLoader will load the
   // rest.

chrome/browser/ui/browser_live_tab_context.h

@@ -52,6 +52,7 @@ class BrowserLiveTabContext : public sessions::LiveTabContext {
       int selected_navigation,
       const std::string& extension_app_id,
       base::Optional<base::Token> group,
+      const TabGroupMetadata* group_metadata,
       bool select,
       bool pin,
       bool from_last_session,

components/sessions/core/live_tab_context.h

@@ -63,6 +63,7 @@ class SESSIONS_EXPORT LiveTabContext {
       int selected_navigation,
       const std::string& extension_app_id,
       base::Optional<base::Token> group,
+      const TabGroupMetadata* group_metadata,
       bool select,
       bool pin,
       bool from_last_session,

components/sessions/core/tab_restore_service.h

@@ -121,6 +121,9 @@ class SESSIONS_EXPORT TabRestoreService : public KeyedService {
 
     // The group the tab belonged to, if any.
     base::Optional<base::Token> group;
+
+    // The group metadata for the tab, if any.
+    base::Optional<TabGroupMetadata> group_metadata;
   };
 
   // Represents a previously open window.

components/sessions/core/tab_restore_service_helper.cc

@@ -318,6 +318,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
           LiveTab* restored_tab = context->AddRestoredTab(
               tab.navigations, context->GetTabCount(),
               tab.current_navigation_index, tab.extension_app_id, tab.group,
+              base::OptionalOrNullptr(tab.group_metadata),
               static_cast<int>(tab_i) == window.selected_tab_index, tab.pinned,
               tab.from_last_session, tab.platform_data.get(),
               tab.user_agent_override);
@@ -555,6 +556,11 @@ void TabRestoreServiceHelper::PopulateTab(Tab* tab,
     tab->browser_id = context->GetSessionID().id();
     tab->pinned = context->IsTabPinned(tab->tabstrip_index);
     tab->group = context->GetTabGroupForTab(tab->tabstrip_index);
+    tab->group_metadata =
+        tab->group.has_value()
+            ? base::Optional<TabGroupMetadata>{context->GetTabGroupMetadata(
+                  tab->group.value())}
+            : base::nullopt;
   }
 }
 
@@ -599,7 +605,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
 
     restored_tab = context->AddRestoredTab(
         tab.navigations, tab_index, tab.current_navigation_index,
-        tab.extension_app_id, base::nullopt,
+        tab.extension_app_id, tab.group,
+        base::OptionalOrNullptr(tab.group_metadata),
         disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB, tab.pinned,
         tab.from_last_session, tab.platform_data.get(),
         tab.user_agent_override);

components/sessions/core/tab_restore_service_impl.cc

@@ -105,6 +105,7 @@ const SessionCommand::id_type kCommandSetExtensionAppID = 6;
 const SessionCommand::id_type kCommandSetWindowAppName = 7;
 const SessionCommand::id_type kCommandSetTabUserAgentOverride = 8;
 const SessionCommand::id_type kCommandWindow = 9;
+const SessionCommand::id_type kCommandGroup = 10;
 
 // Number of entries (not commands) before we clobber the file and write
 // everything.
@@ -695,6 +696,17 @@ void TabRestoreServiceImpl::PersistenceDelegate::ScheduleCommandsForTab(
     base_session_service_->ScheduleCommand(std::move(command));
   }
 
+  if (tab.group.has_value()) {
+    base::Pickle pickle;
+    WriteTokenToPickle(&pickle, tab.group.value());
+    const TabGroupMetadata* metadata = &tab.group_metadata.value();
+    pickle.WriteString16(metadata->title);
+    pickle.WriteUInt32(metadata->color);
+    std::unique_ptr<SessionCommand> command(
+        new SessionCommand(kCommandGroup, pickle));
+    base_session_service_->ScheduleCommand(std::move(command));
+  }
+
   if (!tab.extension_app_id.empty()) {
     base_session_service_->ScheduleCommand(CreateSetTabExtensionAppIDCommand(
         kCommandSetExtensionAppID, tab.id, tab.extension_app_id));
@@ -942,6 +954,27 @@ void TabRestoreServiceImpl::PersistenceDelegate::CreateEntriesFromCommands(
         break;
       }
 
+      case kCommandGroup: {
+        if (!current_tab) {
+          // Should be in a tab when we get this.
+          return;
+        }
+        std::unique_ptr<base::Pickle> pickle(command.PayloadAsPickle());
+        base::PickleIterator iter(*pickle);
+        base::Optional<base::Token> group_id = ReadTokenFromPickle(&iter);
+        base::string16 title;
+        SkColor color;
+        if (!iter.ReadString16(&title)) {
+          break;
+        }
+        if (!iter.ReadUInt32(&color)) {
+          break;
+        }
+        current_tab->group = group_id.value();
+        current_tab->group_metadata = TabGroupMetadata{title, color};
+        break;
+      }
+
       case kCommandSetWindowAppName: {
         if (!current_window) {
           // We should have created a window already.

ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.h

@@ -50,6 +50,7 @@ class TabRestoreServiceDelegateImplIOS : public sessions::LiveTabContext,
       int selected_navigation,
       const std::string& extension_app_id,
       base::Optional<base::Token> group,
+      const TabGroupMetadata* group_metadata,
       bool select,
       bool pin,
       bool from_last_session,

ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.mm

@@ -106,6 +106,7 @@ sessions::LiveTab* TabRestoreServiceDelegateImplIOS::AddRestoredTab(
     int selected_navigation,
     const std::string& extension_app_id,
     base::Optional<base::Token> group,
+    const TabGroupMetadata* group_metadata,
     bool select,
     bool pin,
     bool from_last_session,