[Password Manager] Send username correction votes

[Password Manager] Send username correction votes

If a user changes autofilled username on a form, but the correct username is in |PasswordForm.other_possible_usernames|, send USERNAME vote for the element that contained the correct username. 

This CL changes the data structure of |other_possible_usernames| from vector<string> to vector<pair<string, string>>. 

The CL updates only one local storage - login_database. We need to receive enough votes, there is no need to cover 100% of Chrome instances. But the results based on the votes would propagated to all instances.

BUG=699530
Review-Url: https://codereview.chromium.org/2747733004
Cr-Commit-Position: refs/heads/master@{#458788}
Committed: https://chromium.googlesource.com/chromium/src/+/718d242b21b1964f8a06e79bf00dcce7e073dac7

[kolos1, 2017-03-16 13:48:51]

Patchset #1 (id:1) has been deleted

[kolos1, 2017-03-16 13:48:59]

Patchset #1 (id:20001) has been deleted

[kolos1, 2017-03-16 13:57:55]

Description was changed from

==========
[Password Manager] Send username correction votes

BUG=
==========

to

==========
[Password Manager] Send username correction votes

If a user changes autofilled username on a form, but the correct username is in
|PasswordForm.other_possible_usernames|, send USERNAME vote for the element that
contained the correct username. 

This CL changes the data structure of |other_possible_usernames| from
vector<string> to vector<pair<string, string>>. 

The CL updates only one local storage - login_database. We need to receive
enough votes, there is no need to cover 100% of Chrome instances. But the
results based on the votes would propagated to all instances.

BUG=699530
==========

[kolos1, 2017-03-16 14:09:59]

kolos@chromium.org changed reviewers:
+ dvadym@chromium.org, vabr@chromium.org

[kolos1, 2017-03-16 14:10:00]

vabr@: Please review all files except autofill_types*

dvadym@: Please take a brief look. I added some comments about key points.

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (left):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:779:
field_types[pending_credentials_.username_element] = autofill::USERNAME;
dvadym@: sending USERNAME is removed here.

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:833:
field_types[form_to_upload.username_element] = autofill::USERNAME;
dvadym@: shall we also send some password vote for |password_element|? I might
be useful when <FORM> tag belongs to several forms (signin/signup), but it might
change input data for password generation. 

I would prefer to separate username and generation votes for now, i.e. don't
send both votes.

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:1287:
FormStructure(observed_form_.form_data).FormSignatureAsStr());
dvadym@: send also the signature of the observed form as login_form_signature.

[vabr (Chromium), 2017-03-16 21:00:42]

Thanks, Maxim. It is exciting to see we will have this way of figuring out the
correct username!

LGTM with comments below.

