Remove a bunch of low-level keychain helper tests that are now redundant with tests of the newer higher-level methods.

Start collecting the helpers into a cohesive class that serves as a translation layer between keychain items and password forms.

BUG=none
TEST=Keychain passwords should still fill after a username is typed on the Mac (no change in behavior).
Review URL: http://codereview.chromium.org/151164

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

3 files changed, 252 insertions, 426 deletions

diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index c5eadda..fac6bda 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -18,8 +18,6 @@
 
 using webkit_glue::PasswordForm;
 
-namespace internal_keychain_helpers {
-
 // Utility class to handle the details of constructing and running a keychain
 // search from a set of attributes.
 class KeychainSearch {
@@ -153,6 +151,17 @@ void KeychainSearch::FindMatchingItems(std::vector<SecKeychainItemRef>* items) {
 
 #pragma mark -
 
+// TODO(stuartmorgan): Convert most of this to private helpers in
+// MacKeychainPaswordFormAdapter once it has sufficient higher-level public
+// methods to provide test coverage.
+namespace internal_keychain_helpers {
+
+// Takes a PasswordForm's signon_realm and parses it into its component parts,
+// which are returned though the appropriate out parameters.
+// Returns true if it can be successfully parsed, in which case all out params
+// that are non-NULL will be set. If there is no port, port will be 0.
+// If the return value is false, the state of the our params is undefined.
+//
 // TODO(stuartmorgan): signon_realm for proxies is not yet supported.
 bool ExtractSignonRealmComponents(const std::string& signon_realm,
                                   std::string* server, int* port,
@@ -178,6 +187,8 @@ bool ExtractSignonRealmComponents(const std::string& signon_realm,
   return true;
 }
 
+// Returns a URL built from the given components. To create a URL without a
+// port, pass kAnyPort for the |port| parameter.
 GURL URLFromComponents(bool is_secure, const std::string& host, int port,
                        const std::string& path) {
   GURL::Replacements url_components;
@@ -197,7 +208,9 @@ GURL URLFromComponents(bool is_secure, const std::string& host, int port,
   return url.ReplaceComponents(url_components);
 }
 
-// The time string is of the form "yyyyMMddHHmmss'Z", in UTC time.
+// Converts a Keychain time string to a Time object, returning true if
+// time_string_bytes was parsable. If the return value is false, the value of
+// |time| is unchanged.
 bool TimeFromKeychainTimeString(const char* time_string_bytes,
                                 unsigned int byte_length,
                                 base::Time* time) {
@@ -208,6 +221,7 @@ bool TimeFromKeychainTimeString(const char* time_string_bytes,
   time_string[byte_length] = '\0';
   base::Time::Exploded exploded_time;
   bzero(&exploded_time, sizeof(exploded_time));
+  // The time string is of the form "yyyyMMddHHmmss'Z", in UTC time.
   int assignments = sscanf(time_string, "%4d%2d%2d%2d%2d%2dZ",
                            &exploded_time.year, &exploded_time.month,
                            &exploded_time.day_of_month, &exploded_time.hour,
@@ -221,6 +235,7 @@ bool TimeFromKeychainTimeString(const char* time_string_bytes,
   return false;
 }
 
+// Returns the Keychain SecAuthenticationType type corresponding to |scheme|.
 SecAuthenticationType AuthTypeForScheme(PasswordForm::Scheme scheme) {
   switch (scheme) {
     case PasswordForm::SCHEME_HTML:   return kSecAuthenticationTypeHTMLForm;
@@ -232,6 +247,7 @@ SecAuthenticationType AuthTypeForScheme(PasswordForm::Scheme scheme) {
   return kSecAuthenticationTypeDefault;
 }
 
+// Returns the PasswordForm Scheme corresponding to |auth_type|.
 PasswordForm::Scheme SchemeForAuthType(SecAuthenticationType auth_type) {
   switch (auth_type) {
     case kSecAuthenticationTypeHTMLForm:   return PasswordForm::SCHEME_HTML;
@@ -241,40 +257,8 @@ PasswordForm::Scheme SchemeForAuthType(SecAuthenticationType auth_type) {
   }
 }
 
-// Searches |keychain| for all items usable for the given signon_realm, and
-// puts them in |items|. The caller is responsible for calling keychain->Free
-// on each of them when it is finished with them.
-void FindMatchingKeychainItems(const MacKeychain& keychain,
-                               const std::string& signon_realm,
-                               PasswordForm::Scheme scheme,
-                               std::vector<SecKeychainItemRef>* items) {
-  // Construct a keychain search based on the signon_realm and scheme.
-  std::string server;
-  std::string security_domain;
-  int port;
-  bool is_secure;
-  if (!ExtractSignonRealmComponents(signon_realm, &server, &port, &is_secure,
-                                    &security_domain)) {
-    // TODO(stuartmorgan): Proxies will currently fail here, since their
-    // signon_realm is not a URL. We need to detect the proxy case and handle
-    // it specially.
-    return;
-  }
-
-  SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
-                                       : kSecProtocolTypeHTTP;
-  SecAuthenticationType auth_type = AuthTypeForScheme(scheme);
-
-  KeychainSearch keychain_search(keychain);
-  keychain_search.Init(server.c_str(), port, protocol, auth_type,
-                       (scheme == PasswordForm::SCHEME_HTML) ?
-                           NULL : security_domain.c_str(),
-                       NULL, NULL);
-  keychain_search.FindMatchingItems(items);
-}
-
-SecKeychainItemRef FindMatchingKeychainItem(const MacKeychain& keychain,
-                                            const PasswordForm& form) {
+SecKeychainItemRef MatchingKeychainItem(const MacKeychain& keychain,
+                                        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) {
@@ -433,52 +417,6 @@ bool FillPasswordFormFromKeychainItem(const MacKeychain& keychain,
   return true;
 }
 
-bool AddKeychainEntryForForm(const MacKeychain& keychain,
-                             const PasswordForm& form) {
-  std::string server;
-  std::string security_domain;
-  int port;
-  bool is_secure;
-  if (!ExtractSignonRealmComponents(form.signon_realm, &server, &port,
-                                    &is_secure, &security_domain)) {
-    return false;
-  }
-  std::string username = WideToUTF8(form.username_value);
-  std::string password = WideToUTF8(form.password_value);
-  std::string path = form.origin.path();
-  SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
-                                       : kSecProtocolTypeHTTP;
-  OSStatus result = keychain.AddInternetPassword(
-      NULL, server.size(), server.c_str(),
-      security_domain.size(), security_domain.c_str(),
-      username.size(), username.c_str(),
-      path.size(), path.c_str(),
-      port, protocol, AuthTypeForScheme(form.scheme),
-      password.size(), password.c_str(), NULL);
-
-  // If we collide with an existing item, find and update it instead.
-  if (result == errSecDuplicateItem) {
-    SecKeychainItemRef existing_item = FindMatchingKeychainItem(keychain, form);
-    if (!existing_item) {
-      return false;
-    }
-    bool changed = SetKeychainItemPassword(keychain, existing_item, password);
-    keychain.Free(existing_item);
-    return changed;
-  }
-
-  return (result == noErr);
-}
-
-bool SetKeychainItemPassword(const MacKeychain& keychain,
-                             const SecKeychainItemRef& keychain_item,
-                             const std::string& password) {
-  OSStatus result = keychain.ItemModifyAttributesAndData(keychain_item, NULL,
-                                                         password.size(),
-                                                         password.c_str());
-  return (result == noErr);
-}
-
 bool FormsMatchForMerge(const PasswordForm& form_a, const PasswordForm& form_b,
                         bool* path_matches) {
   // We never merge blacklist entries between our store and the keychain.
@@ -562,40 +500,124 @@ void MergePasswordForms(std::vector<PasswordForm*>* keychain_forms,
   }
 }
 
-// Returns PasswordForms constructed from the given Keychain items.
+}  // namespace internal_keychain_helpers
+
+#pragma mark -
+
+MacKeychainPasswordFormAdapter::MacKeychainPasswordFormAdapter(
+    MacKeychain* keychain) : keychain_(keychain) {
+}
+
+// Returns PasswordForms for each keychain entry matching |form|.
 // Caller is responsible for deleting the returned forms.
-std::vector<PasswordForm*> CreateFormsFromKeychainItems(
-    const MacKeychain& keychain,
-    const std::vector<SecKeychainItemRef>& items) {
+std::vector<PasswordForm*>
+    MacKeychainPasswordFormAdapter::PasswordsMatchingForm(
+        const PasswordForm& query_form) {
+  std::vector<SecKeychainItemRef> keychain_items =
+      MatchingKeychainItems(query_form.signon_realm, query_form.scheme);
+
+  std::vector<PasswordForm*> keychain_forms =
+      CreateFormsFromKeychainItems(keychain_items);
+  for (std::vector<SecKeychainItemRef>::iterator i = keychain_items.begin();
+       i != keychain_items.end(); ++i) {
+    keychain_->Free(*i);
+  }
+  return keychain_forms;
+}
+
+bool MacKeychainPasswordFormAdapter::AddLogin(const PasswordForm& form) {
+  std::string server;
+  std::string security_domain;
+  int port;
+  bool is_secure;
+  if (!internal_keychain_helpers::ExtractSignonRealmComponents(
+           form.signon_realm, &server, &port, &is_secure, &security_domain)) {
+    return false;
+  }
+  std::string username = WideToUTF8(form.username_value);
+  std::string password = WideToUTF8(form.password_value);
+  std::string path = form.origin.path();
+  SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
+                                       : kSecProtocolTypeHTTP;
+  OSStatus result = keychain_->AddInternetPassword(
+      NULL, server.size(), server.c_str(),
+      security_domain.size(), security_domain.c_str(),
+      username.size(), username.c_str(),
+      path.size(), path.c_str(),
+      port, protocol, internal_keychain_helpers::AuthTypeForScheme(form.scheme),
+      password.size(), password.c_str(), NULL);
+
+  // If we collide with an existing item, find and update it instead.
+  if (result == errSecDuplicateItem) {
+    SecKeychainItemRef existing_item =
+        internal_keychain_helpers::MatchingKeychainItem(*keychain_, form);
+    if (!existing_item) {
+      return false;
+    }
+    bool changed = SetKeychainItemPassword(existing_item, password);
+    keychain_->Free(existing_item);
+    return changed;
+  }
+
+  return (result == noErr);
+}
+
+std::vector<PasswordForm*>
+    MacKeychainPasswordFormAdapter::CreateFormsFromKeychainItems(
+        const 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();
-    if (FillPasswordFormFromKeychainItem(keychain, *i, form)) {
+    if (internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
+                                                                    *i, form)) {
       keychain_forms.push_back(form);
     }
   }
   return keychain_forms;
 }
 
-// Returns PasswordForms for each keychain entry matching |form|.
-// Caller is responsible for deleting the returned forms.
-std::vector<PasswordForm*> KeychainFormsMatchingForm(
-    const MacKeychain& keychain, const PasswordForm& query_form) {
-  std::vector<SecKeychainItemRef> keychain_items;
-  FindMatchingKeychainItems(keychain, query_form.signon_realm,
-                            query_form.scheme, &keychain_items);
-
-  std::vector<PasswordForm*> keychain_forms =
-      CreateFormsFromKeychainItems(keychain, keychain_items);
-  for (std::vector<SecKeychainItemRef>::iterator i = keychain_items.begin();
-       i != keychain_items.end(); ++i) {
-    keychain.Free(*i);
+// Searches |keychain| for all items usable for the given signon_realm, and
+// returns them. The caller is responsible for calling keychain->Free
+// on each of them when it is finished with them.
+std::vector<SecKeychainItemRef>
+    MacKeychainPasswordFormAdapter::MatchingKeychainItems(
+        const std::string& signon_realm, PasswordForm::Scheme scheme) {
+  std::vector<SecKeychainItemRef> matches;
+  // Construct a keychain search based on the signon_realm and scheme.
+  std::string server;
+  std::string security_domain;
+  int port;
+  bool is_secure;
+  if (!internal_keychain_helpers::ExtractSignonRealmComponents(
+           signon_realm, &server, &port, &is_secure, &security_domain)) {
+    // TODO(stuartmorgan): Proxies will currently fail here, since their
+    // signon_realm is not a URL. We need to detect the proxy case and handle
+    // it specially.
+    return matches;
   }
-  return keychain_forms;
+
+  SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
+                                       : kSecProtocolTypeHTTP;
+  SecAuthenticationType auth_type =
+      internal_keychain_helpers::AuthTypeForScheme(scheme);
+
+  KeychainSearch keychain_search(*keychain_);
+  keychain_search.Init(server.c_str(), port, protocol, auth_type,
+                       (scheme == PasswordForm::SCHEME_HTML) ?
+                           NULL : security_domain.c_str(),
+                       NULL, NULL);
+  keychain_search.FindMatchingItems(&matches);
+  return matches;
 }
 
-}  // namespace internal_keychain_helpers
+bool MacKeychainPasswordFormAdapter::SetKeychainItemPassword(
+    const SecKeychainItemRef& keychain_item, const std::string& password) {
+  OSStatus result = keychain_->ItemModifyAttributesAndData(keychain_item, NULL,
+                                                           password.size(),
+                                                           password.c_str());
+  return (result == noErr);
+}
 
 #pragma mark -
 
@@ -621,9 +643,9 @@ void PasswordStoreMac::RemoveLoginImpl(const PasswordForm& form) {
 }
 
 void PasswordStoreMac::GetLoginsImpl(GetLoginsRequest* request) {
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_.get());
   std::vector<PasswordForm*> keychain_forms =
-      internal_keychain_helpers::KeychainFormsMatchingForm(*keychain_,
-                                                           request->form);
+      keychainAdapter.PasswordsMatchingForm(request->form);
 
   std::vector<PasswordForm*> database_forms;
   login_metadata_db_->GetLogins(request->form, &database_forms);
diff --git a/chrome/browser/password_manager/password_store_mac_internal.h b/chrome/browser/password_manager/password_store_mac_internal.h
index f8622d4..3d546e0 100644
--- a/chrome/browser/password_manager/password_store_mac_internal.h
+++ b/chrome/browser/password_manager/password_store_mac_internal.h
@@ -13,51 +13,55 @@
 #include "base/time.h"
 #include "chrome/browser/keychain_mac.h"
 
-namespace internal_keychain_helpers {
+// Adapter that wraps a MacKeychain and provides interaction in terms of
+// PasswordForms instead of Keychain items.
+class MacKeychainPasswordFormAdapter {
+ public:
+  // Creates an adapter for |keychain|. This class does not take ownership of
+  // |keychain|, so the caller must make sure that the keychain outlives the
+  // created object.
+  explicit MacKeychainPasswordFormAdapter(MacKeychain* keychain);
+
+  // Returns PasswordForms for each keychain entry matching |form|.
+  // Caller is responsible for deleting the returned forms.
+  std::vector<webkit_glue::PasswordForm*> PasswordsMatchingForm(
+      const webkit_glue::PasswordForm& query_form);
+
+  // Creates a new keychain entry from |form|, or updates the password of an
+  // existing keychain entry if there is a collision. Returns true if a keychain
+  // entry was successfully added/updated.
+  bool AddLogin(const webkit_glue::PasswordForm& form);
+
+ private:
+  // Returns PasswordForms constructed from the given Keychain items.
+  // Caller is responsible for deleting the returned forms.
+  std::vector<webkit_glue::PasswordForm*> CreateFormsFromKeychainItems(
+      const std::vector<SecKeychainItemRef>& items);
+
+  // Searches |keychain| for all items usable for the given signon_realm, and
+  // puts them in |items|. The caller is responsible for calling keychain->Free
+  // on each of them when it is finished with them.
+  std::vector<SecKeychainItemRef> MatchingKeychainItems(
+      const std::string& signon_realm,
+      webkit_glue::PasswordForm::Scheme scheme);
+
+  // Changes the password for keychain_item to |password|; returns true if the
+  // password was successfully changed.
+  bool SetKeychainItemPassword(const SecKeychainItemRef& keychain_item,
+                               const std::string& password);
+
+  MacKeychain* keychain_;
+
+  DISALLOW_COPY_AND_ASSIGN(MacKeychainPasswordFormAdapter);
+};
 
-// Takes a PasswordForm's signon_realm and parses it into its component parts,
-// which are returned though the appropriate out parameters.
-// Returns true if it can be successfully parsed, in which case all out params
-// that are non-NULL will be set. If there is no port, port will be 0.
-// If the return value is false, the state of the our params is undefined.
-bool ExtractSignonRealmComponents(const std::string& signon_realm,
-                                  std::string* server, int* port,
-                                  bool* is_secure,
-                                  std::string* security_domain);
-
-// Returns a URL built from the given components. To create a URL without a
-// port, pass kAnyPort for the |port| parameter.
-GURL URLFromComponents(bool is_secure, const std::string& host, int port,
-                       const std::string& path);
-
-// Converts a Keychain time string to a Time object, returning true if
-// time_string_bytes was parsable. If the return value is false, the value of
-// |time| is unchanged.
-bool TimeFromKeychainTimeString(const char* time_string_bytes,
-                                unsigned int byte_length,
-                                base::Time* time);
-
-// Returns the Keychain SecAuthenticationType type corresponding to |scheme|.
-SecAuthenticationType AuthTypeForScheme(
-    webkit_glue::PasswordForm::Scheme scheme);
-
-// Returns the PasswordForm Scheme corresponding to |auth_type|.
-webkit_glue::PasswordForm::Scheme SchemeForAuthType(
-    SecAuthenticationType auth_type);
-
-// Searches |keychain| for all items usable for the given signon_realm, and
-// puts them in |items|. The caller is responsible for calling keychain->Free
-// on each of them when it is finished with them.
-void FindMatchingKeychainItems(const MacKeychain& keychain,
-                               const std::string& signon_realm,
-                               webkit_glue::PasswordForm::Scheme scheme,
-                               std::vector<SecKeychainItemRef>* items);
+namespace internal_keychain_helpers {
 
 // Searches |keychain| for the specific keychain entry matching the given form,
 // and returns it (or NULL if no match is found).
 // The caller is responsible for calling keychain->Free on the returned item.
-SecKeychainItemRef FindMatchingKeychainItem(
-    const MacKeychain& keychain, const webkit_glue::PasswordForm& form);
+SecKeychainItemRef MatchingKeychainItem(const MacKeychain& keychain,
+                                        const webkit_glue::PasswordForm& form);
 
 // Sets the fields of |form| based on the keychain data from |keychain_item|.
 // Fields that can't be determined from |keychain_item| will be unchanged.
@@ -74,18 +78,6 @@ bool FillPasswordFormFromKeychainItem(const MacKeychain& keychain,
                                       const SecKeychainItemRef& keychain_item,
                                       webkit_glue::PasswordForm* form);
 
-// Creates a new keychain entry from |form|, or updates the password of an
-// existing keychain entry if there is a collision. Returns true if a keychain
-// entry was successfully added/updated.
-bool AddKeychainEntryForForm(const MacKeychain& keychain,
-                             const webkit_glue::PasswordForm& form);
-
-// Changes the password for keychain_item to |password|; returns true if the
-// password was successfully changed.
-bool SetKeychainItemPassword(const MacKeychain& keychain,
-                             const SecKeychainItemRef& keychain_item,
-                             const std::string& password);
-
 // Returns true if the two given forms match based on signon_reaml, scheme, and
 // username_value, and are thus suitable for merging (see MergePasswordForms).
 // If this returns true, and path_matches is non-NULL, *path_matches will be set
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc
index a785a0c..21cf79a 100644
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc
+++ b/chrome/browser/password_manager/password_store_mac_unittest.cc
@@ -5,6 +5,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/basictypes.h"
+#include "base/stl_util-inl.h"
 #include "chrome/browser/keychain_mock_mac.h"
 #include "chrome/browser/password_manager/password_store_mac.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
@@ -48,6 +49,10 @@ class PasswordStoreMacTest : public testing::Test {
       { kSecAuthenticationTypeHTTPDigest, "some.domain.com",
         kSecProtocolTypeHTTPS, NULL, 0, "high_security", "19980330100000Z",
         "digest_auth_user", "digest", false },
+      // An FTP password with an invalid date, for edge-case testing.
+      { kSecAuthenticationTypeDefault, "a.server.com",
+        kSecProtocolTypeFTP, NULL, 0, NULL, "20010203040",
+        "abc", "123", false },
     };
 
     // Save one slot for use by AddInternetPassword.
@@ -177,171 +182,8 @@ static void CheckFormsAgainstExpectations(
   }
 }
 
-// Frees all the Keychain items in |items|, and clears the vector.
-static void FreeKeychainItems(const MacKeychain& keychain,
-                              std::vector<SecKeychainItemRef>* items) {
-  CHECK(items);
-  for (std::vector<SecKeychainItemRef>::iterator i = items->begin();
-       i != items->end(); ++i) {
-    keychain.Free(*i);
-  }
-  items->clear();
-}
-
-// Deletes all the PasswordForms in |forms|, and clears the vector.
-static void DeletePasswordForms(std::vector<PasswordForm*>* forms) {
-  CHECK(forms);
-  for (std::vector<PasswordForm*>::iterator i = forms->begin();
-       i != forms->end(); ++i) {
-    delete *i;
-  }
-  forms->clear();
-}
-
 #pragma mark -
 
-TEST_F(PasswordStoreMacTest, TestSignonRealmParsing) {
-  typedef struct {
-    const char* signon_realm;
-    const bool expected_parsed;
-    const char* expected_server;
-    const bool expected_is_secure;
-    const int expected_port;
-    const char* expected_security_domain;
-  } TestData;
-
-  TestData test_data[] = {
-    // HTML form signon realms.
-    { "http://www.domain.com/",
-        true, "www.domain.com", false, 0, "" },
-    { "https://foo.org:9999/",
-        true, "foo.org", true, 9999, "" },
-    // HTTP auth signon realms.
-    { "http://httpauth.com:8080/lowsecurity",
-        true, "httpauth.com", false, 8080, "lowsecurity" },
-    { "https://httpauth.com/highsecurity",
-        true, "httpauth.com", true, 0 , "highsecurity" },
-    // Bogus realms
-    { "blahblahblah",
-        false, false, "", 0, "" },
-    { "foo/bar/baz",
-        false, false, "", 0, "" },
-  };
-
-  for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
-    std::string server;
-    std::string security_domain;
-    bool is_secure = false;
-    int port = -1;
-    bool parsed = internal_keychain_helpers::ExtractSignonRealmComponents(
-        std::string(test_data[i].signon_realm), &server, &port, &is_secure,
-        &security_domain);
-    EXPECT_EQ(test_data[i].expected_parsed, parsed) << "In iteration " << i;
-
-    if (!parsed) {
-      continue;  // If parse failed, out params are undefined.
-    }
-    EXPECT_EQ(std::string(test_data[i].expected_server), server)
-        << "In iteration " << i;
-    EXPECT_EQ(std::string(test_data[i].expected_security_domain),
-              security_domain)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_is_secure, is_secure)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_port, port) << "In iteration " << i;
-  }
-
-  // NULLs are allowed for out params.
-  bool parsed = internal_keychain_helpers::ExtractSignonRealmComponents(
-      std::string("http://foo.bar.com:1234/baz"), NULL, NULL, NULL, NULL);
-  EXPECT_TRUE(parsed);
-}
-
-TEST_F(PasswordStoreMacTest, TestURLConstruction) {
-  std::string host("exampledomain.com");
-  std::string path("/path/to/page.html");
-
-  GURL full_url = internal_keychain_helpers::URLFromComponents(false, host,
-                                                               1234, path);
-  EXPECT_TRUE(full_url.is_valid());
-  EXPECT_EQ(GURL("http://exampledomain.com:1234/path/to/page.html"), full_url);
-
-  GURL simple_secure_url = internal_keychain_helpers::URLFromComponents(
-      true, host, 0, std::string(""));
-  EXPECT_TRUE(simple_secure_url.is_valid());
-  EXPECT_EQ(GURL("https://exampledomain.com/"), simple_secure_url);
-}
-
-TEST_F(PasswordStoreMacTest, TestKeychainTime) {
-  typedef struct {
-    const char* time_string;
-    const bool expected_parsed;
-    const int expected_year;
-    const int expected_month;
-    const int expected_day;
-    const int expected_hour;
-    const int expected_minute;
-    const int expected_second;
-  } TestData;
-
-  TestData test_data[] = {
-    // HTML form signon realms.
-    { "19980330100000Z", true,  1998,  3, 30, 10,  0,  0 },
-    { "19991231235959Z", true,  1999, 12, 31, 23, 59, 59 },
-    { "20000101000000Z", true,  2000,  1,  1,  0,  0,  0 },
-    { "20011112012843Z", true,  2001, 11, 12,  1, 28, 43 },
-    { "20020601171530Z", true,  2002,  6,  1, 17, 15, 30 },
-    { "20100908070605Z", true,  2010,  9,  8,  7,  6,  5 },
-    { "20010203040",     false, 0, 0, 0, 0, 0, 0 },
-  };
-
-  for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
-    base::Time time;
-    bool parsed = internal_keychain_helpers::TimeFromKeychainTimeString(
-        test_data[i].time_string, strlen(test_data[i].time_string), &time);
-    EXPECT_EQ(test_data[i].expected_parsed, parsed) << "In iteration " << i;
-    if (!parsed) {
-      continue;
-    }
-
-    base::Time::Exploded exploded_time;
-    time.UTCExplode(&exploded_time);
-    EXPECT_EQ(test_data[i].expected_year, exploded_time.year)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_month, exploded_time.month)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_day, exploded_time.day_of_month)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_hour, exploded_time.hour)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_minute, exploded_time.minute)
-        << "In iteration " << i;
-    EXPECT_EQ(test_data[i].expected_second, exploded_time.second)
-        << "In iteration " << i;
-  }
-}
-
-TEST_F(PasswordStoreMacTest, TestAuthTypeSchemeTranslation) {
-  // Our defined types should round-trip correctly.
-  SecAuthenticationType auth_types[] = { kSecAuthenticationTypeHTMLForm,
-                                         kSecAuthenticationTypeHTTPBasic,
-                                         kSecAuthenticationTypeHTTPDigest };
-  const int auth_count = sizeof(auth_types) / sizeof(SecAuthenticationType);
-  for (int i = 0; i < auth_count; ++i) {
-    SecAuthenticationType round_tripped_auth_type =
-        internal_keychain_helpers::AuthTypeForScheme(
-            internal_keychain_helpers::SchemeForAuthType(auth_types[i]));
-    EXPECT_EQ(auth_types[i], round_tripped_auth_type);
-  }
-  // Anything else should become SCHEME_OTHER and come back as Default.
-  PasswordForm::Scheme scheme_for_other_auth_type =
-      internal_keychain_helpers::SchemeForAuthType(kSecAuthenticationTypeNTLM);
-  SecAuthenticationType round_tripped_other_auth_type =
-      internal_keychain_helpers::AuthTypeForScheme(scheme_for_other_auth_type);
-  EXPECT_EQ(PasswordForm::SCHEME_OTHER, scheme_for_other_auth_type);
-  EXPECT_EQ(kSecAuthenticationTypeDefault, round_tripped_other_auth_type);
-}
-
 TEST_F(PasswordStoreMacTest, TestKeychainToFormTranslation) {
   typedef struct {
     const PasswordForm::Scheme scheme;
@@ -383,6 +225,9 @@ TEST_F(PasswordStoreMacTest, TestKeychainToFormTranslation) {
     { PasswordForm::SCHEME_DIGEST, "https://some.domain.com/high_security",
       "https://some.domain.com/", L"digest_auth_user", L"digest", true,
       1998,  3, 30, 10,  0,  0 },
+    { PasswordForm::SCHEME_OTHER, "http://a.server.com/",
+      "http://a.server.com/", L"abc", L"123", false,
+      1970,  1,  1,  0,  0,  0 },
   };
 
   for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(expected); ++i) {
@@ -436,58 +281,46 @@ TEST_F(PasswordStoreMacTest, TestKeychainToFormTranslation) {
 }
 
 TEST_F(PasswordStoreMacTest, TestKeychainSearch) {
-  {  // An HTML form we've seen.
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("http://some.domain.com/"),
-        PasswordForm::SCHEME_HTML, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(2), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
-  }
-
-  {  // An HTML form we haven't seen
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("http://www.unseendomain.com/"),
-        PasswordForm::SCHEME_HTML, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(0), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
-  }
-
-  {  // Basic auth that should match.
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("http://some.domain.com:4567/low_security"),
-        PasswordForm::SCHEME_BASIC, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(1), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
-  }
-
-  {  // Basic auth with the wrong port.
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("http://some.domain.com:1111/low_security"),
-        PasswordForm::SCHEME_BASIC, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(0), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
-  }
-
-  {  // Digest auth we've saved under https, visited with http.
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("http://some.domain.com/high_security"),
-        PasswordForm::SCHEME_DIGEST, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(0), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
-  }
+  struct TestDataAndExpectation {
+    const PasswordFormData data;
+    const size_t expected_matches;
+  };
+  // Most fields are left blank because we don't care about them for searching.
+  TestDataAndExpectation test_data[] = {
+    // An HTML form we've seen.
+    { { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 2 },
+    // An HTML form we haven't seen
+    { { PasswordForm::SCHEME_HTML, "http://www.unseendomain.com/",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 0 },
+    // Basic auth that should match.
+    { { PasswordForm::SCHEME_BASIC, "http://some.domain.com:4567/low_security",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 1 },
+    // Basic auth with the wrong port.
+    { { PasswordForm::SCHEME_BASIC, "http://some.domain.com:1111/low_security",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 0 },
+    // Digest auth we've saved under https, visited with http.
+    { { PasswordForm::SCHEME_DIGEST, "http://some.domain.com/high_security",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 0 },
+    // Digest auth that should match.
+    { { PasswordForm::SCHEME_DIGEST, "https://some.domain.com/high_security",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, true, 0 }, 1 },
+    // Digest auth with the wrong domain.
+    { { PasswordForm::SCHEME_DIGEST, "https://some.domain.com/other_domain",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, true, 0 }, 0 },
+    // Garbage forms should have no matches.
+    { { PasswordForm::SCHEME_HTML, "foo/bar/baz",
+        NULL, NULL, NULL, NULL, NULL, NULL, NULL, false, false, 0 }, 0 },
+  };
 
-  {  // Digest auth that should match.
-    std::vector<SecKeychainItemRef> matching_items;
-    internal_keychain_helpers::FindMatchingKeychainItems(
-        *keychain_, std::string("https://some.domain.com/high_security"),
-        PasswordForm::SCHEME_DIGEST, &matching_items);
-    EXPECT_EQ(static_cast<size_t>(1), matching_items.size());
-    FreeKeychainItems(*keychain_, &matching_items);
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_);
+  for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
+    scoped_ptr<PasswordForm> query_form(
+        CreatePasswordFormFromData(test_data[i].data));
+    std::vector<PasswordForm*> matching_items =
+        keychainAdapter.PasswordsMatchingForm(*query_form);
+    EXPECT_EQ(test_data[i].expected_matches, matching_items.size());
+    STLDeleteElements(&matching_items);
   }
 }
 
@@ -503,35 +336,35 @@ TEST_F(PasswordStoreMacTest, TestKeychainExactSearch) {
     search_form.password_element = std::wstring(L"password");
     search_form.preferred = true;
     SecKeychainItemRef match;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                search_form);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            search_form);
     EXPECT_EQ(reinterpret_cast<SecKeychainItemRef>(2), match);
     keychain_->Free(match);
 
     // Make sure that the matching isn't looser than it should be.
     PasswordForm wrong_username(search_form);
     wrong_username.username_value = std::wstring(L"wrong_user");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_username);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_username);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_path(search_form);
     wrong_path.origin = GURL("http://some.domain.com/elsewhere.html");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_path);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_path);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_scheme(search_form);
     wrong_scheme.scheme = PasswordForm::SCHEME_BASIC;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_scheme);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_scheme);
     EXPECT_EQ(NULL, match);
 
     // With no path, we should match the pathless Keychain entry.
     PasswordForm no_path(search_form);
     no_path.origin = GURL("http://some.domain.com/");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                no_path);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            no_path);
     EXPECT_EQ(reinterpret_cast<SecKeychainItemRef>(1), match);
     keychain_->Free(match);
 
@@ -539,8 +372,8 @@ TEST_F(PasswordStoreMacTest, TestKeychainExactSearch) {
     // those stored by other browsers.
     PasswordForm blacklist(search_form);
     blacklist.blacklisted_by_user = true;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                blacklist);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            blacklist);
     EXPECT_EQ(NULL, match);
   }
 
@@ -554,74 +387,54 @@ TEST_F(PasswordStoreMacTest, TestKeychainExactSearch) {
     search_form.username_value = std::wstring(L"basic_auth_user");
     search_form.scheme = PasswordForm::SCHEME_BASIC;
     SecKeychainItemRef match;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                search_form);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            search_form);
     EXPECT_EQ(reinterpret_cast<SecKeychainItemRef>(7), match);
     keychain_->Free(match);
 
     // Make sure that the matching isn't looser than it should be.
     PasswordForm wrong_username(search_form);
     wrong_username.username_value = std::wstring(L"wrong_user");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_username);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_username);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_path(search_form);
     wrong_path.origin = GURL("http://some.domain.com:4567/elsewhere.html");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_path);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_path);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_scheme(search_form);
     wrong_scheme.scheme = PasswordForm::SCHEME_DIGEST;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_scheme);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_scheme);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_port(search_form);
     wrong_port.signon_realm =
         std::string("http://some.domain.com:1234/low_security");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_port);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_port);
     EXPECT_EQ(NULL, match);
 
     PasswordForm wrong_realm(search_form);
     wrong_realm.signon_realm =
         std::string("http://some.domain.com:4567/incorrect");
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                wrong_realm);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            wrong_realm);
     EXPECT_EQ(NULL, match);
 
     // We don't store blacklist entries in the keychain, and we want to ignore
     // those stored by other browsers.
     PasswordForm blacklist(search_form);
     blacklist.blacklisted_by_user = true;
