PasswordStore: Use ScopedVector to express ownership of forms

chrome/browser/password_manager/password_store_mac_unittest.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 "base/basictypes.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/scoped_vector.h"
 #include "base/scoped_observer.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
 #include "chrome/common/chrome_paths.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
 #include "content/public/test/test_browser_thread.h"
@@ skipping 455 matching lines @@
   };
 
   MacKeychainPasswordFormAdapter keychain_adapter(keychain_);
   MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_);
   owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
   for (unsigned int i = 0; i < arraysize(test_data); ++i) {
     scoped_ptr<PasswordForm> query_form(
         CreatePasswordFormFromData(test_data[i].data));
 
     // Check matches treating the form as a fill target.
-    std::vector<PasswordForm*> matching_items =
+    ScopedVector<autofill::PasswordForm> matching_items(
         keychain_adapter.PasswordsFillingForm(query_form->signon_realm,
-                                              query_form->scheme);
+                                              query_form->scheme));
     EXPECT_EQ(test_data[i].expected_fill_matches, matching_items.size());
-    STLDeleteElements(&matching_items);
+    matching_items.clear();
 
     // Check matches treating the form as a merging target.
     EXPECT_EQ(test_data[i].expected_merge_matches > 0,
               keychain_adapter.HasPasswordsMergeableWithForm(*query_form));
     std::vector<SecKeychainItemRef> keychain_items;
     std::vector<internal_keychain_helpers::ItemFormPair> item_form_pairs =
         internal_keychain_helpers::
             ExtractAllKeychainItemAttributesIntoPasswordForms(&keychain_items,
                                                               *keychain_);
     matching_items =
         internal_keychain_helpers::ExtractPasswordsMergeableWithForm(
             *keychain_, item_form_pairs, *query_form);
     EXPECT_EQ(test_data[i].expected_merge_matches, matching_items.size());
     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);
     }
-    STLDeleteElements(&matching_items);
 
     // None of the pre-seeded items are owned by us, so none should match an
     // owned-passwords-only search.
     matching_items = owned_keychain_adapter.PasswordsFillingForm(
         query_form->signon_realm, query_form->scheme);
     EXPECT_EQ(0U, matching_items.size());
-    STLDeleteElements(&matching_items);
   }
 }
 
 // Changes just the origin path of |form|.
 static void SetPasswordFormPath(PasswordForm* form, const char* path) {
   GURL::Replacements replacement;
   std::string new_value(path);
   replacement.SetPathStr(new_value);
   form->origin = form->origin.ReplaceComponents(replacement);
 }
