[Password Manager] Remove dead profile migration code for GNOME/KWallet

This code has been unused for around 2 years.

BUG=355145

Review URL: https://codereview.chromium.org/196173023

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

chrome/browser/password_manager/native_backend_gnome_x.cc

@@ -495,19 +495,9 @@ void GKRMethod::OnOperationGetList(GnomeKeyringResult result, GList* list,
 
 }  // namespace
 
-NativeBackendGnome::NativeBackendGnome(LocalProfileId id, PrefService* prefs)
-    : profile_id_(id), prefs_(prefs) {
-  // TODO(mdm): after a few more releases, remove the code which is now dead due
-  // to the true || here, and simplify this code. We don't do it yet to make it
-  // easier to revert if necessary.
-  if (true || PasswordStoreX::PasswordsUseLocalProfileId(prefs)) {
-    app_string_ = GetProfileSpecificAppString();
-    // We already did the migration previously. Don't try again.
-    migrate_tried_ = true;
-  } else {
-    app_string_ = kGnomeKeyringAppString;
-    migrate_tried_ = false;
-  }
+NativeBackendGnome::NativeBackendGnome(LocalProfileId id)
+    : profile_id_(id) {
+  app_string_ = GetProfileSpecificAppString();
 }
 
 NativeBackendGnome::~NativeBackendGnome() {
@@ -530,9 +520,6 @@ bool NativeBackendGnome::RawAddLogin(const PasswordForm& form) {
                << gnome_keyring_result_to_message(result);
     return false;
   }
-  // Successful write. Try migration if necessary.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
   return true;
 }
 
@@ -562,11 +549,6 @@ bool NativeBackendGnome::AddLogin(const PasswordForm& form) {
                    << " matching logins already! Will replace only the first.";
     }
 
-    // We try migration before updating the existing logins, since otherwise
-    // we'd do it after making some but not all of the changes below.
-    if (forms.size() > 0 && !migrate_tried_)
-      MigrateToProfileSpecificLogins();
-
     RemoveLogin(*forms[0]);
     for (size_t i = 0; i < forms.size(); ++i)
       delete forms[i];
@@ -595,11 +577,6 @@ bool NativeBackendGnome::UpdateLogin(const PasswordForm& form) {
     return false;
   }
 
-  // We try migration before updating the existing logins, since otherwise
-  // we'd do it after making some but not all of the changes below.
-  if (forms.size() > 0 && !migrate_tried_)
-    MigrateToProfileSpecificLogins();
-
   bool ok = true;
   for (size_t i = 0; i < forms.size(); ++i) {
     if (forms[i]->action != form.action ||
@@ -637,11 +614,6 @@ bool NativeBackendGnome::RemoveLogin(const PasswordForm& form) {
                  << gnome_keyring_result_to_message(result);
     return false;
   }
-  // Successful write. Try migration if necessary. Note that presumably if we've
-  // been asked to delete a login, it's because we returned it previously; thus,
-  // this will probably never happen since we'd have already tried migration.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
   return true;
 }
 
@@ -655,7 +627,6 @@ bool NativeBackendGnome::RemoveLoginsCreatedBetween(
   PasswordFormList forms;
   if (!GetAllLogins(&forms))
     return false;
-  // No need to try migration here: GetAllLogins() does it.
 
   for (size_t i = 0; i < forms.size(); ++i) {
     if (delete_begin <= forms[i]->date_created &&
@@ -684,9 +655,6 @@ bool NativeBackendGnome::GetLogins(const PasswordForm& form,
                << gnome_keyring_result_to_message(result);
     return false;
   }
-  // Successful read of actual data. Try migration if necessary.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
   return true;
 }
 
@@ -699,7 +667,6 @@ bool NativeBackendGnome::GetLoginsCreatedBetween(const base::Time& get_begin,
   PasswordFormList all_forms;
   if (!GetAllLogins(&all_forms))
     return false;
-  // No need to try migration here: GetAllLogins() does it.
 
   forms->reserve(forms->size() + all_forms.size());
   for (size_t i = 0; i < all_forms.size(); ++i) {
@@ -741,9 +708,6 @@ bool NativeBackendGnome::GetLoginsList(PasswordFormList* forms,
                << gnome_keyring_result_to_message(result);
     return false;
   }
-  // Successful read of actual data. Try migration if necessary.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
   return true;
 }
 
@@ -761,9 +725,6 @@ bool NativeBackendGnome::GetAllLogins(PasswordFormList* forms) {
                << gnome_keyring_result_to_message(result);
     return false;
   }
-  // Successful read of actual data. Try migration if necessary.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
   return true;
 }
 
@@ -773,44 +734,3 @@ std::string NativeBackendGnome::GetProfileSpecificAppString() const {
   // for nothing. Now we use it to distinguish passwords for different profiles.
   return base::StringPrintf("%s-%d", kGnomeKeyringAppString, profile_id_);
 }
-
-void NativeBackendGnome::MigrateToProfileSpecificLogins() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-
-  DCHECK(!migrate_tried_);
-  DCHECK_EQ(app_string_, kGnomeKeyringAppString);
-
-  // Record the fact that we've attempted migration already right away, so that
-  // we don't get recursive calls back to MigrateToProfileSpecificLogins().
-  migrate_tried_ = true;
-
-  // First get all the logins, using the old app string.
-  PasswordFormList forms;
-  if (!GetAllLogins(&forms))
-    return;
-
-  // Now switch to a profile-specific app string.
-  app_string_ = GetProfileSpecificAppString();
-
-  // Try to add all the logins with the new app string.
-  bool ok = true;
-  for (size_t i = 0; i < forms.size(); ++i) {
-    if (!RawAddLogin(*forms[i]))
-      ok = false;
-    delete forms[i];
-  }
-
-  if (ok) {
-    // All good! Keep the new app string and set a persistent pref.
-    // NOTE: We explicitly don't delete the old passwords yet. They are
-    // potentially shared with other profiles and other user data dirs!
-    // Each other profile must be able to migrate the shared data as well,
-    // so we must leave it alone. After a few releases, we'll add code to
-    // delete them, and eventually remove this migration code.
-    // TODO(mdm): follow through with the plan above.
-    PasswordStoreX::SetPasswordsUseLocalProfileId(prefs_);
-  } else {
-    // We failed to migrate for some reason. Use the old app string.
-    app_string_ = kGnomeKeyringAppString;
-  }
-}

chrome/browser/password_manager/native_backend_gnome_x.h

@@ -15,8 +15,6 @@
 #include "chrome/browser/password_manager/password_store_x.h"
 #include "chrome/browser/profiles/profile.h"
 
-class PrefService;
-
 namespace autofill {
 struct PasswordForm;
 }
@@ -76,7 +74,7 @@ class GnomeKeyringLoader {
 class NativeBackendGnome : public PasswordStoreX::NativeBackend,
                            public GnomeKeyringLoader {
  public:
-  NativeBackendGnome(LocalProfileId id, PrefService* prefs);
+  explicit NativeBackendGnome(LocalProfileId id);
 
   virtual ~NativeBackendGnome();
 
@@ -109,21 +107,12 @@ class NativeBackendGnome : public PasswordStoreX::NativeBackend,
   // Generates a profile-specific app string based on profile_id_.
   std::string GetProfileSpecificAppString() const;
 
-  // Migrates non-profile-specific logins to be profile-specific.
-  void MigrateToProfileSpecificLogins();
-
   // The local profile id, used to generate the app string.
   const LocalProfileId profile_id_;
 
-  // The pref service to use for persistent migration settings.
-  PrefService* prefs_;
-
   // The app string, possibly based on the local profile id.
   std::string app_string_;
 
-  // True once MigrateToProfileSpecificLogins() has been attempted.
-  bool migrate_tried_;
-
   DISALLOW_COPY_AND_ASSIGN(NativeBackendGnome);
 };
 

chrome/browser/password_manager/native_backend_kwallet_x.cc

@@ -102,23 +102,11 @@ void LogDeserializationWarning(int version,
 
 }  // namespace
 
-NativeBackendKWallet::NativeBackendKWallet(LocalProfileId id,
-                                           PrefService* prefs)
+NativeBackendKWallet::NativeBackendKWallet(LocalProfileId id)
     : profile_id_(id),
-      prefs_(prefs),
       kwallet_proxy_(NULL),
       app_name_(l10n_util::GetStringUTF8(IDS_PRODUCT_NAME)) {
-  // TODO(mdm): after a few more releases, remove the code which is now dead due
-  // to the true || here, and simplify this code. We don't do it yet to make it
-  // easier to revert if necessary.
-  if (true || PasswordStoreX::PasswordsUseLocalProfileId(prefs)) {
-    folder_name_ = GetProfileSpecificFolderName();
-    // We already did the migration previously. Don't try again.
-    migrate_tried_ = true;
-  } else {
-    folder_name_ = kKWalletFolder;
-    migrate_tried_ = false;
-  }
+  folder_name_ = GetProfileSpecificFolderName();
 }
 
 NativeBackendKWallet::~NativeBackendKWallet() {
@@ -926,10 +914,6 @@ int NativeBackendKWallet::WalletHandle() {
     }
   }
 
-  // Successful initialization. Try migration if necessary.
-  if (!migrate_tried_)
-    MigrateToProfileSpecificLogins();
-
   return handle;
 }
 
@@ -938,59 +922,3 @@ std::string NativeBackendKWallet::GetProfileSpecificFolderName() const {
   // Now we use it to distinguish passwords for different profiles.
   return base::StringPrintf("%s (%d)", kKWalletFolder, profile_id_);
 }
-
-void NativeBackendKWallet::MigrateToProfileSpecificLogins() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-
-  DCHECK(!migrate_tried_);
-  DCHECK_EQ(folder_name_, kKWalletFolder);
-
-  // Record the fact that we've attempted migration already right away, so that
-  // we don't get recursive calls back to MigrateToProfileSpecificLogins().
-  migrate_tried_ = true;
-
-  // First get all the logins, using the old folder name.
-  int wallet_handle = WalletHandle();
-  if (wallet_handle == kInvalidKWalletHandle)
-    return;
-  PasswordFormList forms;
-  if (!GetAllLogins(&forms, wallet_handle))
-    return;
-
-  // Now switch to a profile-specific folder name.
-  folder_name_ = GetProfileSpecificFolderName();
-
-  // Try to add all the logins with the new folder name.
-  // This could be done more efficiently by grouping by signon realm and using
-  // SetLoginsList(), but we do this for simplicity since it is only done once.
-  // Note, however, that we do need another call to WalletHandle() to create
-  // this folder if necessary.
-  bool ok = true;
-  for (size_t i = 0; i < forms.size(); ++i) {
-    if (!AddLogin(*forms[i]))
-      ok = false;
-    delete forms[i];
-  }
-  if (forms.empty()) {
-    // If there were no logins to migrate, we do an extra call to WalletHandle()
-    // for its side effect of attempting to create the profile-specific folder.
-    // This is not strictly necessary, but it's safe and helps in testing.
-    wallet_handle = WalletHandle();
-    if (wallet_handle == kInvalidKWalletHandle)
-      ok = false;
-  }
-
-  if (ok) {
-    // All good! Keep the new app string and set a persistent pref.
-    // NOTE: We explicitly don't delete the old passwords yet. They are
-    // potentially shared with other profiles and other user data dirs!
-    // Each other profile must be able to migrate the shared data as well,
-    // so we must leave it alone. After a few releases, we'll add code to
-    // delete them, and eventually remove this migration code.
-    // TODO(mdm): follow through with the plan above.
-    PasswordStoreX::SetPasswordsUseLocalProfileId(prefs_);
-  } else {
-    // We failed to migrate for some reason. Use the old folder name.
-    folder_name_ = kKWalletFolder;
-  }
-}

chrome/browser/password_manager/native_backend_kwallet_x.h

@@ -16,7 +16,6 @@
 
 class Pickle;
 class PickleIterator;
-class PrefService;
 
 namespace autofill {
 struct PasswordForm;
@@ -34,7 +33,7 @@ class ObjectProxy;
 // NativeBackend implementation using KWallet.
 class NativeBackendKWallet : public PasswordStoreX::NativeBackend {
  public:
-  NativeBackendKWallet(LocalProfileId id, PrefService* prefs);
+  explicit NativeBackendKWallet(LocalProfileId id);
 
   virtual ~NativeBackendKWallet();
 
@@ -131,21 +130,12 @@ class NativeBackendKWallet : public PasswordStoreX::NativeBackend {
   // Generates a profile-specific folder name based on profile_id_.
   std::string GetProfileSpecificFolderName() const;
 
-  // Migrates non-profile-specific logins to be profile-specific.
-  void MigrateToProfileSpecificLogins();
-
   // The local profile id, used to generate the folder name.
   const LocalProfileId profile_id_;
 
-  // The pref service to use for persistent migration settings.
-  PrefService* prefs_;
-
   // The KWallet folder name, possibly based on the local profile id.
   std::string folder_name_;
 
-  // True once MigrateToProfileSpecificLogins() has been attempted.
-  bool migrate_tried_;
-
   // DBus handle for communication with klauncher and kwalletd.
   scoped_refptr<dbus::Bus> session_bus_;
   // Object proxy for kwalletd. We do not own this.

chrome/browser/password_manager/password_store_factory.cc

@@ -187,7 +187,7 @@ KeyedService* PasswordStoreFactory::BuildServiceInstanceFor(
   if (desktop_env == base::nix::DESKTOP_ENVIRONMENT_KDE4) {
     // KDE3 didn't use DBus, which our KWallet store uses.
     VLOG(1) << "Trying KWallet for password storage.";
-    backend.reset(new NativeBackendKWallet(id, prefs));
+    backend.reset(new NativeBackendKWallet(id));
     if (backend->Init())
       VLOG(1) << "Using KWallet for password storage.";
     else
@@ -197,7 +197,7 @@ KeyedService* PasswordStoreFactory::BuildServiceInstanceFor(
              desktop_env == base::nix::DESKTOP_ENVIRONMENT_XFCE) {
 #if defined(USE_GNOME_KEYRING)
     VLOG(1) << "Trying GNOME keyring for password storage.";
-    backend.reset(new NativeBackendGnome(id, prefs));
+    backend.reset(new NativeBackendGnome(id));
     if (backend->Init())
       VLOG(1) << "Using GNOME keyring for password storage.";
     else
@@ -233,14 +233,12 @@ KeyedService* PasswordStoreFactory::BuildServiceInstanceFor(
 void PasswordStoreFactory::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
 #if !defined(OS_CHROMEOS) && defined(USE_X11)
+  // Notice that the preprocessor conditions above are exactly those that will
+  // result in using PasswordStoreX in BuildServiceInstanceFor().
   registry->RegisterIntegerPref(
       prefs::kLocalProfileId,
       kInvalidLocalProfileId,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
-
-  // Notice that the preprocessor conditions above are exactly those that will
-  // result in using PasswordStoreX in CreatePasswordStore() below.
-  PasswordStoreX::RegisterProfilePrefs(registry);
 #endif
 }
 

chrome/browser/password_manager/password_store_x.cc

@@ -260,42 +260,3 @@ ssize_t PasswordStoreX::MigrateLogins() {
   STLDeleteElements(&forms);
   return result;
 }
-
-#if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
-// static
-void PasswordStoreX::RegisterProfilePrefs(
-    user_prefs::PrefRegistrySyncable* registry) {
-  // Normally we should be on the UI thread here, but in tests we might not.
-  registry->RegisterBooleanPref(
-      prefs::kPasswordsUseLocalProfileId,
-      // default: passwords don't use local ids
-      false,
-      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
-}
-
-// static
-bool PasswordStoreX::PasswordsUseLocalProfileId(PrefService* prefs) {
-  // Normally we should be on the UI thread here, but in tests we might not.
-  return prefs->GetBoolean(prefs::kPasswordsUseLocalProfileId);
-}
-
-namespace {
-// This function is a hack to do something not entirely thread safe: the pref
-// service comes from the UI thread, but it's not ref counted. We keep a pointer
-// to it on the DB thread, and need to invoke a method on the UI thread. This
-// function does that for us without requiring ref counting the pref service.
-// TODO(mdm): Fix this if it becomes a problem. Given that this function will
-// be called once ever per profile, it probably will not cause a problem...
-void UISetPasswordsUseLocalProfileId(PrefService* prefs) {
-  prefs->SetBoolean(prefs::kPasswordsUseLocalProfileId, true);
-}
-}  // anonymous namespace
-
-// static
-void PasswordStoreX::SetPasswordsUseLocalProfileId(PrefService* prefs) {
-  // This method should work on any thread, but we expect the DB thread.
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::Bind(&UISetPasswordsUseLocalProfileId, prefs));
-}
-#endif  // !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)

chrome/browser/password_manager/password_store_x.h

@@ -58,19 +58,6 @@ class PasswordStoreX : public PasswordStoreDefault {
                  LoginDatabase* login_db,
                  NativeBackend* backend);
 
-#if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
-  // Registers the pref setting used for the methods below.
-  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
-
-  // Returns true if passwords have been tagged with the local profile id.
-  static bool PasswordsUseLocalProfileId(PrefService* prefs);
-
-  // Sets the persistent bit indicating that passwords have been tagged with the
-  // local profile id. This cannot be unset; passwords get migrated only once.
-  // The caller promises that |prefs| will not be deleted any time soon.
-  static void SetPasswordsUseLocalProfileId(PrefService* prefs);
-#endif  // !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
-
  private:
   friend class PasswordStoreXTest;
 

components/password_manager/core/common/password_manager_pref_names.cc

@@ -35,11 +35,6 @@ const char kPasswordManagerGroupsForDomains[] =
 #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
 // The local profile id for this profile.
 const char kLocalProfileId[] = "profile.local_profile_id";
-
-// Whether passwords in external services (e.g. GNOME Keyring) have been tagged
-// with the local profile id yet. (Used for migrating to tagged passwords.)
-const char kPasswordsUseLocalProfileId[] =
-    "profile.passwords_use_local_profile_id";
 #endif
 
 }  // namespace prefs

components/password_manager/core/common/password_manager_pref_names.h

@@ -21,7 +21,6 @@ extern const char kPasswordManagerGroupsForDomains[];
 
 #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS) && defined(OS_POSIX)
 extern const char kLocalProfileId[];
-extern const char kPasswordsUseLocalProfileId[];
 #endif
 
 }  // namespace prefs