[Password Manager] Verify that password is updated if multiple credentials

[Password Manager] Verify that password is updated if multiple credentials
with the same username exist.

I believe r285140 should have fixed any possible underlying issue here.
Unfortunately even before this I could not devise a test that would fail in the
expected way, but we should still have a test verifying this behavior going
forward.

BUG=395190
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287958

[vabr (Chromium), 2014-07-31 10:00:32]

LGTM with nits.
Thanks for adding this test!
Vaclav

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
File components/password_manager/core/browser/password_form_manager_unittest.cc
(right):

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
components/password_manager/core/browser/password_form_manager_unittest.cc:1015:
scoped_ptr<PasswordFormManager> manager(
nit: It takes some thinking to understand that |manager| is actually two
different objects during the test, and what is the difference.
My suggestion is to replace the scoped_ptr with 2 local variables, e.g.:
PasswordFormManager storing_manager(...);  // Manager to store the 3rd login.
...
PasswordFormManager retrieving_manager(...);  // Manager to retrieve and prefer
the 3rd login.

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
components/password_manager/core/browser/password_form_manager_unittest.cc:1039:
manager->FetchMatchingLoginsFromPasswordStore(PasswordStore::ALLOW_PROMPT);
nit: Could the PasswordFormManager be defined just above this line, to make it
apparent that it is not involved in storing the first 2 logins?

[Garrett Casto, 2014-08-06 22:00:47]

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
File components/password_manager/core/browser/password_form_manager_unittest.cc
(right):

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
components/password_manager/core/browser/password_form_manager_unittest.cc:1015:
scoped_ptr<PasswordFormManager> manager(
On 2014/07/31 10:00:32, vabr (Chromium) wrote:
> nit: It takes some thinking to understand that |manager| is actually two
> different objects during the test, and what is the difference.
> My suggestion is to replace the scoped_ptr with 2 local variables, e.g.:
> PasswordFormManager storing_manager(...);  // Manager to store the 3rd login.
> ...
> PasswordFormManager retrieving_manager(...);  // Manager to retrieve and
prefer
> the 3rd login.

Done.

https://codereview.chromium.org/431893003/diff/20001/components/password_mana...
components/password_manager/core/browser/password_form_manager_unittest.cc:1039:
manager->FetchMatchingLoginsFromPasswordStore(PasswordStore::ALLOW_PROMPT);
On 2014/07/31 10:00:32, vabr (Chromium) wrote:
> nit: Could the PasswordFormManager be defined just above this line, to make it
> apparent that it is not involved in storing the first 2 logins?

Done.

[Garrett Casto, 2014-08-06 22:00:55]

The CQ bit was checked by gcasto@chromium.org

[commit-bot: I haz the power, 2014-08-06 22:02:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/gcasto@chromium.org/431893003/40001

[commit-bot: I haz the power, 2014-08-07 03:40:35]

Change committed as 287958