@@ skipping 33 matching lines @@
 
   for (unsigned int i = 0; i < arraysize(base_form_data); ++i) {
     // Create a base form and make sure we find a match.
     scoped_ptr<PasswordForm> base_form(CreatePasswordFormFromData(
         base_form_data[i]));
     EXPECT_TRUE(keychain_adapter.HasPasswordsMergeableWithForm(*base_form));
     EXPECT_TRUE(keychain_adapter.HasPasswordExactlyMatchingForm(*base_form));
 
     // Make sure that the matching isn't looser than it should be by checking
     // that slightly altered forms don't match.
-    std::vector<PasswordForm*> modified_forms;
+    ScopedVector<autofill::PasswordForm> modified_forms;
 
     modified_forms.push_back(new PasswordForm(*base_form));
     modified_forms.back()->username_value = ASCIIToUTF16("wrong_user");
 
     modified_forms.push_back(new PasswordForm(*base_form));
     SetPasswordFormPath(modified_forms.back(), "elsewhere.html");
 
     modified_forms.push_back(new PasswordForm(*base_form));
     modified_forms.back()->scheme = PasswordForm::SCHEME_OTHER;
 
     modified_forms.push_back(new PasswordForm(*base_form));
     SetPasswordFormPort(modified_forms.back(), "1234");
 
     modified_forms.push_back(new PasswordForm(*base_form));
     modified_forms.back()->blacklisted_by_user = true;
 
     if (base_form->scheme == PasswordForm::SCHEME_BASIC ||
         base_form->scheme == PasswordForm::SCHEME_DIGEST) {
       modified_forms.push_back(new PasswordForm(*base_form));
       SetPasswordFormRealm(modified_forms.back(), "incorrect");
     }
 
     for (unsigned int j = 0; j < modified_forms.size(); ++j) {
       bool match = keychain_adapter.HasPasswordExactlyMatchingForm(
           *modified_forms[j]);
       EXPECT_FALSE(match) << "In modified version " << j
           << " of base form " << i;
     }
-    STLDeleteElements(&modified_forms);
   }
 }
 
 TEST_F(PasswordStoreMacInternalsTest, TestKeychainAdd) {
   struct TestDataAndExpectation {
     PasswordFormData data;
     bool should_succeed;
   };
   TestDataAndExpectation test_data[] = {
     // Test a variety of scheme/port/protocol/path variations.
@@ skipping 273 matching lines @@
   CHECK(current_test < kTestCount);
   test_data[KEYCHAIN_INPUT][current_test].push_back(&keychain_user_1);
   test_data[KEYCHAIN_INPUT][current_test].push_back(&keychain_user_1_with_path);
   test_data[DATABASE_INPUT][current_test].push_back(&db_user_1);
   test_data[DATABASE_INPUT][current_test].push_back(&db_user_1_with_path);
   test_data[MERGE_OUTPUT][current_test].push_back(&merged_user_1);
   test_data[MERGE_OUTPUT][current_test].push_back(
       &merged_user_1_with_both_paths);
 
   for (unsigned int test_case = 0; test_case <= current_test; ++test_case) {
-    std::vector<PasswordForm*> keychain_forms;
+    ScopedVector<autofill::PasswordForm> keychain_forms;
     for (std::vector<PasswordFormData*>::iterator i =
              test_data[KEYCHAIN_INPUT][test_case].begin();
          i != test_data[KEYCHAIN_INPUT][test_case].end(); ++i) {
       keychain_forms.push_back(CreatePasswordFormFromData(*(*i)));
     }
-    std::vector<PasswordForm*> database_forms;
+    ScopedVector<autofill::PasswordForm> database_forms;
     for (std::vector<PasswordFormData*>::iterator i =
              test_data[DATABASE_INPUT][test_case].begin();
          i != test_data[DATABASE_INPUT][test_case].end(); ++i) {
       database_forms.push_back(CreatePasswordFormFromData(*(*i)));
     }
 
-    std::vector<PasswordForm*> merged_forms;
+    ScopedVector<autofill::PasswordForm> merged_forms;
     internal_keychain_helpers::MergePasswordForms(&keychain_forms,
                                                   &database_forms,
                                                   &merged_forms);
 
-    CHECK_FORMS(keychain_forms, test_data[KEYCHAIN_OUTPUT][test_case],
+    CHECK_FORMS(keychain_forms.get(), test_data[KEYCHAIN_OUTPUT][test_case],
                 test_case);
-    CHECK_FORMS(database_forms, test_data[DATABASE_OUTPUT][test_case],
+    CHECK_FORMS(database_forms.get(), test_data[DATABASE_OUTPUT][test_case],
                 test_case);
-    CHECK_FORMS(merged_forms, test_data[MERGE_OUTPUT][test_case], test_case);
-
-    STLDeleteElements(&keychain_forms);
-    STLDeleteElements(&database_forms);
-    STLDeleteElements(&merged_forms);
+    CHECK_FORMS(merged_forms.get(), test_data[MERGE_OUTPUT][test_case],
+                test_case);
   }
 }
 
 TEST_F(PasswordStoreMacInternalsTest, TestPasswordBulkLookup) {
   PasswordFormData db_data[] = {
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/", "http://some.domain.com/action.cgi",
       L"submit", L"username", L"password", L"joe_user", L"",
       true, false, 1212121212 },
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/page.html",
       "http://some.domain.com/handlepage.cgi",
       L"submit", L"username", L"password", L"joe_user", L"",
       true, false, 1234567890 },
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/page.html",
       "http://some.domain.com/handlepage.cgi",
       L"submit", L"username", L"password", L"second-account", L"",
       true, false, 1240000000 },
     { PasswordForm::SCHEME_HTML, "http://dont.remember.com/",
       "http://dont.remember.com/",
       "http://dont.remember.com/handlepage.cgi",
       L"submit", L"username", L"password", L"joe_user", L"",
       true, false, 1240000000 },
     { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
       "http://some.domain.com/path.html", "http://some.domain.com/action.cgi",
       L"submit", L"username", L"password", NULL, NULL,
       true, false, 1212121212 },
   };
-  std::vector<PasswordForm*> database_forms;
+  ScopedVector<autofill::PasswordForm> database_forms;
   for (unsigned int i = 0; i < arraysize(db_data); ++i) {
     database_forms.push_back(CreatePasswordFormFromData(db_data[i]));
   }
-  std::vector<PasswordForm*> merged_forms =
-      internal_keychain_helpers::GetPasswordsForForms(*keychain_,
-                                                      &database_forms);
+  ScopedVector<autofill::PasswordForm> merged_forms;
+  internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms,
+                                                  &merged_forms);
   EXPECT_EQ(2U, database_forms.size());
   ASSERT_EQ(3U, merged_forms.size());
   EXPECT_EQ(ASCIIToUTF16("sekrit"), merged_forms[0]->password_value);
   EXPECT_EQ(ASCIIToUTF16("sekrit"), merged_forms[1]->password_value);
   EXPECT_TRUE(merged_forms[2]->blacklisted_by_user);
