PasswordStore: Clean up expectations about rewriting vectors of forms

components/password_manager/core/browser/login_database.h

Index: components/password_manager/core/browser/login_database.h
diff --git a/components/password_manager/core/browser/login_database.h b/components/password_manager/core/browser/login_database.h
index f2adacdb94091e70c8d17ab8c00a590512b4a7f0..597416e542f80493e2918819182cb18d17bef6af 100644
--- a/components/password_manager/core/browser/login_database.h
+++ b/components/password_manager/core/browser/login_database.h
@@ -69,34 +69,37 @@ class LoginDatabase {
   bool RemoveLoginsSyncedBetween(base::Time delete_begin,
                                  base::Time delete_end);
 
-  // Loads a list of matching password forms into the specified vector |forms|.
-  // The list will contain all possibly relevant entries to the observed |form|,
-  // including blacklisted matches.
+  // All Get* methods below overwrite |forms| with the returned credentials. On
+  // success, those methods return true.
+
+  // Gets a list of credentials matching |form|, including blacklisted matches.
   bool GetLogins(const autofill::PasswordForm& form,
-                 ScopedVector<autofill::PasswordForm>* forms) const;
+                 ScopedVector<autofill::PasswordForm>* forms) const
+      WARN_UNUSED_RESULT;
 
-  // Loads all logins created from |begin| onwards (inclusive) and before |end|.
+  // Gets all logins created from |begin| onwards (inclusive) and before |end|.
   // You may use a null Time value to do an unbounded search in either
   // direction.
   bool GetLoginsCreatedBetween(
       base::Time begin,
       base::Time end,
-      ScopedVector<autofill::PasswordForm>* forms) const;
+      ScopedVector<autofill::PasswordForm>* forms) const WARN_UNUSED_RESULT;
 
-  // Loads all logins synced from |begin| onwards (inclusive) and before |end|.
+  // Gets all logins synced from |begin| onwards (inclusive) and before |end|.
   // You may use a null Time value to do an unbounded search in either
   // direction.
-  bool GetLoginsSyncedBetween(
-      base::Time begin,
-      base::Time end,
-      ScopedVector<autofill::PasswordForm>* forms) const;
+  bool GetLoginsSyncedBetween(base::Time begin,
+                              base::Time end,
+                              ScopedVector<autofill::PasswordForm>* forms) const
+      WARN_UNUSED_RESULT;
 
-  // Loads the complete list of autofillable password forms (i.e., not blacklist
-  // entries) into |forms|.
-  bool GetAutofillableLogins(ScopedVector<autofill::PasswordForm>* forms) const;
+  // Gets the complete list of not blacklisted credentials.
+  bool GetAutofillableLogins(ScopedVector<autofill::PasswordForm>* forms) const
+      WARN_UNUSED_RESULT;
 
-  // Loads the complete list of blacklist forms into |forms|.
-  bool GetBlacklistLogins(ScopedVector<autofill::PasswordForm>* forms) const;
+  // Gets the complete list of blacklisted credentials.
+  bool GetBlacklistLogins(ScopedVector<autofill::PasswordForm>* forms) const
+      WARN_UNUSED_RESULT;
 
   // Deletes the login database file on disk, and creates a new, empty database.
   // This can be used after migrating passwords to some other store, to ensure
@@ -124,15 +127,15 @@ class LoginDatabase {
   // successful, or returning false and leaving cipher_text unchanged if
   // encryption fails (e.g., if the underlying OS encryption system is
   // temporarily unavailable).
-  EncryptionResult EncryptedString(const base::string16& plain_text,
-                                   std::string* cipher_text) const;
+  static EncryptionResult EncryptedString(const base::string16& plain_text,
+                                          std::string* cipher_text);
 
   // Decrypts cipher_text, setting the value of plain_text and returning true if
   // successful, or returning false and leaving plain_text unchanged if
   // decryption fails (e.g., if the underlying OS encryption system is
   // temporarily unavailable).
-  EncryptionResult DecryptedString(const std::string& cipher_text,
-                                   base::string16* plain_text) const;
+  static EncryptionResult DecryptedString(const std::string& cipher_text,
+                                          base::string16* plain_text);
 
   bool InitLoginsTable();
   bool MigrateOldVersionsAsNeeded();
@@ -141,15 +144,25 @@ class LoginDatabase {
   // be of the form used by the Get*Logins methods).
   // Returns the EncryptionResult from decrypting the password in |s|; if not
   // ENCRYPTION_RESULT_SUCCESS, |form| is not filled.
-  EncryptionResult InitPasswordFormFromStatement(autofill::PasswordForm* form,
-                                                 sql::Statement& s) const;
+  static EncryptionResult InitPasswordFormFromStatement(
+      autofill::PasswordForm* form,
+      sql::Statement& s);
 
-  // Loads all logins whose blacklist setting matches |blacklisted| into
-  // |forms|.
+  // Gets all blacklisted or all non-blacklisted (depending on |blacklisted|)
+  // credentials. On success returns true and overwrites |forms| with the
+  // result, otherwise returns false and clears |forms|.

[engedy, 2015/03/11 19:25:35]

We should remove the promise about clearing forms on failure.

[vabr (Chromium), 2015/03/12 15:30:51]

Done.

   bool GetAllLoginsWithBlacklistSetting(
       bool blacklisted,
       ScopedVector<autofill::PasswordForm>* forms) const;
 
+  // Overwrites |forms| with credentials retrieved from |statement|. If
+  // |psl_match| is not null, filters out all results but thos PSL-matching
+  // |*psl_match|. On success returns true, otherwise clears |forms| and returns

[engedy, 2015/03/11 19:25:35]

Same here.

[vabr (Chromium), 2015/03/12 15:30:51]

Done.

+  // false.
+  static bool StatementToForms(sql::Statement* statement,
+                               const autofill::PasswordForm* psl_match,
+                               ScopedVector<autofill::PasswordForm>* forms);
+
   base::FilePath db_path_;
   mutable sql::Connection db_;
   sql::MetaTable meta_table_;