Add support for setting password value gated on user's gesture in a page

Add support for setting password value gated on user's gesture in a page

WebViewImpl will now be able to notify WebAutofillClient when a user gesture is handled for the first time after a load.
This has security motivation, detailed here:
https://docs.google.com/document/d/1_Es0qQVrKZSZoXuQBuPmNW8-EAUDsuQGSMIlRFPzs_0/edit?usp=sharing

This improves the current status, when a workaround involving checking for a user gesture outside of Blink is used. That workaround does not work for Android, and can be seen as a layering violation in the sense, that checking for user gesture is a Blink internal thing.

There was one technical challenge to tackle in this approach: the need to know in WebViewImpl, whether a event satisfies isUserGestureEventType, because such events can get created higher up the stack, in EventRouter. For example, the GestureTap event does not satisfy isUserGestureEventType, but causes a mouse click event to be created, which satisfies isUserGestureEventType. That event is never seen by WebViewImpl. That's why this CL also needs to add a check for UserGestureIndicator being created during the handleEvent call.

This is the Blink part, two more CLs are planned as follow-ups:
1) Chromium part: PasswordAutofillAgent receiving the newly added notification + clean-up (basically getting rid of content/ changes from https://codereview.chromium.org/163843002)
2) Blink clean-up: making WebInputEvent::isUserGestureEventType Blink-internal

BUG=163072
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171759

[vabr (Chromium), 2014-04-15 17:09:37]

Jochen FYI.

Hi Kent,

This is my attempt at your suggestion in
https://codereview.chromium.org/225853005/#msg4.

My biggest issue was the need to know in WebViewImpl, whether a event satisfies
isUserGestureEventType, because such events can get created higher up the stack,
in EventRouter. For example, the GestureTap event does not satisfy
isUserGestureEventType, but causes a mouse click event to be created, which
satisfies isUserGestureEventType. That event is never seen by WebViewImpl.
That's why I needed to check for UserGestureIndicator being created during the
handleEvent call.

An alternative would just be to have the isUserGestureEventType check in
WebViewImpl, and manually add also the gesture events (those suggested in the
discussion starting on http://crbug.com/345510#c5). It would be shorter, but
would create hidden dependencies in the code.

What do you think?

Cheers,
Vaclav

[tkent, 2014-04-16 02:09:04]

lgtm.

On 2014/04/15 17:09:37, vabr (Chromium) wrote:
> My biggest issue was the need to know in WebViewImpl, whether a event
satisfies
> isUserGestureEventType, because such events can get created higher up the
stack,
> in EventRouter. For example, the GestureTap event does not satisfy
> isUserGestureEventType, but causes a mouse click event to be created, which
> satisfies isUserGestureEventType. That event is never seen by WebViewImpl.
> That's why I needed to check for UserGestureIndicator being created during the
> handleEvent call.

This paragraph is very helpful to understand the CL.  The description should
have it.

[tkent, 2014-04-16 02:09:11]

https://codereview.chromium.org/235983016/diff/20001/Source/web/tests/WebView...
File Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/235983016/diff/20001/Source/web/tests/WebView...
Source/web/tests/WebViewTest.cpp:1148: virtual void firstUserGestureObserved() {
++m_userGestureNotificationsCount; }
nit: add OVERRIDE

[vabr (Chromium), 2014-04-16 07:21:59]

Thanks, Kent!

I updated the CL description with the explanation about checking for user
gesture events, and addressed your comment in the code, so I'll send it to the
CQ now.

Cheers,
Vaclav

https://codereview.chromium.org/235983016/diff/20001/Source/web/tests/WebView...
File Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/235983016/diff/20001/Source/web/tests/WebView...
Source/web/tests/WebViewTest.cpp:1148: virtual void firstUserGestureObserved() {
++m_userGestureNotificationsCount; }
On 2014/04/16 02:09:12, tkent wrote:
> nit: add OVERRIDE

Done.

[vabr (Chromium), 2014-04-16 07:22:14]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-04-16 07:22:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/235983016/30001

[commit-bot: I haz the power, 2014-04-16 08:37:19]

Change committed as 171759

[jochen (gone - plz use gerrit), 2014-04-16 14:05:15]

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
File Source/platform/UserGestureIndicator.cpp (right):

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
Source/platform/UserGestureIndicator.cpp:134: s_processedUserGestureInPast =
true;
i think you only want to do this when we actually invoke addGesture()

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
Source/platform/UserGestureIndicator.cpp:204: s_processedUserGestureInPast =
true;
false?

[vabr (Chromium), 2014-04-16 16:24:16]

Thanks, Jochen!

Created a new CL to address your comments:
https://codereview.chromium.org/239963005/

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
File Source/platform/UserGestureIndicator.cpp (right):

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
Source/platform/UserGestureIndicator.cpp:134: s_processedUserGestureInPast =
true;
On 2014/04/16 14:05:16, jochen wrote:
> i think you only want to do this when we actually invoke addGesture()

Done.

https://codereview.chromium.org/235983016/diff/30001/Source/platform/UserGest...
Source/platform/UserGestureIndicator.cpp:204: s_processedUserGestureInPast =
true;
On 2014/04/16 14:05:16, jochen wrote:
> false?

True! :)
(Thanks for catching this!)