Add an exact search method to the Keychain adapter, and modify unit tests to us that instead of the raw keychain version.

Overhaul the single-item-search test to be shorter, clearer, and more complete.
Convert more namespaced methods that now have no public callers to private helpers of the Keychain adapter, and refactor the helpers.

BUG=none
TEST=none; no behavioral changes
Review URL: http://codereview.chromium.org/149160

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

3 files changed, 230 insertions, 243 deletions

diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index 56980fc..a451bed 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -143,7 +143,7 @@ void KeychainSearch::FindMatchingItems(std::vector<SecKeychainItemRef>* items) {
 
   SecKeychainItemRef keychain_item;
   while (keychain_->SearchCopyNext(search_ref_, &keychain_item) == noErr) {
-    // Consumer is responsible for deleting the forms when they are done.
+    // Consumer is responsible for freeing the items.
     items->push_back(keychain_item);
   }
 
@@ -158,37 +158,6 @@ void KeychainSearch::FindMatchingItems(std::vector<SecKeychainItemRef>* items) {
 // 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,
-                                  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.
-  GURL realm_as_url(signon_realm);
-  if (!realm_as_url.is_valid()) {
-    return false;
-  }
-
-  if (server)
-    *server = realm_as_url.host();
-  if (is_secure)
-    *is_secure = realm_as_url.SchemeIsSecure();
-  if (port)
-    *port = realm_as_url.has_port() ? atoi(realm_as_url.port().c_str()) : 0;
-  if (security_domain) {
-    // Strip the leading '/' off of the path to get the security domain.
-    *security_domain = realm_as_url.path().substr(1);
-  }
-  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,
@@ -237,18 +206,6 @@ 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;
-    case PasswordForm::SCHEME_BASIC:  return kSecAuthenticationTypeHTTPBasic;
-    case PasswordForm::SCHEME_DIGEST: return kSecAuthenticationTypeHTTPDigest;
-    case PasswordForm::SCHEME_OTHER:  return kSecAuthenticationTypeDefault;
-  }
-  NOTREACHED();
-  return kSecAuthenticationTypeDefault;
-}
-
 // Returns the PasswordForm Scheme corresponding to |auth_type|.
 PasswordForm::Scheme SchemeForAuthType(SecAuthenticationType auth_type) {
   switch (auth_type) {
@@ -259,53 +216,6 @@ PasswordForm::Scheme SchemeForAuthType(SecAuthenticationType auth_type) {
   }
 }
 
-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) {
-    return NULL;
-  }
-
-  // Construct a keychain search based on all the unique attributes.
-  std::string server;
-  std::string security_domain;
-  int port;
-  bool is_secure;
-  if (!ExtractSignonRealmComponents(form.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 NULL;
-  }
-
-  SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
-                                       : kSecProtocolTypeHTTP;
-  SecAuthenticationType auth_type = AuthTypeForScheme(form.scheme);
-  std::string path = form.origin.path();
-  std::string username = WideToUTF8(form.username_value);
-
-  KeychainSearch keychain_search(keychain);
-  keychain_search.Init(server.c_str(), port, protocol, auth_type,
-                       (form.scheme == PasswordForm::SCHEME_HTML) ?
-                           NULL : security_domain.c_str(),
-                       path.c_str(), username.c_str());
-
-  std::vector<SecKeychainItemRef> matches;
-  keychain_search.FindMatchingItems(&matches);
-
-  if (matches.size() == 0) {
-    return NULL;
-  }
-  // Free all items after the first, since we won't be returning them.
-  for (std::vector<SecKeychainItemRef>::iterator i = matches.begin() + 1;
-       i != matches.end(); ++i) {
-    keychain.Free(*i);
-  }
-  return matches[0];
-}
-
 bool FillPasswordFormFromKeychainItem(const MacKeychain& keychain,
                                       const SecKeychainItemRef& keychain_item,
                                       PasswordForm* form) {
@@ -510,13 +420,11 @@ 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*>
     MacKeychainPasswordFormAdapter::PasswordsMatchingForm(
         const PasswordForm& query_form) {
   std::vector<SecKeychainItemRef> keychain_items =
-      MatchingKeychainItems(query_form.signon_realm, query_form.scheme);
+      KeychainItemsForFillingForm(query_form);
 
   std::vector<PasswordForm*> keychain_forms =
       CreateFormsFromKeychainItems(keychain_items);
@@ -527,13 +435,27 @@ std::vector<PasswordForm*>
   return keychain_forms;
 }
 
+PasswordForm* MacKeychainPasswordFormAdapter::PasswordExactlyMatchingForm(
+    const PasswordForm& query_form) {
+  SecKeychainItemRef keychain_item = KeychainItemForForm(query_form);
+  if (keychain_item) {
+    PasswordForm* form = new PasswordForm();
+    internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
+                                                                keychain_item,
+                                                                form);
+    keychain_->Free(keychain_item);
+    return form;
+  }
+  return NULL;
+}
+
 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)) {
+  if (!ExtractSignonRealmComponents(form.signon_realm, &server, &port,
+                                    &is_secure, &security_domain)) {
     return false;
   }
   std::string username = WideToUTF8(form.username_value);
@@ -547,7 +469,7 @@ bool MacKeychainPasswordFormAdapter::AddLogin(const PasswordForm& form) {
       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),
+      port, protocol, AuthTypeForScheme(form.scheme),
       password.size(), password.c_str(), &new_item);
 
   if (result == noErr) {
@@ -555,8 +477,7 @@ bool MacKeychainPasswordFormAdapter::AddLogin(const PasswordForm& form) {
     keychain_->Free(new_item);
   } else if (result == errSecDuplicateItem) {
     // If we collide with an existing item, find and update it instead.
-    SecKeychainItemRef existing_item =
-        internal_keychain_helpers::MatchingKeychainItem(*keychain_, form);
+    SecKeychainItemRef existing_item = KeychainItemForForm(form);
     if (!existing_item) {
       return false;
     }
@@ -583,40 +504,104 @@ std::vector<PasswordForm*>
   return keychain_forms;
 }
 
-// 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::KeychainItemsForFillingForm(
+        const PasswordForm& form) {
+  return MatchingKeychainItems(form.signon_realm, form.scheme, NULL, NULL);
+}
+
+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;
+  }
+
+  std::string path = form.origin.path();
+  std::string username = WideToUTF8(form.username_value);
+  std::vector<SecKeychainItemRef> matches = MatchingKeychainItems(
+      form.signon_realm, form.scheme, path.c_str(), username.c_str());
+
+  if (matches.size() == 0) {
+    return NULL;
+  }
+  // Free all items after the first, since we won't be returning them.
+  for (std::vector<SecKeychainItemRef>::iterator i = matches.begin() + 1;
+       i != matches.end(); ++i) {
+    keychain_->Free(*i);
+  }
+  return matches[0];
+}
+
 std::vector<SecKeychainItemRef>
     MacKeychainPasswordFormAdapter::MatchingKeychainItems(
-        const std::string& signon_realm, PasswordForm::Scheme scheme) {
+        const std::string& signon_realm,
+        webkit_glue::PasswordForm::Scheme scheme,
+        const char* path, const char* username) {
   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)) {
+  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 matches;
   }
