Implement Add and Update for PasswordStoreMac....

chrome/browser/password_manager/password_store_mac.cc

 // Copyright (c) 2009 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 <string>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/browser/keychain_mac.h"
 #include "chrome/browser/password_manager/login_database_mac.h"
 
 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 {
  public:
   KeychainSearch(const MacKeychain& keychain);
   ~KeychainSearch();
 
   // Sets up a keycahin search based on an non "null" (NULL for char*,
   // The appropriate "Any" entry for other types) arguments.
   //
@@ skipping 501 matching lines @@
   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;
+  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 =
         internal_keychain_helpers::MatchingKeychainItem(*keychain_, form);
     if (!existing_item) {
       return false;
     }
     bool changed = SetKeychainItemPassword(existing_item, password);
     keychain_->Free(existing_item);
     return changed;
   }
 
@@ skipping 50 matching lines @@
 }
 
 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);
 }
 
+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,
                                    LoginDatabaseMac* login_db)
     : keychain_(keychain), login_metadata_db_(login_db) {
   DCHECK(keychain_.get());
   DCHECK(login_metadata_db_.get());
 }
 
 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) {
   NOTIMPLEMENTED();
 }
 
 void PasswordStoreMac::GetLoginsImpl(GetLoginsRequest* request) {
   MacKeychainPasswordFormAdapter keychainAdapter(keychain_.get());
   std::vector<PasswordForm*> keychain_forms =
       keychainAdapter.PasswordsMatchingForm(request->form);
@@ skipping 10 matching lines @@
   for (std::vector<PasswordForm*>::iterator i = database_forms.begin();
        i != database_forms.end(); ++i) {
     login_metadata_db_->RemoveLogin(**i);
   }
   // Delete the forms we aren't returning.
   STLDeleteElements(&database_forms);
   STLDeleteElements(&keychain_forms);
 
   NotifyConsumer(request, merged_forms);
 }