Commit 7069d4dc authored by Vincent Boisselle's avatar Vincent Boisselle Committed by Commit Bot

Add a direct check to know if sync is paused before providing user demographics.

In its current implementation, IsSyncEnable() does not cover by default the case where sync is in the "paused" state where it will return true when sync is "paused".

Considering that we don't want demographics to be provided when sync is "paused", we need to add an explicit check for the "paused" state before providing demographics.

Bug: 988825
Change-Id: Ic62a2a6a43468ad3980d7d1fc0e127ba6241579e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724638
Commit-Queue: Vincent Boisselle <vincb@google.com>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682735}
parent 20f5c582
......@@ -1672,9 +1672,10 @@ void ProfileSyncService::SetInvalidationsForSessionsEnabled(bool enabled) {
base::Optional<UserDemographics> ProfileSyncService::GetUserDemographics(
base::Time now) {
// Do not provide demographics when sync is disabled because user demographics
// should only be provided when the other sync data is also provided.
if (!IsSyncFeatureEnabled())
// Do not provide demographics when sync is disabled or paused because user
// demographics should only be provided when the other sync data can be
// uploaded to the sync server.
if (!IsSyncFeatureEnabled() || auth_manager_->IsSyncPaused())
return base::nullopt;
return sync_prefs_.GetUserDemographics(now);
......
......@@ -1367,5 +1367,99 @@ TEST_F(ProfileSyncServiceTest, GetUserDemographics_SyncTemporarilyDisabled) {
EXPECT_TRUE(HasBirthYearOffset(prefs()));
}
// Test whether sync service does not provide user demographics and does not
// clear demographic prefs when sync is paused and enabled, which represents the
// case where the kStopSyncInPausedState feature is disabled.
TEST_F(ProfileSyncServiceTest,
GetUserDemographics_SyncPausedAndFeatureDisabled) {
base::test::ScopedFeatureList feature;
// Disable the feature that stops the sync engine (disables sync) when sync is
// paused.
feature.InitAndDisableFeature(switches::kStopSyncInPausedState);
// Initialize service with sync enabled at start.
SignIn();
CreateService(ProfileSyncService::AUTO_START);
InitializeForNthSync();
ASSERT_EQ(SyncService::TransportState::ACTIVE,
service()->GetTransportState());
// Set demographic prefs that are normally fetched from server when syncing.
SetDemographics(kOldEnoughForDemographicsUserBirthYear,
metrics::UserDemographicsProto_Gender_GENDER_FEMALE);
// Set birth year noise offset that is usually set when calling
// SyncPrefs::GetUserDemographics.
prefs()->SetInteger(prefs::kSyncDemographicsBirthYearOffset, 2);
// Verify that demographic prefs exist (i.e., the test is set up).
ASSERT_TRUE(HasBirthYearDemographic(prefs()));
ASSERT_TRUE(HasGenderDemographic(prefs()));
ASSERT_TRUE(HasBirthYearOffset(prefs()));
// Simulate sign out using an invalid auth error.
identity_test_env()->SetInvalidRefreshTokenForPrimaryAccount();
ASSERT_EQ(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS,
service()->GetAuthError().state());
ASSERT_EQ(SyncService::DISABLE_REASON_NONE, service()->GetDisableReasons());
// Verify that sync service does not provide demographics when sync is paused.
base::Optional<UserDemographics> user_demographics =
service()->GetUserDemographics(GetNowTime());
EXPECT_FALSE(user_demographics.has_value());
// Verify that demographic prefs are not cleared.
EXPECT_TRUE(HasBirthYearDemographic(prefs()));
EXPECT_TRUE(HasGenderDemographic(prefs()));
EXPECT_TRUE(HasBirthYearOffset(prefs()));
}
// Test whether sync service does not provide user demographics and does not
// clear demographic prefs when sync is paused and disabled, which represents
// the case where the kStopSyncInPausedState feature is enabled.
TEST_F(ProfileSyncServiceTest,
GetUserDemographics_SyncPausedAndFeatureEnabled) {
base::test::ScopedFeatureList feature;
// Enable the feature that stops the sync engine (disables sync) when sync is
// paused.
feature.InitAndEnableFeature(switches::kStopSyncInPausedState);
// Initialize service with sync enabled at start.
SignIn();
CreateService(ProfileSyncService::AUTO_START);
InitializeForNthSync();
ASSERT_EQ(SyncService::TransportState::ACTIVE,
service()->GetTransportState());
// Set demographic prefs that are normally fetched from server when syncing.
SetDemographics(kOldEnoughForDemographicsUserBirthYear,
metrics::UserDemographicsProto_Gender_GENDER_FEMALE);
// Set birth year noise offset that is usually set when calling
// SyncPrefs::GetUserDemographics.
prefs()->SetInteger(prefs::kSyncDemographicsBirthYearOffset, 2);
// Verify that demographic prefs exist (i.e., the test is set up).
ASSERT_TRUE(HasBirthYearDemographic(prefs()));
ASSERT_TRUE(HasGenderDemographic(prefs()));
ASSERT_TRUE(HasBirthYearOffset(prefs()));
// Simulate sign out using an invalid auth error.
identity_test_env()->SetInvalidRefreshTokenForPrimaryAccount();
ASSERT_EQ(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS,
service()->GetAuthError().state());
ASSERT_EQ(SyncService::DISABLE_REASON_PAUSED, service()->GetDisableReasons());
// Verify that sync service does not provide demographics when sync is paused.
base::Optional<UserDemographics> user_demographics =
service()->GetUserDemographics(GetNowTime());
EXPECT_FALSE(user_demographics.has_value());
// Verify that demographic prefs are not cleared.
EXPECT_TRUE(HasBirthYearDemographic(prefs()));
EXPECT_TRUE(HasGenderDemographic(prefs()));
EXPECT_TRUE(HasBirthYearOffset(prefs()));
}
} // namespace
} // namespace syncer
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