Commit c3b3eeff authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Remove ProfileSyncService::IsManaged

https://crrev.com/c/1113746 introduced a GetDisableReasons which
replaces it. This CL updates all callers to use the new
GetDisableReasons instead.

Bug: 839834
Change-Id: If832ffb12f24308e08890a7c073d5827c0ef44a7
Reviewed-on: https://chromium-review.googlesource.com/1114959
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarJacob Dufault <jdufault@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571059}
parent 8a61b90c
...@@ -128,8 +128,8 @@ SyncConsentScreen::SyncScreenBehavior SyncConsentScreen::GetSyncScreenBehavior() ...@@ -128,8 +128,8 @@ SyncConsentScreen::SyncScreenBehavior SyncConsentScreen::GetSyncScreenBehavior()
// Skip for sync-disabled case. // Skip for sync-disabled case.
const browser_sync::ProfileSyncService* sync_service = const browser_sync::ProfileSyncService* sync_service =
GetSyncService(profile_); GetSyncService(profile_);
// IsManaged() is true for both 'sync is managed' and 'sync is disabled'. if (sync_service->HasDisableReason(
if (sync_service->IsManaged()) { syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY)) {
return SyncScreenBehavior::SKIP; return SyncScreenBehavior::SKIP;
} }
......
...@@ -62,13 +62,17 @@ base::string16 GetSyncedStateStatusLabel(ProfileSyncService* service, ...@@ -62,13 +62,17 @@ base::string16 GetSyncedStateStatusLabel(ProfileSyncService* service,
const SigninManagerBase& signin, const SigninManagerBase& signin,
StatusLabelStyle style, StatusLabelStyle style,
bool sync_everything) { bool sync_everything) {
if (!service || service->IsManaged()) { if (!service || service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY)) {
// User is signed in, but sync is disabled. // User is signed in, but sync is disabled.
return l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_DISABLED); return l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_DISABLED);
} else if (!service->IsSyncRequested()) { }
if (service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_USER_CHOICE)) {
// User is signed in, but sync has been stopped. // User is signed in, but sync has been stopped.
return l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_SUPPRESSED); return l10n_util::GetStringUTF16(IDS_SIGNED_IN_WITH_SYNC_SUPPRESSED);
} else if (!service->IsSyncActive()) { }
if (!service->IsSyncActive()) {
// User is not signed in, or sync is still initializing. // User is not signed in, or sync is still initializing.
return base::string16(); return base::string16();
} }
...@@ -198,8 +202,11 @@ MessageType GetStatusInfo(Profile* profile, ...@@ -198,8 +202,11 @@ MessageType GetStatusInfo(Profile* profile,
if (!signin.IsAuthenticated()) if (!signin.IsAuthenticated())
return PRE_SYNCED; return PRE_SYNCED;
if (!service || service->IsManaged() || service->IsFirstSetupComplete() || if (!service || service->IsFirstSetupComplete() ||
!service->IsSyncRequested()) { service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY) ||
service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_USER_CHOICE)) {
// The order or priority is going to be: 1. Unrecoverable errors. // The order or priority is going to be: 1. Unrecoverable errors.
// 2. Auth errors. 3. Protocol errors. 4. Passphrase errors. // 2. Auth errors. 3. Protocol errors. 4. Passphrase errors.
...@@ -263,7 +270,8 @@ MessageType GetStatusInfo(Profile* profile, ...@@ -263,7 +270,8 @@ MessageType GetStatusInfo(Profile* profile,
// Check to see if sync has been disabled via the dasboard and needs to be // Check to see if sync has been disabled via the dasboard and needs to be
// set up once again. // set up once again.
if (!service->IsSyncRequested() && if (service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_USER_CHOICE) &&
status.sync_protocol_error.error_type == syncer::NOT_MY_BIRTHDAY) { status.sync_protocol_error.error_type == syncer::NOT_MY_BIRTHDAY) {
if (status_label) { if (status_label) {
status_label->assign(GetSyncedStateStatusLabel(service, signin, style, status_label->assign(GetSyncedStateStatusLabel(service, signin, style,
......
...@@ -877,10 +877,13 @@ PeopleHandler::GetSyncStatusDictionary() { ...@@ -877,10 +877,13 @@ PeopleHandler::GetSyncStatusDictionary() {
// makes Profile::IsSyncAllowed() false. // makes Profile::IsSyncAllowed() false.
ProfileSyncService* service = ProfileSyncService* service =
ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile_); ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile_);
bool disallowed_by_policy =
service && service->HasDisableReason(
syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY);
sync_status->SetBoolean("signinAllowed", signin->IsSigninAllowed()); sync_status->SetBoolean("signinAllowed", signin->IsSigninAllowed());
sync_status->SetBoolean("syncSystemEnabled", (service != nullptr)); sync_status->SetBoolean("syncSystemEnabled", (service != nullptr));
sync_status->SetBoolean("setupInProgress", sync_status->SetBoolean("setupInProgress",
service && !service->IsManaged() && service && !disallowed_by_policy &&
service->IsFirstSetupInProgress() && service->IsFirstSetupInProgress() &&
signin->IsAuthenticated()); signin->IsAuthenticated());
...@@ -896,7 +899,7 @@ PeopleHandler::GetSyncStatusDictionary() { ...@@ -896,7 +899,7 @@ PeopleHandler::GetSyncStatusDictionary() {
sync_status->SetBoolean("hasError", status_has_error); sync_status->SetBoolean("hasError", status_has_error);
sync_status->SetString("statusAction", GetSyncErrorAction(action_type)); sync_status->SetString("statusAction", GetSyncErrorAction(action_type));
sync_status->SetBoolean("managed", service && service->IsManaged()); sync_status->SetBoolean("managed", disallowed_by_policy);
sync_status->SetBoolean("signedIn", signin->IsAuthenticated()); sync_status->SetBoolean("signedIn", signin->IsAuthenticated());
sync_status->SetString("signedInUsername", sync_status->SetString("signedInUsername",
signin_ui_util::GetAuthenticatedUsername(signin)); signin_ui_util::GetAuthenticatedUsername(signin));
......
...@@ -325,7 +325,9 @@ void ProfileSyncService::Initialize() { ...@@ -325,7 +325,9 @@ void ProfileSyncService::Initialize() {
// Only clear data if disallowed by policy. // Only clear data if disallowed by policy.
// TODO(crbug.com/839834): Should this call StopImpl, so that SyncRequested // TODO(crbug.com/839834): Should this call StopImpl, so that SyncRequested
// doesn't get set to false? // doesn't get set to false?
RequestStop(IsManaged() ? CLEAR_DATA : KEEP_DATA); RequestStop((disable_reasons & DISABLE_REASON_ENTERPRISE_POLICY)
? CLEAR_DATA
: KEEP_DATA);
return; return;
} }
...@@ -756,7 +758,7 @@ int ProfileSyncService::GetDisableReasons() const { ...@@ -756,7 +758,7 @@ int ProfileSyncService::GetDisableReasons() const {
if (!IsSyncAllowedByFlag() || !IsSyncAllowedByPlatform()) { if (!IsSyncAllowedByFlag() || !IsSyncAllowedByPlatform()) {
result = result | DISABLE_REASON_PLATFORM_OVERRIDE; result = result | DISABLE_REASON_PLATFORM_OVERRIDE;
} }
if (IsManaged()) { if (sync_prefs_.IsManaged() || sync_disabled_by_admin_) {
result = result | DISABLE_REASON_ENTERPRISE_POLICY; result = result | DISABLE_REASON_ENTERPRISE_POLICY;
} }
// Local sync doesn't require sign-in. // Local sync doesn't require sign-in.
...@@ -1302,7 +1304,8 @@ ProfileSyncService::GetSetupInProgressHandle() { ...@@ -1302,7 +1304,8 @@ ProfileSyncService::GetSetupInProgressHandle() {
bool ProfileSyncService::IsSyncAllowed() const { bool ProfileSyncService::IsSyncAllowed() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return IsSyncAllowedByFlag() && !IsManaged() && IsSyncAllowedByPlatform(); return !HasDisableReason(DISABLE_REASON_PLATFORM_OVERRIDE) &&
!HasDisableReason(DISABLE_REASON_ENTERPRISE_POLICY);
} }
bool ProfileSyncService::IsSyncActive() const { bool ProfileSyncService::IsSyncActive() const {
...@@ -1986,11 +1989,6 @@ bool ProfileSyncService::IsSyncAllowedByPlatform() const { ...@@ -1986,11 +1989,6 @@ bool ProfileSyncService::IsSyncAllowedByPlatform() const {
platform_sync_allowed_provider_.Run(); platform_sync_allowed_provider_.Run();
} }
bool ProfileSyncService::IsManaged() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return sync_prefs_.IsManaged() || sync_disabled_by_admin_;
}
void ProfileSyncService::RequestStop(SyncStopDataFate data_fate) { void ProfileSyncService::RequestStop(SyncStopDataFate data_fate) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
sync_prefs_.SetSyncRequested(false); sync_prefs_.SetSyncRequested(false);
......
...@@ -406,13 +406,6 @@ class ProfileSyncService : public syncer::SyncService, ...@@ -406,13 +406,6 @@ class ProfileSyncService : public syncer::SyncService,
// TODO(crbug.com/839834): This method is somewhat misnamed. // TODO(crbug.com/839834): This method is somewhat misnamed.
virtual bool IsSyncConfirmationNeeded() const; virtual bool IsSyncConfirmationNeeded() const;
// Returns whether sync is managed, i.e. controlled by configuration
// management. If so, the user is not allowed to configure sync.
// TODO(crbug.com/839834): This is misnamed, it means "is force-disabled by
// policy".
// DEPRECATED! Use GetDisableReasons instead.
bool IsManaged() const;
// syncer::UnrecoverableErrorHandler implementation. // syncer::UnrecoverableErrorHandler implementation.
void OnUnrecoverableError(const base::Location& from_here, void OnUnrecoverableError(const base::Location& from_here,
const std::string& message) override; const std::string& message) override;
......
...@@ -431,11 +431,9 @@ TEST_F(ProfileSyncServiceStartupTest, ManagedStartup) { ...@@ -431,11 +431,9 @@ TEST_F(ProfileSyncServiceStartupTest, ManagedStartup) {
CreateSyncService(ProfileSyncService::MANUAL_START); CreateSyncService(ProfileSyncService::MANUAL_START);
// Disable sync through policy. // Disable sync through policy.
ASSERT_FALSE(sync_service()->IsManaged());
ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE, ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
sync_service()->GetDisableReasons()); sync_service()->GetDisableReasons());
pref_service()->SetBoolean(syncer::prefs::kSyncManaged, true); pref_service()->SetBoolean(syncer::prefs::kSyncManaged, true);
ASSERT_TRUE(sync_service()->IsManaged());
ASSERT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY, ASSERT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY,
sync_service()->GetDisableReasons()); sync_service()->GetDisableReasons());
EXPECT_CALL(*component_factory(), CreateSyncEngine(_, _, _, _)).Times(0); EXPECT_CALL(*component_factory(), CreateSyncEngine(_, _, _, _)).Times(0);
...@@ -467,7 +465,6 @@ TEST_F(ProfileSyncServiceStartupTest, SwitchManaged) { ...@@ -467,7 +465,6 @@ TEST_F(ProfileSyncServiceStartupTest, SwitchManaged) {
.WillOnce(Return(DataTypeManager::CONFIGURED)); .WillOnce(Return(DataTypeManager::CONFIGURED));
EXPECT_CALL(*data_type_manager, Stop(syncer::DISABLE_SYNC)); EXPECT_CALL(*data_type_manager, Stop(syncer::DISABLE_SYNC));
pref_service()->SetBoolean(syncer::prefs::kSyncManaged, true); pref_service()->SetBoolean(syncer::prefs::kSyncManaged, true);
ASSERT_TRUE(sync_service()->IsManaged());
ASSERT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY, ASSERT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY,
sync_service()->GetDisableReasons()); sync_service()->GetDisableReasons());
EXPECT_FALSE(sync_service()->IsEngineInitialized()); EXPECT_FALSE(sync_service()->IsEngineInitialized());
...@@ -481,7 +478,6 @@ TEST_F(ProfileSyncServiceStartupTest, SwitchManaged) { ...@@ -481,7 +478,6 @@ TEST_F(ProfileSyncServiceStartupTest, SwitchManaged) {
EXPECT_CALL(*component_factory(), CreateDataTypeManager(_, _, _, _, _, _)) EXPECT_CALL(*component_factory(), CreateDataTypeManager(_, _, _, _, _, _))
.Times(0); .Times(0);
pref_service()->ClearPref(syncer::prefs::kSyncManaged); pref_service()->ClearPref(syncer::prefs::kSyncManaged);
ASSERT_FALSE(sync_service()->IsManaged());
ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE, ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
sync_service()->GetDisableReasons()); sync_service()->GetDisableReasons());
EXPECT_FALSE(sync_service()->IsEngineInitialized()); EXPECT_FALSE(sync_service()->IsEngineInitialized());
......
...@@ -343,7 +343,6 @@ TEST_F(ProfileSyncServiceTest, SuccessfulInitialization) { ...@@ -343,7 +343,6 @@ TEST_F(ProfileSyncServiceTest, SuccessfulInitialization) {
.WillOnce( .WillOnce(
ReturnNewFakeDataTypeManager(GetDefaultConfigureCalledCallback())); ReturnNewFakeDataTypeManager(GetDefaultConfigureCalledCallback()));
InitializeForNthSync(); InitializeForNthSync();
EXPECT_FALSE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_TRUE(service()->IsSyncActive()); EXPECT_TRUE(service()->IsSyncActive());
...@@ -357,7 +356,6 @@ TEST_F(ProfileSyncServiceTest, SuccessfulLocalBackendInitialization) { ...@@ -357,7 +356,6 @@ TEST_F(ProfileSyncServiceTest, SuccessfulLocalBackendInitialization) {
.WillOnce( .WillOnce(
ReturnNewFakeDataTypeManager(GetDefaultConfigureCalledCallback())); ReturnNewFakeDataTypeManager(GetDefaultConfigureCalledCallback()));
InitializeForNthSync(); InitializeForNthSync();
EXPECT_FALSE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_TRUE(service()->IsSyncActive()); EXPECT_TRUE(service()->IsSyncActive());
...@@ -413,7 +411,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInit) { ...@@ -413,7 +411,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyBeforeInit) {
SignIn(); SignIn();
CreateService(ProfileSyncService::AUTO_START); CreateService(ProfileSyncService::AUTO_START);
InitializeForNthSync(); InitializeForNthSync();
EXPECT_TRUE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY | EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY |
syncer::SyncService::DISABLE_REASON_USER_CHOICE, syncer::SyncService::DISABLE_REASON_USER_CHOICE,
service()->GetDisableReasons()); service()->GetDisableReasons());
...@@ -427,7 +424,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) { ...@@ -427,7 +424,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) {
CreateService(ProfileSyncService::AUTO_START); CreateService(ProfileSyncService::AUTO_START);
InitializeForNthSync(); InitializeForNthSync();
ASSERT_FALSE(service()->IsManaged());
ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE, ASSERT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
service()->GetDisableReasons()); service()->GetDisableReasons());
ASSERT_TRUE(service()->IsSyncActive()); ASSERT_TRUE(service()->IsSyncActive());
...@@ -435,7 +431,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) { ...@@ -435,7 +431,6 @@ TEST_F(ProfileSyncServiceTest, DisabledByPolicyAfterInit) {
prefs()->SetManagedPref(syncer::prefs::kSyncManaged, prefs()->SetManagedPref(syncer::prefs::kSyncManaged,
std::make_unique<base::Value>(true)); std::make_unique<base::Value>(true));
EXPECT_TRUE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_FALSE(service()->IsSyncActive()); EXPECT_FALSE(service()->IsSyncActive());
...@@ -1175,7 +1170,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) { ...@@ -1175,7 +1170,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) {
std::make_unique<base::Value>(false)); std::make_unique<base::Value>(false));
CreateServiceWithLocalSyncBackend(); CreateServiceWithLocalSyncBackend();
InitializeForNthSync(); InitializeForNthSync();
EXPECT_FALSE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_TRUE(service()->IsSyncActive()); EXPECT_TRUE(service()->IsSyncActive());
...@@ -1183,7 +1177,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) { ...@@ -1183,7 +1177,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) {
prefs()->SetManagedPref(syncer::prefs::kSyncManaged, prefs()->SetManagedPref(syncer::prefs::kSyncManaged,
std::make_unique<base::Value>(true)); std::make_unique<base::Value>(true));
EXPECT_TRUE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_ENTERPRISE_POLICY,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_FALSE(service()->IsSyncActive()); EXPECT_FALSE(service()->IsSyncActive());
...@@ -1192,7 +1185,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) { ...@@ -1192,7 +1185,6 @@ TEST_F(ProfileSyncServiceTest, LocalBackendDisabledByPolicy) {
std::make_unique<base::Value>(false)); std::make_unique<base::Value>(false));
service()->RequestStart(); service()->RequestStart();
EXPECT_FALSE(service()->IsManaged());
EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE, EXPECT_EQ(syncer::SyncService::DISABLE_REASON_NONE,
service()->GetDisableReasons()); service()->GetDisableReasons());
EXPECT_TRUE(service()->IsSyncActive()); EXPECT_TRUE(service()->IsSyncActive());
......
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