-
-  STLDeleteElements(&database_forms);
-  STLDeleteElements(&merged_forms);
 }
 
 TEST_F(PasswordStoreMacInternalsTest, TestBlacklistedFiltering) {
   PasswordFormData db_data[] = {
     { PasswordForm::SCHEME_HTML, "http://dont.remember.com/",
       "http://dont.remember.com/",
       "http://dont.remember.com/handlepage.cgi",
       L"submit", L"username", L"password", L"joe_user", L"non_empty_password",
       true, false, 1240000000 },
     { PasswordForm::SCHEME_HTML, "https://dont.remember.com/",
       "https://dont.remember.com/",
       "https://dont.remember.com/handlepage_secure.cgi",
       L"submit", L"username", L"password", L"joe_user", L"non_empty_password",
       true, false, 1240000000 },
   };
-  std::vector<PasswordForm*> database_forms;
+  ScopedVector<autofill::PasswordForm> database_forms;
   for (unsigned int i = 0; i < arraysize(db_data); ++i) {
     database_forms.push_back(CreatePasswordFormFromData(db_data[i]));
   }
-  std::vector<PasswordForm*> merged_forms =
-      internal_keychain_helpers::GetPasswordsForForms(*keychain_,
-                                                      &database_forms);
+  ScopedVector<autofill::PasswordForm> merged_forms;
+  internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms,
+                                                  &merged_forms);
   EXPECT_EQ(2U, database_forms.size());
   ASSERT_EQ(0U, merged_forms.size());
