Fix a memory leak in PasswordStoreMac

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>
@@ skipping 600 matching lines @@
 
 std::vector<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);
 }
 
-PasswordForm* MacKeychainPasswordFormAdapter::PasswordExactlyMatchingForm(
-    const PasswordForm& query_form) {
+scoped_ptr<PasswordForm> MacKeychainPasswordFormAdapter::
+    PasswordExactlyMatchingForm(const PasswordForm& query_form) {
   SecKeychainItemRef keychain_item = KeychainItemForForm(query_form);
   if (keychain_item) {
-    PasswordForm* form = new PasswordForm();
+    scoped_ptr<PasswordForm> form(new PasswordForm);
     internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
                                                                 keychain_item,
-                                                                form,
+                                                                form.get(),
                                                                 true);
     keychain_->Free(keychain_item);
-    return form;
+    return form.Pass();
   }
-  return NULL;
+  return scoped_ptr<PasswordForm>();
 }
 
 bool MacKeychainPasswordFormAdapter::HasPasswordsMergeableWithForm(
     const PasswordForm& query_form) {
   std::string username = base::UTF16ToUTF8(query_form.username_value);
   std::vector<SecKeychainItemRef> matches =
       MatchingKeychainItems(query_form.signon_realm, query_form.scheme,
                             NULL, username.c_str());
   for (std::vector<SecKeychainItemRef>::iterator i = matches.begin();
        i != matches.end(); ++i) {
@@ skipping 291 matching lines @@
   if (login_metadata_db_->RemoveLogin(form)) {
     // See if we own a Keychain item associated with this item. We can do an
     // exact search rather than messing around with trying to do fuzzy matching
     // because passwords that we created will always have an exact-match
     // database entry.
     // (If a user does lose their profile but not their keychain we'll treat the
     // entries we find like other imported entries anyway, so it's reasonable to
     // handle deletes on them the way we would for an imported item.)
     MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get());
     owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
-    PasswordForm* owned_password_form =
-        owned_keychain_adapter.PasswordExactlyMatchingForm(form);
-    if (owned_password_form) {
+    if (owned_keychain_adapter.PasswordExactlyMatchingForm(form)) {

[Ilya Sherman, 2014/08/12 00:31:51]

Hmm, could the method be changed to instead just return a boolean value, rather
than returning a pointer to the form?

[vasilii, 2014/08/12 09:17:37]

The returned value is used in unit tests. With boolean it'll be more difficult
to test.

[Ilya Sherman, 2014/08/12 17:21:25]

Hmm, why do the tests need more information that the production code?  In
general, it's better to simplify production code and to test public APIs.

[vasilii, 2014/08/13 09:56:46]

Done.

       // If we don't have other forms using it (i.e., a form differing only by
       // the names of the form elements), delete the keychain entry.
       if (!DatabaseHasFormMatchingKeychainForm(form)) {
         owned_keychain_adapter.RemovePassword(form);
       }
     }
 
     changes.push_back(PasswordStoreChange(PasswordStoreChange::REMOVE, form));
   }
   return changes;
@@ skipping 195 matching lines @@
 
   ScopedVector<PasswordForm> merged_forms;
   merged_forms.get() = internal_keychain_helpers::GetPasswordsForForms(
       *keychain_, &database_forms);
 
   // Clean up any orphaned database entries.
   RemoveDatabaseForms(database_forms);
 
   forms->insert(forms->end(), database_forms.begin(), database_forms.end());
 }