Implement Add and Update for PasswordStoreMac....

chrome/browser/password_manager/password_store_mac.cc

Index: chrome/browser/password_manager/password_store_mac.cc
===================================================================
--- chrome/browser/password_manager/password_store_mac.cc	(revision 19773)
+++ chrome/browser/password_manager/password_store_mac.cc	(working copy)
@@ -18,6 +18,8 @@
 
 using webkit_glue::PasswordForm;
 
+static const OSType kChromeKeychainCreatorCode = 'GCHR';
+
 // Utility class to handle the details of constructing and running a keychain
 // search from a set of attributes.
 class KeychainSearch {
@@ -539,14 +541,20 @@
   std::string path = form.origin.path();
   SecProtocolType protocol = is_secure ? kSecProtocolTypeHTTPS
                                        : kSecProtocolTypeHTTP;
+  SecKeychainItemRef new_item = NULL;
   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);
+      password.size(), password.c_str(), &new_item);
 
+  if (result == noErr) {
+    SetKeychainItemCreatorCode(new_item, kChromeKeychainCreatorCode);
+    keychain_->Free(new_item);
+  }
+
   // If we collide with an existing item, find and update it instead.
   if (result == errSecDuplicateItem) {

[Mark Mentovai, 2009/07/02 03:03:33]

This ought to be |else if| now.

     SecKeychainItemRef existing_item =
@@ -619,6 +627,16 @@
   return (result == noErr);
 }
 
+bool MacKeychainPasswordFormAdapter::SetKeychainItemCreatorCode(
+    const SecKeychainItemRef& keychain_item, OSType creator_code) {
+  SecKeychainAttribute attr = { kSecCreatorItemAttr, sizeof(creator_code),
+                                &creator_code };
+  SecKeychainAttributeList attrList = { 1, &attr };
+  OSStatus result = keychain_->ItemModifyAttributesAndData(keychain_item,
+                                                           &attrList, 0, NULL);
+  return (result == noErr);

[Mark Mentovai, 2009/07/02 03:03:33]

Parens aren't necessary here or in the preceding function.  I don't think they
improve clarity, but if you do, you can keep them.

+}
+
 #pragma mark -
 
 PasswordStoreMac::PasswordStoreMac(MacKeychain* keychain,
@@ -631,11 +649,25 @@
 PasswordStoreMac::~PasswordStoreMac() {}
 
 void PasswordStoreMac::AddLoginImpl(const PasswordForm& form) {
-  NOTIMPLEMENTED();
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_.get());
+  if (keychainAdapter.AddLogin(form)) {
+    login_metadata_db_->AddLogin(form);
+  }
 }
 
 void PasswordStoreMac::UpdateLoginImpl(const PasswordForm& form) {
-  NOTIMPLEMENTED();
+  MacKeychainPasswordFormAdapter keychainAdapter(keychain_.get());
+  // The keychain AddLogin will update if there is a collision and add if there
+  // isn't, which is the behavior we want, so there's no separate UpdateLogin.
+  if (keychainAdapter.AddLogin(form)) {
+    int update_count = 0;
+    login_metadata_db_->UpdateLogin(form, &update_count);
+    // Update will catch any database entries that we already had, but we could
+    // also be updating a keychain-only form, in which case we need to add.
+    if (update_count == 0) {
+      login_metadata_db_->AddLogin(form);
+    }
+  }
 }
 
 void PasswordStoreMac::RemoveLoginImpl(const PasswordForm& form) {