-
-  STLDeleteElements(&database_forms);
-  STLDeleteElements(&merged_forms);
 }
 
 TEST_F(PasswordStoreMacInternalsTest, TestFillPasswordFormFromKeychainItem) {
   // When |extract_password_data| is false, the password field must be empty,
   // and |blacklisted_by_user| must be false.
   SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(1);
   PasswordForm form_without_extracted_password;
   bool parsed = internal_keychain_helpers::FillPasswordFormFromKeychainItem(
       *keychain_,
       keychain_item,
@@ skipping 79 matching lines @@
     { PasswordForm::SCHEME_DIGEST, "https://digest.site.com/differentrealm",
       "https://digest.site.com/secure.html", NULL, NULL, NULL, NULL,
       L"testname", L"testpass", false, false, 0 },
   };
   for (unsigned int i = 0; i < arraysize(owned_password_data); ++i) {
     scoped_ptr<PasswordForm> form(CreatePasswordFormFromData(
         owned_password_data[i]));
     owned_keychain_adapter.AddPassword(*form);
   }
 
-  std::vector<PasswordForm*> all_passwords =
-      keychain_adapter.GetAllPasswordFormPasswords();
+  ScopedVector<autofill::PasswordForm> all_passwords(

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

nit: here and below I'd prefer ScopedVector<autofill::PasswordForm> var = ...

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

Done.

+      keychain_adapter.GetAllPasswordFormPasswords());
   EXPECT_EQ(8 + arraysize(owned_password_data), all_passwords.size());
-  STLDeleteElements(&all_passwords);
 
-  std::vector<PasswordForm*> owned_passwords =
-      owned_keychain_adapter.GetAllPasswordFormPasswords();
+  ScopedVector<autofill::PasswordForm> owned_passwords(
+      owned_keychain_adapter.GetAllPasswordFormPasswords());
   EXPECT_EQ(arraysize(owned_password_data), owned_passwords.size());
-  STLDeleteElements(&owned_passwords);
 }
 
 #pragma mark -
 
 class PasswordStoreMacTest : public testing::Test {
  public:
   PasswordStoreMacTest() : ui_thread_(BrowserThread::UI, &message_loop_) {}
 
   void SetUp() override {
     ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
@@ skipping 121 matching lines @@
     store_->UpdateLogin(*form);
   }
 
   FinishAsyncProcessing();
 
   MacKeychainPasswordFormAdapter keychain_adapter(keychain());
   for (unsigned int i = 0; i < arraysize(updates); ++i) {
     scoped_ptr<PasswordForm> query_form(
         CreatePasswordFormFromData(updates[i].form_data));
 
-    std::vector<PasswordForm*> matching_items =
+    ScopedVector<autofill::PasswordForm> matching_items(
         keychain_adapter.PasswordsFillingForm(query_form->signon_realm,
-                                              query_form->scheme);
+                                              query_form->scheme));
     if (updates[i].password) {
       EXPECT_GT(matching_items.size(), 0U) << "iteration " << i;
       if (matching_items.size() >= 1)
         EXPECT_EQ(ASCIIToUTF16(updates[i].password),
                   matching_items[0]->password_value) << "iteration " << i;
     } else {
       EXPECT_EQ(0U, matching_items.size()) << "iteration " << i;
     }
-    STLDeleteElements(&matching_items);
+    matching_items.clear();
 
     login_db()->GetLogins(*query_form, &matching_items);
     EXPECT_EQ(updates[i].password ? 1U : 0U, matching_items.size())
         << "iteration " << i;
-    STLDeleteElements(&matching_items);
   }
 }
 
 TEST_F(PasswordStoreMacTest, TestDBKeychainAssociation) {
   // Tests that association between the keychain and login database parts of a
   // password added by fuzzy (PSL) matching works.
   // 1. Add a password for www.facebook.com
   // 2. Get a password for m.facebook.com. This fuzzy matches and returns the
   //    www.facebook.com password.
   // 3. Add the returned password for m.facebook.com.
@@ skipping 30 matching lines @@
 
   // 3. Add the returned password for m.facebook.com.
   login_db()->AddLogin(consumer.last_result[0]);
   owned_keychain_adapter.AddPassword(m_form);
 
   // 4. Remove both passwords.
   store_->RemoveLogin(*www_form);
   store_->RemoveLogin(m_form);
   FinishAsyncProcessing();
 
-  std::vector<PasswordForm*> matching_items;
   // No trace of www.facebook.com.
-  matching_items = owned_keychain_adapter.PasswordsFillingForm(
-      www_form->signon_realm, www_form->scheme);
+  ScopedVector<autofill::PasswordForm> matching_items(
+      owned_keychain_adapter.PasswordsFillingForm(www_form->signon_realm,
+                                                  www_form->scheme));
   EXPECT_EQ(0u, matching_items.size());
   login_db()->GetLogins(*www_form, &matching_items);
   EXPECT_EQ(0u, matching_items.size());
   // No trace of m.facebook.com.
   matching_items = owned_keychain_adapter.PasswordsFillingForm(
       m_form.signon_realm, m_form.scheme);
   EXPECT_EQ(0u, matching_items.size());
   login_db()->GetLogins(m_form, &matching_items);
   EXPECT_EQ(0u, matching_items.size());
 }
@@ skipping 69 matching lines @@
   test->store()->AddLogin(*form_facebook);
   test->store()->AddLogin(*form_other);
   EXPECT_CALL(observer, OnLoginsChanged(GetAddChangeList(*form_facebook_old)));
   EXPECT_CALL(observer, OnLoginsChanged(GetAddChangeList(*form_facebook)));
   EXPECT_CALL(observer, OnLoginsChanged(GetAddChangeList(*form_other)));
   observer.WaitAndVerify(test);
 
   // Check the keychain content.
   MacKeychainPasswordFormAdapter owned_keychain_adapter(test->keychain());
   owned_keychain_adapter.SetFindsOnlyOwnedItems(false);
-  ScopedVector<PasswordForm> matching_items;
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
-      form_facebook->signon_realm, form_facebook->scheme);
+  ScopedVector<PasswordForm> matching_items(
+      owned_keychain_adapter.PasswordsFillingForm(form_facebook->signon_realm,
+                                                  form_facebook->scheme));
   EXPECT_EQ(1u, matching_items.size());
