[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. Fixed: 389107698 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-Commit-Position: refs/heads/main@{#1407984}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
779c154167
commit
40d45a6c13
@ -194,6 +194,7 @@ source_set("unit_tests") {
|
||||
"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) {
|
||||
|
@ -563,6 +563,31 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTabOrGroupFromWindow(
|
||||
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 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
|
||||
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 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
|
||||
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.
|
||||
|
@ -87,6 +87,19 @@ class SESSIONS_EXPORT TabRestoreServiceHelper
|
||||
|
||||
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);
|
||||
|
@ -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
|
Reference in New Issue
Block a user