[Password Generation] Sends the flag whether a field has nonempty user input

[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from |field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the field properties masks will be sent to the browser on form submission, inpage nvaigation, force saving.

4) On the browser side, copy the field properties to |pending_credentials_|. See PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.

BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature, PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit, PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation, PasswordFormManagerTest.FieldPropertiesMasksUpload

Committed: https://crrev.com/66b4c2950fda6e71c891ef57ac0354f16a0cf9d3
Cr-Commit-Position: refs/heads/master@{#408684}

[kolos1, 2016-07-15 16:33:32]

Description was changed from

==========
[Password Generation] Sends the flag whether a field has nonempty user input

BUG=552420
==========

to

==========
[Password Generation] Sends the flag whether a field has nonempty user input

BUG=552420
==========

[kolos1, 2016-07-21 14:44:53]

Patchset #10 (id:130017) has been deleted

[kolos1, 2016-07-21 15:12:02]

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

[kolos1, 2016-07-21 15:12:04]

Hi Vadym,

Please review these files: 
password_autofill_agent.*
password_form.h
content_password_manager_driver.*
password_form_manager*
password_manager*

Thanks,
Maxim

[kolos1, 2016-07-25 14:45:25]

Patchset #13 (id:250001) has been deleted

[kolos1, 2016-07-25 14:45:35]

Patchset #13 (id:270001) has been deleted

[kolos1, 2016-07-25 14:46:42]

Hi Vadym,

I changed the design as we discussed in person. Please review.

Best regards,
Maxim

[kolos1, 2016-07-27 16:00:27]

Changed design one more time to handle duplicates IDs. Please review.

[kolos1, 2016-07-28 08:58:06]

Description was changed from

==========
[Password Generation] Sends the flag whether a field has nonempty user input

BUG=552420
==========

to

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

[kolos1, 2016-07-28 08:59:17]

Description was changed from

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

to

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

[dvadym, 2016-07-28 09:03:06]

Thanks, it looks good. There are some comments, mostly nits.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:355: void
UpdateFieldValueAndPropertiesMaskMap(
Nit: By style, output arguments should be after input argumetns

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:367:
std::pair<base::string16, FieldPropertiesMask>(value, added_flags);
nit: make_pair

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:1397:
control_element, control_element.value(),
It seems that here control_element.value() could have js origin. It looks like
in this case we don't want to update stored in the map element value.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
File components/autofill/content/renderer/password_form_conversion_utils.h
(right):

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils.h:12:
#include <components/autofill/core/common/password_form.h>
NIt: It seems that you don't need to add this header

[kolos1, 2016-07-28 11:01:05]

Patchset #20 (id:430001) has been deleted

[kolos1, 2016-07-28 11:04:34]

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

[kolos1, 2016-07-28 11:04:36]

dvadym@: made changes addressed to your comments.

vabr@: please review the rest of files except autofill_messages. I will update
mojo in a separate CL.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:355: void
UpdateFieldValueAndPropertiesMaskMap(
On 2016/07/28 09:03:06, dvadym wrote:
> Nit: By style, output arguments should be after input argumetns

Done.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:367:
std::pair<base::string16, FieldPropertiesMask>(value, added_flags);
On 2016/07/28 09:03:06, dvadym wrote:
> nit: make_pair

Done.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:1397:
control_element, control_element.value(),
You are right, it might js content. Fixed.

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
File components/autofill/content/renderer/password_form_conversion_utils.h
(right):

https://codereview.chromium.org/2148303005/diff/410001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils.h:12:
#include <components/autofill/core/common/password_form.h>
On 2016/07/28 09:03:06, dvadym wrote:
> NIt: It seems that you don't need to add this header

PasswordForm is used in this header.

[vabr (Chromium), 2016-07-28 11:33:53]

Hi Maxim,

Looks good, but I have some comments.

Thanks!
Vaclav

https://codereview.chromium.org/2148303005/diff/450001/chrome/renderer/autofi...
File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):

https://codereview.chromium.org/2148303005/diff/450001/chrome/renderer/autofi...
chrome/renderer/autofill/password_autofill_agent_browsertest.cc:491: void
ExpectFieldPropertiesMasks(uint32_t expected_message_id) {
nit: It is not clear what expectation this function verifies. Either the
function name (preferably) or at leas a comment above this line should make it
clear.

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/content/renderer/form_autofill_util.h (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/content/renderer/form_autofill_util.h:150: const
FieldValueAndPropertiesMaskMap* field_value_and_properties_map,
nit: Please describe the added argument in the method declaration comment.
Please state that whether is optional (i.e., can be null).

Also below.

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.h (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.h:17: #include
"components/autofill/core/common/password_form.h"
What do you need this #include for? Could you move it back to the .cc?

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/core/browser/proto/server.proto (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/core/browser/proto/server.proto:95: optional uint32
properties_mask = 20;
note: Please acknowledge, that you are taking care of synchronising the
server-side proto with this change.

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/core/common/form_field_data.cc (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/core/common/form_field_data.cc:17: const int kPickleVersion
= 6;
Please also update components/autofill/core/common/form_field_data_unittest.cc
for the new version.

[kolos1, 2016-07-28 13:44:03]

https://codereview.chromium.org/2148303005/diff/450001/chrome/renderer/autofi...
File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):

https://codereview.chromium.org/2148303005/diff/450001/chrome/renderer/autofi...
chrome/renderer/autofill/password_autofill_agent_browsertest.cc:491: void
ExpectFieldPropertiesMasks(uint32_t expected_message_id) {
On 2016/07/28 11:33:53, vabr (Chromium) wrote:
> nit: It is not clear what expectation this function verifies. Either the
> function name (preferably) or at leas a comment above this line should make it
> clear.

Done.

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/content/renderer/form_autofill_util.h (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/content/renderer/form_autofill_util.h:150: const
FieldValueAndPropertiesMaskMap* field_value_and_properties_map,
On 2016/07/28 11:33:53, vabr (Chromium) wrote:
> nit: Please describe the added argument in the method declaration comment.
> Please state that whether is optional (i.e., can be null).
> 
> Also below.

Done.

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.h (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.h:17: #include
"components/autofill/core/common/password_form.h"
On 2016/07/28 11:33:53, vabr (Chromium) wrote:
> What do you need this #include for? Could you move it back to the .cc?

|provisionally_saved_form_| is declared as |std::unique_ptr<PasswordForm>|. do
we not need an include for PasswordForm here?

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/core/browser/proto/server.proto (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/core/browser/proto/server.proto:95: optional uint32
properties_mask = 20;
On 2016/07/28 11:33:53, vabr (Chromium) wrote:
> note: Please acknowledge, that you are taking care of synchronising the
> server-side proto with this change. 

of course, thanks. It was created a week ago and is still waiting his
client-side twin  :)

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
File components/autofill/core/common/form_field_data.cc (right):

https://codereview.chromium.org/2148303005/diff/450001/components/autofill/co...
components/autofill/core/common/form_field_data.cc:17: const int kPickleVersion
= 6;
On 2016/07/28 11:33:53, vabr (Chromium) wrote:
> Please also update components/autofill/core/common/form_field_data_unittest.cc
> for the new version.

Done.

[dvadym, 2016-07-28 13:49:44]

Thanks Maxim for implementing this.
Password part and general logic LGTM

[vabr (Chromium), 2016-07-28 14:30:59]

LGTM with minor comments.
Thanks!
Vaclav

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:369: value ?
std::unique_ptr<base::string16>(new base::string16(*value))
optional nit: Consider using base::WrapUnique instead of
std::unique_ptr<base::string16>. The former does not duplicate the type (which
is also spelled out after "new"), and makes the code more readable.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:370: :
std::unique_ptr<base::string16>(),
nit: Just nullptr will do the same job and is easier to read.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:756: const
base::string16 username_value = username;
Note: Once you rebase https://codereview.chromium.org/2187253002/ on top of
this, don't forget to remove this line (and line 764) in
https://codereview.chromium.org/2187253002/.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File components/autofill/content/renderer/password_form_conversion_utils.cc
(right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils.cc:318:
return field_map && field_map->find(element) != field_map->end() &&
optional: The "find" and "at" seem a little wasteful. If you stored the
iterator, you could use it to query the value, without the second lookup.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File
components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
(right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc:190:
std::unique_ptr<base::string16>(new base::string16(element_value)),
optional: Also here, a base::WrapUnique would remove the redundant mention of
the type.

[kolos1, 2016-07-28 15:24:27]

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:369: value ?
std::unique_ptr<base::string16>(new base::string16(*value))
On 2016/07/28 14:30:59, vabr (Chromium) wrote:
> optional nit: Consider using base::WrapUnique instead of
> std::unique_ptr<base::string16>. The former does not duplicate the type (which
> is also spelled out after "new"), and makes the code more readable.

Done.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:370: :
std::unique_ptr<base::string16>(),
On 2016/07/28 14:30:59, vabr (Chromium) wrote:
> nit: Just nullptr will do the same job and is easier to read.

Done.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:756: const
base::string16 username_value = username;
On 2016/07/28 14:30:59, vabr (Chromium) wrote:
> Note: Once you rebase https://codereview.chromium.org/2187253002/ on top of
> this, don't forget to remove this line (and line 764) in
> https://codereview.chromium.org/2187253002/.

Done.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File components/autofill/content/renderer/password_form_conversion_utils.cc
(right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils.cc:318:
return field_map && field_map->find(element) != field_map->end() &&
On 2016/07/28 14:30:59, vabr (Chromium) wrote:
> optional: The "find" and "at" seem a little wasteful. If you stored the
> iterator, you could use it to query the value, without the second lookup.

Good point, thanks. Fixed.

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
File
components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc
(right):

https://codereview.chromium.org/2148303005/diff/490001/components/autofill/co...
components/autofill/content/renderer/password_form_conversion_utils_browsertest.cc:190:
std::unique_ptr<base::string16>(new base::string16(element_value)),
On 2016/07/28 14:30:59, vabr (Chromium) wrote:
> optional: Also here, a base::WrapUnique would remove the redundant mention of
> the type.

Done.

[kolos1, 2016-07-28 15:36:18]

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

[kolos1, 2016-07-28 15:36:19]

mkwst@: please review tiny change in autofill_messages.h

[vabr (Chromium), 2016-07-28 17:03:51]

One last comment, but still LGTM.

Cheers,
Vaclav

https://codereview.chromium.org/2148303005/diff/510001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/510001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:371: :
std::unique_ptr<base::string16>(nullptr),
What I meant is replace this whole line with
     : nullptr,

(nullptr is implicitly cast into a null unique_ptr, no need to spell out the
constructor)

[Mike West, 2016-07-28 18:49:14]

IPC LGTM (except insofar as it ought to be mojo :) )

[kolos1, 2016-07-29 06:15:57]

On 2016/07/28 18:49:14, Mike West wrote:
> IPC LGTM (except insofar as it ought to be mojo :) )

Thanks, Mike. update for mojo is in this CL
(https://codereview.chromium.org/2166773002)

[kolos1, 2016-07-29 06:16:19]

https://codereview.chromium.org/2148303005/diff/510001/components/autofill/co...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/2148303005/diff/510001/components/autofill/co...
components/autofill/content/renderer/password_autofill_agent.cc:371: :
std::unique_ptr<base::string16>(nullptr),
On 2016/07/28 17:03:51, vabr (Chromium) wrote:
> What I meant is replace this whole line with
>      : nullptr,
> 
> (nullptr is implicitly cast into a null unique_ptr, no need to spell out the
> constructor)

my concern was about implicit constructor, but since the type is mentioned   in
the first branch of ternary operator, it is ok to skip it her.

[kolos1, 2016-07-29 06:16:29]

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

[commit-bot: I haz the power, 2016-07-29 06:16:46]

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, 2016-07-29 07:00:42]

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

[commit-bot: I haz the power, 2016-07-29 07:00:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[vabr (Chromium), 2016-07-29 07:26:53]

Speaking of Mojo...

https://codereview.chromium.org/2148303005/diff/530001/components/autofill/co...
File components/autofill/content/common/autofill_messages.h (right):

https://codereview.chromium.org/2148303005/diff/530001/components/autofill/co...
components/autofill/content/common/autofill_messages.h:62:
IPC_STRUCT_TRAITS_MEMBER(properties_mask)
I guess this change needs also to be mirrored in
components/autofill/content/public/interfaces/autofill_types.mojom.

[kolos1, 2016-07-29 16:47:48]

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

[commit-bot: I haz the power, 2016-07-29 16:48:09]

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

[kolos1, 2016-07-29 17:09:43]

The CQ bit was checked by kolos@chromium.org

[kolos1, 2016-07-29 17:09:44]

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

[commit-bot: I haz the power, 2016-07-29 17:10:12]

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, 2016-07-29 17:55:10]

Description was changed from

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

to

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

[commit-bot: I haz the power, 2016-07-29 17:55:11]

Committed patchset #26 (id:570001)

[commit-bot: I haz the power, 2016-07-29 17:58:24]

Description was changed from

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving. 

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.


BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload
==========

to

==========
[Password Generation] Sends field properties mask in autofill upload

1. Extend |PasswordAutofillAgent.nonscript_modified_values_| to
FieldValueAndPropertiesMaskMap that is a map from WebFormControlElement to the
pair of:
a) The most recent text that user typed or PasswordManager autofilled in  input
elements. Used for storing username/password before JavaScript changes them.
a) Field properties mask, i.e. whether the field was autofilled, modified by
user, etc. (see FieldPropertiesMask). THIS ONE was added.

2) Update field properties mask on (PasswordAutofillAgent)
a) autofilling form field
b) user input in a field
c) user moves focus to a field

3) On FormData initialization, copy field properties masks from
|field_value_and_properties_map_| to |FormFieldData.properties_mask|. Thus, the
field properties masks will be sent to the browser on form submission, inpage
nvaigation, force saving.

4) On the browser side, copy the field properties to |pending_credentials_|. See
PasswordFormManager.CreatePendingCredentials()

5) Add the field properties mask to autofill upload request.

BUG=552420
TEST=FormStructureTest.EncodeUploadRequestWithAdditionalPasswordFormSignature,
PasswordAutofillAgentTest.RememberFieldPropertiesOnSubmit,
PasswordAutofillAgentTest, RememberFieldPropertiesOnInPageNavigation,
PasswordFormManagerTest.FieldPropertiesMasksUpload

Committed: https://crrev.com/66b4c2950fda6e71c891ef57ac0354f16a0cf9d3
Cr-Commit-Position: refs/heads/master@{#408684}
==========

[commit-bot: I haz the power, 2016-07-29 17:58:27]

Patchset 26 (id:??) landed as
https://crrev.com/66b4c2950fda6e71c891ef57ac0354f16a0cf9d3
Cr-Commit-Position: refs/heads/master@{#408684}

[vabr (Chromium), 2016-07-29 18:04:51]

Hi Maxim,
Post-landing comment below.

Also, what was the problem with the tests failing on Mac? (Cannot tell from the
diff, as it includes a rebase.)

Cheers,
Vaclav

https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):

https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
chrome/renderer/autofill/password_autofill_agent_browsertest.cc:22: #include
<components/autofill/core/common/password_form.h>
Please change < and > to " and ".

[kolos1, 2016-07-29 19:32:04]

On 2016/07/29 18:04:51, vabr (holiday until 8 Aug) wrote:
> Hi Maxim,
> Post-landing comment below.
> 
> Also, what was the problem with the tests failing on Mac? (Cannot tell from
the
> diff, as it includes a rebase.)
> 
> Cheers,
> Vaclav
> 
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):
> 
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> chrome/renderer/autofill/password_autofill_agent_browsertest.cc:22: #include
> <components/autofill/core/common/password_form.h>
> Please change < and > to " and ".

Hi Vaclav,

It failed on tests on InPageNavigation where we hide the form with changing
elements' visibility (username_element_.setAttribute("style",
"display:none;");).
I added new field ('random_field') to kNoForm, but didn't change its visibility
in the test. So, for Mac such form remains visible even if username and password
was hidden.

