0

[SharedTabGroups] add new fieldtrialconfig for experiment.

Change-Id: Ied438a12c2c84fe093440a5ce21a08c66a16f943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6444214
Commit-Queue: David Pennington <dpenning@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Auto-Submit: David Pennington <dpenning@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1446592}
This commit is contained in:
David Pennington
2025-04-14 09:46:39 -07:00
committed by Chromium LUCI CQ
parent 47c66c045a
commit b460695179
21 changed files with 126 additions and 47 deletions

@ -17,7 +17,6 @@
#include "chrome/browser/tab_group_sync/tab_group_sync_service_factory.h"
#include "components/collaboration/internal/messaging/configuration.h"
#include "components/collaboration/internal/messaging/data_sharing_change_notifier_impl.h"
#include "components/collaboration/internal/messaging/empty_messaging_backend_service.h"
#include "components/collaboration/internal/messaging/instant_message_processor_impl.h"
#include "components/collaboration/internal/messaging/messaging_backend_service_impl.h"
#include "components/collaboration/internal/messaging/storage/empty_messaging_backend_database.h"
@ -25,6 +24,7 @@
#include "components/collaboration/internal/messaging/storage/messaging_backend_store_impl.h"
#include "components/collaboration/internal/messaging/tab_group_change_notifier_impl.h"
#include "components/collaboration/public/features.h"
#include "components/collaboration/public/messaging/empty_messaging_backend_service.h"
#include "components/data_sharing/public/features.h"
#include "components/saved_tab_groups/public/features.h"
#include "components/saved_tab_groups/public/tab_group_sync_service.h"

