PasswordStore: Clean up expectations about rewriting vectors of forms

chrome/browser/password_manager/native_backend_libsecret.cc

 // Copyright (c) 2015 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_libsecret.h"
 
 #include <dlfcn.h>
 #include <list>
 
 #include "base/basictypes.h"
@@ skipping 218 matching lines @@
   name_values_.push_back(value);
   gpointer value_str =
       static_cast<gpointer>(const_cast<char*>(name_values_.back().c_str()));
   g_hash_table_insert(attrs_, name_str, value_str);
 }
 
 void LibsecretAttributesBuilder::Append(const std::string& name, int64 value) {
   Append(name, base::Int64ToString(value));
 }
 
+// Generates a profile-specific app string based on profile_id_.
+std::string GetProfileSpecificAppString(LocalProfileId id) {
+  // 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", kLibsecretAppString, id);
+}
+
 }  // namespace
 
 bool LibsecretLoader::LibsecretIsAvailable() {
   if (!libsecret_loaded)
     return false;
   // A dummy query is made to check for availability, because libsecret doesn't
   // have a dedicated availability function. For performance reasons, the query
   // is meant to return an empty result.
   LibsecretAttributesBuilder attrs;
   attrs.Append("application", "chrome-string_to_get_empty_result");
@@ skipping 24 matching lines @@
   return LoadLibsecret() && LibsecretIsAvailable();
 }
 
 password_manager::PasswordStoreChangeList NativeBackendLibsecret::AddLogin(
     const PasswordForm& form) {
   // Based on LoginDatabase::AddLogin(), we search for an existing match based
   // on origin_url, username_element, username_value, password_element, submit
   // element, and signon_realm first, remove that, 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!
-  ScopedVector<autofill::PasswordForm> forms;
-  AddUpdateLoginSearch(form, SEARCH_USE_SUBMIT, &forms);
+  ScopedVector<autofill::PasswordForm> forms =
+      AddUpdateLoginSearch(form, SEARCH_USE_SUBMIT);
   password_manager::PasswordStoreChangeList changes;
   if (forms.size() > 0) {
     if (forms.size() > 1) {
       VLOG(1) << "Adding login when there are " << forms.size()
               << " matching logins already! Will replace only the first.";
     }
 
     if (RemoveLogin(*forms[0])) {
       changes.push_back(password_manager::PasswordStoreChange(
           password_manager::PasswordStoreChange::REMOVE, *forms[0]));
@@ skipping 11 matching lines @@
     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(changes);
   changes->clear();
 
-  ScopedVector<autofill::PasswordForm> forms;
-  AddUpdateLoginSearch(form, SEARCH_IGNORE_SUBMIT, &forms);
+  ScopedVector<autofill::PasswordForm> forms =
+      AddUpdateLoginSearch(form, SEARCH_IGNORE_SUBMIT);
 
   bool removed = false;
   for (size_t i = 0; i < forms.size(); ++i) {
     if (*forms[i] != form) {
       RemoveLogin(*forms[i]);
       removed = true;
     }
   }
   if (!removed)
     return true;
@@ skipping 40 matching lines @@
     password_manager::PasswordStoreChangeList* changes) {
   return RemoveLoginsBetween(delete_begin, delete_end, SYNC_TIMESTAMP, changes);
 }
 
 bool NativeBackendLibsecret::GetLogins(
     const PasswordForm& form,
     ScopedVector<autofill::PasswordForm>* forms) {
   return GetLoginsList(&form, ALL_LOGINS, forms);
 }
 
-void NativeBackendLibsecret::AddUpdateLoginSearch(
+ScopedVector<autofill::PasswordForm>
+NativeBackendLibsecret::AddUpdateLoginSearch(
     const autofill::PasswordForm& lookup_form,
-    AddUpdateLoginSearchOptions options,
-    ScopedVector<autofill::PasswordForm>* forms) {
+    AddUpdateLoginSearchOptions options) {
   LibsecretAttributesBuilder attrs;
   attrs.Append("origin_url", lookup_form.origin.spec());
   attrs.Append("username_element", UTF16ToUTF8(lookup_form.username_element));
   attrs.Append("username_value", UTF16ToUTF8(lookup_form.username_value));
   attrs.Append("password_element", UTF16ToUTF8(lookup_form.password_element));
   if (options == SEARCH_USE_SUBMIT)
     attrs.Append("submit_element", UTF16ToUTF8(lookup_form.submit_element));
   attrs.Append("signon_realm", lookup_form.signon_realm);
   attrs.Append("application", app_string_);
 
   GError* error = nullptr;
   GList* found = secret_service_search_sync(nullptr,  // default secret service
                                             &kLibsecretSchema, attrs.Get(),
                                             SECRET_SEARCH_ALL,
                                             nullptr,  // no cancellable ojbect
                                             &error);
   if (error) {
     VLOG(1) << "Unable to get logins " << error->message;
     g_error_free(error);
     if (found)
       g_list_free(found);
-    return;
+    return ScopedVector<autofill::PasswordForm>();
   }
 
-  ConvertFormList(found, &lookup_form, forms);
+  return ConvertFormList(found, &lookup_form);
 }
 
 bool NativeBackendLibsecret::RawAddLogin(const PasswordForm& form) {
   int64 date_created = form.date_created.ToInternalValue();
   // If we are asked to save a password with 0 date, use the current time.
   // We don't want to actually save passwords as though on January 1, 1601.
   if (!date_created)
     date_created = base::Time::Now().ToInternalValue();
   int64 date_synced = form.date_synced.ToInternalValue();
   std::string form_data;
@@ skipping 68 matching lines @@
                                             nullptr,  // no cancellable ojbect
                                             &error);
   if (error) {
     VLOG(1) << "Unable to get logins " << error->message;
     g_error_free(error);
     if (found)
       g_list_free(found);
     return false;
   }
 
-  return ConvertFormList(found, lookup_form, forms);
-}
-
-bool NativeBackendLibsecret::GetAllLogins(
-    ScopedVector<autofill::PasswordForm>* forms) {
-  return GetLoginsList(nullptr, ALL_LOGINS, forms);
+  *forms = ConvertFormList(found, lookup_form);
+  return true;
 }
 
 bool NativeBackendLibsecret::GetLoginsBetween(
     base::Time get_begin,
     base::Time get_end,
     TimestampToCompare date_to_compare,
     ScopedVector<autofill::PasswordForm>* forms) {
+  forms->clear();
   ScopedVector<autofill::PasswordForm> all_forms;
-  if (!GetAllLogins(&all_forms))
+  if (!GetLoginsList(nullptr, ALL_LOGINS, &all_forms))
     return false;
 
   base::Time autofill::PasswordForm::*date_member =
       date_to_compare == CREATION_TIMESTAMP
           ? &autofill::PasswordForm::date_created
           : &autofill::PasswordForm::date_synced;
   for (auto& saved_form : all_forms) {
     if (get_begin <= saved_form->*date_member &&
         (get_end.is_null() || saved_form->*date_member < get_end)) {
       forms->push_back(saved_form);
@@ skipping 20 matching lines @@
     if (RemoveLogin(*forms[i])) {
       changes->push_back(password_manager::PasswordStoreChange(
           password_manager::PasswordStoreChange::REMOVE, *forms[i]));
     } else {
       ok = false;
     }
   }
   return ok;
 }
 
-bool NativeBackendLibsecret::ConvertFormList(
+ScopedVector<autofill::PasswordForm> NativeBackendLibsecret::ConvertFormList(
     GList* found,
-    const PasswordForm* lookup_form,
-    ScopedVector<autofill::PasswordForm>* forms) {
+    const PasswordForm* lookup_form) {
+  ScopedVector<autofill::PasswordForm> forms;
   password_manager::PSLDomainMatchMetric psl_domain_match_metric =
       password_manager::PSL_DOMAIN_MATCH_NONE;
   GError* error = nullptr;
   for (GList* element = g_list_first(found); element != nullptr;
        element = g_list_next(element)) {
     SecretItem* secretItem = static_cast<SecretItem*>(element->data);
     LibsecretLoader::secret_item_load_secret_sync(secretItem, nullptr, &error);
     if (error) {
       VLOG(1) << "Unable to load secret item" << error->message;
       g_error_free(error);
@@ skipping 17 matching lines @@
         form->signon_realm = lookup_form->signon_realm;
         form->origin = lookup_form->origin;
       }
       SecretValue* secretValue = secret_item_get_secret(secretItem);
       if (secretValue) {
         form->password_value = UTF8ToUTF16(secret_value_get_text(secretValue));
         secret_value_unref(secretValue);
       } else {
         VLOG(1) << "Unable to access password from list element!";
       }
-      forms->push_back(form.release());
+      forms.push_back(form.release());
     } else {
       VLOG(1) << "Could not initialize PasswordForm from attributes!";
     }
   }
 
   if (lookup_form) {
     const GURL signon_realm(lookup_form->signon_realm);
     std::string registered_domain =
         password_manager::GetRegistryControlledDomain(signon_realm);
     UMA_HISTOGRAM_ENUMERATION(
         "PasswordManager.PslDomainMatchTriggering",
         password_manager::ShouldPSLDomainMatchingApply(registered_domain)
             ? psl_domain_match_metric
             : password_manager::PSL_DOMAIN_MATCH_NOT_USED,
         password_manager::PSL_DOMAIN_MATCH_COUNT);
   }
   g_list_free(found);
-  return true;
+  return forms.Pass();
 }
-
-std::string NativeBackendLibsecret::GetProfileSpecificAppString(
-    LocalProfileId id) {
-  // 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", kLibsecretAppString, id);
-}