[Smart Lock, Prefs reconciliation] Prefs migration logic for desktop platforms.

chrome/browser/password_manager/password_manager_setting_migrater_service.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/password_manager/password_manager_setting_migrater_service.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/sync_driver/sync_service.h"
+#include "content/public/browser/notification_source.h"
+
+namespace {
+void ChangeOnePrefBecauseAnotherPrefHasChanged(
+    PrefService* prefs,
+    const std::string& other_pref_name,
+    const std::string& changed_pref_name) {
+  bool changed_pref = prefs->GetBoolean(changed_pref_name.c_str());
+  bool other_pref = prefs->GetBoolean(other_pref_name.c_str());
+  if (changed_pref != other_pref)
+    prefs->SetBoolean(other_pref_name, changed_pref);
+  // TODO(melandory): add histograms in order to track when we can stop
+  // migration.
+}
+
+}  // namespace
+
+PasswordManagerSettingMigraterService::PasswordManagerSettingMigraterService(
+    Profile* profile)
+    : profile_(profile), sync_service_(nullptr), both_pref_off_(true) {
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
+                 content::Source<Profile>(profile));
+  // There should be user set values for preferences we're migrating, otherwise
+  // sync will overwrite default values with server data and this will generate
+  // pref changes notification even if prefs have not changed.
+  PrefService* prefs = profile_->GetPrefs();
+  bool legacy_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled);
+  bool new_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kCredentialsEnableService);
+  prefs->SetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled,
+                    legacy_pref_value);
+  prefs->SetBoolean(password_manager::prefs::kCredentialsEnableService,
+                    new_pref_value);
+}
+
+PasswordManagerSettingMigraterService::
+    ~PasswordManagerSettingMigraterService() {}
+
+void PasswordManagerSettingMigraterService::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_PROFILE_ADDED, type);
+  if (ProfileSyncServiceFactory::HasProfileSyncService(profile_))
+    sync_service_ = ProfileSyncServiceFactory::GetForProfile(profile_);
+  InitObservers();
+}
+
+void PasswordManagerSettingMigraterService::InitObservers() {
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  bool legacy_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled);
+  bool new_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kCredentialsEnableService);
+  both_pref_off_ = (!legacy_pref_value) && (!new_pref_value);
+  pref_change_registrar_.Init(profile_->GetPrefs());
+  pref_change_registrar_.Add(
+      password_manager::prefs::kCredentialsEnableService,
+      base::Bind(&PasswordManagerSettingMigraterService::
+                     OnCredentialsEnableServicePrefChanged,
+                 base::Unretained(this)));
+  pref_change_registrar_.Add(
+      password_manager::prefs::kPasswordManagerSavingEnabled,
+      base::Bind(&PasswordManagerSettingMigraterService::
+                     OnPasswordManagerSavingEnabledPrefChanged,
+                 base::Unretained(this)));
+  if (!(sync_service_ && sync_service_->CanSyncStart())) {
+    PasswordManagerSettingMigraterService::MigrateOffState(prefs);
+  }
+  // This causes OnIsSyncingChanged to be called when the value of
+  // PrefService::IsSyncing() changes.
+  prefs->AddObserver(this);
+}
+
+void PasswordManagerSettingMigraterService::Shutdown() {
+  PrefService* prefs = profile_->GetPrefs();
+  MigrateOffState(prefs);

[vabr (Chromium), 2015/08/26 14:42:43]

I still think we should call prefs->RemoveObserver(this) and not
MigrateOffState. Shutdown is a place to clean-up, not try to do any more work.
Is there a good reason to try and do migration here?

[melandory, 2015/08/28 14:49:24]

Done.

+}
+
+void PasswordManagerSettingMigraterService::
+    OnCredentialsEnableServicePrefChanged(
+        const std::string& changed_pref_name) {
+  PrefService* prefs = profile_->GetPrefs();
+  ChangeOnePrefBecauseAnotherPrefHasChanged(
+      prefs, password_manager::prefs::kPasswordManagerSavingEnabled,
+      password_manager::prefs::kCredentialsEnableService);
+}
+
+void PasswordManagerSettingMigraterService::
+    OnPasswordManagerSavingEnabledPrefChanged(
+        const std::string& changed_pref_name) {
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  // In order to preserve off value we need to hold a propagation of the update.

[vabr (Chromium), 2015/08/26 14:42:43]

nit: What does "hold a propagation" mean? Did you mean "hold on with
propagation"?

[melandory, 2015/08/28 14:49:24]

Not applicable any more.

+  if (!prefs->GetBoolean(password_manager::prefs::kCredentialsEnableService) &&
+      !both_pref_off_) {
+    return;
+  }
+  ChangeOnePrefBecauseAnotherPrefHasChanged(
+      prefs, password_manager::prefs::kCredentialsEnableService,
+      password_manager::prefs::kPasswordManagerSavingEnabled);
+}
+
+void PasswordManagerSettingMigraterService::OnIsSyncingChanged() {
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  if (prefs->IsSyncing() && prefs->IsPrioritySyncing()) {
+    // Initial sync has finished.
+    MigrateOffState(prefs);
+  }
+  bool legacy_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled);
+  bool new_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kCredentialsEnableService);
+  if (prefs->IsPrioritySyncing() && prefs->IsSyncing()) {
+    both_pref_off_ = (!legacy_pref_value) && (!new_pref_value);
+  }
+}
+
+void PasswordManagerSettingMigraterService::MigrateOffState(
+    PrefService* prefs) {
+  bool new_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kCredentialsEnableService);
+  bool legacy_pref_value =
+      prefs->GetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled);
+  if (new_pref_value != legacy_pref_value) {
+    // Values are not equal, so we reconcile preferences to the off state.
+    prefs->SetBoolean(password_manager::prefs::kPasswordManagerSavingEnabled,
+                      false);
+    prefs->SetBoolean(password_manager::prefs::kCredentialsEnableService,
+                      false);
+  }
+}
+
+// static
+PasswordManagerSettingMigraterService::Factory*
+PasswordManagerSettingMigraterService::Factory::GetInstance() {
+  return Singleton<PasswordManagerSettingMigraterService::Factory>::get();
+}
+
+// static
+PasswordManagerSettingMigraterService*
+PasswordManagerSettingMigraterService::Factory::GetForProfile(
+    Profile* profile) {
+  return static_cast<PasswordManagerSettingMigraterService*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+PasswordManagerSettingMigraterService::Factory::Factory()
+    : BrowserContextKeyedServiceFactory(
+          "PasswordManagerSettingMigraterService",
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(ProfileSyncServiceFactory::GetInstance());
+}
+
+PasswordManagerSettingMigraterService::Factory::~Factory() {}
+
+KeyedService*
+PasswordManagerSettingMigraterService::Factory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  return new PasswordManagerSettingMigraterService(
+      static_cast<Profile*>(context));
+}
+
+bool PasswordManagerSettingMigraterService::Factory::
+    ServiceIsCreatedWithBrowserContext() const {
+  return true;
+}