[M132][STG] Fix data duplications for shared tab groups on crash/power down.
When a window is created via TabRestore as the first tab restore entry,
the tabs in the window (and group) are not populated with saved
information if the browser had crashed or powered down in an unclean
state. This causes new groups to be created, even though they should be
mapped to saved groups.
This Cl adds methods that assign the correct saved information to the
tabs so they dont duplicate groups.
(cherry picked from commit 40d45a6c1379b3a8c157e7ceb13ba29544946bcd)
Fixed: 391655073
Change-Id: Ic6f387c533263def3102e35d013abf8c903feeee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6177194
Reviewed-by: Eshwar Stalin <estalin@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Commit-Queue: David Pennington <dpenning@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1407984}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191744
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Daniel Yip <danielyip@google.com>
Cr-Commit-Position: refs/branch-heads/6834@{#4394}
Cr-Branched-From: 47a3549fac11ee8cb7be6606001ede605b302b9f-refs/heads/main@{#1381561}
diff --git a/components/sessions/BUILD.gn b/components/sessions/BUILD.gn
index 6db298c..11c3af7 100644
--- a/components/sessions/BUILD.gn
+++ b/components/sessions/BUILD.gn
@@ -195,6 +195,7 @@
"core/command_storage_manager_unittest.cc",
"core/serialized_navigation_entry_unittest.cc",
"core/session_id_generator_unittest.cc",
+ "core/tab_restore_service_helper_unittest.cc",
]
if (!is_ios) {
diff --git a/components/sessions/core/tab_restore_service_helper.cc b/components/sessions/core/tab_restore_service_helper.cc
index b8cccdf..d8ee2cc 100644
--- a/components/sessions/core/tab_restore_service_helper.cc
+++ b/components/sessions/core/tab_restore_service_helper.cc
@@ -563,6 +563,31 @@
return context;
}
+void TabRestoreServiceHelper::UpdateSavedGroupIDsForTabEntries(
+ std::vector<std::unique_ptr<tab_restore::Tab>>& tabs,
+ const std::map<tab_groups::TabGroupId, base::Uuid>& group_mapping) {
+ for (auto& tab : tabs) {
+ if (tab->group.has_value() && group_mapping.contains(tab->group.value())) {
+ tab->saved_group_id = group_mapping.at(tab->group.value());
+ }
+ }
+}
+
+std::map<tab_groups::TabGroupId, base::Uuid>
+TabRestoreServiceHelper::CreateLocalSavedGroupIDMapping(
+ const std::map<tab_groups::TabGroupId, std::unique_ptr<tab_restore::Group>>&
+ groups) {
+ std::map<tab_groups::TabGroupId, base::Uuid> group_mapping;
+
+ for (const auto& [group_id, group] : groups) {
+ if (group->saved_group_id.has_value()) {
+ group_mapping[group_id] = group->saved_group_id.value();
+ }
+ }
+
+ return group_mapping;
+}
+
std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
LiveTabContext* context,
SessionID id,
@@ -617,6 +642,12 @@
TimeNow() - window.timestamp);
}
+ // In cases where restoring from a tab restore entry that was created
+ // through session service, or in other cases, the tab objects saved group
+ // information may be missing. Add it in before repopulating tabs.
+ UpdateSavedGroupIDsForTabEntries(
+ window.tabs, CreateLocalSavedGroupIDMapping(window.tab_groups));
+
// When restoring a window, either the entire window can be restored, or a
// single tab within it. If the entry's ID matches the one to restore, or
// the entry corresponds to an application, then the entire window will be
@@ -683,6 +714,14 @@
TimeNow() - group.timestamp);
}
+ // In cases where restoring from a tab restore entry that was created
+ // through session service, or in other cases, the tab objects saved group
+ // information may be missing. Add it in before repopulating tabs.
+ if (group.saved_group_id.has_value()) {
+ UpdateSavedGroupIDsForTabEntries(
+ group.tabs, {{group.group_id, group.saved_group_id.value()}});
+ }
+
// When restoring a group, either the entire group can be restored, or a
// single tab within it. If the entry's ID matches the one to restore,
// then the entire group will be restored.
diff --git a/components/sessions/core/tab_restore_service_helper.h b/components/sessions/core/tab_restore_service_helper.h
index 39becd91..84942348 100644
--- a/components/sessions/core/tab_restore_service_helper.h
+++ b/components/sessions/core/tab_restore_service_helper.h
@@ -87,6 +87,19 @@
void SetHelperObserver(Observer* observer);
+ // Creates a mapping of local to saved ids for groups that have a
+ // saved_group_id. Take in a window's group storage type, and outputs a type
+ // usable for UpdateSavedGroupIDsForTabEntries.
+ static std::map<tab_groups::TabGroupId, base::Uuid>
+ CreateLocalSavedGroupIDMapping(
+ const std::map<tab_groups::TabGroupId, std::unique_ptr<Group>>& groups);
+
+ // Updates the saved group IDs for tabs based on the provided group mapping.
+ // Used by RestoreEntryByID.
+ static void UpdateSavedGroupIDsForTabEntries(
+ std::vector<std::unique_ptr<tab_restore::Tab>>& tabs,
+ const std::map<tab_groups::TabGroupId, base::Uuid>& group_mapping);
+
// Helper methods used to implement TabRestoreService.
void AddObserver(TabRestoreServiceObserver* observer);
void RemoveObserver(TabRestoreServiceObserver* observer);
diff --git a/components/sessions/core/tab_restore_service_helper_unittest.cc b/components/sessions/core/tab_restore_service_helper_unittest.cc
new file mode 100644
index 0000000..356ff38
--- /dev/null
+++ b/components/sessions/core/tab_restore_service_helper_unittest.cc
@@ -0,0 +1,99 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/sessions/core/tab_restore_service_helper.h"
+
+#include <optional>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sessions {
+
+std::unique_ptr<tab_restore::Group> CreateTabRestoreGroup(
+ tab_groups::TabGroupId group_id,
+ const std::optional<base::Uuid>& saved_group_id) {
+ auto group = std::make_unique<tab_restore::Group>();
+ group->group_id = group_id;
+
+ group->saved_group_id = saved_group_id;
+ return group;
+}
+
+std::unique_ptr<tab_restore::Tab> CreateTabWithGroupInfo(
+ tab_groups::TabGroupId group_id,
+ const std::optional<base::Uuid>& saved_group_id) {
+ auto tab = std::make_unique<tab_restore::Tab>();
+ tab->group = group_id;
+ tab->saved_group_id = saved_group_id;
+ return tab;
+}
+
+class TabRestoreServiceHelperTest : public testing::Test {};
+
+TEST_F(TabRestoreServiceHelperTest, CreateLocalSavedGroupIDMapping) {
+ std::map<tab_groups::TabGroupId, std::unique_ptr<tab_restore::Group>> groups;
+
+ tab_groups::TabGroupId local_group_id_1 =
+ tab_groups::TabGroupId::GenerateNew();
+ tab_groups::TabGroupId local_group_id_2 =
+ tab_groups::TabGroupId::GenerateNew();
+
+ base::Uuid saved_uuid_1 = base::Uuid::GenerateRandomV4();
+
+ groups[local_group_id_1] =
+ CreateTabRestoreGroup(local_group_id_1, saved_uuid_1);
+ groups[local_group_id_2] =
+ CreateTabRestoreGroup(local_group_id_2, std::nullopt);
+
+ std::map<tab_groups::TabGroupId, base::Uuid> result =
+ TabRestoreServiceHelper::CreateLocalSavedGroupIDMapping(groups);
+
+ // Verify that only groups with saved_group_id appear in the mapping.
+ ASSERT_EQ(result.size(), 1u);
+ EXPECT_EQ(result[local_group_id_1], saved_uuid_1);
+}
+
+TEST_F(TabRestoreServiceHelperTest, UpdateSavedGroupIDsForTabEntries) {
+ tab_groups::TabGroupId local_group_id_1 =
+ tab_groups::TabGroupId::GenerateNew();
+ tab_groups::TabGroupId local_group_id_2 =
+ tab_groups::TabGroupId::GenerateNew();
+ tab_groups::TabGroupId local_group_id_3 =
+ tab_groups::TabGroupId::GenerateNew();
+
+ base::Uuid saved_guid_1 = base::Uuid::GenerateRandomV4();
+ base::Uuid saved_guid_2 = base::Uuid::GenerateRandomV4();
+
+ std::vector<std::unique_ptr<tab_restore::Tab>> tabs;
+ tabs.push_back(CreateTabWithGroupInfo(local_group_id_1, std::nullopt));
+ tabs.push_back(CreateTabWithGroupInfo(local_group_id_2, std::nullopt));
+ tabs.push_back(CreateTabWithGroupInfo(local_group_id_3, std::nullopt));
+ tabs.push_back(std::make_unique<tab_restore::Tab>());
+
+ std::map<tab_groups::TabGroupId, std::unique_ptr<tab_restore::Group>> groups;
+ groups[local_group_id_1] =
+ CreateTabRestoreGroup(local_group_id_1, saved_guid_1);
+ groups[local_group_id_2] =
+ CreateTabRestoreGroup(local_group_id_2, saved_guid_2);
+ groups[local_group_id_3] =
+ CreateTabRestoreGroup(local_group_id_3, std::nullopt);
+
+ TabRestoreServiceHelper::UpdateSavedGroupIDsForTabEntries(
+ tabs, TabRestoreServiceHelper::CreateLocalSavedGroupIDMapping(groups));
+
+ // Verify that only tabs with a group in the mapping are updated.
+ ASSERT_TRUE(tabs[0]->saved_group_id.has_value());
+ EXPECT_EQ(tabs[0]->saved_group_id.value(), saved_guid_1);
+
+ ASSERT_TRUE(tabs[1]->saved_group_id.has_value());
+ EXPECT_EQ(tabs[1]->saved_group_id.value(), saved_guid_2);
+
+ // 3rd tab's group isn't in the mapping, so Tab 2 remains unchanged.
+ EXPECT_FALSE(tabs[2]->saved_group_id.has_value());
+
+ // The last tab has no group, so it should also remain nullopt.
+ EXPECT_FALSE(tabs[3]->saved_group_id.has_value());
+}
+
+} // namespace sessions
