[password generation] Always allow generated passwords to be shown.

[password generation] Always allow generated passwords to be shown.

Currently if you have a password saved from login and you create and save new
credentials on the site, they will not autofill on the login form because the
login credentials are considered a better match. This is not ideal in general,
but is particularly bad for generated passwords.

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

[Ilya Sherman, 2013-09-19 01:02:20]

LGTM, thanks.

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
File chrome/browser/password_manager/password_form_manager.cc (right):

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager.cc:295: // We will make
sure these forms are in the final result regardless of score.
nit: Please try to phrase comments without using "we" or "us".  For this
comment, you might phrase it as "These forms will be in the final result
regardless of score."

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager.cc:331: }
nit: No need for curlies.

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
File chrome/browser/password_manager/password_form_manager_unittest.cc (right):

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager_unittest.cc:57:
autofill::PasswordFormMap GetLatestBestMatches() { return best_matches_; }
Return by const-ref?

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager_unittest.cc:60: // Marked
mutable to get around constness of Autofill().
Hmm, it's rather odd for the Autofill() method to be const in the first place. 
There are definitely externally-observable results from calling it...  Oh well,
not really something that's necessarily ripe for addressing in this particular
CL.

[zysxqn, 2013-09-19 17:02:00]

lgtm

[zysxqn, 2013-09-19 17:02:02]

lgtm

[Garrett Casto, 2013-09-19 17:40:14]

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
File chrome/browser/password_manager/password_form_manager.cc (right):

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager.cc:295: // We will make
sure these forms are in the final result regardless of score.
On 2013/09/19 01:02:20, Ilya Sherman wrote:
> nit: Please try to phrase comments without using "we" or "us".  For this
> comment, you might phrase it as "These forms will be in the final result
> regardless of score."

Done.

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager.cc:331: }
On 2013/09/19 01:02:20, Ilya Sherman wrote:
> nit: No need for curlies.

Done.

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
File chrome/browser/password_manager/password_form_manager_unittest.cc (right):

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager_unittest.cc:57:
autofill::PasswordFormMap GetLatestBestMatches() { return best_matches_; }
On 2013/09/19 01:02:20, Ilya Sherman wrote:
> Return by const-ref?

I'm generally wary of returning references as it complicates lifetime issues,
but it's fine here. Changed.

https://codereview.chromium.org/23533069/diff/1/chrome/browser/password_manag...
chrome/browser/password_manager/password_form_manager_unittest.cc:60: // Marked
mutable to get around constness of Autofill().
On 2013/09/19 01:02:20, Ilya Sherman wrote:
> Hmm, it's rather odd for the Autofill() method to be const in the first place.

> There are definitely externally-observable results from calling it...  Oh
well,
> not really something that's necessarily ripe for addressing in this particular
> CL.

Yeah, I had a similar thought as I was writing this.

[commit-bot: I haz the power, 2013-09-19 17:41:56]

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

[commit-bot: I haz the power, 2013-09-19 21:57:49]

Change committed as 224215