PasswordStore: Clean up expectations about rewriting vectors of forms

chrome/browser/password_manager/native_backend_libsecret.h

Index: chrome/browser/password_manager/native_backend_libsecret.h
diff --git a/chrome/browser/password_manager/native_backend_libsecret.h b/chrome/browser/password_manager/native_backend_libsecret.h
index 15dfc3997b7d6ea99d07f35c64b69a0029213080..b3cd459f78f51fe47e85c21052752fe8fde5cd35 100644
--- a/chrome/browser/password_manager/native_backend_libsecret.h
+++ b/chrome/browser/password_manager/native_backend_libsecret.h
@@ -86,10 +86,10 @@ class NativeBackendLibsecret : public PasswordStoreX::NativeBackend,
     SEARCH_IGNORE_SUBMIT,
   };
 
-  // Search that is used in AddLogin and UpdateLogin methods
-  void AddUpdateLoginSearch(const autofill::PasswordForm& lookup_form,
-                            AddUpdateLoginSearchOptions options,
-                            ScopedVector<autofill::PasswordForm>* forms);
+  // Returns credentials matching |lookup_form| and |options|.
+  ScopedVector<autofill::PasswordForm> AddUpdateLoginSearch(
+      const autofill::PasswordForm& lookup_form,
+      AddUpdateLoginSearchOptions options);
 
   // Adds a login form without checking for one to replace first.
   bool RawAddLogin(const autofill::PasswordForm& form);
@@ -100,14 +100,13 @@ class NativeBackendLibsecret : public PasswordStoreX::NativeBackend,
     BLACKLISTED_LOGINS,
   };
 
-  // Reads PasswordForms from the keyring with the given autofillability state.
+  // Overwrites |forms| with credentials matching |options| from the keyring. If

[engedy, 2015/02/25 15:17:48]

Phrasing suggestion (just an idea, I do not feel strongly about this):

Retrieves credentials matching |options| from the keyring into |forms|,
overwriting the original contents of |forms|. If |lookup_form| is not NULL, only
retrieves credentials PSL-matching it. Returns true on success; false on
failure, in which case |forms| gets erased.

[vabr (Chromium), 2015/03/09 10:56:19]

Done.

+  // |lookup_form| is not NULL, only retrieves credentials PSL-matching it. In
+  // case of failures returns false and erases |forms|.
   bool GetLoginsList(const autofill::PasswordForm* lookup_form,
                      GetLoginsListOptions options,
                      ScopedVector<autofill::PasswordForm>* forms);
 
-  // Helper for GetLoginsCreatedBetween().
-  bool GetAllLogins(ScopedVector<autofill::PasswordForm>* forms);
-
   // Retrieves password created/synced in the time interval. Returns |true| if
   // the operation succeeded.
   bool GetLoginsBetween(base::Time get_begin,

[engedy, 2015/02/25 15:17:48]

Need to elaborate on the fate of |forms|.

[vabr (Chromium), 2015/03/09 10:56:19]

Done.

@@ -123,9 +122,9 @@ class NativeBackendLibsecret : public PasswordStoreX::NativeBackend,
                            password_manager::PasswordStoreChangeList* changes);
 
   // convert data get from Libsecret to Passwordform
-  bool ConvertFormList(GList* found,
-                       const autofill::PasswordForm* lookup_form,
-                       ScopedVector<autofill::PasswordForm>* forms);
+  ScopedVector<autofill::PasswordForm> ConvertFormList(
+      GList* found,
+      const autofill::PasswordForm* lookup_form);
 
   // Generates a profile-specific app string based on profile_id_.
   static std::string GetProfileSpecificAppString(LocalProfileId id);

[engedy, 2015/02/25 15:17:48]

I think we can only move this to an anonymous namespace.

[vabr (Chromium), 2015/03/09 10:56:19]

Done. Also for the Gnome backend.

[engedy, 2015/03/09 13:33:17]

Acknowledged.