[Password Generation] Add FormFieldData.css_classes to mojo

[Password Generation] Add FormFieldData.css_classes to mojo

Add FormFieldData.css_classes to mojo, otherwise css_classes wouldn't be sent from rendered to browser. The similar CL for IPC https://codereview.chromium.org/2162933003/

BUG=582434
Committed: https://crrev.com/21d7ab8c712c5793719d8283dd9635589a2f1143
Cr-Commit-Position: refs/heads/master@{#406810}

[kolos1, 2016-07-20 11:12:02]

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

[kolos1, 2016-07-20 11:12:02]

Hi Vaclav,

I added css_classes to mojo. Is there any way to test it at this point? 

Regards,
Maxim

[vabr (Chromium), 2016-07-20 11:19:22]

On 2016/07/20 11:12:02, kolos1 wrote:
> Hi Vaclav,
> 
> I added css_classes to mojo. Is there any way to test it at this point? 
> 
> Regards,
> Maxim

Hi Maxim,

Looking at https://codereview.chromium.org/1999623002, which added the
FormFieldData part, the test to run is components_unittests
--gtest_filter=AutofillTypeTraitsTestImpl*. Please also have a look at that CL
for what else needs to be changed. It seems that, e.g.,
components/autofill/content/public/cpp/autofill_types_struct_traits.h needs to
be modified, etc.

Cheers,
Vaclav

[kolos1, 2016-07-20 12:26:14]

On 2016/07/20 11:19:22, vabr (Chromium) wrote:
> On 2016/07/20 11:12:02, kolos1 wrote:
> > Hi Vaclav,
> > 
> > I added css_classes to mojo. Is there any way to test it at this point? 
> > 
> > Regards,
> > Maxim
> 
> Hi Maxim,
> 
> Looking at https://codereview.chromium.org/1999623002, which added the
> FormFieldData part, the test to run is components_unittests
> --gtest_filter=AutofillTypeTraitsTestImpl*. Please also have a look at that CL
> for what else needs to be changed. It seems that, e.g.,
> components/autofill/content/public/cpp/autofill_types_struct_traits.h needs to
> be modified, etc.
> 
> Cheers,
> Vaclav

Thanks, Vaclav. I added other necessary stuff, I believe.

[kolos1, 2016-07-20 12:41:11]

On 2016/07/20 12:26:14, kolos1 wrote:
> On 2016/07/20 11:19:22, vabr (Chromium) wrote:
> > On 2016/07/20 11:12:02, kolos1 wrote:
> > > Hi Vaclav,
> > > 
> > > I added css_classes to mojo. Is there any way to test it at this point? 
> > > 
> > > Regards,
> > > Maxim
> > 
> > Hi Maxim,
> > 
> > Looking at https://codereview.chromium.org/1999623002, which added the
> > FormFieldData part, the test to run is components_unittests
> > --gtest_filter=AutofillTypeTraitsTestImpl*. Please also have a look at that
CL
> > for what else needs to be changed. It seems that, e.g.,
> > components/autofill/content/public/cpp/autofill_types_struct_traits.h needs
to
> > be modified, etc.
> > 
> > Cheers,
> > Vaclav
> 
> Thanks, Vaclav. I added other necessary stuff, I believe.

Vaclav, are there any specific persons in ipc/SECURITY_OWNERS who should review
mojo stuff?

[vabr (Chromium), 2016-07-20 12:42:18]

Thanks, Maxim, LGTM, but please get this reviewed by somebody from
ipc/SECURITY_OWNERS.

Cheers,
Vaclav

[kolos1, 2016-07-20 12:59:47]

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

[kolos1, 2016-07-20 12:59:47]

dcheng@: Please review this CL. Thanks.

[dcheng, 2016-07-21 01:45:00]

Is this a list of css classes, or is it just the literal contents of the class
attribute?

[kolos1, 2016-07-21 06:29:26]

On 2016/07/21 01:45:00, dcheng wrote:
> Is this a list of css classes, or is it just the literal contents of the class
> attribute?

Literal contents of the class attribute
(https://cs.chromium.org/chromium/src/components/autofill/content/renderer/for...)

[dcheng, 2016-07-21 06:30:44]

On 2016/07/21 06:29:26, kolos1 wrote:
> On 2016/07/21 01:45:00, dcheng wrote:
> > Is this a list of css classes, or is it just the literal contents of the
class
> > attribute?
> 
> Literal contents of the class attribute
>
(https://cs.chromium.org/chromium/src/components/autofill/content/renderer/for...)

How will this be used in the browser process? Will we need to parse or interpret
the data in any form?

(I couldn't find somewhere that uses it today, so I guess this is new)

[kolos1, 2016-07-21 07:26:29]

On 2016/07/21 06:30:44, dcheng wrote:
> On 2016/07/21 06:29:26, kolos1 wrote:
> > On 2016/07/21 01:45:00, dcheng wrote:
> > > Is this a list of css classes, or is it just the literal contents of the
> class
> > > attribute?
> > 
> > Literal contents of the class attribute
> >
>
(https://cs.chromium.org/chromium/src/components/autofill/content/renderer/for...)
> 
> How will this be used in the browser process? Will we need to parse or
interpret
> the data in any form?
> 
> (I couldn't find somewhere that uses it today, so I guess this is new)

It will be uploaded to the autofill server where we will analyse CSS classes
(among others) to distinguish signup and signin forms
(https://cs.chromium.org/chromium/src/components/autofill/core/browser/form_st...).
E.g. 'signup' in CSS attribute might be a very strong signal of signup form. We
need the signup forms to provide  password generation on them.

[vabr (Chromium), 2016-07-21 08:16:07]

On 2016/07/21 07:26:29, kolos1 wrote:
> On 2016/07/21 06:30:44, dcheng wrote:
> > On 2016/07/21 06:29:26, kolos1 wrote:
> > > On 2016/07/21 01:45:00, dcheng wrote:
> > > > Is this a list of css classes, or is it just the literal contents of the
> > class
> > > > attribute?
> > > 
> > > Literal contents of the class attribute
> > >
> >
>
(https://cs.chromium.org/chromium/src/components/autofill/content/renderer/for...)
> > 
> > How will this be used in the browser process? Will we need to parse or
> interpret
> > the data in any form?
> > 
> > (I couldn't find somewhere that uses it today, so I guess this is new)
> 
> It will be uploaded to the autofill server where we will analyse CSS classes
> (among others) to distinguish signup and signin forms
>
(https://cs.chromium.org/chromium/src/components/autofill/core/browser/form_st...).
> E.g. 'signup' in CSS attribute might be a very strong signal of signup form.
We
> need the signup forms to provide  password generation on them.

Also, note that the CSS classes are already passed to the browser in messages
sent through the old IPC:
https://cs.chromium.org/chromium/src/components/autofill/content/common/autof....

[dcheng, 2016-07-21 09:19:14]

On 2016/07/21 08:16:07, vabr (Chromium) wrote:
> On 2016/07/21 07:26:29, kolos1 wrote:
> > On 2016/07/21 06:30:44, dcheng wrote:
> > > On 2016/07/21 06:29:26, kolos1 wrote:
> > > > On 2016/07/21 01:45:00, dcheng wrote:
> > > > > Is this a list of css classes, or is it just the literal contents of
the
> > > class
> > > > > attribute?
> > > > 
> > > > Literal contents of the class attribute
> > > >
> > >
> >
>
(https://cs.chromium.org/chromium/src/components/autofill/content/renderer/for...)
> > > 
> > > How will this be used in the browser process? Will we need to parse or
> > interpret
> > > the data in any form?
> > > 
> > > (I couldn't find somewhere that uses it today, so I guess this is new)
> > 
> > It will be uploaded to the autofill server where we will analyse CSS classes
> > (among others) to distinguish signup and signin forms
> >
>
(https://cs.chromium.org/chromium/src/components/autofill/core/browser/form_st...).
> > E.g. 'signup' in CSS attribute might be a very strong signal of signup form.
> We
> > need the signup forms to provide  password generation on them.
> 
> Also, note that the CSS classes are already passed to the browser in messages
> sent through the old IPC:
>
https://cs.chromium.org/chromium/src/components/autofill/content/common/autof....

Right, I just wanted to understand how it's used: the main thing I was concerned
about was any potential parsing. But it looks like we just send it verbatim to
the autofill server, so that's fine.

LGTM

[kolos1, 2016-07-21 09:28:39]

The CQ bit was checked by kolos@chromium.org

[commit-bot: I haz the power, 2016-07-21 09:28:57]

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-21 10:16:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-21 10:18:03]

Description was changed from

==========
[Password Generation] Add FormFieldData.css_classes to mojo

Add FormFieldData.css_classes to mojo, otherwise css_classes wouldn't be sent
from rendered to browser. The similar CL for IPC
https://codereview.chromium.org/2162933003/

BUG=582434
==========

to

==========
[Password Generation] Add FormFieldData.css_classes to mojo

Add FormFieldData.css_classes to mojo, otherwise css_classes wouldn't be sent
from rendered to browser. The similar CL for IPC
https://codereview.chromium.org/2162933003/

BUG=582434

Committed: https://crrev.com/21d7ab8c712c5793719d8283dd9635589a2f1143
Cr-Commit-Position: refs/heads/master@{#406810}
==========

[commit-bot: I haz the power, 2016-07-21 10:18:05]

Patchset 3 (id:??) landed as
https://crrev.com/21d7ab8c712c5793719d8283dd9635589a2f1143
Cr-Commit-Position: refs/heads/master@{#406810}