Introducing a synced pref for tabgroup sync
This pref allows a device to check whether tabgroup sync is enabled
on the profile. And thus allows it to sync tabgroups from other devices.
This is currently only used by Android, but will be used later by other platforms
Bug: 336134530
Change-Id: Idbb31449b5340455430192a391e89af5e62a2338
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5497984
Reviewed-by: Tommy Nyquist <[email protected]>
Reviewed-by: Rushan Suleymanov <[email protected]>
Commit-Queue: Min Qin <[email protected]>
Reviewed-by: Dominic Battre <[email protected]>
Reviewed-by: Victor Vianna <[email protected]>
Reviewed-by: Shakti Sahu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1296077}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
index 462be34..9c4559d 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java
@@ -962,7 +962,7 @@
@CallSuper
protected void initProfileDependentFeatures(Profile currentlySelectedProfile) {
Profile originalProfile = currentlySelectedProfile.getOriginalProfile();
- if (TabGroupSyncFeatures.isTabGroupSyncEnabled()) {
+ if (TabGroupSyncFeatures.isTabGroupSyncEnabled(currentlySelectedProfile)) {
mTabGroupSyncController =
new TabGroupSyncController(
mTabModelSelectorSupplier.get(),
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 67e5c4e..4c8ad4e4 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -10996,7 +10996,7 @@
{"android-tab-group-stable-ids",
flag_descriptions::kAndroidTabGroupStableIdsName,
flag_descriptions::kAndroidTabGroupStableIdsDescription, kOsAndroid,
- FEATURE_VALUE_TYPE(chrome::android::kAndroidTabGroupStableIds)},
+ FEATURE_VALUE_TYPE(tab_groups::kAndroidTabGroupStableIds)},
{"tab-group-sync-android", flag_descriptions::kTabGroupSyncAndroidName,
flag_descriptions::kTabGroupSyncAndroidDescription, kOsAndroid,
diff --git a/chrome/browser/flags/android/chrome_feature_list.cc b/chrome/browser/flags/android/chrome_feature_list.cc
index a628117..cd942b5 100644
--- a/chrome/browser/flags/android/chrome_feature_list.cc
+++ b/chrome/browser/flags/android/chrome_feature_list.cc
@@ -160,7 +160,6 @@
&kAndroidNoVisibleHintForDifferentTLD,
&kAndroidTabDeclutter,
&kAndroidTabDeclutterRescueKillswitch,
- &kAndroidTabGroupStableIds,
&kAndroidToolbarScrollAblation,
&kAnimatedImageDragShadow,
&kAppSpecificHistory,
@@ -352,6 +351,7 @@
&syncer::kSyncAndroidLimitNTPPromoImpressions,
&syncer::kSyncEnableContactInfoDataTypeInTransportMode,
&syncer::kWebApkBackupAndRestoreBackend,
+ &tab_groups::kAndroidTabGroupStableIds,
&tab_groups::kTabGroupSyncAndroid,
&webapps::features::kPwaUniversalInstallUi,
&webapps::features::kWebApkInstallFailureNotification,
@@ -432,10 +432,6 @@
"AndroidTabDeclutterRescueKillswitch",
base::FEATURE_ENABLED_BY_DEFAULT);
-BASE_FEATURE(kAndroidTabGroupStableIds,
- "AndroidTabGroupStableIds",
- base::FEATURE_DISABLED_BY_DEFAULT);
-
BASE_FEATURE(kAndroidToolbarScrollAblation,
"AndroidToolbarScrollAblation",
base::FEATURE_DISABLED_BY_DEFAULT);
diff --git a/chrome/browser/flags/android/chrome_feature_list.h b/chrome/browser/flags/android/chrome_feature_list.h
index 63a469d..614e17a0 100644
--- a/chrome/browser/flags/android/chrome_feature_list.h
+++ b/chrome/browser/flags/android/chrome_feature_list.h
@@ -28,7 +28,6 @@
BASE_DECLARE_FEATURE(kAndroidNoVisibleHintForDifferentTLD);
BASE_DECLARE_FEATURE(kAndroidTabDeclutter);
BASE_DECLARE_FEATURE(kAndroidTabDeclutterRescueKillswitch);
-BASE_DECLARE_FEATURE(kAndroidTabGroupStableIds);
BASE_DECLARE_FEATURE(kAndroidToolbarScrollAblation);
BASE_DECLARE_FEATURE(kAnimatedImageDragShadow);
BASE_DECLARE_FEATURE(kAppSpecificHistory);
diff --git a/chrome/browser/preferences/BUILD.gn b/chrome/browser/preferences/BUILD.gn
index 7a57e4f..6c99524a5 100644
--- a/chrome/browser/preferences/BUILD.gn
+++ b/chrome/browser/preferences/BUILD.gn
@@ -53,6 +53,7 @@
"//components/privacy_sandbox/privacy_sandbox_prefs.h",
"//components/privacy_sandbox/tracking_protection_prefs.h",
"//components/safe_browsing/core/common/safe_browsing_prefs.h",
+ "//components/saved_tab_groups/pref_names.h",
"//components/signin/public/base/signin_pref_names.cc",
"//components/supervised_user/core/common/pref_names.h",
"//components/translate/core/browser/translate_pref_names.h",
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index f27fa495..e0fd02f3b 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -159,6 +159,7 @@
#include "components/proxy_config/pref_proxy_config_tracker_impl.h"
#include "components/safe_browsing/content/common/file_type_policies_prefs.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
+#include "components/saved_tab_groups/pref_names.h"
#include "components/search_engines/template_url_prepopulate_data.h"
#include "components/security_interstitials/content/insecure_form_blocking_page.h"
#include "components/security_interstitials/content/stateful_ssl_host_state_delegate.h"
@@ -259,7 +260,6 @@
#include "chrome/browser/readaloud/android/prefs.h"
#include "chrome/browser/safety_hub/android/prefs.h"
#include "chrome/browser/ssl/known_interception_disclosure_infobar_delegate.h"
-#include "chrome/browser/tab_group_sync/prefs.h"
#include "components/cdm/browser/media_drm_storage_impl.h" // nogncheck crbug.com/1125897
#include "components/ntp_snippets/register_prefs.h"
#include "components/ntp_tiles/popular_sites_impl.h"
@@ -1915,6 +1915,7 @@
syncer::SyncPrefs::RegisterProfilePrefs(registry);
syncer::SyncTransportDataPrefs::RegisterProfilePrefs(registry);
TemplateURLPrepopulateData::RegisterProfilePrefs(registry);
+ tab_groups::prefs::RegisterProfilePrefs(registry);
tpcd::experiment::RegisterProfilePrefs(registry);
translate::TranslatePrefs::RegisterProfilePrefs(registry);
omnibox::RegisterProfilePrefs(registry);
@@ -1982,7 +1983,6 @@
readaloud::RegisterProfilePrefs(registry);
RecentTabsPagePrefs::RegisterProfilePrefs(registry);
safety_hub_prefs::RegisterSafetyHubAndroidProfilePrefs(registry);
- tab_groups::RegisterProfilePrefs(registry);
usage_stats::UsageStatsBridge::RegisterProfilePrefs(registry);
variations::VariationsService::RegisterProfilePrefs(registry);
webapps::InstallPromptPrefs::RegisterProfilePrefs(registry);
diff --git a/chrome/browser/sync/chrome_sync_client.cc b/chrome/browser/sync/chrome_sync_client.cc
index 720b366..7840507 100644
--- a/chrome/browser/sync/chrome_sync_client.cc
+++ b/chrome/browser/sync/chrome_sync_client.cc
@@ -470,7 +470,7 @@
BUILDFLAG(IS_WIN)
enable_tab_group_sync = true;
#elif BUILDFLAG(IS_ANDROID)
- enable_tab_group_sync = tab_groups::IsTabGroupSyncEnabled();
+ enable_tab_group_sync = tab_groups::IsTabGroupSyncEnabled(GetPrefService());
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) ||
// BUILDFLAG(IS_WIN)
@@ -661,7 +661,7 @@
CHECK(keyed_service);
return keyed_service->GetSavedTabGroupControllerDelegate();
#elif BUILDFLAG(IS_ANDROID)
- DCHECK(tab_groups::IsTabGroupSyncEnabled());
+ DCHECK(tab_groups::IsTabGroupSyncEnabled(GetPrefService()));
return tab_groups::TabGroupSyncServiceFactory::GetForProfile(profile_)
->GetSavedTabGroupControllerDelegate();
#else
diff --git a/chrome/browser/tab_group_sync/BUILD.gn b/chrome/browser/tab_group_sync/BUILD.gn
index 5b421ed..7bc328f 100644
--- a/chrome/browser/tab_group_sync/BUILD.gn
+++ b/chrome/browser/tab_group_sync/BUILD.gn
@@ -13,12 +13,11 @@
sources = [
"feature_utils.cc",
"feature_utils.h",
- "prefs.cc",
- "prefs.h",
]
deps = [
"//base:base",
+ "//chrome/browser/profiles",
"//chrome/common:constants",
"//components/pref_registry",
"//components/prefs",
@@ -51,6 +50,7 @@
}
android_library("java") {
+ srcjar_deps = [ ":utils_jni_headers" ]
sources = [
"android/java/src/org/chromium/chrome/browser/tab_group_sync/LocalTabGroupMutationHelper.java",
"android/java/src/org/chromium/chrome/browser/tab_group_sync/LogUtils.java",
@@ -72,6 +72,7 @@
"//build/android:build_java",
"//chrome/browser/flags:java",
"//chrome/browser/preferences:java",
+ "//chrome/browser/profiles/android:java",
"//chrome/browser/tab:java",
"//chrome/browser/tab_group:java",
"//chrome/browser/tabmodel:java",
diff --git a/chrome/browser/tab_group_sync/android/java/src/org/chromium/chrome/browser/tab_group_sync/TabGroupSyncFeatures.java b/chrome/browser/tab_group_sync/android/java/src/org/chromium/chrome/browser/tab_group_sync/TabGroupSyncFeatures.java
index 9949612..8f4a000 100644
--- a/chrome/browser/tab_group_sync/android/java/src/org/chromium/chrome/browser/tab_group_sync/TabGroupSyncFeatures.java
+++ b/chrome/browser/tab_group_sync/android/java/src/org/chromium/chrome/browser/tab_group_sync/TabGroupSyncFeatures.java
@@ -4,17 +4,22 @@
package org.chromium.chrome.browser.tab_group_sync;
-import org.jni_zero.CalledByNative;
+import org.jni_zero.JNINamespace;
+import org.jni_zero.JniType;
+import org.jni_zero.NativeMethods;
-import org.chromium.chrome.browser.flags.ChromeFeatureList;
+import org.chromium.chrome.browser.profiles.Profile;
/** Utility class for defining features and params related to tab group sync. */
+@JNINamespace("tab_groups")
public final class TabGroupSyncFeatures {
-
/** Whether tab group sync is enabled. */
- @CalledByNative
- public static boolean isTabGroupSyncEnabled() {
- return ChromeFeatureList.sAndroidTabGroupStableIds.isEnabled()
- && ChromeFeatureList.isEnabled(ChromeFeatureList.TAB_GROUP_SYNC_ANDROID);
+ public static boolean isTabGroupSyncEnabled(Profile profile) {
+ return TabGroupSyncFeaturesJni.get().isTabGroupSyncEnabled(profile);
+ }
+
+ @NativeMethods
+ interface Natives {
+ boolean isTabGroupSyncEnabled(@JniType("Profile*") Profile profile);
}
}
diff --git a/chrome/browser/tab_group_sync/feature_utils.cc b/chrome/browser/tab_group_sync/feature_utils.cc
index a8ab92c7..23351de 100644
--- a/chrome/browser/tab_group_sync/feature_utils.cc
+++ b/chrome/browser/tab_group_sync/feature_utils.cc
@@ -5,22 +5,59 @@
#include "chrome/browser/tab_group_sync/feature_utils.h"
#include "build/build_config.h"
-#include "components/saved_tab_groups/features.h"
+#include "components/prefs/pref_service.h"
#if BUILDFLAG(IS_ANDROID)
#include "base/android/jni_android.h"
+#include "base/android/scoped_java_ref.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/tab_group_sync/utils_jni_headers/TabGroupSyncFeatures_jni.h"
-#endif
+#include "components/saved_tab_groups/features.h"
+#include "components/saved_tab_groups/pref_names.h"
+#endif // BUILDFLAG(IS_ANDROID)
namespace tab_groups {
-bool IsTabGroupSyncEnabled() {
#if BUILDFLAG(IS_ANDROID)
- JNIEnv* env = base::android::AttachCurrentThread();
- return Java_TabGroupSyncFeatures_isTabGroupSyncEnabled(env);
+// static
+jboolean JNI_TabGroupSyncFeatures_IsTabGroupSyncEnabled(JNIEnv* env,
+ Profile* profile) {
+ DCHECK(profile);
+ return IsTabGroupSyncEnabled(profile->GetPrefs());
+}
+#endif // BUILDFLAG(IS_ANDROID)
+
+bool IsTabGroupSyncEnabled(PrefService* pref_service) {
+#if BUILDFLAG(IS_ANDROID)
+ if (!base::FeatureList::IsEnabled(tab_groups::kAndroidTabGroupStableIds)) {
+ return false;
+ }
+
+ // Enabled the feature if both of the following condition is true:
+ // 1. If kTabGroupSyncAndroid is enabled, or kSyncableTabGroups is on.
+ // 2. And kTabGroupSyncForceOff is disabled.
+ // kTabGroupSyncForceOff will turn off the feature on the current device, so
+ // tab groups will not be synced.
+ if (base::FeatureList::IsEnabled(tab_groups::kTabGroupSyncForceOff)) {
+ return false;
+ }
+
+ if (base::FeatureList::IsEnabled(tab_groups::kTabGroupSyncAndroid)) {
+ // The user is in an experiment group that enables the feature, push
+ // kSyncableTabGroups preference to other devices so that the feature can
+ // work on those devices too for the same user..
+ pref_service->SetBoolean(tab_groups::prefs::kSyncableTabGroups, true);
+ return true;
+ }
+
+ // If kSyncableTabGroups is true, the feature is enabled for the user on
+ // another device through experiments. Enable the feature on the current
+ // device too.
+ return pref_service->GetBoolean(tab_groups::prefs::kSyncableTabGroups);
#else
return false;
-#endif
+#endif // BUILDFLAG(IS_ANDROID)
}
} // namespace tab_groups
diff --git a/chrome/browser/tab_group_sync/feature_utils.h b/chrome/browser/tab_group_sync/feature_utils.h
index bd615d6..50946528 100644
--- a/chrome/browser/tab_group_sync/feature_utils.h
+++ b/chrome/browser/tab_group_sync/feature_utils.h
@@ -5,9 +5,10 @@
#ifndef CHROME_BROWSER_TAB_GROUP_SYNC_FEATURE_UTILS_H_
#define CHROME_BROWSER_TAB_GROUP_SYNC_FEATURE_UTILS_H_
+class PrefService;
namespace tab_groups {
-bool IsTabGroupSyncEnabled();
+bool IsTabGroupSyncEnabled(PrefService* pref_service);
} // namespace tab_groups
diff --git a/chrome/browser/tab_group_sync/prefs.cc b/chrome/browser/tab_group_sync/prefs.cc
deleted file mode 100644
index eb6e7a6..0000000
--- a/chrome/browser/tab_group_sync/prefs.cc
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright 2024 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/tab_group_sync/prefs.h"
-
-#include "base/feature_list.h"
-#include "base/metrics/field_trial_params.h"
-#include "chrome/common/pref_names.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/saved_tab_groups/features.h"
-
-namespace tab_groups {
-
-void RegisterProfilePrefs(PrefRegistrySimple* registry) {
- if (base::FeatureList::IsEnabled(tab_groups::kTabGroupSyncAndroid)) {
- registry->RegisterBooleanPref(prefs::kAutoOpenSyncedTabGroups,
- base::GetFieldTrialParamByFeatureAsBool(
- tab_groups::kTabGroupSyncAndroid,
- "auto_open_synced_tab_groups", true));
- registry->RegisterBooleanPref(
- prefs::kStopShowingTabGroupConfirmationOnClose, false);
- registry->RegisterBooleanPref(
- prefs::kStopShowingTabGroupConfirmationOnUngroup, false);
- registry->RegisterBooleanPref(
- prefs::kStopShowingTabGroupConfirmationOnTabRemove, false);
- registry->RegisterBooleanPref(
- prefs::kStopShowingTabGroupConfirmationOnTabClose, false);
- }
-}
-
-} // namespace tab_groups
diff --git a/chrome/browser/tab_group_sync/prefs.h b/chrome/browser/tab_group_sync/prefs.h
deleted file mode 100644
index 2b33227..0000000
--- a/chrome/browser/tab_group_sync/prefs.h
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright 2024 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_BROWSER_TAB_GROUP_SYNC_PREFS_H_
-#define CHROME_BROWSER_TAB_GROUP_SYNC_PREFS_H_
-
-class PrefRegistrySimple;
-
-namespace tab_groups {
-
-// Register prefs for tab group sync. Presently only used on Android.
-void RegisterProfilePrefs(PrefRegistrySimple* registry);
-
-} // namespace tab_groups
-
-#endif // CHROME_BROWSER_TAB_GROUP_SYNC_PREFS_H_
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index ffd9e7b..04350fe 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -4132,19 +4132,6 @@
inline constexpr char kTabGroupSavesUIUpdateMigrated[] =
"tab_group_saves_ui_update_migrated";
-#if BUILDFLAG(IS_ANDROID)
-inline constexpr char kAutoOpenSyncedTabGroups[] =
- "auto_open_synced_tab_groups";
-inline constexpr char kStopShowingTabGroupConfirmationOnClose[] =
- "stop_showing_tab_group_confirmation_on_close";
-inline constexpr char kStopShowingTabGroupConfirmationOnUngroup[] =
- "stop_showing_tab_group_confirmation_on_ungroup";
-inline constexpr char kStopShowingTabGroupConfirmationOnTabRemove[] =
- "stop_showing_tab_group_confirmation_on_tab_remove";
-inline constexpr char kStopShowingTabGroupConfirmationOnTabClose[] =
- "stop_showing_tab_group_confirmation_on_tab_close";
-#endif // BUILDFLAG(IS_ANDROID)
-
} // namespace prefs
#endif // CHROME_COMMON_PREF_NAMES_H_