Cheers,
Vaclav

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
File components/autofill/core/common/password_form.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:19: base::string16
OtherPossibleUsernamesToString(
You can also implement it as two pass processing:
(1) using std::transform to create a vector of "first+second" strings, then
(2) using base::JoinString to join them with ",".

There are two advantages to using JoinString: it is easier to read than the
for-cycle with the special clause for the non-empty result, and it is also
faster thanks to pre-allocating the needed space upfront.

The disadvantage is that more space is locally taken to store the "first+second"
strings, but at least it is not slower, because the current code on line 25 also
creates all those strings.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:20: PossibleUsernamesVector
possible_usernames) {
const PossibleUsernamesVector&

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:22: for (const
PossibleUsernamePair pair : possible_usernames) {
use const ref not const value:
const PossibleUsernamePair& pair

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
File components/autofill/core/common/password_form.h (right):

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.h:21: typedef
std::pair<base::string16, base::string16> PossibleUsernamePair;
Here and below please use:
using new_alias = old_type
instead of typedef. See also https://chromium-cpp.appspot.com/.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.h:314: base::string16
OtherPossibleUsernamesToStr(const PasswordForm& form);
Having both OtherPossibleUsernamesToStr and OtherPossibleUsernamesToString is
confusing, and the difference in the names does not hint at the difference in
behaviour.

Please only keep OtherPossibleUsernamesToString and declare it here. The only
place outside password_form.* where OtherPossibleUsernamesToStr is currently
used will be easily converted to use OtherPossibleUsernamesToString.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
File components/autofill/core/common/password_form_fill_data.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form_fill_data.cc:22: for (auto& pair :
values_and_field_names)
nit: const auto&
While the compiler deduces that the "auto" is a const type, it does not hurt to
make that explicit. We follow a similar logic in enforcing auto* for pointer
types in a presubmit script.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/login_database.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/login_database.cc:70:
DCHECK(iterator.ReadString16(&field_name));
Don't push the ReadString16 into a DCHECK! This would not get executed on
release builds (note that trybots do compile DCHECKS even in release, but
official Chrome does not). Instead do:

bool name_success = iterator.ReadString16(&field_name);
DCHECK(name_success);

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:709: const
autofill::PossibleUsernamePair& pair =
This could be a range-based for loop, you don't use |i| for anything else than
defining |pair|.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.h (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.h:358: void
SendVoteOnCredentialsReusage(const autofill::PasswordForm& observed,
nit: Reusage -> Reuse

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.h:484: // then
|username_correction_vote_| stores this credentials.
Not sure what "this credentials" refer to. It is a PasswordForm, but how does it
look like?
Also, either use "this credential" or "these credentials".

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager_unittest.cc
(right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager_unittest.cc:2913:
// Checks the username correction vote is saved.
Does this duplicate the check on line 2940?
If yes, and if the only reason for the username_correction_vote() accessor in
PasswordFormManager is peeking at it in tests, then I strongly suggest removing
the accessor. PasswordFormManager has unfortunately grown to a state when most
of its internal data is exposed through accessors, making the tests dependent on
its internal representation. That's bad and means brittle tests and a lot of
changes if we ever change how data is stored in PasswordFormManager.

[kolos1, 2017-03-17 14:07:38]

Patchset #18 (id:380001) has been deleted

[kolos1, 2017-03-17 14:09:34]

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
File components/autofill/core/common/password_form.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:19: base::string16
OtherPossibleUsernamesToString(
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> You can also implement it as two pass processing:
> (1) using std::transform to create a vector of "first+second" strings, then
> (2) using base::JoinString to join them with ",".
> 
> There are two advantages to using JoinString: it is easier to read than the
> for-cycle with the special clause for the non-empty result, and it is also
> faster thanks to pre-allocating the needed space upfront.
> 
> The disadvantage is that more space is locally taken to store the
"first+second"
> strings, but at least it is not slower, because the current code on line 25
also
> creates all those strings.

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:20: PossibleUsernamesVector
possible_usernames) {
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> const PossibleUsernamesVector&

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.cc:22: for (const
PossibleUsernamePair pair : possible_usernames) {
On 2017/03/16 21:00:41, vabr (Chromium) wrote:
> use const ref not const value:
> const PossibleUsernamePair& pair

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
File components/autofill/core/common/password_form.h (right):

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.h:21: typedef
std::pair<base::string16, base::string16> PossibleUsernamePair;
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> Here and below please use:
> using new_alias = old_type
> instead of typedef. See also https://chromium-cpp.appspot.com/.

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/autofill/co...
components/autofill/core/common/password_form.h:314: base::string16
OtherPossibleUsernamesToStr(const PasswordForm& form);
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> Having both OtherPossibleUsernamesToStr and OtherPossibleUsernamesToString is
> confusing, and the difference in the names does not hint at the difference in
> behaviour.
> 
> Please only keep OtherPossibleUsernamesToString and declare it here. The only
> place outside password_form.* where OtherPossibleUsernamesToStr is currently
> used will be easily converted to use OtherPossibleUsernamesToString.

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/login_database.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/login_database.cc:70:
DCHECK(iterator.ReadString16(&field_name));
ah, of course. 

I though the release compiler would manage to separate DCHECK and the
expression. Just realized that it is impossible for machine.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:709: const
autofill::PossibleUsernamePair& pair =
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> This could be a range-based for loop, you don't use |i| for anything else than
> defining |pair|.

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.h (right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.h:358: void
SendVoteOnCredentialsReusage(const autofill::PasswordForm& observed,
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> nit: Reusage -> Reuse

Done.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager.h:484: // then
|username_correction_vote_| stores this credentials.
On 2017/03/16 21:00:42, vabr (Chromium) wrote:
> Not sure what "this credentials" refer to. It is a PasswordForm, but how does
it
> look like?
> Also, either use "this credential" or "these credentials".

Fixed.

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
File components/password_manager/core/browser/password_form_manager_unittest.cc
(right):

https://codereview.chromium.org/2747733004/diff/360001/components/password_ma...
components/password_manager/core/browser/password_form_manager_unittest.cc:2913:
// Checks the username correction vote is saved.
Yes, we could get rid of this check. Fixed.

[kolos1, 2017-03-17 14:12:37]

kolos@chromium.org changed reviewers:
+ mkwst@chromium.org

[kolos1, 2017-03-17 14:12:38]

mkwst@: please review changes in autofill_types*

Thanks.

Regards,
Maxim

[Mike West, 2017-03-17 14:16:08]

Converting the string to a pair LGTM.

[vabr (Chromium), 2017-03-17 15:30:07]

Still LGTM, thanks for addressing the comments.

[kolos1, 2017-03-18 21:53:09]

The CQ bit was checked by kolos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-18 21:53:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-18 23:06:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-18 23:06:13]

Dry run: This issue passed the CQ dry run.

[kolos1, 2017-03-21 20:42:01]

dvadym@: Friendly ping. Please let me know if you have any concerns. Sending
username correction votes doesn't affect neither form matching nor sending
generation votes. I believe it minimizes the risks to distort existing votes.

[dvadym, 2017-03-22 10:56:19]

In general it looks good. I have some comments.

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (left):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:779:
field_types[pending_credentials_.username_element] = autofill::USERNAME;
On 2017/03/16 14:10:00, kolos1 wrote:
> dvadym@: sending USERNAME is removed here.

Acknowledged. In this setup USERNAME vote is useless.

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:833:
field_types[form_to_upload.username_element] = autofill::USERNAME;
On 2017/03/16 14:10:00, kolos1 wrote:
> dvadym@: shall we also send some password vote for |password_element|? I might
> be useful when <FORM> tag belongs to several forms (signin/signup), but it
might
> change input data for password generation. 
> 
> I would prefer to separate username and generation votes for now, i.e. don't
> send both votes.

It looks like you should send ACCOUNT_CREATION_PASSWORD for |password_element|,
this is reuse of credentials, that's the exactly places where we send
ACCOUNT_CREATION_PASSWORD.

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
File components/autofill/core/common/password_form.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form.cc:175: pairs.begin(),
[](PossibleUsernamePair p) {
Nit: const PossibleUsernamePair&

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
File components/autofill/core/common/password_form_fill_data.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form_fill_data.cc:22: for (auto& pair :
values_and_field_names)
Nit: const auto&

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form_fill_data.cc:102:
&result->other_possible_usernames[key]);
Could you please remove sending OtherPossibleUsername to the renderer at all
(here and in mojo files)? We don't want to show for now OtherPossible usernames
for the user, so we don't need to send them to the renderer. Not sending to the
renderer will help us:
1.To save time/memory
2.To improve security, since less data will be in renderer.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:116:
[&username_value](PossibleUsernamePair pair) {
Nit: PossibleUsernamePair -> const PossibleUsernamePair&

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:122:
std::remove_if(usernames.begin(), new_end, [](PossibleUsernamePair pair) {
Nit: PossibleUsernamePair -> const PossibleUsernamePair&

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:127:
[](PossibleUsernamePair pair) { return autofill::IsSSN(pair.first); });
Nit: PossibleUsernamePair -> const PossibleUsernamePair&

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:703: bool
PasswordFormManager::FindUsernameInOtherPossibleUsernames(
I'm wondering whether we should match credentials by password also. If a
submitted password is different from password in credentials in which we
OtherPossibleUsername found, it would look strange, which would probably mean
that we are incorrect in password element detection. From the current logic we
would be able to understand whether it happens on the server.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:708: for
(const autofill::PossibleUsernamePair& pair :
Nit: autofill::PossibleUsernamePair->PossibleUsernamePair (there using for
autofill::PossibleUsernamePair)

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:710: if
(pair.second.empty())
I feel that we shouldn't skip fields with empty ids. Yeah it would be strange if
a site use such fields for username, and also there could be multiple fields
with no ids. But I think if such field has a value that equal to username, we
should send it. Then server-side we can investigate how often this happens and
to think what to do, but if we skipped we would be blind here.

[dvadym, 2017-03-22 10:57:38]

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:1287:
FormStructure(observed_form_.form_data).FormSignatureAsStr());
On 2017/03/16 14:10:00, kolos1 wrote:
> dvadym@: send also the signature of the observed form as login_form_signature.

Acknowledged.

[kolos1, 2017-03-22 13:40:39]

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/320001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:833:
field_types[form_to_upload.username_element] = autofill::USERNAME;
On 2017/03/22 10:56:18, dvadym wrote:
> On 2017/03/16 14:10:00, kolos1 wrote:
> > dvadym@: shall we also send some password vote for |password_element|? I
might
> > be useful when <FORM> tag belongs to several forms (signin/signup), but it
> might
> > change input data for password generation. 
> > 
> > I would prefer to separate username and generation votes for now, i.e. don't
> > send both votes.
> 
> It looks like you should send ACCOUNT_CREATION_PASSWORD for
|password_element|,
> this is reuse of credentials, that's the exactly places where we send
> ACCOUNT_CREATION_PASSWORD.

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
File components/autofill/core/common/password_form.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form.cc:175: pairs.begin(),
[](PossibleUsernamePair p) {
On 2017/03/22 10:56:18, dvadym wrote:
> Nit: const PossibleUsernamePair&

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
File components/autofill/core/common/password_form_fill_data.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form_fill_data.cc:22: for (auto& pair :
values_and_field_names)
On 2017/03/22 10:56:18, dvadym wrote:
> Nit: const auto&

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/autofill/co...
components/autofill/core/common/password_form_fill_data.cc:102:
&result->other_possible_usernames[key]);
Agree. Created a separate CL for that
(https://codereview.chromium.org/2760353003/)

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:116:
[&username_value](PossibleUsernamePair pair) {
On 2017/03/22 10:56:18, dvadym wrote:
> Nit: PossibleUsernamePair -> const PossibleUsernamePair&

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:122:
std::remove_if(usernames.begin(), new_end, [](PossibleUsernamePair pair) {
On 2017/03/22 10:56:18, dvadym wrote:
> Nit: PossibleUsernamePair -> const PossibleUsernamePair&

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:127:
[](PossibleUsernamePair pair) { return autofill::IsSSN(pair.first); });
On 2017/03/22 10:56:19, dvadym wrote:
> Nit: PossibleUsernamePair -> const PossibleUsernamePair&

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:703: bool
PasswordFormManager::FindUsernameInOtherPossibleUsernames(
Great point! I was going to check the password value, but forgot.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:708: for
(const autofill::PossibleUsernamePair& pair :
On 2017/03/22 10:56:18, dvadym wrote:
> Nit: autofill::PossibleUsernamePair->PossibleUsernamePair (there using for
> autofill::PossibleUsernamePair)

Done.

https://codereview.chromium.org/2747733004/diff/400001/components/password_ma...
components/password_manager/core/browser/password_form_manager.cc:710: if
(pair.second.empty())
On 2017/03/22 10:56:19, dvadym wrote:
> I feel that we shouldn't skip fields with empty ids. Yeah it would be strange
if
> a site use such fields for username, and also there could be multiple fields
> with no ids. But I think if such field has a value that equal to username, we
> should send it. Then server-side we can investigate how often this happens and
> to think what to do, but if we skipped we would be blind here.

Done.

[dvadym, 2017-03-22 14:14:46]

LGTM Thanks for implementing sending USERNAME votes!

[kolos1, 2017-03-22 14:20:51]

The CQ bit was checked by kolos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 14:21:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 15:35:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 15:35:22]

Dry run: This issue passed the CQ dry run.

[kolos1, 2017-03-22 16:03:34]

The CQ bit was checked by kolos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-22 16:04:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kolos1, 2017-03-22 16:10:30]

The CQ bit was unchecked by kolos@chromium.org

[kolos1, 2017-03-22 16:10:31]

The CQ bit was checked by kolos@chromium.org

[kolos1, 2017-03-22 16:10:32]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org,
mkwst@chromium.org, dvadym@chromium.org
Link to the patchset: https://codereview.chromium.org/2747733004/#ps440001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-22 16:10:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 17:16:34]

CQ is committing da patch.

Bot data: {"patchset_id": 440001, "attempt_start_ts": 1490199031536210,
"parent_rev": "68d51c6dd7715dcb357341845d7f8103b82febb1", "commit_rev":
"718d242b21b1964f8a06e79bf00dcce7e073dac7"}

[commit-bot: I haz the power, 2017-03-22 17:17:43]

Description was changed from

==========
[Password Manager] Send username correction votes

If a user changes autofilled username on a form, but the correct username is in
|PasswordForm.other_possible_usernames|, send USERNAME vote for the element that
contained the correct username. 

This CL changes the data structure of |other_possible_usernames| from
vector<string> to vector<pair<string, string>>. 

The CL updates only one local storage - login_database. We need to receive
enough votes, there is no need to cover 100% of Chrome instances. But the
results based on the votes would propagated to all instances.

BUG=699530
==========

to

==========
[Password Manager] Send username correction votes

If a user changes autofilled username on a form, but the correct username is in
|PasswordForm.other_possible_usernames|, send USERNAME vote for the element that
contained the correct username. 

This CL changes the data structure of |other_possible_usernames| from
vector<string> to vector<pair<string, string>>. 

The CL updates only one local storage - login_database. We need to receive
enough votes, there is no need to cover 100% of Chrome instances. But the
results based on the votes would propagated to all instances.

BUG=699530

Review-Url: https://codereview.chromium.org/2747733004
Cr-Commit-Position: refs/heads/master@{#458788}
Committed:
https://chromium.googlesource.com/chromium/src/+/718d242b21b1964f8a06e79bf00d...
==========

[commit-bot: I haz the power, 2017-03-22 17:17:45]

Committed patchset #20 (id:440001) as
https://chromium.googlesource.com/chromium/src/+/718d242b21b1964f8a06e79bf00d...

[adithyas, 2017-07-31 21:15:51]

A revert of this CL (patchset #20 id:440001) has been created in
https://codereview.chromium.org/2985373002/ by adithyas@chromium.org.

The reason for reverting is: Suspect that this is causing browser_tests and
interactive_ui_tests failures.