Password Login Database: report correct changes from UpdateLogin().

chrome/browser/password_manager/native_backend_gnome_x.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/native_backend_gnome_x.h"
 
 #include <dlfcn.h>
 #include <gnome-keyring.h>
 
 #include <map>
@@ skipping 556 matching lines @@
           password_manager::PasswordStoreChange::REMOVE, *forms[0]));
     }
   }
   if (RawAddLogin(form)) {
     changes.push_back(password_manager::PasswordStoreChange(
         password_manager::PasswordStoreChange::ADD, form));
   }
   return changes;
 }
 
-bool NativeBackendGnome::UpdateLogin(const PasswordForm& form) {
+bool NativeBackendGnome::UpdateLogin(
+    const PasswordForm& form,
+    password_manager::PasswordStoreChangeList* changes) {
   // Based on LoginDatabase::UpdateLogin(), we search for forms to update by
   // origin_url, username_element, username_value, password_element, and
   // signon_realm. We then compare the result to the updated form. If they
   // differ in any of the mutable fields, then we remove the original, and
   // then add the new entry. We'd add the new one first, and then delete the
   // original, but then the delete might actually delete the newly-added entry!
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  DCHECK(changes);
+  changes->clear();
   GKRMethod method;
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&GKRMethod::UpdateLoginSearch,
                                      base::Unretained(&method),
                                      form, app_string_.c_str()));
-  PasswordFormList forms;
-  GnomeKeyringResult result = method.WaitResult(&forms);
+  ScopedVector<autofill::PasswordForm> forms;
+  GnomeKeyringResult result = method.WaitResult(&forms.get());
   if (result != GNOME_KEYRING_RESULT_OK) {
     LOG(ERROR) << "Keyring find failed: "
                << gnome_keyring_result_to_message(result);
     return false;
   }
 
   bool ok = true;
   for (size_t i = 0; i < forms.size(); ++i) {
-    if (forms[i]->action != form.action ||
-        forms[i]->password_value != form.password_value ||
-        forms[i]->ssl_valid != form.ssl_valid ||
-        forms[i]->preferred != form.preferred ||
-        forms[i]->times_used != form.times_used) {
+    if (*forms[i] != form) {
       RemoveLogin(*forms[i]);
 
-      forms[i]->action = form.action;
-      forms[i]->password_value = form.password_value;
-      forms[i]->ssl_valid = form.ssl_valid;
-      forms[i]->preferred = form.preferred;
-      forms[i]->times_used = form.times_used;
-      if (!RawAddLogin(*forms[i]))
+      if (RawAddLogin(form)) {

[Garrett Casto, 2014/05/21 19:14:20]

Now that we completely overwrite the saved form, is there any reason for this to
be in the loop? Seems like it should just be done once afterwards (assuming
!forms.empty()).

[vasilii, 2014/05/22 11:38:14]

Done.

+        password_manager::PasswordStoreChange change(
+            password_manager::PasswordStoreChange::UPDATE, form);
+        changes->assign(&change, &change + 1);
+      } else {
         ok = false;
+      }
     }
-    delete forms[i];
   }
   return ok;
 }
 
 bool NativeBackendGnome::RemoveLogin(const PasswordForm& form) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
   GKRMethod method;
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&GKRMethod::RemoveLogin,
                                      base::Unretained(&method),
@@ skipping 119 matching lines @@
   }
   return true;
 }
 
 std::string NativeBackendGnome::GetProfileSpecificAppString() const {
   // Originally, the application string was always just "chrome" and used only
   // so that we had *something* to search for since GNOME Keyring won't search
   // for nothing. Now we use it to distinguish passwords for different profiles.
   return base::StringPrintf("%s-%d", kGnomeKeyringAppString, profile_id_);
 }