-    match = internal_keychain_helpers::FindMatchingKeychainItem(*keychain_,
-                                                                blacklist);
+    match = internal_keychain_helpers::MatchingKeychainItem(*keychain_,
+                                                            blacklist);
     EXPECT_EQ(NULL, match);
   }
 }
 
-TEST_F(PasswordStoreMacTest, TestKeychainModify) {
-  SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(1);
-  EXPECT_TRUE(internal_keychain_helpers::SetKeychainItemPassword(
-      *keychain_, keychain_item, std::string("allnewpassword")));
-  PasswordForm form;
-  internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
-                                                              keychain_item,
-                                                              &form);
-  EXPECT_EQ(L"allnewpassword", form.password_value);
-
-  // Check that invalid items fail to update
-  SecKeychainItemRef invalid_item = reinterpret_cast<SecKeychainItemRef>(1000);
-  EXPECT_FALSE(internal_keychain_helpers::SetKeychainItemPassword(
-      *keychain_, invalid_item, std::string("allnewpassword")));
-
-  // Check that other errors are reported (using the magic failure value).
-  EXPECT_FALSE(internal_keychain_helpers::SetKeychainItemPassword(
-      *keychain_, keychain_item, std::string("fail_me")));
-}
-
 TEST_F(PasswordStoreMacTest, TestKeychainAdd) {
   struct TestDataAndExpectation {
     PasswordFormData data;
@@ -652,15 +465,15 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) {
         L"joe_user", L"fail_me", false, false, 0 }, false },
   };
 
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_);
+
   for (unsigned int i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) {
     PasswordForm* in_form = CreatePasswordFormFromData(test_data[i].data);
-    bool add_succeeded =
-        internal_keychain_helpers::AddKeychainEntryForForm(*keychain_,
-                                                           *in_form);
+    bool add_succeeded = keychainAdapter.AddLogin(*in_form);
     EXPECT_EQ(test_data[i].should_succeed, add_succeeded);
     if (add_succeeded) {
       SecKeychainItemRef matching_item;
-      matching_item = internal_keychain_helpers::FindMatchingKeychainItem(
+      matching_item = internal_keychain_helpers::MatchingKeychainItem(
           *keychain_, *in_form);
       EXPECT_TRUE(matching_item != NULL);
       PasswordForm out_form;
@@ -684,8 +497,7 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) {
       NULL, NULL, NULL, L"joe_user", L"updated_password", false, false, 0
     };
     PasswordForm* update_form = CreatePasswordFormFromData(data);
-    EXPECT_TRUE(internal_keychain_helpers::AddKeychainEntryForForm(
-        *keychain_, *update_form));
+    EXPECT_TRUE(keychainAdapter.AddLogin(*update_form));
     SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(2);
     PasswordForm stored_form;
     internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
@@ -917,8 +729,8 @@ TEST_F(PasswordStoreMacTest, TestFormMerge) {
                 test_case);
     CHECK_FORMS(merged_forms, test_data[MERGE_OUTPUT][test_case], test_case);
 
-    DeletePasswordForms(&keychain_forms);
-    DeletePasswordForms(&database_forms);
-    DeletePasswordForms(&merged_forms);
+    STLDeleteElements(&keychain_forms);
+    STLDeleteElements(&database_forms);
+    STLDeleteElements(&merged_forms);
   }
 }