PasswordStore: Use ScopedVector to express ownership of forms

chrome/browser/password_manager/native_backend_kwallet_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_kwallet_x.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 254 matching lines @@
   return INIT_SUCCESS;
 }
 
 password_manager::PasswordStoreChangeList NativeBackendKWallet::AddLogin(
     const PasswordForm& form) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return password_manager::PasswordStoreChangeList();
 
   ScopedVector<autofill::PasswordForm> forms;
-  GetLoginsList(&forms.get(), form.signon_realm, wallet_handle);
+  GetLoginsList(form.signon_realm, wallet_handle, &forms);
 
   // We search for a login to update, rather than unconditionally appending the
   // login, because in some cases (especially involving sync) we can be asked to
   // add a login that already exists. In these cases we want to just update.
   bool updated = false;
   password_manager::PasswordStoreChangeList changes;
   for (size_t i = 0; i < forms.size(); ++i) {
     // Use the more restrictive removal comparison, so that we never have
     // duplicate logins that would all be removed together by RemoveLogin().
     if (CompareForms(form, *forms[i], false)) {
@@ skipping 18 matching lines @@
 bool NativeBackendKWallet::UpdateLogin(
     const PasswordForm& form,
     password_manager::PasswordStoreChangeList* changes) {
   DCHECK(changes);
   changes->clear();
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
 
   ScopedVector<autofill::PasswordForm> forms;
-  GetLoginsList(&forms.get(), form.signon_realm, wallet_handle);
+  GetLoginsList(form.signon_realm, wallet_handle, &forms);
 
   bool updated = false;
   for (size_t i = 0; i < forms.size(); ++i) {
     if (CompareForms(form, *forms[i], true)) {
       *forms[i] = form;
       updated = true;
     }
   }
   if (!updated)
     return true;
 
   if (SetLoginsList(forms.get(), form.signon_realm, wallet_handle)) {
     changes->push_back(password_manager::PasswordStoreChange(
         password_manager::PasswordStoreChange::UPDATE, form));
     return true;
   }
 
   return false;
 }
 
 bool NativeBackendKWallet::RemoveLogin(const PasswordForm& form) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
 
-  PasswordFormList all_forms;
-  GetLoginsList(&all_forms, form.signon_realm, wallet_handle);
+  ScopedVector<autofill::PasswordForm> all_forms;
+  GetLoginsList(form.signon_realm, wallet_handle, &all_forms);
 
-  PasswordFormList kept_forms;
+  ScopedVector<autofill::PasswordForm> kept_forms;
   kept_forms.reserve(all_forms.size());
-  for (size_t i = 0; i < all_forms.size(); ++i) {
-    if (CompareForms(form, *all_forms[i], false))
-      delete all_forms[i];
-    else
-      kept_forms.push_back(all_forms[i]);
+  for (auto& saved_form : all_forms) {
+    if (!CompareForms(form, *saved_form, false)) {
+      kept_forms.push_back(saved_form);
+      saved_form = nullptr;
+    }
   }
 
   // Update the entry in the wallet, possibly deleting it.
-  bool ok = SetLoginsList(kept_forms, form.signon_realm, wallet_handle);
-
-  STLDeleteElements(&kept_forms);
-  return ok;
+  return SetLoginsList(kept_forms.get(), form.signon_realm, wallet_handle);
 }
 
 bool NativeBackendKWallet::RemoveLoginsCreatedBetween(
     base::Time delete_begin,
     base::Time delete_end,
     password_manager::PasswordStoreChangeList* changes) {
   return RemoveLoginsBetween(
       delete_begin, delete_end, CREATION_TIMESTAMP, changes);
 }
 
 bool NativeBackendKWallet::RemoveLoginsSyncedBetween(
     base::Time delete_begin,
     base::Time delete_end,
     password_manager::PasswordStoreChangeList* changes) {
   return RemoveLoginsBetween(delete_begin, delete_end, SYNC_TIMESTAMP, changes);
 }
 
-bool NativeBackendKWallet::GetLogins(const PasswordForm& form,
-                                     PasswordFormList* forms) {
+bool NativeBackendKWallet::GetLogins(
+    const PasswordForm& form,
+    ScopedVector<autofill::PasswordForm>* forms) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
-  return GetLoginsList(forms, form.signon_realm, wallet_handle);
+  return GetLoginsList(form.signon_realm, wallet_handle, forms);
 }
 
-bool NativeBackendKWallet::GetAutofillableLogins(PasswordFormList* forms) {
+bool NativeBackendKWallet::GetAutofillableLogins(
+    ScopedVector<autofill::PasswordForm>* forms) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
-  return GetLoginsList(forms, true, wallet_handle);
+  return GetLoginsList(true, wallet_handle, forms);
 }
 
-bool NativeBackendKWallet::GetBlacklistLogins(PasswordFormList* forms) {
+bool NativeBackendKWallet::GetBlacklistLogins(
+    ScopedVector<autofill::PasswordForm>* forms) {
   int wallet_handle = WalletHandle();
   if (wallet_handle == kInvalidKWalletHandle)
     return false;
-  return GetLoginsList(forms, false, wallet_handle);
+  return GetLoginsList(false, wallet_handle, forms);
 }
 
-bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms,
-                                         const std::string& signon_realm,
-                                         int wallet_handle) {
+bool NativeBackendKWallet::GetLoginsList(
+    const std::string& signon_realm,
+    int wallet_handle,
+    ScopedVector<autofill::PasswordForm>* forms) {
   // Is there an entry in the wallet?
   {
     dbus::MethodCall method_call(kKWalletInterface, "hasEntry");
     dbus::MessageWriter builder(&method_call);
     builder.AppendInt32(wallet_handle);  // handle
     builder.AppendString(folder_name_);  // folder
     builder.AppendString(signon_realm);  // key
     builder.AppendString(app_name_);     // appid
     scoped_ptr<dbus::Response> response(
         kwallet_proxy_->CallMethodAndBlock(
@@ skipping 42 matching lines @@
     if (!CheckSerializedValue(bytes, length, signon_realm)) {
       // This is weird, but we choose not to call it an error. There is an
       // invalid entry somehow, but by just ignoring it, we make it easier to
       // repair without having to delete it using kwalletmanager (that is, by
       // just saving a new password within this realm to overwrite it).
       return true;
     }
 
     // Can't we all just agree on whether bytes are signed or not? Please?
     Pickle pickle(reinterpret_cast<const char*>(bytes), length);
-    PasswordFormList all_forms;
     DeserializeValue(signon_realm, pickle, forms);
   }
 
   return true;
 }
 
-bool NativeBackendKWallet::GetLoginsList(PasswordFormList* forms,
-                                         bool autofillable,
-                                         int wallet_handle) {
-  PasswordFormList all_forms;
-  if (!GetAllLogins(&all_forms, wallet_handle))
+bool NativeBackendKWallet::GetLoginsList(
+    bool autofillable,
+    int wallet_handle,
+    ScopedVector<autofill::PasswordForm>* forms) {
+  ScopedVector<autofill::PasswordForm> all_forms;
+  if (!GetAllLogins(wallet_handle, &all_forms))
     return false;
 
   // We have to read all the entries, and then filter them here.
   forms->reserve(forms->size() + all_forms.size());
-  for (size_t i = 0; i < all_forms.size(); ++i) {
-    if (all_forms[i]->blacklisted_by_user == !autofillable)
-      forms->push_back(all_forms[i]);
-    else
-      delete all_forms[i];
+  for (auto& saved_form : all_forms) {
+    if (saved_form->blacklisted_by_user == !autofillable) {
+      forms->push_back(saved_form);
+      saved_form = nullptr;
+    }
   }
 
   return true;
 }
 
-bool NativeBackendKWallet::GetAllLogins(PasswordFormList* forms,
-                                        int wallet_handle) {
+bool NativeBackendKWallet::GetAllLogins(
+    int wallet_handle,
+    ScopedVector<autofill::PasswordForm>* forms) {
   // We could probably also use readEntryList here.
   std::vector<std::string> realm_list;
   {
     dbus::MethodCall method_call(kKWalletInterface, "entryList");
     dbus::MessageWriter builder(&method_call);
     builder.AppendInt32(wallet_handle);  // handle
     builder.AppendString(folder_name_);  // folder
     builder.AppendString(app_name_);     // appid
     scoped_ptr<dbus::Response> response(
         kwallet_proxy_->CallMethodAndBlock(
@@ skipping 31 matching lines @@
     if (!reader.PopArrayOfBytes(&bytes, &length)) {
       LOG(ERROR) << "Error reading response from kwalletd (readEntry): "
                  << response->ToString();
       return false;
     }
     if (!bytes || !CheckSerializedValue(bytes, length, signon_realm))
       continue;
 
     // Can't we all just agree on whether bytes are signed or not? Please?
     Pickle pickle(reinterpret_cast<const char*>(bytes), length);
-    PasswordFormList all_forms;
     DeserializeValue(signon_realm, pickle, forms);
   }
   return true;
 }
 
-bool NativeBackendKWallet::SetLoginsList(const PasswordFormList& forms,
-                                         const std::string& signon_realm,
-                                         int wallet_handle) {
+bool NativeBackendKWallet::SetLoginsList(
+    const std::vector<autofill::PasswordForm*>& forms,
+    const std::string& signon_realm,
+    int wallet_handle) {
   if (forms.empty()) {
     // No items left? Remove the entry from the wallet.
     dbus::MethodCall method_call(kKWalletInterface, "removeEntry");
     dbus::MessageWriter builder(&method_call);
     builder.AppendInt32(wallet_handle);  // handle
     builder.AppendString(folder_name_);  // folder
     builder.AppendString(signon_realm);  // key
     builder.AppendString(app_name_);     // appid
     scoped_ptr<dbus::Response> response(
         kwallet_proxy_->CallMethodAndBlock(
@@ skipping 108 matching lines @@
     if (!reader.PopArrayOfBytes(&bytes, &length)) {
       LOG(ERROR) << "Error reading response from kwalletd (readEntry): "
                  << response->ToString();
       continue;
     }
     if (!bytes || !CheckSerializedValue(bytes, length, signon_realm))
       continue;
 
     // Can't we all just agree on whether bytes are signed or not? Please?
     Pickle pickle(reinterpret_cast<const char*>(bytes), length);
-    PasswordFormList all_forms;
+    ScopedVector<autofill::PasswordForm> all_forms;
     DeserializeValue(signon_realm, pickle, &all_forms);
 
-    PasswordFormList kept_forms;
+    ScopedVector<autofill::PasswordForm> kept_forms;
     kept_forms.reserve(all_forms.size());
     base::Time autofill::PasswordForm::*date_member =
         date_to_compare == CREATION_TIMESTAMP
             ? &autofill::PasswordForm::date_created
             : &autofill::PasswordForm::date_synced;
-    for (size_t i = 0; i < all_forms.size(); ++i) {
-      if (delete_begin <= all_forms[i]->*date_member &&
-          (delete_end.is_null() || all_forms[i]->*date_member < delete_end)) {
+    for (auto& saved_form : all_forms) {
+      if (delete_begin <= saved_form->*date_member &&
+          (delete_end.is_null() || saved_form->*date_member < delete_end)) {
         changes->push_back(password_manager::PasswordStoreChange(
-            password_manager::PasswordStoreChange::REMOVE, *all_forms[i]));
-        delete all_forms[i];
+            password_manager::PasswordStoreChange::REMOVE, *saved_form));
       } else {
-        kept_forms.push_back(all_forms[i]);
+        kept_forms.push_back(saved_form);
+        saved_form = nullptr;
       }
     }
 
-    if (!SetLoginsList(kept_forms, signon_realm, wallet_handle)) {
+    if (!SetLoginsList(kept_forms.get(), signon_realm, wallet_handle)) {
       ok = false;
       changes->clear();
     }
-    STLDeleteElements(&kept_forms);
   }
   return ok;
 }
 
 // static
-void NativeBackendKWallet::SerializeValue(const PasswordFormList& forms,
-                                          Pickle* pickle) {
+void NativeBackendKWallet::SerializeValue(
+    const std::vector<autofill::PasswordForm*>& forms,
+    Pickle* pickle) {
   pickle->WriteInt(kPickleVersion);
   pickle->WriteSizeT(forms.size());
-  for (PasswordFormList::const_iterator it = forms.begin();
-       it != forms.end(); ++it) {
-    const PasswordForm* form = *it;
+  for (const auto& form : forms) {

[vasilii, 2015/01/27 20:45:51]

autofill::PasswordForm* form

[vabr (Chromium), 2015/01/28 13:27:35]

Done.
I guess I was a bit confused about the new for loop syntax, should have taken
look before. At least I finally did take a look, sorry for those silly mistakes.

     pickle->WriteInt(form->scheme);
     pickle->WriteString(form->origin.spec());
     pickle->WriteString(form->action.spec());
     pickle->WriteString16(form->username_element);
     pickle->WriteString16(form->username_value);
     pickle->WriteString16(form->password_element);
     pickle->WriteString16(form->password_value);
     pickle->WriteString16(form->submit_element);
     pickle->WriteBool(form->ssl_valid);
     pickle->WriteBool(form->preferred);
     pickle->WriteBool(form->blacklisted_by_user);
     pickle->WriteInt64(form->date_created.ToTimeT());
     pickle->WriteInt(form->type);
     pickle->WriteInt(form->times_used);
     autofill::SerializeFormData(form->form_data, pickle);
     pickle->WriteInt64(form->date_synced.ToInternalValue());
     pickle->WriteString16(form->display_name);
     pickle->WriteString(form->avatar_url.spec());
     pickle->WriteString(form->federation_url.spec());
     pickle->WriteBool(form->is_zero_click);
   }
 }
 
 // static
-bool NativeBackendKWallet::DeserializeValueSize(const std::string& signon_realm,
-                                                const PickleIterator& init_iter,
-                                                int version,
-                                                bool size_32,
-                                                bool warn_only,
-                                                PasswordFormList* forms) {
+bool NativeBackendKWallet::DeserializeValueSize(
+    const std::string& signon_realm,
+    const PickleIterator& init_iter,
+    int version,
+    bool size_32,
+    bool warn_only,
+    ScopedVector<autofill::PasswordForm>* forms) {
   PickleIterator iter = init_iter;
 
   size_t count = 0;
   if (size_32) {
     uint32_t count_32 = 0;
     if (!iter.ReadUInt32(&count_32)) {
       LOG(ERROR) << "Failed to deserialize KWallet entry "
                  << "(realm: " << signon_realm << ")";
       return false;
     }
@@ skipping 77 matching lines @@
       }
     }
 
     forms->push_back(form.release());
   }
 
   return true;
 }
 
 // static
-void NativeBackendKWallet::DeserializeValue(const std::string& signon_realm,
-                                            const Pickle& pickle,
-                                            PasswordFormList* forms) {
+void NativeBackendKWallet::DeserializeValue(
+    const std::string& signon_realm,
+    const Pickle& pickle,
+    ScopedVector<autofill::PasswordForm>* forms) {
   PickleIterator iter(pickle);
 
   int version = -1;
   if (!iter.ReadInt(&version) ||
       version < 0 || version > kPickleVersion) {
     LOG(ERROR) << "Failed to deserialize KWallet entry "
                << "(realm: " << signon_realm << ")";
     return;
   }
 
@@ skipping 98 matching lines @@
   }
 
   return handle;
 }
 
 std::string NativeBackendKWallet::GetProfileSpecificFolderName() const {
   // Originally, the folder name was always just "Chrome Form Data".
   // Now we use it to distinguish passwords for different profiles.
   return base::StringPrintf("%s (%d)", kKWalletFolder, profile_id_);
 }