@ -34,6 +34,7 @@
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/buildflags.h"
#include "chrome/browser/collaboration/messaging/messaging_backend_service_factory.h"
#include "chrome/browser/defaults.h"
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/browser/prefs/session_startup_pref.h"
@ -92,6 +93,7 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/collaboration/public/messaging/empty_messaging_backend_service.h"
#include "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h"
#include "components/memory_pressure/fake_memory_pressure_monitor.h"
@ -188,7 +190,13 @@ void WaitForTabsToLoad(Browser* browser) {
class SessionRestoreTest : public InProcessBrowserTest {
public:
SessionRestoreTest() = default;
SessionRestoreTest() {
dependency_manager_subscription_ =
BrowserContextDependencyManager::GetInstance()
->RegisterCreateServicesCallbackForTesting(
base::BindRepeating(&SessionRestoreTest::RegisterFakeServices,
base::Unretained(this)));
}
~SessionRestoreTest() override = default;
protected:
@ -199,6 +207,16 @@ class SessionRestoreTest : public InProcessBrowserTest {
}
#endif
void RegisterFakeServices(content::BrowserContext* context) {
collaboration::messaging::MessagingBackendServiceFactory::GetInstance()
->SetTestingFactory(
context, base::BindRepeating([](content::BrowserContext* context)
-> std::unique_ptr<KeyedService> {
return std::make_unique<
collaboration::messaging::EmptyMessagingBackendService>();
}));
}
void SetUpOnMainThread() override {
active_browser_list_ = BrowserList::GetInstance();
@ -377,6 +395,8 @@ class SessionRestoreTest : public InProcessBrowserTest {
memory_pressure::test::FakeMemoryPressureMonitor
fake_memory_pressure_monitor_;
base::CallbackListSubscription dependency_manager_subscription_;
};
// Activates the smart restore behaviour.

@ -128,8 +128,12 @@ IN_PROC_BROWSER_TEST_F(SingleClientCommonSyncTest,
// TODO(crbug.com/40264154): remove once GetUpdates is not issued anymore.
GetUpdatesObserver::GetUpdatesOriginSet get_updates_origins_to_exclude{
SyncEnums::PROGRAMMATIC};
DataTypeSet types_to_exclude{DataType::ARC_PACKAGE, DataType::HISTORY,
DataType::CONTACT_INFO, DataType::NIGORI};
DataTypeSet types_to_exclude{
DataType::ARC_PACKAGE, DataType::HISTORY, DataType::CONTACT_INFO,
DataType::NIGORI,
// TODO(crbug.com/410116020): Remove once these types pass this test.
DataType::SHARED_TAB_GROUP_DATA, DataType::SHARED_TAB_GROUP_ACCOUNT_DATA,
DataType::COLLABORATION_GROUP};
// Verify that there were no unexpected GetUpdates requests during Sync
// initialization.

@ -11,6 +11,7 @@
#include "base/test/run_until.h"
#include "base/test/scoped_feature_list.h"
#include "base/uuid.h"
#include "chrome/browser/collaboration/messaging/messaging_backend_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/data_type_store_service_factory.h"
#include "chrome/browser/sync/device_info_sync_service_factory.h"
@ -27,6 +28,8 @@
#include "chrome/browser/ui/views/bookmarks/saved_tab_groups/saved_tab_group_bar.h"
#include "chrome/common/channel_info.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/collaboration/public/messaging/empty_messaging_backend_service.h"
#include "components/collaboration/public/messaging/messaging_backend_service.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_service.h"
@ -63,6 +66,22 @@ class TabGroupSyncDelegateBrowserTest : public InProcessBrowserTest,
public:
TabGroupSyncDelegateBrowserTest() {
features_.InitWithFeatures({kTabGroupSyncServiceDesktopMigration}, {});
dependency_manager_subscription_ =
BrowserContextDependencyManager::GetInstance()
->RegisterCreateServicesCallbackForTesting(base::BindRepeating(
&TabGroupSyncDelegateBrowserTest::RegisterFakeServices,
base::Unretained(this)));
}
void RegisterFakeServices(content::BrowserContext* context) {
collaboration::messaging::MessagingBackendServiceFactory::GetInstance()
->SetTestingFactory(
context, base::BindRepeating([](content::BrowserContext* context)
-> std::unique_ptr<KeyedService> {
return std::make_unique<
collaboration::messaging::EmptyMessagingBackendService>();
}));
}
void OnWillBeDestroyed() override {
@ -150,7 +169,6 @@ class TabGroupSyncDelegateBrowserTest : public InProcessBrowserTest,
std::make_unique<TabGroupSyncDelegateDesktop>(service.get(), profile);
service->SetTabGroupSyncDelegate(std::move(delegate));
service->SetIsInitializedForTesting(true);
service_ = service.get();
return std::move(service);
}
@ -161,6 +179,7 @@ class TabGroupSyncDelegateBrowserTest : public InProcessBrowserTest,
raw_ptr<TabGroupSyncService> service_;
base::OnceClosure quit_;
bool callback_received_ = false;
base::CallbackListSubscription dependency_manager_subscription_;
};
IN_PROC_BROWSER_TEST_F(TabGroupSyncDelegateBrowserTest,
@ -695,11 +714,17 @@ IN_PROC_BROWSER_TEST_F(TabGroupSyncDelegateBrowserTest,
EXPECT_EQ(local_tab_group->visual_data()->title(), u"Title");
EXPECT_EQ(local_tab_group->visual_data()->color(), TabGroupColorId::kBlue);
EXPECT_EQ(local_tab_group->ListTabs().length(), 2u);
// iterate through all of the tabs activating them so that deferred
// navigations are resolved.
const gfx::Range tab_range = local_tab_group->ListTabs();
for (size_t i = tab_range.start(); i < tab_range.end(); ++i) {
browser()->tab_strip_model()->ActivateTabAt(i);
}
// Verify that a new tab was added, and the existing one navigated to the
// correct URL.
const gfx::Range tab_range = local_tab_group->ListTabs();
ASSERT_EQ(tab_range.length(), 2u);
EXPECT_EQ(browser()
->tab_strip_model()
->GetWebContentsAt(tab_range.start())
@ -748,8 +773,14 @@ IN_PROC_BROWSER_TEST_F(TabGroupSyncDelegateBrowserTest,
EXPECT_EQ(local_tab_group->visual_data()->title(), u"Title");
EXPECT_EQ(local_tab_group->visual_data()->color(), TabGroupColorId::kBlue);
// Verify that only one tab remains and it's navigated to the correct URL.
// iterate through all of the tabs activating them so that deferred
// navigations are resolved.
const gfx::Range tab_range = local_tab_group->ListTabs();
for (size_t i = tab_range.start(); i < tab_range.end(); ++i) {
browser()->tab_strip_model()->ActivateTabAt(i);
}
// Verify that only one tab remains and it's navigated to the correct URL.
ASSERT_EQ(tab_range.length(), 1u);
EXPECT_EQ(browser()
->tab_strip_model()

@ -128,7 +128,6 @@ class TabGroupSyncNavigationIntegrationTest : public InProcessBrowserTest {
static_cast<TabGroupSyncServiceImpl*>(service());
SavedTabGroupModel* model = service_impl->GetModelForTesting();
model->AddObserver(&sync_bridge_model_observer_);
service_impl->SetIsInitializedForTesting(true);
}
content::WebContents* AddTabToBrowser(int index) {

@ -98,12 +98,6 @@ IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest, MoveToNewWindow) {
}
IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest, AddToExistingGroupSubmenu) {
// Prevents flakes by ensuring the TabGroupSyncService is initialized before
// creating any tab groups.
tab_groups::TabGroupSyncService* service =
tab_groups::SavedTabGroupUtils::GetServiceForProfile(profile());
service->SetIsInitializedForTesting(true);
chrome::NewTab(browser());
chrome::NewTab(browser());
chrome::NewTab(browser());
@ -138,12 +132,6 @@ IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest, AddToExistingGroupSubmenu) {
IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest,
AddToExistingGroupSubmenu_DoesNotIncludeCurrentGroup) {
// Prevents flakes by ensuring the TabGroupSyncService is initialized before
// creating any tab groups.
tab_groups::TabGroupSyncService* service =
tab_groups::SavedTabGroupUtils::GetServiceForProfile(profile());
service->SetIsInitializedForTesting(true);
chrome::NewTab(browser());
chrome::NewTab(browser());
chrome::NewTab(browser());
@ -181,12 +169,6 @@ IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest,
// Regression test for crbug.com/1197875
IN_PROC_BROWSER_TEST_F(TabMenuModelBrowserTest,
AddToExistingGroupAfterGroupDestroyed) {
// Prevents flakes by ensuring the TabGroupSyncService is initialized before
// creating any tab groups.
tab_groups::TabGroupSyncService* service =
tab_groups::SavedTabGroupUtils::GetServiceForProfile(profile());
service->SetIsInitializedForTesting(true);
chrome::NewTab(browser());
chrome::NewTab(browser());

@ -25,6 +25,7 @@
#include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/data_sharing/public/features.h"
#include "components/saved_tab_groups/internal/saved_tab_group_model.h"
#include "components/saved_tab_groups/internal/tab_group_sync_service_impl.h"
#include "components/saved_tab_groups/public/features.h"
@ -45,10 +46,14 @@ class SavedTabGroupBarBrowserTest : public InProcessBrowserTest,
SavedTabGroupBarBrowserTest() {
if (GetParam()) {
features_.InitWithFeatures(
{tab_groups::kTabGroupSyncServiceDesktopMigration}, {});
{tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature},
{data_sharing::features::kDataSharingJoinOnly});
} else {
features_.InitWithFeatures(
{}, {tab_groups::kTabGroupSyncServiceDesktopMigration});
{}, {tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature,
data_sharing::features::kDataSharingJoinOnly});
}
}
@ -257,7 +262,6 @@ IN_PROC_BROWSER_TEST_P(SavedTabGroupBarBrowserTest,
SavedTabGroupUtils::GetServiceForProfile(browser()->profile());
TabGroupSyncServiceImpl* service_impl =
static_cast<TabGroupSyncServiceImpl*>(service);
service_impl->SetIsInitializedForTesting(true);
SavedTabGroupModel* model = service_impl->GetModelForTesting();
{ // Create 1 pinned group
@ -337,7 +341,7 @@ IN_PROC_BROWSER_TEST_P(SavedTabGroupBarBrowserTest,
SavedTabGroupUtils::GetServiceForProfile(browser()->profile());
TabGroupSyncServiceImpl* service_impl =
static_cast<TabGroupSyncServiceImpl*>(service);
service_impl->SetIsInitializedForTesting(true);
SavedTabGroupModel* model = service_impl->GetModelForTesting();
{ // Create an empty group.
ScopedAddObservation observer(service);
@ -345,7 +349,6 @@ IN_PROC_BROWSER_TEST_P(SavedTabGroupBarBrowserTest,
SavedTabGroup group{
u"group_title", TabGroupColorId::kGrey, {}, 0, group_guid};
SavedTabGroupModel* model = service_impl->GetModelForTesting();
model->AddedFromSync(std::move(group));
observer.Wait();

@ -40,6 +40,7 @@
#include "chrome/test/interaction/tracked_element_webcontents.h"
#include "chrome/test/interaction/webcontents_interaction_test_util.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/data_sharing/public/features.h"
#include "components/favicon/content/content_favicon_driver.h"
#include "components/favicon/core/favicon_driver.h"
#include "components/favicon/core/favicon_driver_observer.h"
@ -207,10 +208,14 @@ class SavedTabGroupInteractiveTest
void SetUp() override {
if (IsMigrationEnabled()) {
scoped_feature_list_.InitWithFeatures(
{tab_groups::kTabGroupSyncServiceDesktopMigration}, {});
{tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature},
{data_sharing::features::kDataSharingJoinOnly});
} else {
scoped_feature_list_.InitWithFeatures(
{}, {tab_groups::kTabGroupSyncServiceDesktopMigration});
{}, {tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature,
data_sharing::features::kDataSharingJoinOnly});
}
SavedTabGroupInteractiveTestBase::SetUp();

@ -10,6 +10,7 @@
#include "chrome/test/user_education/interactive_feature_promo_test.h"
#include "chrome/test/user_education/interactive_feature_promo_test_common.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/data_sharing/public/features.h"
#include "components/feature_engagement/public/feature_list.h"
#include "components/prefs/pref_service.h"
#include "components/saved_tab_groups/public/features.h"
@ -26,7 +27,14 @@ class SavedTabGroupV2PromoTest : public InteractiveFeaturePromoTest,
{feature_engagement::kIPHTabGroupsSaveV2CloseGroupFeature})) {
if (GetParam()) {
feature_list_.InitWithFeatures(
{{tab_groups::kTabGroupSyncServiceDesktopMigration}}, {});
{tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature},
{data_sharing::features::kDataSharingJoinOnly});
} else {
feature_list_.InitWithFeatures(
{}, {tab_groups::kTabGroupSyncServiceDesktopMigration,
data_sharing::features::kDataSharingFeature,
data_sharing::features::kDataSharingJoinOnly});
}
}
@ -39,7 +47,6 @@ class SavedTabGroupV2PromoTest : public InteractiveFeaturePromoTest,
tab_groups::SavedTabGroupUtils::GetServiceForProfile(
browser()->profile());
ASSERT_TRUE(service);
service->SetIsInitializedForTesting(true);
chrome::AddTabAt(browser(), GURL(), 0, true);
chrome::AddTabAt(browser(), GURL(), 1, true);

@ -21,6 +21,7 @@
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/data_sharing/public/features.h"
#include "components/saved_tab_groups/public/features.h"
#include "components/tab_groups/tab_group_id.h"
#include "content/public/test/browser_test.h"
@ -30,6 +31,13 @@
#include "ui/views/test/button_test_api.h"
class TabGroupEditorBubbleViewDialogBrowserTest : public DialogBrowserTest {
public:
TabGroupEditorBubbleViewDialogBrowserTest() {
scoped_feature_list_.InitWithFeatures(
{}, {data_sharing::features::kDataSharingFeature,
data_sharing::features::kDataSharingJoinOnly});
}
protected:
void ShowUi(const std::string& name) override {
group_ = browser()->tab_strip_model()->AddToNewGroup({0});
@ -53,6 +61,7 @@ class TabGroupEditorBubbleViewDialogBrowserTest : public DialogBrowserTest {
}
std::optional<tab_groups::TabGroupId> group_;
base::test::ScopedFeatureList scoped_feature_list_;
};
IN_PROC_BROWSER_TEST_F(TabGroupEditorBubbleViewDialogBrowserTest,

@ -341,6 +341,7 @@ std::string ToolbarController::GetActionNameFromElementIdentifier(
{kActionTaskManager, "PinnedTaskManagerButton"},
{kActionSidePanelShowLensOverlayResults,
"PinnedShowLensOverlayResultsSidePanelButton"},
{kActionSendSharedTabGroupFeedback, "SharedTabGroupFeedbackButton"},
});
const auto it = identifier_to_action_name_map->find(identifier);

@ -2353,6 +2353,7 @@ if (!is_android) {
"//components/cbor",
"//components/certificate_transparency",
"//components/certificate_transparency:proto",
"//components/collaboration/public:empty_messaging_backend_service",
"//components/commerce/content/browser",
"//components/commerce/content/renderer",
"//components/commerce/core:cart_db_content_proto",

@ -86,8 +86,6 @@ static_library("messaging_internal") {
"messaging/data_sharing_change_notifier.h",
"messaging/data_sharing_change_notifier_impl.cc",
"messaging/data_sharing_change_notifier_impl.h",
"messaging/empty_messaging_backend_service.cc",
"messaging/empty_messaging_backend_service.h",
"messaging/instant_message_processor.h",
"messaging/instant_message_processor_impl.cc",
"messaging/instant_message_processor_impl.h",
@ -113,6 +111,7 @@ static_library("messaging_internal") {
deps = [
"//base",
"//components/collaboration/internal/messaging/storage/protocol",
"//components/collaboration/public:empty_messaging_backend_service",
"//components/collaboration/public:messaging_public",
"//components/data_sharing/public",
"//components/saved_tab_groups/public",

@ -11,6 +11,7 @@ if (is_android) {
group("public") {
public_deps = [
":core_public",
":empty_messaging_backend_service",
":messaging_public",
]
}
@ -217,3 +218,12 @@ if (is_android) {
]
}
}
static_library("empty_messaging_backend_service") {
visibility = [ "*" ]
sources = [
"messaging/empty_messaging_backend_service.cc",
"messaging/empty_messaging_backend_service.h",
]
deps = [ ":messaging_public" ]
}

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/collaboration/internal/messaging/empty_messaging_backend_service.h"
#include "components/collaboration/public/messaging/empty_messaging_backend_service.h"
#include <optional>
#include <vector>

@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_COLLABORATION_INTERNAL_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_
#define COMPONENTS_COLLABORATION_INTERNAL_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_
#ifndef COMPONENTS_COLLABORATION_PUBLIC_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_
#define COMPONENTS_COLLABORATION_PUBLIC_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_
#include <memory>
#include <vector>
@ -49,4 +49,4 @@ class EmptyMessagingBackendService : public MessagingBackendService {
} // namespace collaboration::messaging
#endif // COMPONENTS_COLLABORATION_INTERNAL_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_
#endif // COMPONENTS_COLLABORATION_PUBLIC_MESSAGING_EMPTY_MESSAGING_BACKEND_SERVICE_H_

@ -413,7 +413,9 @@ class TabGroupSyncService : public KeyedService, public base::SupportsUserData {
// For testing only. This is needed to test the API calls received before
// service init as we need to explicitly un-initialize the service for these
// scenarios.
// scenarios. When calling this method the MessagingBackendService will need
// to be faked or have its store callbacks set first. (see
// EmptyMessagingBackendService)
virtual void SetIsInitializedForTesting(bool initialized) {}
// For testing only. This is needed to test shared tab groups flow without

@ -6,7 +6,7 @@ include_rules = [
# Allows service construction.
"+components/collaboration/internal/messaging/configuration.h",
"+components/collaboration/internal/messaging/data_sharing_change_notifier_impl.h",
"+components/collaboration/internal/messaging/empty_messaging_backend_service.h",
"+components/collaboration/public/messaging/empty_messaging_backend_service.h",
"+components/collaboration/internal/messaging/messaging_backend_service_impl.h",
"+components/collaboration/internal/messaging/storage/messaging_backend_store_impl.h",
"+components/collaboration/internal/messaging/tab_group_change_notifier_impl.h",

@ -8,7 +8,6 @@
#import "components/collaboration/internal/messaging/configuration.h"
#import "components/collaboration/internal/messaging/data_sharing_change_notifier_impl.h"
#import "components/collaboration/internal/messaging/empty_messaging_backend_service.h"
#import "components/collaboration/internal/messaging/instant_message_processor_impl.h"
#import "components/collaboration/internal/messaging/messaging_backend_service_impl.h"
#import "components/collaboration/internal/messaging/storage/empty_messaging_backend_database.h"
@ -16,6 +15,7 @@
#import "components/collaboration/internal/messaging/storage/messaging_backend_store_impl.h"
#import "components/collaboration/internal/messaging/tab_group_change_notifier_impl.h"
#import "components/collaboration/public/features.h"
#import "components/collaboration/public/messaging/empty_messaging_backend_service.h"
#import "components/data_sharing/public/features.h"
#import "ios/chrome/browser/collaboration/model/features.h"
#import "ios/chrome/browser/collaboration/model/messaging/instant_messaging_service.h"

@ -22874,10 +22874,14 @@
]
}
],
"SharedTabGroupsTeamfood": [
"SharedTabGroups": [
{
"platforms": [
"android"
"android",
"chromeos",
"linux",
"mac",
"windows"
],
"experiments": [
{

@ -49302,6 +49302,8 @@ should be able to be added at any place in this file.
<suffix name="PinnedShowTranslateButton"
label="Pinned show translate button"/>
<suffix name="PinnedTaskManagerButton" label="Pinned task manager button"/>
<suffix name="SharedTabGroupFeedbackButton"
label="Feedback button for the shared tab groups feature"/>
<suffix name="ShowPerformanceSidePanelButton"
label="Show performance side panel button"/>
<suffix name="SidePanelButton" label="Side Panel button"/>