AutofillDriverFactory manages information about user gestures

AutofillDriverFactory manages information about user gestures

When a password field is autofilled, the value is not accessible to JavaScript
until the user clicks or types in the page. To keep track of this, the frames
report such user gestures, and ChromeAutofillClient distributes such a signal
to all other frames in the same WebContent, via their common
AutofillDriverFactory.

This does not work when a new frame is added later, because the browser process
does not keep track whether the user gestures were seen already. Newly added
frames never learn about the user already having clicked in the page.

This CL fixes that by modifying AutofillDriverFactory to remember the
user gestures and notify also drivers added later. The flag is reset on main
page navigation.

The CL also makes parts of the AutofillDriverFactory API public, as long as they
do not modify the drivers map. This results in a slight reordering of methods.

BUG=669045
Review-Url: https://codereview.chromium.org/2603623002
Cr-Commit-Position: refs/heads/master@{#453245}
Committed: https://chromium.googlesource.com/chromium/src/+/ce7eba36c420dabb86b57f1851d5c86115d7d13c

[vabr (Chromium), 2016-12-23 20:49:23]

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

[commit-bot: I haz the power, 2016-12-23 20:49:28]

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-12-23 21:10:30]

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

[commit-bot: I haz the power, 2016-12-23 21:10:31]

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

[vabr (Chromium), 2016-12-23 21:21:03]

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

[commit-bot: I haz the power, 2016-12-23 21:21:10]

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

[vabr (Chromium), 2016-12-23 21:45:21]

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

[commit-bot: I haz the power, 2016-12-23 21:45:28]

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-12-23 22:41:26]

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

[commit-bot: I haz the power, 2016-12-23 22:41:27]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2017-02-24 09:56:32]

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

[commit-bot: I haz the power, 2017-02-24 09:56:50]

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-02-24 10:06:38]

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

[commit-bot: I haz the power, 2017-02-24 10:06:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vabr (Chromium), 2017-02-24 10:28:58]

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

[commit-bot: I haz the power, 2017-02-24 10:29:10]

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

[vabr (Chromium), 2017-02-24 10:49:09]

Description was changed from

==========
ContentAutofillDriverFactory manages information about user gestures

When a password field is autofilled, the value is not accessible to JavaScript
until the user clicks or types in the page. To keep track of this, the frames
report such user gestures, and ChromeAutofillClient distributes such a signal
to all other frames in the same WebContent, via their common
ContentAutofillDriverFactory.

This does not work when a new frame is added later, because the browser process
does not keep track whether the user gestures were seen already. Newly added
frames never learn about the user already having clicked in the page.

This CL fixes that by modifying ContentAutofillDriverFactory to remember the
user gestures and notify also drivers added later. The flag is reset on main
page navigation.

BUG=669045
==========

to

==========
AutofillDriverFactory manages information about user gestures

When a password field is autofilled, the value is not accessible to JavaScript
until the user clicks or types in the page. To keep track of this, the frames
report such user gestures, and ChromeAutofillClient distributes such a signal
to all other frames in the same WebContent, via their common
AutofillDriverFactory.

This does not work when a new frame is added later, because the browser process
does not keep track whether the user gestures were seen already. Newly added
frames never learn about the user already having clicked in the page.

This CL fixes that by modifying AutofillDriverFactory to remember the
user gestures and notify also drivers added later. The flag is reset on main
page navigation.

The CL also makes parts of the AutofillDriverFactory API public, as long as they
do not modify the drivers map. This results in a slight reordering of methods.

BUG=669045
==========

[vabr (Chromium), 2017-02-24 10:49:32]

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

[vabr (Chromium), 2017-02-24 10:49:33]

Hi Vadym,

Could you please review this CL?

Thanks!
Vaclav

[commit-bot: I haz the power, 2017-02-24 11:37:57]

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

[commit-bot: I haz the power, 2017-02-24 11:37:58]

Dry run: This issue passed the CQ dry run.

[dvadym, 2017-02-24 12:48:21]

LGTM, thanks for fixing this.

https://codereview.chromium.org/2603623002/diff/100001/components/autofill/co...
File components/autofill/core/browser/autofill_driver_factory.cc (right):

