PasswordStore: Clean up expectations about rewriting vectors of forms

components/password_manager/core/browser/login_database.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 51 matching lines @@
   // delete in either direction.
   bool RemoveLoginsCreatedBetween(base::Time delete_begin,
                                   base::Time delete_end);
 
   // Removes all logins synced from |delete_begin| onwards (inclusive) and
   // before |delete_end|. You may use a null Time value to do an unbounded
   // delete in either direction.
   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
   // that SQLite doesn't leave fragments of passwords in the database file.
   // Returns true on success; otherwise, whether the file was deleted and
   // whether further use of this login database will succeed is unspecified.
   bool DeleteAndRecreateDatabaseFile();
 
  private:
   // Result values for encryption/decryption actions.
   enum EncryptionResult {
     // Success.
     ENCRYPTION_RESULT_SUCCESS,
     // Failure for a specific item (e.g., the encrypted value was manually
     // moved from another machine, and can't be decrypted on this machine).
     // This is presumed to be a permanent failure.
     ENCRYPTION_RESULT_ITEM_FAILURE,
     // A service-level failure (e.g., on a platform using a keyring, the keyring
     // is temporarily unavailable).
     // This is presumed to be a temporary failure.
     ENCRYPTION_RESULT_SERVICE_FAILURE,
   };
 
   // Encrypts plain_text, setting the value of cipher_text and returning true if
   // 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();
 
   // Fills |form| from the values in the given statement (which is assumed to
   // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginDatabase);
 };
 
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_