-  matching_items.clear();
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       form_other->signon_realm, form_other->scheme);
   EXPECT_EQ(1u, matching_items.size());
-  matching_items.clear();
 
   // Remove facebook.
   void (PasswordStore::*method)(base::Time, base::Time) =
       check_created ? &PasswordStore::RemoveLoginsCreatedBetween
                     : &PasswordStore::RemoveLoginsSyncedBetween;
   (test->store()->*method)(base::Time(), next_day);
   password_manager::PasswordStoreChangeList list;
   form_facebook_old->password_value.clear();
   form_facebook->password_value.clear();
   list.push_back(password_manager::PasswordStoreChange(
       password_manager::PasswordStoreChange::REMOVE, *form_facebook_old));
   list.push_back(password_manager::PasswordStoreChange(
       password_manager::PasswordStoreChange::REMOVE, *form_facebook));
   EXPECT_CALL(observer, OnLoginsChanged(list));
   list.clear();
   observer.WaitAndVerify(test);
 
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       form_facebook->signon_realm, form_facebook->scheme);
   EXPECT_EQ(0u, matching_items.size());
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       form_other->signon_realm, form_other->scheme);
   EXPECT_EQ(1u, matching_items.size());
-  matching_items.clear();
 
   // Remove form_other.
   (test->store()->*method)(next_day, base::Time());
   form_other->password_value.clear();
   list.push_back(password_manager::PasswordStoreChange(
       password_manager::PasswordStoreChange::REMOVE, *form_other));
   EXPECT_CALL(observer, OnLoginsChanged(list));
   observer.WaitAndVerify(test);
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       form_other->signon_realm, form_other->scheme);
   EXPECT_EQ(0u, matching_items.size());
 }
 
 }  // namespace
 
 TEST_F(PasswordStoreMacTest, TestRemoveLoginsCreatedBetween) {
   CheckRemoveLoginsBetween(this, true);
 }
 
@@ skipping 26 matching lines @@
   // Add a password from the current profile.
   PasswordFormData www_form_data2 = {
       PasswordForm::SCHEME_HTML, "http://www.facebook.com/",
       "http://www.facebook.com/index.html", "login", L"username", L"password",
       L"submit", L"not_joe_user", L"12345", true, false, 1 };
   www_form.reset(CreatePasswordFormFromData(www_form_data2));
   store_->AddLogin(*www_form);
   FinishAsyncProcessing();
 
   ScopedVector<PasswordForm> matching_items;
-  login_db()->GetLogins(*www_form, &matching_items.get());
+  login_db()->GetLogins(*www_form, &matching_items);
   EXPECT_EQ(1u, matching_items.size());
   matching_items.clear();
 
   store_->RemoveLoginsCreatedBetween(base::Time(), base::Time());
   FinishAsyncProcessing();
 
   // Check the second facebook form is gone.
-  login_db()->GetLogins(*www_form, &matching_items.get());
+  login_db()->GetLogins(*www_form, &matching_items);
   EXPECT_EQ(0u, matching_items.size());
 
   // Check the first facebook form is still there.
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       www_form->signon_realm, www_form->scheme);
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(ASCIIToUTF16("joe_user"), matching_items[0]->username_value);
   matching_items.clear();
 
   // Check the third-party password is still there.
   owned_keychain_adapter.SetFindsOnlyOwnedItems(false);
-  matching_items.get() = owned_keychain_adapter.PasswordsFillingForm(
+  matching_items = owned_keychain_adapter.PasswordsFillingForm(
       "http://some.domain.com/insecure.html", PasswordForm::SCHEME_HTML);
   ASSERT_EQ(1u, matching_items.size());
 }
 
 // Open the store and immediately write to it and try to read it back, without
 // first waiting for the initialization to finish. If tasks are processed in
 // order, read/write operations will correctly be performed only after the
 // initialization has finished.
 TEST_F(PasswordStoreMacTest, StoreIsUsableImmediatelyAfterConstruction) {
   ClosePasswordStore();
@@ skipping 71 matching lines @@
 
   // Delete one login; a range of logins.
   store()->RemoveLogin(*form);
   store()->RemoveLoginsCreatedBetween(base::Time(), base::Time::Max());
   store()->RemoveLoginsSyncedBetween(base::Time(), base::Time::Max());
   FinishAsyncProcessing();
 
   // Verify no notifications are fired during shutdown either.
   ClosePasswordStore();
 }