Regards,
Maxim

[kolos1, 2016-07-29 19:32:30]

On 2016/07/29 18:04:51, vabr (holiday until 8 Aug) wrote:
> Hi Maxim,
> Post-landing comment below.
> 
> Also, what was the problem with the tests failing on Mac? (Cannot tell from
the
> diff, as it includes a rebase.)
> 
> Cheers,
> Vaclav
> 
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right):
> 
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> chrome/renderer/autofill/password_autofill_agent_browsertest.cc:22: #include
> <components/autofill/core/common/password_form.h>
> Please change < and > to " and ".

Hi Vaclav,

It failed on tests on InPageNavigation where we hide the form with changing
elements' visibility (username_element_.setAttribute("style",
"display:none;");).
I added new field ('random_field') to kNoForm, but didn't change its visibility
in the test. So, for Mac such form remains visible even if username and password
was hidden.

I will fix the include.

Regards,
Maxim

[vabr (Chromium), 2016-07-29 19:59:20]

On 2016/07/29 19:32:04, kolos1 wrote:
> On 2016/07/29 18:04:51, vabr (holiday until 8 Aug) wrote:
> > Hi Maxim,
> > Post-landing comment below.
> > 
> > Also, what was the problem with the tests failing on Mac? (Cannot tell from
> the
> > diff, as it includes a rebase.)
> > 
> > Cheers,
> > Vaclav
> > 
> >
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> > File chrome/renderer/autofill/password_autofill_agent_browsertest.cc
(right):
> > 
> >
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> > chrome/renderer/autofill/password_autofill_agent_browsertest.cc:22: #include
> > <components/autofill/core/common/password_form.h>
> > Please change < and > to " and ".
> 
> Hi Vaclav,
> 
> It failed on tests on InPageNavigation where we hide the form with changing
> elements' visibility (username_element_.setAttribute("style",
> "display:none;");).
> I added new field ('random_field') to kNoForm, but didn't change its
visibility
> in the test. So, for Mac such form remains visible even if username and
password
> was hidden.
> 
> Regards,
> Maxim

