[sync] Decouple priority prefs from user toggle #2
This CL is part of the changes to decouple priority preferences from
prefs sync user toggle. The priority prefs will still only be synced
with the user toggle on, except for the allowlisted prefs which will
continue to sync even with the toggle off. This feature is dependent on
EnablePreferencesAccountStorage launch.
This CL adds support in the SyncablePrefsDatabase to define an allowlist of syncable prefs which always sync, irrespective of the preferences sync user toggle. Note that the allowlist is not currently used and will be put to use in follow-up CLs.
Bug: 412602018
Change-Id: Ib3698fc05b212310f6b0555105d96450655ab38d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6491986
Auto-Submit: Ankush Singh <ankushkush@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Rushan Suleymanov <rushans@google.com>
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/main@{#1451926}
diff --git a/chrome/browser/sync/prefs/chrome_syncable_prefs_database.cc b/chrome/browser/sync/prefs/chrome_syncable_prefs_database.cc
index 9aaf71f..04968cb 100644
--- a/chrome/browser/sync/prefs/chrome_syncable_prefs_database.cc
+++ b/chrome/browser/sync/prefs/chrome_syncable_prefs_database.cc
@@ -1709,4 +1709,9 @@
std::inserter(syncable_prefs, syncable_prefs.end()));
return syncable_prefs;
}
+
+bool ChromeSyncablePrefsDatabase::IsPreferenceAlwaysSyncing(
+ std::string_view pref_name) const {
+ return common_syncable_prefs_database_.IsPreferenceAlwaysSyncing(pref_name);
+}
} // namespace browser_sync
diff --git a/chrome/browser/sync/prefs/chrome_syncable_prefs_database.h b/chrome/browser/sync/prefs/chrome_syncable_prefs_database.h
index 3ae6e999..8d172d6 100644
--- a/chrome/browser/sync/prefs/chrome_syncable_prefs_database.h
+++ b/chrome/browser/sync/prefs/chrome_syncable_prefs_database.h
@@ -24,6 +24,8 @@
std::map<std::string_view, sync_preferences::SyncablePrefMetadata>
GetAllSyncablePrefsForTest() const;
+ bool IsPreferenceAlwaysSyncing(std::string_view pref_name) const override;
+
private:
// This defines the list of preferences that are syncable across all
// platforms.
diff --git a/chrome/browser/sync/prefs/chrome_syncable_prefs_database_unittest.cc b/chrome/browser/sync/prefs/chrome_syncable_prefs_database_unittest.cc
index 66e13b1..89d754b 100644
--- a/chrome/browser/sync/prefs/chrome_syncable_prefs_database_unittest.cc
+++ b/chrome/browser/sync/prefs/chrome_syncable_prefs_database_unittest.cc
@@ -7,6 +7,8 @@
#include <string_view>
#include "base/test/metrics/histogram_enum_reader.h"
+#include "base/test/scoped_feature_list.h"
+#include "components/sync/base/features.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -30,4 +32,23 @@
}
}
+TEST(ChromeSyncablePrefsDatabaseTest, IsPreferenceAlwaysSyncing) {
+ base::test::ScopedFeatureList scoped_feature_list(
+ syncer::kSyncSupportAlwaysSyncingPriorityPreferences);
+ browser_sync::ChromeSyncablePrefsDatabase db;
+ EXPECT_TRUE(db.IsPreferenceAlwaysSyncing(
+ sync_preferences::kSyncableAlwaysSyncingPriorityPrefForTesting));
+ EXPECT_FALSE(db.IsPreferenceAlwaysSyncing(
+ sync_preferences::kSyncablePriorityPrefForTesting));
+
+ // Currently, only priority preferences are allowed in the allowlist.
+ const std::map<std::string_view, sync_preferences::SyncablePrefMetadata>
+ syncable_prefs = db.GetAllSyncablePrefsForTest();
+ for (const auto& [pref_name, metadata] : syncable_prefs) {
+ if (db.IsPreferenceAlwaysSyncing(pref_name)) {
+ EXPECT_EQ(metadata.data_type(), syncer::PRIORITY_PREFERENCES);
+ }
+ }
+}
+
} // namespace
diff --git a/components/sync_preferences/common_syncable_prefs_database.cc b/components/sync_preferences/common_syncable_prefs_database.cc
index c7aae364..09626e0 100644
--- a/components/sync_preferences/common_syncable_prefs_database.cc
+++ b/components/sync_preferences/common_syncable_prefs_database.cc
@@ -7,6 +7,8 @@
#include <string_view>
#include "base/containers/fixed_flat_map.h"
+#include "base/containers/fixed_flat_set.h"
+#include "base/feature_list.h"
#include "build/build_config.h"
#include "components/autofill/core/common/autofill_prefs.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
@@ -27,6 +29,7 @@
#include "components/search_engines/search_engines_pref_names.h"
#include "components/sharing_message/pref_names.h"
#include "components/sync/base/data_type.h"
+#include "components/sync/base/features.h"
#include "components/translate/core/browser/translate_pref_names.h"
#include "components/translate/core/browser/translate_prefs.h"
#include "components/variations/service/google_groups_manager_prefs.h"
@@ -128,6 +131,8 @@
kAutofillBnplEnabled = 81,
kAutofillHasSeenBnpl = 82,
kAutomaticPasskeyUpgrades = 83,
+ kSyncablePriorityPrefForTesting = 84, // For tests.
+ kSyncableAlwaysSyncingPriorityPrefForTesting = 85, // For tests.
// See components/sync_preferences/README.md about adding new entries here.
// vvvvv IMPORTANT! vvvvv
// Note to the reviewer: IT IS YOUR RESPONSIBILITY to ensure that new syncable
@@ -343,6 +348,14 @@
PrefSensitivity::kNone, MergeBehavior::kNone}},
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) ||
// BUILDFLAG(IS_CHROMEOS)
+ {kSyncablePriorityPrefForTesting,
+ {syncable_prefs_ids::kSyncablePriorityPrefForTesting,
+ syncer::PRIORITY_PREFERENCES, PrefSensitivity::kNone,
+ MergeBehavior::kNone}},
+ {kSyncableAlwaysSyncingPriorityPrefForTesting,
+ {syncable_prefs_ids::kSyncableAlwaysSyncingPriorityPrefForTesting,
+ syncer::PRIORITY_PREFERENCES, PrefSensitivity::kNone,
+ MergeBehavior::kNone}},
});
} // namespace
@@ -363,4 +376,18 @@
kCommonSyncablePrefsAllowlist.end()};
}
+bool CommonSyncablePrefsDatabase::IsPreferenceAlwaysSyncing(
+ std::string_view pref_name) const {
+ CHECK(base::FeatureList::IsEnabled(
+ syncer::kSyncSupportAlwaysSyncingPriorityPreferences));
+ // TODO(crbug.com/412602018): Consider renaming and using the
+ // `PrefSensitivity` enum instead, since it currently helps with something
+ // similar in regards to history opt-in and the options would be mutually
+ // exclusive.
+ static constexpr auto kAlwaysSyncingPrefsAllowlist =
+ base::MakeFixedFlatSet<std::string_view>(
+ {kSyncableAlwaysSyncingPriorityPrefForTesting});
+ return kAlwaysSyncingPrefsAllowlist.contains(pref_name);
+}
+
} // namespace sync_preferences
diff --git a/components/sync_preferences/common_syncable_prefs_database.h b/components/sync_preferences/common_syncable_prefs_database.h
index d5a03bb..2c3520b 100644
--- a/components/sync_preferences/common_syncable_prefs_database.h
+++ b/components/sync_preferences/common_syncable_prefs_database.h
@@ -20,6 +20,10 @@
"syncable-mergeable-list-test-preference";
inline constexpr char kSyncableHistorySensitiveListPrefForTesting[] =
"syncable-history-sensitive-list-test-preference";
+inline constexpr char kSyncablePriorityPrefForTesting[] =
+ "syncable-test-priority-preference";
+inline constexpr char kSyncableAlwaysSyncingPriorityPrefForTesting[] =
+ "syncable-always-syncing-test-priority-preference";
// This class provides an implementation for SyncablePrefsDatabase for common
// syncable preferences, i.e. preferences which are shared between all
@@ -33,6 +37,8 @@
std::map<std::string_view, SyncablePrefMetadata> GetAllSyncablePrefsForTest()
const;
+
+ bool IsPreferenceAlwaysSyncing(std::string_view pref_name) const override;
};
} // namespace sync_preferences
diff --git a/components/sync_preferences/syncable_prefs_database.h b/components/sync_preferences/syncable_prefs_database.h
index f8dfbbac..b66bb35 100644
--- a/components/sync_preferences/syncable_prefs_database.h
+++ b/components/sync_preferences/syncable_prefs_database.h
@@ -112,6 +112,10 @@
// Return true if `pref_name` is a mergeable syncable preference.
// Note: `pref_name` must be syncable.
bool IsPreferenceMergeable(std::string_view pref_name) const;
+
+ // Returns whether `pref_name` is part of the allowlist of preferences that
+ // are always synced, irrespective of the preference sync user toggle.
+ virtual bool IsPreferenceAlwaysSyncing(std::string_view pref_name) const = 0;
};
} // namespace sync_preferences
diff --git a/components/sync_preferences/test_syncable_prefs_database.cc b/components/sync_preferences/test_syncable_prefs_database.cc
index 0354a8b..c738ad46 100644
--- a/components/sync_preferences/test_syncable_prefs_database.cc
+++ b/components/sync_preferences/test_syncable_prefs_database.cc
@@ -24,4 +24,9 @@
return std::nullopt;
}
+bool TestSyncablePrefsDatabase::IsPreferenceAlwaysSyncing(
+ std::string_view pref_name) const {
+ return false;
+}
+
} // namespace sync_preferences
diff --git a/components/sync_preferences/test_syncable_prefs_database.h b/components/sync_preferences/test_syncable_prefs_database.h
index c2c76b5b..6c63f47 100644
--- a/components/sync_preferences/test_syncable_prefs_database.h
+++ b/components/sync_preferences/test_syncable_prefs_database.h
@@ -25,6 +25,8 @@
std::optional<sync_preferences::SyncablePrefMetadata> GetSyncablePrefMetadata(
std::string_view pref_name) const override;
+ bool IsPreferenceAlwaysSyncing(std::string_view pref_name) const override;
+
private:
PrefsMap syncable_prefs_map_;
};
diff --git a/ios/chrome/browser/sync/model/prefs/BUILD.gn b/ios/chrome/browser/sync/model/prefs/BUILD.gn
index c6978b3b..ca3cafe 100644
--- a/ios/chrome/browser/sync/model/prefs/BUILD.gn
+++ b/ios/chrome/browser/sync/model/prefs/BUILD.gn
@@ -46,6 +46,7 @@
":prefs",
":test_data",
"//base/test:test_support",
+ "//components/sync/base:features",
"//testing/gtest",
]
}
diff --git a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.cc b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.cc
index 12e84d5..cd938bd 100644
--- a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.cc
+++ b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.cc
@@ -116,4 +116,9 @@
std::inserter(syncable_prefs, syncable_prefs.end()));
return syncable_prefs;
}
+
+bool IOSChromeSyncablePrefsDatabase::IsPreferenceAlwaysSyncing(
+ std::string_view pref_name) const {
+ return common_syncable_prefs_database_.IsPreferenceAlwaysSyncing(pref_name);
+}
} // namespace browser_sync
diff --git a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.h b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.h
index b75dfc0..84363e46 100644
--- a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.h
+++ b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database.h
@@ -24,6 +24,8 @@
std::map<std::string_view, sync_preferences::SyncablePrefMetadata>
GetAllSyncablePrefsForTest() const;
+ bool IsPreferenceAlwaysSyncing(std::string_view pref_name) const override;
+
private:
// This defines the list of preferences that are syncable across all
// platforms.
diff --git a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database_unittest.cc b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database_unittest.cc
index f7635057..1827a93 100644
--- a/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database_unittest.cc
+++ b/ios/chrome/browser/sync/model/prefs/ios_chrome_syncable_prefs_database_unittest.cc
@@ -7,6 +7,8 @@
#include <string_view>
#include "base/test/metrics/histogram_enum_reader.h"
+#include "base/test/scoped_feature_list.h"
+#include "components/sync/base/features.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -30,4 +32,23 @@
}
}
+TEST(IOSChromeSyncablePrefsDatabaseTest, IsPreferenceAlwaysSyncing) {
+ base::test::ScopedFeatureList scoped_feature_list(
+ syncer::kSyncSupportAlwaysSyncingPriorityPreferences);
+ browser_sync::IOSChromeSyncablePrefsDatabase db;
+ EXPECT_TRUE(db.IsPreferenceAlwaysSyncing(
+ sync_preferences::kSyncableAlwaysSyncingPriorityPrefForTesting));
+ EXPECT_FALSE(db.IsPreferenceAlwaysSyncing(
+ sync_preferences::kSyncablePriorityPrefForTesting));
+
+ // Currently, only priority preferences are allowed in the allowlist.
+ const std::map<std::string_view, sync_preferences::SyncablePrefMetadata>
+ syncable_prefs = db.GetAllSyncablePrefsForTest();
+ for (const auto& [pref_name, metadata] : syncable_prefs) {
+ if (db.IsPreferenceAlwaysSyncing(pref_name)) {
+ EXPECT_EQ(metadata.data_type(), syncer::PRIORITY_PREFERENCES);
+ }
+ }
+}
+
} // namespace
diff --git a/tools/metrics/histograms/metadata/sync/enums.xml b/tools/metrics/histograms/metadata/sync/enums.xml
index c3830a0eb..8d19492 100644
--- a/tools/metrics/histograms/metadata/sync/enums.xml
+++ b/tools/metrics/histograms/metadata/sync/enums.xml
@@ -399,6 +399,8 @@
<int value="81" label="AutofillBnplEnabled"/>
<int value="82" label="AutofillHasSeenBnpl"/>
<int value="83" label="AutomaticPasskeyUpgrades"/>
+ <int value="84" label="(test) SyncablePriorityPrefForTesting"/>
+ <int value="85" label="(test) SyncableAlwaysSyncingPriorityPrefForTesting"/>
<!-- LINT.ThenChange(/components/sync_preferences/common_syncable_prefs_database.cc:CommonSyncablePref)-->
<!-- LINT.IfChange(ChromeSyncablePref) -->