-
   SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
                                        : kSecProtocolTypeHTTP;
-  SecAuthenticationType auth_type =
-      internal_keychain_helpers::AuthTypeForScheme(scheme);
+  SecAuthenticationType auth_type = AuthTypeForScheme(scheme);
+  const char* auth_domain = (scheme == PasswordForm::SCHEME_HTML) ?
+      NULL : security_domain.c_str();
 
   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);
+                       auth_domain, path, username);
   keychain_search.FindMatchingItems(&matches);
   return matches;
 }
 
+// TODO(stuartmorgan): signon_realm for proxies is not yet supported.
+bool MacKeychainPasswordFormAdapter::ExtractSignonRealmComponents(
+    const std::string& signon_realm, std::string* server, int* 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.
+  GURL realm_as_url(signon_realm);
+  if (!realm_as_url.is_valid()) {
+    return false;
+  }
+
+  if (server)
+    *server = realm_as_url.host();
+  if (is_secure)
+    *is_secure = realm_as_url.SchemeIsSecure();
+  if (port)
+    *port = realm_as_url.has_port() ? atoi(realm_as_url.port().c_str()) : 0;
+  if (security_domain) {
+    // Strip the leading '/' off of the path to get the security domain.
+    *security_domain = realm_as_url.path().substr(1);
+  }
+  return true;
+}
+
+// Returns the Keychain SecAuthenticationType type corresponding to |scheme|.
+SecAuthenticationType MacKeychainPasswordFormAdapter::AuthTypeForScheme(
+    PasswordForm::Scheme scheme) {
+  switch (scheme) {
+    case PasswordForm::SCHEME_HTML:   return kSecAuthenticationTypeHTMLForm;
+    case PasswordForm::SCHEME_BASIC:  return kSecAuthenticationTypeHTTPBasic;
+    case PasswordForm::SCHEME_DIGEST: return kSecAuthenticationTypeHTTPDigest;
+    case PasswordForm::SCHEME_OTHER:  return kSecAuthenticationTypeDefault;
+  }
+  NOTREACHED();
+  return kSecAuthenticationTypeDefault;
+}
+
 bool MacKeychainPasswordFormAdapter::SetKeychainItemPassword(
     const SecKeychainItemRef& keychain_item, const std::string& password) {
   OSStatus result = keychain_->ItemModifyAttributesAndData(keychain_item, NULL,
diff --git a/chrome/browser/password_manager/password_store_mac_internal.h b/chrome/browser/password_manager/password_store_mac_internal.h
index 6b7712b..35d0d8a 100644
--- a/chrome/browser/password_manager/password_store_mac_internal.h
+++ b/chrome/browser/password_manager/password_store_mac_internal.h
@@ -22,11 +22,18 @@ class MacKeychainPasswordFormAdapter {
   // created object.
   explicit MacKeychainPasswordFormAdapter(MacKeychain* keychain);
 
-  // Returns PasswordForms for each keychain entry matching |form|.
-  // Caller is responsible for deleting the returned forms.
+  // Returns PasswordForms for each keychain entry that could be used to fill
+  // |form|. Caller is responsible for deleting the returned forms.
   std::vector<webkit_glue::PasswordForm*> PasswordsMatchingForm(
       const webkit_glue::PasswordForm& query_form);
 
+  // Returns the PasswordForm for the Keychain entry that matches |form| on all
+  // of the fields that uniquely identify a Keychain item, or NULL if there is
+  // no such entry.
+  // Caller is responsible for deleting the returned form.
+  webkit_glue::PasswordForm* PasswordExactlyMatchingForm(
+      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.
@@ -38,11 +45,38 @@ class MacKeychainPasswordFormAdapter {
   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.
+  // Searches |keychain| for all items usable for the given form, and returns
+  // them. The caller is responsible for calling MacKeychain::Free on the
+  // returned items.
+  std::vector<SecKeychainItemRef> KeychainItemsForFillingForm(
+      const webkit_glue::PasswordForm& form);
+
+  // Searches |keychain| for the specific keychain entry that corresponds to the
+  // given form, and returns it (or NULL if no match is found).  The caller is
+  // responsible for calling MacKeychain::Free on on the returned item.
+  SecKeychainItemRef KeychainItemForForm(
+      const webkit_glue::PasswordForm& form);
+
+  // Returns the Keychain items matching the given signon_realm, scheme, and
+  // optionally path and username (either of both can be NULL).
+  // them. The caller is responsible for calling MacKeychain::Free on the
+  // returned items.
   std::vector<SecKeychainItemRef> MatchingKeychainItems(
-      const std::string& signon_realm,
+      const std::string& signon_realm, webkit_glue::PasswordForm::Scheme scheme,
+      const char* path, const char* username);
+
+  // 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 out params is undefined.
+  bool ExtractSignonRealmComponents(const std::string& signon_realm,
+                                    std::string* server, int* port,
+                                    bool* is_secure,
+                                    std::string* security_domain);
+
+  // Returns the Keychain SecAuthenticationType type corresponding to |scheme|.
+  SecAuthenticationType AuthTypeForScheme(
       webkit_glue::PasswordForm::Scheme scheme);
 
   // Changes the password for keychain_item to |password|; returns true if the
@@ -62,12 +96,6 @@ class MacKeychainPasswordFormAdapter {
 
 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 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.
 //
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc
index f26fffe..2b85b9a 100644
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc
+++ b/chrome/browser/password_manager/password_store_mac_unittest.cc
@@ -331,114 +331,93 @@ TEST_F(PasswordStoreMacTest, TestKeychainSearch) {
   }
 }
 
+// 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);
+}
+
+// Changes just the signon_realm port of |form|.
+static void SetPasswordFormPort(PasswordForm* form, const char* port) {
+  GURL::Replacements replacement;
+  std::string new_value(port);
+  replacement.SetPortStr(new_value);
+  GURL signon_gurl = GURL(form->signon_realm);
+  form->signon_realm = signon_gurl.ReplaceComponents(replacement).spec();
+}
+
+// Changes just the signon_ream auth realm of |form|.
+static void SetPasswordFormRealm(PasswordForm* form, const char* realm) {
+  GURL::Replacements replacement;
+  std::string new_value(realm);
+  replacement.SetPathStr(new_value);
+  GURL signon_gurl = GURL(form->signon_realm);
+  form->signon_realm = signon_gurl.ReplaceComponents(replacement).spec();
+}
+
 TEST_F(PasswordStoreMacTest, TestKeychainExactSearch) {
-  // Test a web form entry (SCHEME_HTML).
-  {
-    PasswordForm search_form;
-    search_form.signon_realm = std::string("http://some.domain.com/");
-    search_form.origin = GURL("http://some.domain.com/insecure.html");
-    search_form.action = GURL("http://some.domain.com/submit.cgi");
-    search_form.username_element = std::wstring(L"username");
-    search_form.username_value = std::wstring(L"joe_user");
-    search_form.password_element = std::wstring(L"password");
-    search_form.preferred = true;
-    SecKeychainItemRef match;
-    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::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::MatchingKeychainItem(*keychain_,
-                                                            wrong_path);
-    EXPECT_EQ(NULL, match);
-
-    PasswordForm wrong_scheme(search_form);
-    wrong_scheme.scheme = PasswordForm::SCHEME_BASIC;
-    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::MatchingKeychainItem(*keychain_,
-                                                            no_path);
-    EXPECT_EQ(reinterpret_cast<SecKeychainItemRef>(1), match);
-    keychain_->Free(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::MatchingKeychainItem(*keychain_,
-                                                            blacklist);
-    EXPECT_EQ(NULL, match);
-  }
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_);
 
-  // Test an http auth entry (SCHEME_BASIC, but SCHEME_DIGEST works is searched
-  // the same way, so this gives sufficient coverage of both).
-  {
-    PasswordForm search_form;
-    search_form.signon_realm =
-        std::string("http://some.domain.com:4567/low_security");
-    search_form.origin = GURL("http://some.domain.com:4567/insecure.html");
-    search_form.username_value = std::wstring(L"basic_auth_user");
-    search_form.scheme = PasswordForm::SCHEME_BASIC;
-    SecKeychainItemRef match;
-    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::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::MatchingKeychainItem(*keychain_,
-                                                            wrong_path);
-    EXPECT_EQ(NULL, match);
-
-    PasswordForm wrong_scheme(search_form);
-    wrong_scheme.scheme = PasswordForm::SCHEME_DIGEST;
-    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::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::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::MatchingKeychainItem(*keychain_,
-                                                            blacklist);
-    EXPECT_EQ(NULL, match);
+  PasswordFormData base_form_data[] = {
+    { PasswordForm::SCHEME_HTML, "http://some.domain.com/",
+      "http://some.domain.com/insecure.html",
+      NULL, NULL, NULL, NULL, L"joe_user", NULL, true, false, 0 },
+    { PasswordForm::SCHEME_BASIC, "http://some.domain.com:4567/low_security",
+      "http://some.domain.com:4567/insecure.html",
+      NULL, NULL, NULL, NULL, L"basic_auth_user", NULL, true, false, 0 },
+    { PasswordForm::SCHEME_DIGEST, "https://some.domain.com/high_security",
+      "https://some.domain.com",
+      NULL, NULL, NULL, NULL, L"digest_auth_user", NULL, true, true, 0 },
+  };
+
+  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]));
+    PasswordForm* match =
+        keychainAdapter.PasswordExactlyMatchingForm(*base_form);
+    EXPECT_TRUE(match != NULL);
+    if (match) {
+      EXPECT_EQ(base_form->scheme, match->scheme);
+      EXPECT_EQ(base_form->origin, match->origin);
+      EXPECT_EQ(base_form->username_value, match->username_value);
+      delete match;
+    }
+
+    // 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;
+
+    modified_forms.push_back(new PasswordForm(*base_form));
+    modified_forms.back()->username_value = std::wstring(L"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) {
+      PasswordForm* match =
+          keychainAdapter.PasswordExactlyMatchingForm(*modified_forms[j]);
+      EXPECT_EQ(NULL, match) << "In modified version " << j << " of base form "
+                             << i;
+    }
+    STLDeleteElements(&modified_forms);
   }
 }
 
@@ -479,19 +458,14 @@ TEST_F(PasswordStoreMacTest, TestKeychainAdd) {
     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::MatchingKeychainItem(
-          *keychain_, *in_form);
-      EXPECT_TRUE(matching_item != NULL);
-      PasswordForm out_form;
-      internal_keychain_helpers::FillPasswordFormFromKeychainItem(
-          *keychain_, matching_item, &out_form);
-      EXPECT_EQ(out_form.scheme, in_form->scheme);
-      EXPECT_EQ(out_form.signon_realm, in_form->signon_realm);
-      EXPECT_EQ(out_form.origin, in_form->origin);
-      EXPECT_EQ(out_form.username_value, in_form->username_value);
-      EXPECT_EQ(out_form.password_value, in_form->password_value);
-      keychain_->Free(matching_item);
+      scoped_ptr<PasswordForm> out_form(
+          keychainAdapter.PasswordExactlyMatchingForm(*in_form));
+      EXPECT_TRUE(out_form.get() != NULL);
+      EXPECT_EQ(out_form->scheme, in_form->scheme);
+      EXPECT_EQ(out_form->signon_realm, in_form->signon_realm);
+      EXPECT_EQ(out_form->origin, in_form->origin);
+      EXPECT_EQ(out_form->username_value, in_form->username_value);
+      EXPECT_EQ(out_form->password_value, in_form->password_value);
     }
     delete in_form;
   }