PasswordStore: Use ScopedVector to express ownership of forms

chrome/browser/password_manager/password_store_mac.cc

 // Copyright (c) 2012 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_store_mac.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
 
 #include <CoreServices/CoreServices.h>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_logging.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"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/mac/security_wrappers.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_store_change.h"
 #include "content/public/browser/browser_thread.h"
 #include "crypto/apple_keychain.h"
 
@@ skipping 153 matching lines @@
 PasswordStoreChangeList FormsToRemoveChangeList(
     const std::vector<PasswordForm*>& forms) {
   PasswordStoreChangeList changes;
   for (std::vector<PasswordForm*>::const_iterator i = forms.begin();
        i != forms.end(); ++i) {
     changes.push_back(PasswordStoreChange(PasswordStoreChange::REMOVE, **i));
   }
   return changes;
 }
 
+// Moves the content of |second| to the end of |first|.
+void AppendSecondToFirst(ScopedVector<autofill::PasswordForm>* first,
+                         ScopedVector<autofill::PasswordForm>* second) {
+  first->insert(first->end(), second->begin(), second->end());
+  second->weak_clear();
+}
+
 }  // namespace
 
 #pragma mark -
 
 // TODO(stuartmorgan): Convert most of this to private helpers in
 // MacKeychainPasswordFormAdapter once it has sufficient higher-level public
 // methods to provide test coverage.
 namespace internal_keychain_helpers {
 
 // Returns a URL built from the given components. To create a URL without a
@@ skipping 209 matching lines @@
       if (base_form.origin == (*i)->origin) {
         return *i;
       } else if (!partial_match) {
         partial_match = *i;
       }
     }
   }
   return partial_match;
 }
 