Thanks, Maxim, for the explanation (and fixing the test)!
I'm still wondering why this was Mac specific.
Cheers,
Vaclav

[kolos1, 2016-07-29 20:05:07]

On 2016/07/29 19:59:20, vabr (holiday until 8 Aug) wrote:
> On 2016/07/29 19:32:04, kolos1 wrote:
> > On 2016/07/29 18:04:51, vabr (holiday until 8 Aug) wrote:
> > > Hi Maxim,
> > > Post-landing comment below.
> > > 
> > > Also, what was the problem with the tests failing on Mac? (Cannot tell
from
> > the
> > > diff, as it includes a rebase.)
> > > 
> > > Cheers,
> > > Vaclav
> > > 
> > >
> >
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> > > File chrome/renderer/autofill/password_autofill_agent_browsertest.cc
> (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2148303005/diff/570001/chrome/renderer/autofi...
> > > chrome/renderer/autofill/password_autofill_agent_browsertest.cc:22:
#include
> > > <components/autofill/core/common/password_form.h>
> > > Please change < and > to " and ".
> > 
> > Hi Vaclav,
> > 
> > It failed on tests on InPageNavigation where we hide the form with changing
> > elements' visibility (username_element_.setAttribute("style",
> > "display:none;");).
> > I added new field ('random_field') to kNoForm, but didn't change its
> visibility
> > in the test. So, for Mac such form remains visible even if username and
> password
> > was hidden.
> > 
> > Regards,
> > Maxim
> 
> Thanks, Maxim, for the explanation (and fixing the test)!
> I'm still wondering why this was Mac specific.
> Cheers,
> Vaclav

Probably because of this macros
https://cs.chromium.org/chromium/src/components/autofill/content/renderer/pas...