[Sync] Fix up users who were affected by bug 154940

These users did not have their Keep Everything Synced preference set
properly, or the preferences for the preferred datatypes. We detect this
and manually fix up the preference before reconfiguring sync.

BUG=158391


Review URL: https://chromiumcodereview.appspot.com/11342022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@164975 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/profile_sync_service.cc

@@ -28,6 +28,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/net/chrome_cookie_notification_details.h"
+#include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/signin_manager.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -52,6 +53,7 @@
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/time_format.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/notification_details.h"
@@ -216,9 +218,45 @@ void ProfileSyncService::Initialize() {
     DisableForUser();
   }
 
+  TrySyncDatatypePrefRecovery();
+
   TryStart();
 }
 
+void ProfileSyncService::TrySyncDatatypePrefRecovery() {
+  DCHECK(!sync_initialized());
+  if (!HasSyncSetupCompleted())
+    return;
+
+  // There was a bug where OnUserChoseDatatypes was not properly called on
+  // configuration (see crbug.com/154940). We detect this by checking whether
+  // kSyncKeepEverythingSynced has a default value. If so, and sync setup has
+  // completed, it means sync was not properly configured, so we manually
+  // set kSyncKeepEverythingSynced.
+  PrefService* const pref_service = profile_->GetPrefs();
+  if (!pref_service)
+    return;
+  if (sync_prefs_.HasKeepEverythingSynced())
+    return;
+  const syncer::ModelTypeSet registered_types = GetRegisteredDataTypes();
+  if (sync_prefs_.GetPreferredDataTypes(registered_types).Size() > 1)
+    return;
+
+  const PrefService::Preference* keep_everything_synced =
+      pref_service->FindPreference(prefs::kSyncKeepEverythingSynced);
+  // This will be false if the preference was properly set or if it's controlled
+  // by policy.
+  if (!keep_everything_synced->IsDefaultValue())
+    return;
+
+  // kSyncKeepEverythingSynced was not properly set. Set it and the preferred
+  // types now, before we configure.
+  UMA_HISTOGRAM_COUNTS("Sync.DatatypePrefRecovery", 1);
+  sync_prefs_.SetKeepEverythingSynced(true);
+  sync_prefs_.SetPreferredDataTypes(registered_types,
+                                    registered_types);
+}
+
 void ProfileSyncService::TryStart() {
   if (!IsSyncEnabledAndLoggedIn())
     return;

chrome/browser/sync/profile_sync_service.h

@@ -648,6 +648,11 @@ class ProfileSyncService : public ProfileSyncServiceBase,
   friend class TestProfileSyncService;
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceTest, InitialState);
 
+  // Detects and attempts to recover from a previous improper datatype
+  // configuration where Keep Everything Synced and the preferred types were
+  // not correctly set.
+  void TrySyncDatatypePrefRecovery();
+
   // Starts up sync if it is not suppressed and preconditions are met.
   // Called from Initialize() and UnsuppressAndStart().
   void TryStart();

chrome/browser/sync/profile_sync_service_startup_unittest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/sync/glue/data_type_manager_mock.h"
 #include "chrome/browser/sync/profile_sync_components_factory_mock.h"
 #include "chrome/browser/sync/profile_sync_test_util.h"
+#include "chrome/browser/sync/sync_prefs.h"
 #include "chrome/browser/sync/test_profile_sync_service.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
@@ -268,6 +269,61 @@ TEST_F(ProfileSyncServiceStartupTest, StartNormal) {
   service_->Initialize();
 }
 
+// Test that we can recover from a case where a bug in the code resulted in
+// OnUserChoseDatatypes not being properly called and datatype preferences
+// therefore being left unset.
+TEST_F(ProfileSyncServiceStartupTest, StartRecoverDatatypePrefs) {
+  DataTypeManagerMock* data_type_manager = SetUpDataTypeManager();
+  EXPECT_CALL(*data_type_manager, Configure(_, _));
+  EXPECT_CALL(*data_type_manager, state()).
+      WillRepeatedly(Return(DataTypeManager::CONFIGURED));
+  EXPECT_CALL(*data_type_manager, Stop()).Times(1);
+
+  EXPECT_CALL(observer_, OnStateChanged()).Times(AnyNumber());
+
+  // Clear the datatype preference fields (simulating bug 154940).
+  profile_->GetPrefs()->ClearPref(prefs::kSyncKeepEverythingSynced);
+  for (syncer::ModelTypeSet::Iterator iter = syncer::UserTypes().First();
+       iter.Good(); iter.Inc()) {
+    profile_->GetPrefs()->ClearPref(
+        browser_sync::SyncPrefs::GetPrefNameForDataType(iter.Get()));
+  }
+
+  // Pre load the tokens
+  TokenServiceFactory::GetForProfile(profile_.get())->IssueAuthTokenForTest(
+      GaiaConstants::kSyncService, "sync_token");
+  profile_->GetPrefs()->SetString(prefs::kGoogleServicesUsername, "test_user");
+  service_->Initialize();
+
+  EXPECT_TRUE(profile_->GetPrefs()->GetBoolean(
+      prefs::kSyncKeepEverythingSynced));
+}
+
+// Verify that the recovery of datatype preferences doesn't overwrite a valid
+// case where only bookmarks are enabled.
+TEST_F(ProfileSyncServiceStartupTest, StartDontRecoverDatatypePrefs) {
+  DataTypeManagerMock* data_type_manager = SetUpDataTypeManager();
+  EXPECT_CALL(*data_type_manager, Configure(_, _));
+  EXPECT_CALL(*data_type_manager, state()).
+      WillRepeatedly(Return(DataTypeManager::CONFIGURED));
+  EXPECT_CALL(*data_type_manager, Stop()).Times(1);
+
+  EXPECT_CALL(observer_, OnStateChanged()).Times(AnyNumber());
+
+  // Explicitly set Keep Everything Synced to false and have only bookmarks
+  // enabled.
+  profile_->GetPrefs()->SetBoolean(prefs::kSyncKeepEverythingSynced, false);
+
+  // Pre load the tokens
+  TokenServiceFactory::GetForProfile(profile_.get())->IssueAuthTokenForTest(
+      GaiaConstants::kSyncService, "sync_token");
+  profile_->GetPrefs()->SetString(prefs::kGoogleServicesUsername, "test_user");
+  service_->Initialize();
+
+  EXPECT_FALSE(profile_->GetPrefs()->GetBoolean(
+      prefs::kSyncKeepEverythingSynced));
+}
+
 TEST_F(ProfileSyncServiceStartupTest, ManagedStartup) {
   // Disable sync through policy.
   profile_->GetPrefs()->SetBoolean(prefs::kSyncManaged, true);