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

chrome/browser/password_manager/native_backend_gnome_x.cc

Index: chrome/browser/password_manager/native_backend_gnome_x.cc
diff --git a/chrome/browser/password_manager/native_backend_gnome_x.cc b/chrome/browser/password_manager/native_backend_gnome_x.cc
index 460326776cc8c1db7704c7f988725e4f5196c3ac..e7294a53dc7b6a7a93c7d66b5fcd33c84d443a72 100644
--- a/chrome/browser/password_manager/native_backend_gnome_x.cc
+++ b/chrome/browser/password_manager/native_backend_gnome_x.cc
@@ -574,7 +574,9 @@ password_manager::PasswordStoreChangeList NativeBackendGnome::AddLogin(
   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
@@ -582,13 +584,15 @@ bool NativeBackendGnome::UpdateLogin(const PasswordForm& form) {
   // 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);
@@ -597,22 +601,17 @@ bool NativeBackendGnome::UpdateLogin(const PasswordForm& form) {
 
   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;
 }