https://codereview.chromium.org/2603623002/diff/100001/components/autofill/co...
components/autofill/core/browser/autofill_driver_factory.cc:36: for (auto&
driver : driver_map_) {
Nit: do we need {} here?

[vabr (Chromium), 2017-02-24 12:52:46]

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

[commit-bot: I haz the power, 2017-02-24 12:53:02]

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

[vabr (Chromium), 2017-02-24 12:54:24]

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

[vabr (Chromium), 2017-02-24 12:54:25]

Thanks, Vadym!

Mathieu, Could you please do an OWNERS review for
chrome/browser/ui/autofill/chrome_autofill_client.cc ?

Thanks!
Vaclav

https://codereview.chromium.org/2603623002/diff/100001/components/autofill/co...
File components/autofill/core/browser/autofill_driver_factory.cc (right):

https://codereview.chromium.org/2603623002/diff/100001/components/autofill/co...
components/autofill/core/browser/autofill_driver_factory.cc:36: for (auto&
driver : driver_map_) {
On 2017/02/24 12:48:21, dvadym wrote:
> Nit: do we need {} here?

We don't need them, but they are allowed [1]. However, I should at least be
consistent with lines 50-51 below, so I'm removing the braces here. Thanks for
pointing this out.

[1]
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

[commit-bot: I haz the power, 2017-02-24 13:31:03]

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

[commit-bot: I haz the power, 2017-02-24 13:31:04]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2017-02-27 15:41:37]

Hi Mathieu,

Just a friendly ping -- could you please do an OWNERS review for
chrome/browser/ui/autofill/chrome_autofill_client.cc ?

Thanks!
Vaclav

[Mathieu, 2017-02-27 15:46:29]

Sorry I had forgotten to send my blessing, lgtm

https://codereview.chromium.org/2603623002/diff/120001/components/autofill/co...
File components/autofill/core/browser/autofill_driver_factory.h (right):

https://codereview.chromium.org/2603623002/diff/120001/components/autofill/co...
components/autofill/core/browser/autofill_driver_factory.h:20: // creating,
notifying, retrieveing and deleting on demand.
*retrieving

[vabr (Chromium), 2017-02-27 16:45:46]

Thanks, Mathieu!

https://codereview.chromium.org/2603623002/diff/120001/components/autofill/co...
File components/autofill/core/browser/autofill_driver_factory.h (right):

https://codereview.chromium.org/2603623002/diff/120001/components/autofill/co...
components/autofill/core/browser/autofill_driver_factory.h:20: // creating,
notifying, retrieveing and deleting on demand.
On 2017/02/27 15:46:29, Mathieu Perreault wrote:
> *retrieving

Done.

[vabr (Chromium), 2017-02-27 16:45:52]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2017-02-27 16:45:52]

The patchset sent to the CQ was uploaded after l-g-t-m from dvadym@chromium.org,
mathp@chromium.org
Link to the patchset: https://codereview.chromium.org/2603623002/#ps140001
(title: "Fix typo")

[commit-bot: I haz the power, 2017-02-27 16:46:22]

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-02-27 17:41:52]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488213952096730,
"parent_rev": "8544d8e9988ef733601b5675d6f7dbc3a27454b9", "commit_rev":
"ce7eba36c420dabb86b57f1851d5c86115d7d13c"}

[commit-bot: I haz the power, 2017-02-27 17:42:32]

Description was changed from

==========
AutofillDriverFactory manages information about user gestures

When a password field is autofilled, the value is not accessible to JavaScript
until the user clicks or types in the page. To keep track of this, the frames
report such user gestures, and ChromeAutofillClient distributes such a signal
to all other frames in the same WebContent, via their common
AutofillDriverFactory.

This does not work when a new frame is added later, because the browser process
does not keep track whether the user gestures were seen already. Newly added
frames never learn about the user already having clicked in the page.

This CL fixes that by modifying AutofillDriverFactory to remember the
user gestures and notify also drivers added later. The flag is reset on main
page navigation.

The CL also makes parts of the AutofillDriverFactory API public, as long as they
do not modify the drivers map. This results in a slight reordering of methods.

BUG=669045
==========

to

==========
AutofillDriverFactory manages information about user gestures

When a password field is autofilled, the value is not accessible to JavaScript
until the user clicks or types in the page. To keep track of this, the frames
report such user gestures, and ChromeAutofillClient distributes such a signal
to all other frames in the same WebContent, via their common
AutofillDriverFactory.

This does not work when a new frame is added later, because the browser process
does not keep track whether the user gestures were seen already. Newly added
frames never learn about the user already having clicked in the page.

This CL fixes that by modifying AutofillDriverFactory to remember the
user gestures and notify also drivers added later. The flag is reset on main
page navigation.

The CL also makes parts of the AutofillDriverFactory API public, as long as they
do not modify the drivers map. This results in a slight reordering of methods.

BUG=669045

Review-Url: https://codereview.chromium.org/2603623002
Cr-Commit-Position: refs/heads/master@{#453245}
Committed:
https://chromium.googlesource.com/chromium/src/+/ce7eba36c420dabb86b57f1851d5...
==========

[commit-bot: I haz the power, 2017-02-27 17:42:33]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ce7eba36c420dabb86b57f1851d5...