-// Returns entries from |forms| that are blacklist entries, after removing
-// them from |forms|.
-std::vector<PasswordForm*> ExtractBlacklistForms(
-    std::vector<PasswordForm*>* forms) {
-  std::vector<PasswordForm*> blacklist_forms;
-  for (std::vector<PasswordForm*>::iterator i = forms->begin();
-       i != forms->end();) {
-    PasswordForm* form = *i;
-    if (form->blacklisted_by_user) {
-      blacklist_forms.push_back(form);
-      i = forms->erase(i);
-    } else {
-      ++i;
-    }
+// Moves entries from |forms| that are blacklist entries into |blacklist|.
+void ExtractBlacklistForms(ScopedVector<autofill::PasswordForm>* forms,
+                           ScopedVector<autofill::PasswordForm>* blacklist) {
+  blacklist->reserve(blacklist->size() + forms->size());
+  ScopedVector<autofill::PasswordForm> non_blacklist;
+  for (auto& form : *forms) {
+    if (form->blacklisted_by_user)
+      blacklist->push_back(form);
+    else
+      non_blacklist.push_back(form);
+    form = nullptr;
   }
-  return blacklist_forms;
+  forms->swap(non_blacklist);
 }
 
-// Deletes and removes from v any element that exists in s.
-template <class T>
-void DeleteVectorElementsInSet(std::vector<T*>* v, const std::set<T*>& s) {
-  for (typename std::vector<T*>::iterator i = v->begin(); i != v->end();) {
-    T* element = *i;
-    if (s.find(element) != s.end()) {
-      delete element;
-      i = v->erase(i);
-    } else {
-      ++i;
-    }
-  }
-}
-
-void MergePasswordForms(std::vector<PasswordForm*>* keychain_forms,
-                        std::vector<PasswordForm*>* database_forms,
-                        std::vector<PasswordForm*>* merged_forms) {
+// Takes |keychain_forms| and |database_forms| and moves the following 2 types
+// of forms to |merged_forms|: (1) blacklisted |database_forms|, (2)
+// |database_forms| which have a corresponding entry in |keychain_forms|. The
+// database forms of type (2) have their password value updated from the
+// corresponding keychain form, and all the keychain forms corresponding to some
+// database form are removed from |keychain_forms| and deleted.
+void MergePasswordForms(ScopedVector<autofill::PasswordForm>* keychain_forms,
+                        ScopedVector<autofill::PasswordForm>* database_forms,
+                        ScopedVector<autofill::PasswordForm>* merged_forms) {
   // Pull out the database blacklist items, since they are used as-is rather
   // than being merged with keychain forms.
-  std::vector<PasswordForm*> database_blacklist_forms =
-      ExtractBlacklistForms(database_forms);
+  ExtractBlacklistForms(database_forms, merged_forms);
 
   // Merge the normal entries.
-  std::set<PasswordForm*> used_keychain_forms;
-  for (std::vector<PasswordForm*>::iterator i = database_forms->begin();
-       i != database_forms->end();) {
-    PasswordForm* db_form = *i;
-    PasswordForm* best_match = BestKeychainFormForForm(*db_form,
-                                                       keychain_forms);
+  ScopedVector<autofill::PasswordForm> unused_database_forms;
+  unused_database_forms.reserve(database_forms->size());
+  std::set<autofill::PasswordForm*> used_keychain_forms;
+  for (auto& db_form : *database_forms) {
+    PasswordForm* best_match =
+        BestKeychainFormForForm(*db_form, &keychain_forms->get());
     if (best_match) {
       used_keychain_forms.insert(best_match);
       db_form->password_value = best_match->password_value;
       merged_forms->push_back(db_form);
-      i = database_forms->erase(i);
     } else {
-      ++i;
+      unused_database_forms.push_back(db_form);
+    }
+    db_form = nullptr;
+  }
+  database_forms->swap(unused_database_forms);
+
+  // Clear out all the Keychain entries we used.
+  ScopedVector<autofill::PasswordForm> unused_keychain_forms;
+  unused_keychain_forms.reserve(keychain_forms->size());
+  for (auto& keychain_form : *keychain_forms) {
+    if (!ContainsKey(used_keychain_forms, keychain_form)) {
+      unused_keychain_forms.push_back(keychain_form);
+      keychain_form = nullptr;
     }
   }
-
-  // Add in the blacklist entries from the database.
-  merged_forms->insert(merged_forms->end(),
-                       database_blacklist_forms.begin(),
-                       database_blacklist_forms.end());
-
-  // Clear out all the Keychain entries we used.
-  DeleteVectorElementsInSet(keychain_forms, used_keychain_forms);
+  keychain_forms->swap(unused_keychain_forms);
 }
 
 std::vector<ItemFormPair> ExtractAllKeychainItemAttributesIntoPasswordForms(
     std::vector<SecKeychainItemRef>* keychain_items,
     const AppleKeychain& keychain) {
   DCHECK(keychain_items);
   MacKeychainPasswordFormAdapter keychain_adapter(&keychain);
   *keychain_items = keychain_adapter.GetAllPasswordFormKeychainItems();
   std::vector<ItemFormPair> item_form_pairs;
   for (std::vector<SecKeychainItemRef>::iterator i = keychain_items->begin();
        i != keychain_items->end(); ++i) {
     PasswordForm* form_without_password = new PasswordForm();
     internal_keychain_helpers::FillPasswordFormFromKeychainItem(
         keychain,
         *i,
         form_without_password,
         false);  // Load password attributes, but not password data.
     item_form_pairs.push_back(std::make_pair(&(*i), form_without_password));
   }
   return item_form_pairs;
 }
 
-std::vector<PasswordForm*> GetPasswordsForForms(
-    const AppleKeychain& keychain,
-    std::vector<PasswordForm*>* database_forms) {
+void GetPasswordsForForms(const AppleKeychain& keychain,
+                          ScopedVector<autofill::PasswordForm>* database_forms,
+                          ScopedVector<autofill::PasswordForm>* passwords) {
   // First load the attributes of all items in the keychain without loading
   // their password data, and then match items in |database_forms| against them.
   // This avoids individually searching through the keychain for passwords
   // matching each form in |database_forms|, and results in a significant
   // performance gain, replacing O(N) keychain search operations with a single
   // operation that loads all keychain items, and then selective reads of only
   // the relevant passwords. See crbug.com/263685.
   std::vector<SecKeychainItemRef> keychain_items;
   std::vector<ItemFormPair> item_form_pairs =
       ExtractAllKeychainItemAttributesIntoPasswordForms(&keychain_items,
                                                         keychain);
 
   // Next, compare the attributes of the PasswordForms in |database_forms|
   // against those in |item_form_pairs|, and extract password data for each
   // matching PasswordForm using its corresponding SecKeychainItemRef.
-  std::vector<PasswordForm*> merged_forms;
-  for (std::vector<PasswordForm*>::iterator i = database_forms->begin();
-       i != database_forms->end();) {
-    std::vector<PasswordForm*> db_form_container(1, *i);
-    std::vector<PasswordForm*> keychain_matches =
-        ExtractPasswordsMergeableWithForm(keychain, item_form_pairs, **i);
-    MergePasswordForms(&keychain_matches, &db_form_container, &merged_forms);
-    if (db_form_container.empty()) {
-      i = database_forms->erase(i);
-    } else {
-      ++i;
-    }
-    STLDeleteElements(&keychain_matches);
+  ScopedVector<autofill::PasswordForm> unused_db_forms;
+  unused_db_forms.reserve(database_forms->size());
+  for (auto& db_form : *database_forms) {
+    ScopedVector<autofill::PasswordForm> keychain_matches(

[vasilii, 2015/01/28 15:09:03]

nit: ScopedVector<autofill::PasswordForm> keychain_matches = ... would be easier
to read while being semantically identical.

[vabr (Chromium), 2015/01/28 16:36:27]

Done.

You are right: Both the r-value constructor and the operator= of ScopedVector do
the same swap(). Generally, I prefer initialising variables through constructors
rather than assignments, because assignment always means -- first assign default
values to the data members, then replace them -- whereas constructors could
potentially skip the default-value step. However, here there is no distinction,
so I'll rewrite it to assignment.

[vasilii, 2015/01/28 17:27:22]

There is no operator= here. "MyObject o = something" invokes the copy
constructor. This is just a difference in syntax.

+        ExtractPasswordsMergeableWithForm(keychain, item_form_pairs, *db_form));
+
+    ScopedVector<autofill::PasswordForm> db_form_container;
+    db_form_container.push_back(db_form);
+    db_form = nullptr;
+    MergePasswordForms(&keychain_matches, &db_form_container, passwords);
+    unused_db_forms.insert(unused_db_forms.end(), db_form_container.begin(),
+                           db_form_container.end());
+    db_form_container.weak_clear();
   }
+  database_forms->swap(unused_db_forms);
 
-  // Clean up temporary PasswordForms and SecKeychainItemRefs.
   STLDeleteContainerPairSecondPointers(item_form_pairs.begin(),
                                        item_form_pairs.end());
-  for (std::vector<SecKeychainItemRef>::iterator i = keychain_items.begin();
-       i != keychain_items.end(); ++i) {
-    keychain.Free(*i);
+  for (SecKeychainItemRef item : keychain_items) {
+    keychain.Free(item);
   }
-  return merged_forms;
 }
 
 // TODO(stuartmorgan): signon_realm for proxies is not yet supported.
 bool ExtractSignonRealmComponents(const std::string& signon_realm,
                                   std::string* server,
                                   UInt32* port,
                                   bool* is_secure,
                                   std::string* security_domain) {
   // The signon_realm will be the Origin portion of a URL for an HTML form,
   // and the same but with the security domain as a path for HTTP auth.
@@ skipping 25 matching lines @@
   UInt32 port;
   bool is_secure;
   if (!ExtractSignonRealmComponents(query_form.signon_realm, &server, &port,
                                     &is_secure, &security_domain)) {
     return false;
   }
   return internal_keychain_helpers::FormsMatchForMerge(
       query_form, other_form, STRICT_FORM_MATCH);
 }
 
-std::vector<PasswordForm*> ExtractPasswordsMergeableWithForm(
+ScopedVector<autofill::PasswordForm> ExtractPasswordsMergeableWithForm(
     const AppleKeychain& keychain,
     const std::vector<ItemFormPair>& item_form_pairs,
     const PasswordForm& query_form) {
-  std::vector<PasswordForm*> matches;
+  ScopedVector<autofill::PasswordForm> matches;
   for (std::vector<ItemFormPair>::const_iterator i = item_form_pairs.begin();
        i != item_form_pairs.end(); ++i) {
     if (FormIsValidAndMatchesOtherForm(query_form, *(i->second))) {
       // Create a new object, since the caller is responsible for deleting the
       // returned forms.
       scoped_ptr<PasswordForm> form_with_password(new PasswordForm());
       internal_keychain_helpers::FillPasswordFormFromKeychainItem(
-          keychain,
-          *(i->first),
-          form_with_password.get(),
+          keychain, *(i->first), form_with_password.get(),
           true);  // Load password attributes and data.
       // Do not include blacklisted items found in the keychain.
       if (!form_with_password->blacklisted_by_user)
         matches.push_back(form_with_password.release());
     }
   }
-  return matches;
+  return matches.Pass();
 }
 
 }  // namespace internal_keychain_helpers
 
 #pragma mark -
 
 MacKeychainPasswordFormAdapter::MacKeychainPasswordFormAdapter(
     const AppleKeychain* keychain)
     : keychain_(keychain), finds_only_owned_(false) {
 }
 
-std::vector<PasswordForm*> MacKeychainPasswordFormAdapter::PasswordsFillingForm(
+ScopedVector<autofill::PasswordForm>
+MacKeychainPasswordFormAdapter::PasswordsFillingForm(
     const std::string& signon_realm,
     PasswordForm::Scheme scheme) {
   std::vector<SecKeychainItemRef> keychain_items =
       MatchingKeychainItems(signon_realm, scheme, NULL, NULL);
-
   return ConvertKeychainItemsToForms(&keychain_items);
 }
 
 bool MacKeychainPasswordFormAdapter::HasPasswordExactlyMatchingForm(
     const PasswordForm& query_form) {
   SecKeychainItemRef keychain_item = KeychainItemForForm(query_form);
   if (keychain_item) {
     keychain_->Free(keychain_item);
     return true;
   }
@@ skipping 32 matching lines @@
                          &supported_auth_types[i],
                          NULL,
                          NULL,
                          NULL,
                          creator ? &creator : NULL);
     keychain_search.FindMatchingItems(&matches);
   }
   return matches;
 }
 
-std::vector<PasswordForm*>
-    MacKeychainPasswordFormAdapter::GetAllPasswordFormPasswords() {
+ScopedVector<autofill::PasswordForm>
+MacKeychainPasswordFormAdapter::GetAllPasswordFormPasswords() {
   std::vector<SecKeychainItemRef> items = GetAllPasswordFormKeychainItems();
   return ConvertKeychainItemsToForms(&items);
 }
 
 bool MacKeychainPasswordFormAdapter::AddPassword(const PasswordForm& form) {
   // We should never be trying to store a blacklist in the keychain.
   DCHECK(!form.blacklisted_by_user);
 
   std::string server;
   std::string security_domain;
@@ skipping 42 matching lines @@
   OSStatus result = keychain_->ItemDelete(keychain_item);
   keychain_->Free(keychain_item);
   return result == noErr;
 }
 
 void MacKeychainPasswordFormAdapter::SetFindsOnlyOwnedItems(
     bool finds_only_owned) {
   finds_only_owned_ = finds_only_owned;
 }
 
-std::vector<PasswordForm*>
-    MacKeychainPasswordFormAdapter::ConvertKeychainItemsToForms(
-        std::vector<SecKeychainItemRef>* items) {
-  std::vector<PasswordForm*> keychain_forms;
-  for (std::vector<SecKeychainItemRef>::const_iterator i = items->begin();
-       i != items->end(); ++i) {
-    PasswordForm* form = new PasswordForm();
+ScopedVector<autofill::PasswordForm>
+MacKeychainPasswordFormAdapter::ConvertKeychainItemsToForms(
+    std::vector<SecKeychainItemRef>* items) {
+  ScopedVector<autofill::PasswordForm> forms;
+  for (SecKeychainItemRef item : *items) {
+    scoped_ptr<PasswordForm> form(new PasswordForm());
     if (internal_keychain_helpers::FillPasswordFormFromKeychainItem(
-            *keychain_, *i, form, true)) {
-      keychain_forms.push_back(form);
+            *keychain_, item, form.get(), true)) {
+      forms.push_back(form.release());
     }
-    keychain_->Free(*i);
+    keychain_->Free(item);
   }
   items->clear();
-  return keychain_forms;
+  return forms.Pass();
 }
 
 SecKeychainItemRef MacKeychainPasswordFormAdapter::KeychainItemForForm(
     const PasswordForm& form) {
   // We don't store blacklist entries in the keychain, so the answer to "what
   // Keychain item goes with this form" is always "nothing" for blacklists.
   if (form.blacklisted_by_user) {
     return NULL;
   }
 
@@ skipping 203 matching lines @@
 
     changes.push_back(PasswordStoreChange(PasswordStoreChange::REMOVE, form));
   }
   return changes;
 }
 
 PasswordStoreChangeList PasswordStoreMac::RemoveLoginsCreatedBetweenImpl(
     base::Time delete_begin,
     base::Time delete_end) {
   PasswordStoreChangeList changes;
-  ScopedVector<PasswordForm> forms;
+  ScopedVector<PasswordForm> forms_to_remove;
   if (login_metadata_db_ &&
       login_metadata_db_->GetLoginsCreatedBetween(delete_begin, delete_end,
-                                                  &forms.get()) &&
+                                                  &forms_to_remove) &&
       login_metadata_db_->RemoveLoginsCreatedBetween(delete_begin,
                                                      delete_end)) {
-    RemoveKeychainForms(forms.get());
-    CleanOrphanedForms(&forms.get());
-    changes = FormsToRemoveChangeList(forms.get());
+    RemoveKeychainForms(forms_to_remove.get());
+    CleanOrphanedForms(&forms_to_remove);  // Add the orphaned forms.
+    changes = FormsToRemoveChangeList(forms_to_remove.get());
     LogStatsForBulkDeletion(changes.size());
   }
   return changes;
 }
 
 PasswordStoreChangeList PasswordStoreMac::RemoveLoginsSyncedBetweenImpl(
     base::Time delete_begin,
     base::Time delete_end) {
   PasswordStoreChangeList changes;
-  ScopedVector<PasswordForm> forms;
+  ScopedVector<PasswordForm> forms_to_remove;
   if (login_metadata_db_ &&
       login_metadata_db_->GetLoginsSyncedBetween(delete_begin, delete_end,
-                                                 &forms.get()) &&
+                                                 &forms_to_remove) &&
       login_metadata_db_->RemoveLoginsSyncedBetween(delete_begin, delete_end)) {
-    RemoveKeychainForms(forms.get());
-    CleanOrphanedForms(&forms.get());
-    changes = FormsToRemoveChangeList(forms.get());
+    RemoveKeychainForms(forms_to_remove.get());
+    CleanOrphanedForms(&forms_to_remove);  // Add the orphaned forms_to_remove.
+    changes = FormsToRemoveChangeList(forms_to_remove.get());
     LogStatsForBulkDeletionDuringRollback(changes.size());
   }
   return changes;
 }
 
 void PasswordStoreMac::GetLoginsImpl(
     const autofill::PasswordForm& form,
     AuthorizationPromptPolicy prompt_policy,
     const ConsumerCallbackRunner& callback_runner) {
   chrome::ScopedSecKeychainSetUserInteractionAllowed user_interaction_allowed(
       prompt_policy == ALLOW_PROMPT);
 
   if (!login_metadata_db_) {
-    callback_runner.Run(std::vector<PasswordForm*>());
+    callback_runner.Run(ScopedVector<autofill::PasswordForm>());
     return;
   }
 
   ScopedVector<PasswordForm> database_forms;
-  login_metadata_db_->GetLogins(form, &database_forms.get());
+  login_metadata_db_->GetLogins(form, &database_forms);
 
   // Let's gather all signon realms we want to match with keychain entries.
   std::set<std::string> realm_set;
   realm_set.insert(form.signon_realm);
-  for (std::vector<PasswordForm*>::const_iterator db_form =
-           database_forms.begin();
-       db_form != database_forms.end();
-       ++db_form) {
+  for (const autofill::PasswordForm* db_form : database_forms) {
     // TODO(vabr): We should not be getting different schemes here.
     // http://crbug.com/340112
-    if (form.scheme != (*db_form)->scheme)
+    if (form.scheme != db_form->scheme)
       continue;  // Forms with different schemes never match.
-    const std::string& original_singon_realm((*db_form)->original_signon_realm);
+    const std::string& original_singon_realm(db_form->original_signon_realm);
     if (!original_singon_realm.empty())
       realm_set.insert(original_singon_realm);
   }
-  std::vector<PasswordForm*> keychain_forms;
+  ScopedVector<autofill::PasswordForm> keychain_forms;
   for (std::set<std::string>::const_iterator realm = realm_set.begin();
-       realm != realm_set.end();
-       ++realm) {
+       realm != realm_set.end(); ++realm) {
     MacKeychainPasswordFormAdapter keychain_adapter(keychain_.get());
-    std::vector<PasswordForm*> temp_keychain_forms =
-        keychain_adapter.PasswordsFillingForm(*realm, form.scheme);
-    keychain_forms.insert(keychain_forms.end(),
-                          temp_keychain_forms.begin(),
-                          temp_keychain_forms.end());
+    ScopedVector<autofill::PasswordForm> temp_keychain_forms(
+        keychain_adapter.PasswordsFillingForm(*realm, form.scheme));
+    AppendSecondToFirst(&keychain_forms, &temp_keychain_forms);
   }
 
-  std::vector<PasswordForm*> matched_forms;
-  internal_keychain_helpers::MergePasswordForms(&keychain_forms,
-                                                &database_forms.get(),
-                                                &matched_forms);
+  ScopedVector<autofill::PasswordForm> matched_forms;
+  internal_keychain_helpers::MergePasswordForms(
+      &keychain_forms, &database_forms, &matched_forms);
 
   // Strip any blacklist entries out of the unused Keychain array, then take
   // all the entries that are left (which we can use as imported passwords).
   ScopedVector<PasswordForm> keychain_blacklist_forms;
-  internal_keychain_helpers::ExtractBlacklistForms(&keychain_forms).swap(
-      keychain_blacklist_forms.get());
+  internal_keychain_helpers::ExtractBlacklistForms(&keychain_forms,
+                                                   &keychain_blacklist_forms);
   matched_forms.insert(matched_forms.end(),
                        keychain_forms.begin(),
                        keychain_forms.end());
-  keychain_forms.clear();
+  keychain_forms.weak_clear();
 
   if (!database_forms.empty()) {
     RemoveDatabaseForms(database_forms.get());
     NotifyLoginsChanged(FormsToRemoveChangeList(database_forms.get()));
   }
 
-  callback_runner.Run(matched_forms);
+  callback_runner.Run(matched_forms.Pass());
 }
 
 void PasswordStoreMac::GetBlacklistLoginsImpl(GetLoginsRequest* request) {
-  FillBlacklistLogins(request->result());
+  ScopedVector<autofill::PasswordForm> obtained_forms;
+  FillBlacklistLogins(&obtained_forms);
+  request->result()->swap(obtained_forms.get());
   ForwardLoginsResult(request);
 }
 
 void PasswordStoreMac::GetAutofillableLoginsImpl(GetLoginsRequest* request) {
-  FillAutofillableLogins(request->result());
+  ScopedVector<autofill::PasswordForm> obtained_forms;
+  FillAutofillableLogins(&obtained_forms);
+  request->result()->swap(obtained_forms.get());
   ForwardLoginsResult(request);
 }
 
 bool PasswordStoreMac::FillAutofillableLogins(
-         std::vector<PasswordForm*>* forms) {
+    ScopedVector<autofill::PasswordForm>* forms) {
   DCHECK(thread_->message_loop() == base::MessageLoop::current());
 
   ScopedVector<PasswordForm> database_forms;
   if (!login_metadata_db_ ||
-      !login_metadata_db_->GetAutofillableLogins(&database_forms.get()))
+      !login_metadata_db_->GetAutofillableLogins(&database_forms))
     return false;
 
-  std::vector<PasswordForm*> merged_forms =
-      internal_keychain_helpers::GetPasswordsForForms(*keychain_,
-                                                      &database_forms.get());
+  internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms,
+                                                  forms);
 
   if (!database_forms.empty()) {
     RemoveDatabaseForms(database_forms.get());
     NotifyLoginsChanged(FormsToRemoveChangeList(database_forms.get()));
   }
 
-  forms->insert(forms->end(), merged_forms.begin(), merged_forms.end());
   return true;
 }
 
 bool PasswordStoreMac::FillBlacklistLogins(
-         std::vector<PasswordForm*>* forms) {
+    ScopedVector<autofill::PasswordForm>* forms) {
   DCHECK(thread_->message_loop() == base::MessageLoop::current());
   return login_metadata_db_ && login_metadata_db_->GetBlacklistLogins(forms);
 }
 
 bool PasswordStoreMac::AddToKeychainIfNecessary(const PasswordForm& form) {
   if (form.blacklisted_by_user) {
     return true;
   }
   MacKeychainPasswordFormAdapter keychainAdapter(keychain_.get());
   return keychainAdapter.AddPassword(form);
 }
 
 bool PasswordStoreMac::DatabaseHasFormMatchingKeychainForm(
     const autofill::PasswordForm& form) {
   DCHECK(login_metadata_db_);
   bool has_match = false;
-  std::vector<PasswordForm*> database_forms;
+  ScopedVector<autofill::PasswordForm> database_forms;
   login_metadata_db_->GetLogins(form, &database_forms);
-  for (std::vector<PasswordForm*>::iterator i = database_forms.begin();
-       i != database_forms.end(); ++i) {
+  for (const autofill::PasswordForm* db_form : database_forms) {
     // Below we filter out forms with non-empty original_signon_realm, because
     // those signal fuzzy matches, and we are only interested in exact ones.
-    if ((*i)->original_signon_realm.empty() &&
+    if (db_form->original_signon_realm.empty() &&
         internal_keychain_helpers::FormsMatchForMerge(
-            form, **i, internal_keychain_helpers::STRICT_FORM_MATCH) &&
-        (*i)->origin == form.origin) {
+            form, *db_form, internal_keychain_helpers::STRICT_FORM_MATCH) &&
+        db_form->origin == form.origin) {
       has_match = true;
       break;
     }
   }
-  STLDeleteElements(&database_forms);
   return has_match;
 }
 
 void PasswordStoreMac::RemoveDatabaseForms(
     const std::vector<PasswordForm*>& forms) {
   DCHECK(login_metadata_db_);
   for (std::vector<PasswordForm*>::const_iterator i = forms.begin();
        i != forms.end(); ++i) {
     login_metadata_db_->RemoveLogin(**i);
   }
 }
 
 void PasswordStoreMac::RemoveKeychainForms(
     const std::vector<PasswordForm*>& forms) {
   MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get());
   owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
   for (std::vector<PasswordForm*>::const_iterator i = forms.begin();
        i != forms.end(); ++i) {
     owned_keychain_adapter.RemovePassword(**i);
   }
 }
 
-void PasswordStoreMac::CleanOrphanedForms(std::vector<PasswordForm*>* forms) {
-  DCHECK(forms);
+void PasswordStoreMac::CleanOrphanedForms(
+    ScopedVector<autofill::PasswordForm>* orphaned_forms) {
+  DCHECK(orphaned_forms);
   DCHECK(login_metadata_db_);
 
-  std::vector<PasswordForm*> database_forms;
+  ScopedVector<autofill::PasswordForm> database_forms;
   login_metadata_db_->GetAutofillableLogins(&database_forms);
 
-  ScopedVector<PasswordForm> merged_forms;
-  merged_forms.get() = internal_keychain_helpers::GetPasswordsForForms(
-      *keychain_, &database_forms);
+  // Filter forms with corresponding Keychain entry out of |database_forms|.
+  ScopedVector<PasswordForm> forms_with_keychain_entry;
+  internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms,
+                                                  &forms_with_keychain_entry);
 
   // Clean up any orphaned database entries.
-  RemoveDatabaseForms(database_forms);
+  RemoveDatabaseForms(database_forms.get());
 
-  forms->insert(forms->end(), database_forms.begin(), database_forms.end());
+  // Move the orphaned DB forms to the output parameter.
+  orphaned_forms->insert(orphaned_forms->end(), database_forms.begin(),
+                         database_forms.end());
+  database_forms.weak_clear();
 }