[sync] Significantly speed up password model association on mac

chrome/browser/password_manager/password_store_mac.cc

Index: chrome/browser/password_manager/password_store_mac.cc
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index 74456e0a14452e354e109009a7b5377a6f7019ef..b25962d571fd095a50cd9b4ba8aeb4ae62ec73e3 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -446,14 +446,31 @@ void MergePasswordForms(std::vector<PasswordForm*>* keychain_forms,
 std::vector<PasswordForm*> GetPasswordsForForms(
     const AppleKeychain& keychain,
     std::vector<PasswordForm*>* database_forms) {
+  // We load all passwords added to the keychain by Chrome at the start, instead

[stuartmorgan, 2013/08/29 20:24:23]

This comment is incorrect. You aren't loading all the passwords added to the
keychain "by Chrome", you are loading *all* form-type passwords ever stored in
the keychain by any application. This will potentially trigger N copies of a
dialog asking the user to authorize access to keychain entries that Chrome has
never used, which is unacceptable.

This would have to be done in conjunction with something like
https://codereview.chromium.org/19775014/ for it to be viable.

[Raghu Simha, 2013/08/29 23:28:46]

You are correct. I only stepped through one level of the code and noticed that
GetAllPasswordFormPasswords() was passing in CreatorCodeForSearch() while
initializing keychain_search, and this led me to believe we were searching only
for passwords owned by Chrome.

I took a closer look and tried modifying my code to first call
keychain_adapter.SetFindsOnlyOwnedItems(true) before calling
GetAllPasswordFormPasswords(), but this only returned a subset of the passwords
added by chrome. This is presumably because I've used Google Chrome to add
passwords in some cases, and Chromium in other cases. Either way, setting
finds_only_owned_ to true doesn't appear to work.

+  // of individually searching through the keychain for passwords matching each
+  // form in |database_forms|. This will result in a significant performance
+  // gain, since we are replacing O(N) keychain search operations with a single
+  // operation that loads all of Chrome's passwords. See crbug.com/263685.
   MacKeychainPasswordFormAdapter keychain_adapter(&keychain);
+  std::vector<PasswordForm*> keychain_forms =
+      keychain_adapter.GetAllPasswordFormPasswords();
 
   std::vector<PasswordForm*> merged_forms;
   for (std::vector<PasswordForm*>::iterator i = database_forms->begin();
        i != database_forms->end();) {
     std::vector<PasswordForm*> db_form_container(1, *i);
-    std::vector<PasswordForm*> keychain_matches =
-        keychain_adapter.PasswordsMergeableWithForm(**i);
+    std::vector<PasswordForm*> keychain_matches;
+    for (std::vector<PasswordForm*>::iterator j = keychain_forms.begin();
+         j != keychain_forms.end(); ++j) {
+      if ((*i)->username_value == (*j)->username_value &&
+          (*i)->scheme == (*j)->scheme &&
+          GURL((*i)->signon_realm) == GURL((*j)->signon_realm)) {
+        // Create a new object, since the caller is responsible for deleting the
+        // returned forms.
+        PasswordForm* form = new PasswordForm(**j);
+        keychain_matches.push_back(form);
+      }
+    }
     MergePasswordForms(&keychain_matches, &db_form_container, &merged_forms);
     if (db_form_container.empty()) {
       i = database_forms->erase(i);
@@ -462,6 +479,7 @@ std::vector<PasswordForm*> GetPasswordsForForms(
     }
     STLDeleteElements(&keychain_matches);
   }
+  STLDeleteElements(&keychain_forms);
   return merged_forms;
 }