PasswordStore: Clean up expectations about rewriting vectors of forms

chrome/browser/password_manager/password_store_x.h

 // 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.
 
 #ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_X_H_
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_X_H_
 
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 40 matching lines @@
     // delete in either direction.
     virtual bool RemoveLoginsCreatedBetween(
         base::Time delete_begin,
         base::Time delete_end,
         password_manager::PasswordStoreChangeList* changes) = 0;
     virtual bool RemoveLoginsSyncedBetween(
         base::Time delete_begin,
         base::Time delete_end,
         password_manager::PasswordStoreChangeList* changes) = 0;
 
+    // The three methods below overwrite |forms| with all stored credentials
+    // matching |form|, all stored non-blacklisted credentials, and all stored
+    // blacklisted credentials, respectively. On success, they return true,
+    // otherwise they return false and erase |forms|.
     virtual bool GetLogins(const autofill::PasswordForm& form,

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

The FailingBackend in password_store_x_unittest.cc should clear |forms| too. (3
methods.)

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

Done.
After dropping the promise about erasing on failure in the comment above, this
is was no longer needed, but I added the clearing calls to the FailingBackend as
well. I'm not entirely sure whether it is actually an improvement (maybe keeping
possible garbage there would help discover the callsite relying on non-existing
promises), but neither clearing, nor keeping seemed like the ideal choice.

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

Acknowledged.

                            ScopedVector<autofill::PasswordForm>* forms) = 0;
     virtual bool GetAutofillableLogins(
         ScopedVector<autofill::PasswordForm>* forms) = 0;
     virtual bool GetBlacklistLogins(
         ScopedVector<autofill::PasswordForm>* forms) = 0;
   };
 
   // Takes ownership of |login_db| and |backend|. |backend| may be NULL in which
   // case this PasswordStoreX will act the same as PasswordStoreDefault.
   PasswordStoreX(scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner,
@@ skipping 57 matching lines @@
   // Whether we should allow falling back to the default store. If there is
   // nothing to migrate, then the first attempt to use the native store will
   // be the first time we try to use it and we should allow falling back. If
   // we have migrated successfully, then we do not allow falling back.
   bool allow_fallback_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordStoreX);
 };
 
 #endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_X_H_