[sync] Significantly speed up password model association on mac

chrome/browser/password_manager/password_store_mac.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/password_manager/password_store_mac.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
 
 #include <CoreServices/CoreServices.h>
 #include <set>
 #include <string>
@@ skipping 428 matching lines @@
                        database_blacklist_forms.begin(),
                        database_blacklist_forms.end());
 
   // Clear out all the Keychain entries we used.
   DeleteVectorElementsInSet(keychain_forms, used_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);
     } else {
       ++i;
     }
     STLDeleteElements(&keychain_matches);
   }
+  STLDeleteElements(&keychain_forms);
   return merged_forms;
 }
 
 }  // namespace internal_keychain_helpers
 
 #pragma mark -
 
 MacKeychainPasswordFormAdapter::MacKeychainPasswordFormAdapter(
     const AppleKeychain* keychain)
     : keychain_(keychain), finds_only_owned_(false) {
@@ skipping 543 matching lines @@
   owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
   for (std::vector<PasswordForm*>::const_iterator i = forms.begin();
        i != forms.end(); ++i) {
     owned_keychain_adapter.RemovePassword(**i);
   }
 }
 
 void PasswordStoreMac::CreateNotificationService() {
   notification_service_.reset(content::NotificationService::Create());
 }