[TabGroups] Fix crash when restoring grouped tab in app like browsers

The crash happens when a grouped tab is restored into a browser that
does not support tab groups. In this scenario, we will touch multiple
different kinds of CHECKS / nullptr dereferences which will crash the
browser.

Changes:
- CHECK TabGroupModel, TabStripModel, and TabGroup have values before
use in BrowserLiveTabContext::SetVisualDataForGroup and
BrowserLiveTabContext::GetVisualDataForGroup
- Restore the grouped tab as an ungrouped tab if there is no
TabGroupModel for the browser
- Adds a regression test for this specific scenario

Change-Id: I0230b7a7c86c31c77740488ce8959de77bd20865
Bug: 368139715
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5888488
Commit-Queue: Darryl James <dljames@chromium.org>
Reviewed-by: Eshwar Stalin <estalin@chromium.org>
Reviewed-by: Steven Luong <stluong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1360585}

chrome/browser/sessions/tab_restore_browsertest.cc

@@ -20,6 +20,8 @@
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/keep_alive/profile_keep_alive_types.h"
 #include "chrome/browser/profiles/keep_alive/scoped_profile_keep_alive.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/chrome_tab_restore_service_client.h"
 #include "chrome/browser/sessions/session_restore_test_helper.h"
 #include "chrome/browser/sessions/tab_loader_tester.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
@@ -52,7 +54,9 @@
 #include "components/saved_tab_groups/features.h"
 #include "components/saved_tab_groups/saved_tab_group_tab.h"
 #include "components/saved_tab_groups/types.h"
+#include "components/sessions/content/content_test_helper.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"
 #include "components/sessions/core/tab_restore_types.h"
 #include "components/tab_groups/tab_group_color.h"
@@ -77,6 +81,7 @@
 #include "chrome/browser/sessions/tab_loader.h"
 #endif  // BUILDFLAG(ENABLE_SESSION_SERVICE)
 
+namespace sessions {
 class TabRestoreTest : public InProcessBrowserTest {
  public:
   TabRestoreTest()
@@ -629,6 +634,44 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreGroup) {
             gfx::Range(2, 4));
 }
 
+// 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,
+                       RestoreGroupInBrowserThatDoesNotSupportGroups) {
+  // Create a browser that does not support groups and try to restore a
+  // grouped tab. This should restore the tab and not recreate the group.
+  Browser::CreateParams app_browser_params =
+      Browser::CreateParams::CreateForApp("App Name", true, gfx::Rect(),
+                                          browser()->profile(), false);
+  Browser* app_browser = Browser::Create(app_browser_params);
+  EXPECT_FALSE(app_browser->tab_strip_model()->group_model());
+
+  // Create a tab entry with a group and add it to TabRestoreService directly.
+  auto service = std::make_unique<sessions::TabRestoreServiceImpl>(
+      std::make_unique<ChromeTabRestoreServiceClient>(app_browser->profile()),
+      app_browser->profile()->GetPrefs(), nullptr);
+
+  tab_groups::TabGroupId group_id = tab_groups::TabGroupId::GenerateNew();
+  std::unique_ptr<sessions::tab_restore::Tab> tab =
+      std::make_unique<sessions::tab_restore::Tab>();
+  tab->current_navigation_index = 0;
+  tab->group = group_id;
+  tab->navigations.push_back(
+      sessions::ContentTestHelper::CreateNavigation("https://1.com", "1"));
+  tab->group_visual_data = tab_groups::TabGroupVisualData(
+      u"Group Title", tab_groups::TabGroupColorId::kBlue);
+
+  service->mutable_entries()->push_front(std::move(tab));
+
+  EXPECT_EQ(1u, service->entries().size());
+  EXPECT_EQ(0, app_browser->tab_strip_model()->count());
+
+  service->RestoreMostRecentEntry(app_browser->live_tab_context());
+
+  EXPECT_EQ(0u, service->entries().size());
+  EXPECT_EQ(1, app_browser->tab_strip_model()->count());
+}
+
 // Close a grouped tab, then the entire group. Restore both. The group should be
 // opened at the end of the tabstrip.
 IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreGroupedTabThenGroup) {
@@ -3091,3 +3134,4 @@ IN_PROC_BROWSER_TEST_F(TabRestoreSycnedServiceTest, RestoreCollapsedGroupTab) {
   EXPECT_EQ(data->color(), visual_data.color());
   EXPECT_TRUE(data->is_collapsed());
 }
+}  // namespace sessions

chrome/browser/ui/browser_live_tab_context.cc

@@ -154,10 +154,13 @@ std::optional<tab_groups::TabGroupId> BrowserLiveTabContext::GetTabGroupForTab(
 const tab_groups::TabGroupVisualData*
 BrowserLiveTabContext::GetVisualDataForGroup(
     const tab_groups::TabGroupId& group) const {
-  return browser_->tab_strip_model()
-      ->group_model()
-      ->GetTabGroup(group)
-      ->visual_data();
+  TabStripModel* tab_strip_model = browser_->tab_strip_model();
+  CHECK(tab_strip_model);
+  TabGroupModel* group_model = tab_strip_model->group_model();
+  CHECK(group_model);
+  TabGroup* tab_group = group_model->GetTabGroup(group);
+  CHECK(tab_group);
+  return tab_group->visual_data();
 }
 
 const std::optional<base::Uuid>
@@ -186,8 +189,13 @@ bool BrowserLiveTabContext::IsTabPinned(int index) const {
 void BrowserLiveTabContext::SetVisualDataForGroup(
     const tab_groups::TabGroupId& group,
     const tab_groups::TabGroupVisualData& visual_data) {
-  browser_->tab_strip_model()->group_model()->GetTabGroup(group)->SetVisualData(
-      std::move(visual_data));
+  TabStripModel* tab_strip_model = browser_->tab_strip_model();
+  CHECK(tab_strip_model);
+  TabGroupModel* group_model = tab_strip_model->group_model();
+  CHECK(group_model);
+  TabGroup* tab_group = group_model->GetTabGroup(group);
+  CHECK(tab_group);
+  tab_group->SetVisualData(std::move(visual_data));
 }
 
 const gfx::Rect BrowserLiveTabContext::GetRestoredBounds() const {
@@ -218,7 +226,11 @@ sessions::LiveTab* BrowserLiveTabContext::AddRestoredTab(
                 ->session_storage_namespace()
           : nullptr;
 
-  std::optional<tab_groups::TabGroupId> group_id = tab.group;
+  // If the browser does not support tabs groups, restore the grouped tab as a
+  // normal tab instead. See crbug.com/368139715.
+  std::optional<tab_groups::TabGroupId> group_id =
+      browser_->tab_strip_model()->SupportsTabGroups() ? tab.group
+                                                       : std::nullopt;
   std::optional<base::Uuid> saved_group_id = tab.saved_group_id;
   content::WebContents* web_contents = nullptr;
 

components/sessions/core/tab_restore_service_impl.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/sessions/core/sessions_export.h"
 #include "components/sessions/core/tab_restore_service.h"
@@ -65,6 +66,8 @@ class SESSIONS_EXPORT TabRestoreServiceImpl : public TabRestoreService {
 
  private:
   friend class ::TabRestoreServiceImplTest;
+  FRIEND_TEST_ALL_PREFIXES(TabRestoreTest,
+                           RestoreGroupInBrowserThatDoesNotSupportGroups);
 
   class PersistenceDelegate;
   void UpdatePersistenceDelegate();