[Autofill] Parse forms that have autocomplete attributes.

[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid confusion with the new type HTML_TYPE_UNRECOGNIZED.

BUG=576853
Committed: https://crrev.com/7ce5a7d8db3a859c027a56f263267a67c7fd208a
Cr-Commit-Position: refs/heads/master@{#369409}

[sebsg, 2016-01-12 21:03:47]

Description was changed from

==========
[Autofill] Parse forms that have autocomplete attributes.

BUG=576853
==========

to

==========
[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid
confusion with the new type HTML_TYPE_UNRECOGNIZED.
 
BUG=576853
==========

[sebsg, 2016-01-12 21:37:57]

Patchset #1 (id:1) has been deleted

[sebsg, 2016-01-12 21:39:16]

sebsg@chromium.org changed reviewers:
+ mathp@chromium.org, vabr@chromium.org

[sebsg, 2016-01-12 21:40:02]

Could you please take a look at this CL? Thanks!

[Mathieu, 2016-01-13 01:03:20]

lgtm, nice tests too!

see nits

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:369: if
(autocomplete_attribute_value == "")
this should probably be first in the function, because it will occur more often
(assuming this code is run for every field?)

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:1173: if (field_type ==
HTML_TYPE_UNSPECIFIED)
shouldn't we also "continue" if it's unrecognized?

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure_unittest.cc (right):

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure_unittest.cc:488: // The
heuristics and server predictions should if there are more than two
*should run

[vabr (Chromium), 2016-01-13 09:01:16]

LGTM! Nothing more to add to Mathieu's comments.

Cheers,
Vaclav

[sebsg, 2016-01-13 16:05:13]

sebsg@chromium.org changed reviewers:
+ thestig@chromium.org

[sebsg, 2016-01-13 16:05:13]

Hi thestig@, could you please review
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc when you get the
chance? Thanks!

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:369: if
(autocomplete_attribute_value == "")
On 2016/01/13 01:03:20, Mathieu Perreault wrote:
> this should probably be first in the function, because it will occur more
often
> (assuming this code is run for every field?)

Absolutely, I should have thought of this. Thanks!

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:1173: if (field_type ==
HTML_TYPE_UNSPECIFIED)
On 2016/01/13 01:03:20, Mathieu Perreault wrote:
> shouldn't we also "continue" if it's unrecognized?

No because if we continue, it's like we give up on the field. So heuristics and
server predictions will be taken into account when deciding the overall type. If
it is unrecognized we don't want the overall type to be unrecognized so we need
to follow the path.

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure_unittest.cc (right):

https://codereview.chromium.org/1583563003/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure_unittest.cc:488: // The
heuristics and server predictions should if there are more than two
On 2016/01/13 01:03:20, Mathieu Perreault wrote:
> *should run

Done.

[Lei Zhang, 2016-01-13 19:34:54]

lgtm

[sebsg, 2016-01-13 19:38:40]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-01-13 19:38:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/1583563003/#ps40001
(title: "Nits")

[commit-bot: I haz the power, 2016-01-13 19:40:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1583563003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1583563003/40001

[commit-bot: I haz the power, 2016-01-13 19:48:00]

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

[commit-bot: I haz the power, 2016-01-13 19:48:01]

Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[sebsg, 2016-01-13 20:25:39]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-01-13 20:25:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
vabr@chromium.org, thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1583563003/#ps60001
(title: "Fixed mac and linux compile error")

[commit-bot: I haz the power, 2016-01-13 20:26:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1583563003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1583563003/60001

[commit-bot: I haz the power, 2016-01-13 23:39:24]

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

[commit-bot: I haz the power, 2016-01-13 23:39:25]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[sebsg, 2016-01-14 02:19:58]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-01-14 02:19:58]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
vabr@chromium.org, thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1583563003/#ps80001
(title: "Fixed android compile and browser test")

[commit-bot: I haz the power, 2016-01-14 02:23:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1583563003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1583563003/80001

[commit-bot: I haz the power, 2016-01-14 03:52:30]

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

[commit-bot: I haz the power, 2016-01-14 03:52:31]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sebsg, 2016-01-14 14:01:11]

The CQ bit was checked by sebsg@chromium.org

[commit-bot: I haz the power, 2016-01-14 14:02:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1583563003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1583563003/80001

[commit-bot: I haz the power, 2016-01-14 14:17:00]

Description was changed from

==========
[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid
confusion with the new type HTML_TYPE_UNRECOGNIZED.
 
BUG=576853
==========

to

==========
[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid
confusion with the new type HTML_TYPE_UNRECOGNIZED.
 
BUG=576853
==========

[commit-bot: I haz the power, 2016-01-14 14:17:01]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-01-14 14:18:52]

Description was changed from

==========
[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid
confusion with the new type HTML_TYPE_UNRECOGNIZED.
 
BUG=576853
==========

to

==========
[Autofill] Parse forms that have autocomplete attributes.

The type HTML_TYPE_UNKNOWN was changed to HTML_TYPE_UNSPECIFIED to avoid
confusion with the new type HTML_TYPE_UNRECOGNIZED.

BUG=576853

Committed: https://crrev.com/7ce5a7d8db3a859c027a56f263267a67c7fd208a
Cr-Commit-Position: refs/heads/master@{#369409}
==========

[commit-bot: I haz the power, 2016-01-14 14:18:53]

Patchset 4 (id:??) landed as
https://crrev.com/7ce5a7d8db3a859c027a56f263267a67c7fd208a
Cr-Commit-Position: refs/heads/master@{#369409}