Commit e74ac106 authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

[sync/android] Make decoupled_from_master_sync a one-shot device pref

Before this CL, sign-out would clear the pref representing whether Sync
should be decoupled from the Android master sync toggle. This CL
removes the clean-up logic and also makes sure that the pref is set
whenever ProfileSyncService detects there's no syncing account (either
on browser startup, or after sign-out). Indeed, by the time the
DecoupleSyncFromAndroidMasterSync feature is enabled, only users who
were already syncing but had the master toggle disabled should remain
in the coupled state.

Bug: 1105795
Change-Id: If5e36080159150fe976885b469696103df890ed4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2539202
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827887}
parent 9a959f6f
...@@ -108,6 +108,7 @@ const char kLocalSyncBackendDir[] = "sync.local_sync_backend_dir"; ...@@ -108,6 +108,7 @@ const char kLocalSyncBackendDir[] = "sync.local_sync_backend_dir";
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Stores whether sync should no longer respect the state of master toggle for // Stores whether sync should no longer respect the state of master toggle for
// this user. // this user.
// TODO(crbug.com/1107904): Clean pref when the decoupling logic is removed.
const char kSyncDecoupledFromAndroidMasterSync[] = const char kSyncDecoupledFromAndroidMasterSync[] =
"sync.decoupled_from_master_sync"; "sync.decoupled_from_master_sync";
#endif // defined(OS_ANDROID) #endif // defined(OS_ANDROID)
......
...@@ -226,9 +226,6 @@ void SyncPrefs::ClearLocalSyncTransportData() { ...@@ -226,9 +226,6 @@ void SyncPrefs::ClearLocalSyncTransportData() {
pref_service_->ClearPref(prefs::kSyncCacheGuid); pref_service_->ClearPref(prefs::kSyncCacheGuid);
pref_service_->ClearPref(prefs::kSyncBirthday); pref_service_->ClearPref(prefs::kSyncBirthday);
pref_service_->ClearPref(prefs::kSyncBagOfChips); pref_service_->ClearPref(prefs::kSyncBagOfChips);
#if defined(OS_ANDROID)
pref_service_->ClearPref(prefs::kSyncDecoupledFromAndroidMasterSync);
#endif // defined(OS_ANDROID)
// No need to clear kManaged, kEnableLocalSyncBackend or kLocalSyncBackendDir, // No need to clear kManaged, kEnableLocalSyncBackend or kLocalSyncBackendDir,
// since they're never actually set as user preferences. // since they're never actually set as user preferences.
......
...@@ -166,7 +166,8 @@ class SyncPrefs : public CryptoSyncPrefs, ...@@ -166,7 +166,8 @@ class SyncPrefs : public CryptoSyncPrefs,
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Sets a boolean pref representing that Sync should no longer respect whether // Sets a boolean pref representing that Sync should no longer respect whether
// Android master sync is enabled/disabled. // Android master sync is enabled/disabled. It is set per-device and never
// gets cleared.
void SetDecoupledFromAndroidMasterSync(); void SetDecoupledFromAndroidMasterSync();
// Gets the value for the boolean pref representing whether Sync should no // Gets the value for the boolean pref representing whether Sync should no
......
...@@ -334,6 +334,16 @@ void ProfileSyncService::Initialize() { ...@@ -334,6 +334,16 @@ void ProfileSyncService::Initialize() {
RecordSyncInitialState(GetDisableReasons(), RecordSyncInitialState(GetDisableReasons(),
user_settings_->IsFirstSetupComplete()); user_settings_->IsFirstSetupComplete());
#if defined(OS_ANDROID)
// If Sync was turned on after the feature toggle was enabled, it should be in
// the decoupled state.
if (!IsAuthenticatedAccountPrimary() &&
base::FeatureList::IsEnabled(
switches::kDecoupleSyncFromAndroidMasterSync)) {
sync_prefs_.SetDecoupledFromAndroidMasterSync();
}
#endif // defined(OS_ANDROID)
// Auto-start means the first time the profile starts up, sync should start up // Auto-start means the first time the profile starts up, sync should start up
// immediately. Since IsSyncRequested() is false by default and nobody else // immediately. Since IsSyncRequested() is false by default and nobody else
// will set it, we need to set it here. // will set it, we need to set it here.
...@@ -386,6 +396,17 @@ WeakHandle<JsEventHandler> ProfileSyncService::GetJsEventHandler() { ...@@ -386,6 +396,17 @@ WeakHandle<JsEventHandler> ProfileSyncService::GetJsEventHandler() {
void ProfileSyncService::AccountStateChanged() { void ProfileSyncService::AccountStateChanged() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
#if defined(OS_ANDROID)
// Once the feature toggle is enabled, Sync and master sync should only remain
// coupled if the former stays enabled and the latter disabled. Upon sign-out
// set the pref so they are decoupled on the next time Sync is turned on.
if (!IsAuthenticatedAccountPrimary() &&
base::FeatureList::IsEnabled(
switches::kDecoupleSyncFromAndroidMasterSync)) {
sync_prefs_.SetDecoupledFromAndroidMasterSync();
}
#endif // defined(OS_ANDROID)
if (!IsSignedIn()) { if (!IsSignedIn()) {
// The account was signed out, so shut down. // The account was signed out, so shut down.
sync_disabled_by_admin_ = false; sync_disabled_by_admin_ = false;
......
...@@ -209,7 +209,8 @@ class ProfileSyncService : public SyncService, ...@@ -209,7 +209,8 @@ class ProfileSyncService : public SyncService,
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Persists the fact that sync should no longer respect whether Android master // Persists the fact that sync should no longer respect whether Android master
// sync is enabled. Only called on Android. // sync is enabled. This will be respected for the current syncing account
// (if one exists) and any future ones. Only called on Android.
void SetDecoupledFromAndroidMasterSync(); void SetDecoupledFromAndroidMasterSync();
// Gets the persisted information of whether sync should no longer respect // Gets the persisted information of whether sync should no longer respect
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h"
#include "components/policy/core/common/policy_service_impl.h" #include "components/policy/core/common/policy_service_impl.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "components/signin/public/identity_manager/account_info.h" #include "components/signin/public/identity_manager/account_info.h"
...@@ -1390,6 +1391,44 @@ TEST_F(ProfileSyncServiceTest, ShouldPopulateAccountIdCachedInPrefs) { ...@@ -1390,6 +1391,44 @@ TEST_F(ProfileSyncServiceTest, ShouldPopulateAccountIdCachedInPrefs) {
EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestUser), sync_prefs.GetGaiaId()); EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestUser), sync_prefs.GetGaiaId());
} }
#if defined(OS_ANDROID)
TEST_F(ProfileSyncServiceTest, DecoupleFromMasterSyncIfInitializedSignedOut) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
switches::kDecoupleSyncFromAndroidMasterSync);
SyncPrefs sync_prefs(prefs());
CreateService(ProfileSyncService::MANUAL_START);
ASSERT_FALSE(sync_prefs.GetDecoupledFromAndroidMasterSync());
service()->Initialize();
EXPECT_TRUE(sync_prefs.GetDecoupledFromAndroidMasterSync());
}
TEST_F(ProfileSyncServiceTest, DecoupleFromMasterSyncIfSignsOut) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
switches::kDecoupleSyncFromAndroidMasterSync);
SyncPrefs sync_prefs(prefs());
SignIn();
CreateService(ProfileSyncService::MANUAL_START);
InitializeForNthSync();
ASSERT_FALSE(sync_prefs.GetDecoupledFromAndroidMasterSync());
// Sign-out.
auto* account_mutator = identity_manager()->GetPrimaryAccountMutator();
DCHECK(account_mutator) << "Account mutator should only be null on ChromeOS.";
account_mutator->ClearPrimaryAccount(
signin::PrimaryAccountMutator::ClearAccountsAction::kDefault,
signin_metrics::SIGNOUT_TEST,
signin_metrics::SignoutDelete::IGNORE_METRIC);
// Wait for ProfileSyncService to be notified.
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(sync_prefs.GetDecoupledFromAndroidMasterSync());
}
#endif // defined(OS_ANDROID)
TEST_F(ProfileSyncServiceTest, TEST_F(ProfileSyncServiceTest,
ShouldNotPopulateAccountIdCachedInPrefsWithLocalSync) { ShouldNotPopulateAccountIdCachedInPrefsWithLocalSync) {
SyncPrefs sync_prefs(prefs()); SyncPrefs sync_prefs(prefs());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment