PasswordStoreMac::RemoveLoginsCreatedBetween() shouldn't affect other profiles.

chrome/browser/password_manager/password_store_mac.cc

Index: chrome/browser/password_manager/password_store_mac.cc
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index 5aa39bde5d87eff87ad68557d5ea3a3ea7ff9cc4..63359dbbc5576ee7560519e4dc25ff57d490b581 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -15,6 +15,7 @@
 #include "base/logging.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/mac_util.h"
+#include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
@@ -959,24 +960,12 @@ PasswordStoreChangeList PasswordStoreMac::RemoveLoginsCreatedBetweenImpl(
     base::Time delete_begin,
     base::Time delete_end) {
   PasswordStoreChangeList changes;
-  std::vector<PasswordForm*> forms;
+  ScopedVector<PasswordForm> forms;

[vabr (Chromium), 2014/07/02 13:15:55]

It's great that you caught the leak.
Looking at the code of LoginDatabase::GetLogins* and similar methods in
LoginDatabase which return a vector of PasswordForm*, they all pass the
ownership to the caller.
Could you please add comment to the declarations of those methods to make that
clear, so that people are less likely to reintroduce the same error again?

[vasilii, 2014/07/02 14:02:58]

Done.

   if (login_metadata_db_->GetLoginsCreatedBetween(delete_begin, delete_end,
-                                                  &forms)) {
+                                                  &forms.get())) {
     if (login_metadata_db_->RemoveLoginsCreatedBetween(delete_begin,
                                                        delete_end)) {
-      // We can't delete from the Keychain by date because we may be sharing
-      // items with database entries that weren't in the delete range. Instead,
-      // we find all the Keychain items we own but aren't using any more and
-      // delete those.
-      std::vector<PasswordForm*> orphan_keychain_forms =
-          GetUnusedKeychainForms();
-      // This is inefficient, since we have to re-look-up each keychain item
-      // one at a time to delete it even though the search step already had a
-      // list of Keychain item references. If this turns out to be noticeably
-      // slow we'll need to rearchitect to allow the search and deletion steps
-      // to share.
-      RemoveKeychainForms(orphan_keychain_forms);
-      STLDeleteElements(&orphan_keychain_forms);
+      RemoveKeychainForms(forms.get());
 
       for (std::vector<PasswordForm*>::const_iterator it = forms.begin();
            it != forms.end(); ++it) {
@@ -993,24 +982,12 @@ PasswordStoreChangeList PasswordStoreMac::RemoveLoginsSyncedBetweenImpl(
     base::Time delete_begin,
     base::Time delete_end) {
   PasswordStoreChangeList changes;
-  std::vector<PasswordForm*> forms;
+  ScopedVector<PasswordForm> forms;
   if (login_metadata_db_->GetLoginsSyncedBetween(
-          delete_begin, delete_end, &forms)) {
+          delete_begin, delete_end, &forms.get())) {
     if (login_metadata_db_->RemoveLoginsSyncedBetween(delete_begin,
                                                       delete_end)) {
-      // We can't delete from the Keychain by date because we may be sharing
-      // items with database entries that weren't in the delete range. Instead,
-      // we find all the Keychain items we own but aren't using any more and
-      // delete those.
-      std::vector<PasswordForm*> orphan_keychain_forms =
-          GetUnusedKeychainForms();
-      // This is inefficient, since we have to re-look-up each keychain item
-      // one at a time to delete it even though the search step already had a
-      // list of Keychain item references. If this turns out to be noticeably
-      // slow we'll need to rearchitect to allow the search and deletion steps
-      // to share.
-      RemoveKeychainForms(orphan_keychain_forms);
-      STLDeleteElements(&orphan_keychain_forms);
+      RemoveKeychainForms(forms.get());
 
       for (std::vector<PasswordForm*>::const_iterator it = forms.begin();
            it != forms.end();
@@ -1146,27 +1123,6 @@ bool PasswordStoreMac::DatabaseHasFormMatchingKeychainForm(
   return has_match;
 }
 
-std::vector<PasswordForm*> PasswordStoreMac::GetUnusedKeychainForms() {
-  std::vector<PasswordForm*> database_forms;
-  login_metadata_db_->GetAutofillableLogins(&database_forms);
-
-  MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get());
-  owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
-  std::vector<PasswordForm*> owned_keychain_forms =
-      owned_keychain_adapter.GetAllPasswordFormPasswords();
-
-  // Run a merge; anything left in owned_keychain_forms when we are done no
-  // longer has a matching database entry.
-  std::vector<PasswordForm*> merged_forms;
-  internal_keychain_helpers::MergePasswordForms(&owned_keychain_forms,
-                                                &database_forms,
-                                                &merged_forms);
-  STLDeleteElements(&merged_forms);
-  STLDeleteElements(&database_forms);
-
-  return owned_keychain_forms;
-}
-
 void PasswordStoreMac::RemoveDatabaseForms(
     const std::vector<PasswordForm*>& forms) {
   for (std::vector<PasswordForm*>::const_iterator i = forms.begin();