Limit form-less Autofilling to pages that look like checkout pages.

Limit form-less Autofilling to pages that look like checkout pages.

This should hopefully stem the tide of false positives on non-checkout pages, while preserving desired behavior on Best Buy and Apple checkout.

Fixing FormStructureBrowserTests: I went back and added <form>s that were present in the source pages but not in the local copies. I also added the original <title> tags, except for ones that did /not/ include any of the checkout keywords. For these, I added the checkout keywords to the title, so that the test still had some value (instead of passing trivially because the title was "Google Calendar" and not something like "Payment information").

BUG=471090, 477466
Committed: https://crrev.com/aa3219ce27ef5471c19670fe5145b136925cbc89
Cr-Commit-Position: refs/heads/master@{#327108}

[Evan Stade, 2015-04-18 00:37:05]

estade@chromium.org changed reviewers:
+ brettw@chromium.org, thestig@chromium.org

[Evan Stade, 2015-04-18 00:37:05]

I think I have some more tests to update, but otherwise this is ready for review

[Evan Stade, 2015-04-20 19:15:18]

In response to your comment on the bug, Lei: yes, I think a few false negatives
are worth the reduced number of false positives.

[brettw, 2015-04-20 19:24:24]

This seems kind of hacky. We also talked about enforcing some minimum number of
form element matches for formless inputs before enabling autofill. I'm wondering
if such an approach would also enable these pages we want while being more
general.

[Lei Zhang, 2015-04-20 19:25:15]

The try jobs for patch set 2 has a bunch of broken FormStructureBrowserTests.

[Evan Stade, 2015-04-20 20:59:06]

On 2015/04/20 19:24:24, brettw wrote:
> This seems kind of hacky. We also talked about enforcing some minimum number
of
> form element matches for formless inputs before enabling autofill. I'm
wondering
> if such an approach would also enable these pages we want while being more
> general.

We already do enforce a certain number of element matches. The false positives
occur in spite of this. If we increase the number of matches required, smaller
forms will no longer be autofilled. I don't think this patch is any hackier than
inferring a UI label and matching it against a regex based on human languages. I
believe the term "heuristic" is a polite form of "hacky guess".

> The try jobs for patch set 2 has a bunch of broken FormStructureBrowserTests.

yes, I'm holding off on fixing them until the reviewers agree this patch is a
good idea.

[Evan Stade, 2015-04-23 00:34:22]

On 2015/04/20 20:59:06, Evan Stade wrote:
> On 2015/04/20 19:24:24, brettw wrote:
> > This seems kind of hacky. We also talked about enforcing some minimum number
> of
> > form element matches for formless inputs before enabling autofill. I'm
> wondering
> > if such an approach would also enable these pages we want while being more
> > general.
> 
> We already do enforce a certain number of element matches. The false positives
> occur in spite of this. If we increase the number of matches required, smaller
> forms will no longer be autofilled. I don't think this patch is any hackier
than
> inferring a UI label and matching it against a regex based on human languages.
I
> believe the term "heuristic" is a polite form of "hacky guess".
> 
> > The try jobs for patch set 2 has a bunch of broken
FormStructureBrowserTests.
> 
> yes, I'm holding off on fixing them until the reviewers agree this patch is a
> good idea.

ping. Your thoughts?

[brettw, 2015-04-23 19:37:22]

I am unhappy but am not sure what to suggest instead. LGTM for now, hopefully we
can come up with something better in the future.

[Evan Stade, 2015-04-23 22:40:44]

FormStructureBrowserTests updated --- see explanation in description. Lei, ptal

[Lei Zhang, 2015-04-24 20:09:31]

There's still some failing tests on the try bots.

https://codereview.chromium.org/1081803003/diff/80001/chrome/test/data/autofi...
File chrome/test/data/autofill/heuristics/input/bug_471831.html (right):

https://codereview.chromium.org/1081803003/diff/80001/chrome/test/data/autofi...
chrome/test/data/autofill/heuristics/input/bug_471831.html:2: <title>Stripe:
Checkout</title>
The stripe page is actually in a <form>.

[Evan Stade, 2015-04-24 22:32:54]

Tests should be fixed.

[Lei Zhang, 2015-04-25 02:39:29]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2015-04-25 02:39:30]

lgtm

[Lei Zhang, 2015-04-25 02:39:30]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1081803003/#ps100001
(title: "test fixes")

[commit-bot: I haz the power, 2015-04-25 02:39:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081803003/100001

[commit-bot: I haz the power, 2015-04-25 03:34:11]

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

[commit-bot: I haz the power, 2015-04-25 03:34:13]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Evan Stade, 2015-04-27 19:02:37]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-04-27 19:02:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1081803003/#ps120001
(title: "suppress msvc warning")

[commit-bot: I haz the power, 2015-04-27 19:03:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1081803003/120001

[commit-bot: I haz the power, 2015-04-27 20:05:40]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-27 20:06:25]

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

[mohsen, 2015-04-27 21:02:06]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1106313002/ by mohsen@chromium.org.

The reason for reverting is: Caused compile error on Win x64 GN bot.

[mohsen, 2015-04-27 21:06:54]

On 2015/04/27 21:02:06, mohsen wrote:
> A revert of this CL (patchset #7 id:120001) has been created in
> https://codereview.chromium.org/1106313002/ by mailto:mohsen@chromium.org.
> 
> The reason for reverting is: Caused compile error on Win x64 GN bot.

Link:
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN/builds/653