Mouse event handlers use user gesture tokens on top-level LocalFrames.

Mouse event handlers use user gesture tokens on top-level LocalFrames.

The last mouseDown and mouseUp user gesture tokens are set on the top
level frame's EventHandler. For the FrameTree to be able to contain
RemoteFrames, we need to account for instances where the current
frame has RemoteFrame ancestors. This change causes the tokens to be
used on the EventHandler attached to the frame that is the root of the
local subtree.

This will not make gestures work with RemoteFrames, as that
will need a much larger refactoring. Eventually gesture
tracking will need to be synchronized across processes.

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

[jochen (gone - plz use gerrit), 2014-04-23 08:11:19]

we also get the top frame's token in WebViewImpl, does that need updating too?

Can you share a plan how to sync the user gesture state across processes?

[kenrb, 2014-04-23 12:45:08]

On 2014/04/23 08:11:19, jochen wrote:
> we also get the top frame's token in WebViewImpl, does that need updating too?
> 
> Can you share a plan how to sync the user gesture state across processes?

It looks like the token in WebViewImpl is retrieved from the mainFrame member,
which should already be the root of the local frame tree.

Gesture handling across processes isn't fully worked out yet. We have high level
designs for cross-process focus management and hit testing, but for gestures I
have only had conversations with rjkroege and rbyers about it. You are welcome
to help us on those details!

[jochen (gone - plz use gerrit), 2014-04-23 12:51:50]

essentially, there most be only one gesture token on the entire page.

We can't store one mouse down token in one event handler, and another one for
the same gesture in another handler.

I could imagine we have some way to hand the tokens between processes, or
something, but I think the current change makes it worse, because it makes it
look like it was ok to have a token per local frame tree.

[kenrb, 2014-04-23 13:35:55]

On 2014/04/23 12:51:50, jochen wrote:
> essentially, there most be only one gesture token on the entire page.
> 
> We can't store one mouse down token in one event handler, and another one for
> the same gesture in another handler.
> 
> I could imagine we have some way to hand the tokens between processes, or
> something, but I think the current change makes it worse, because it makes it
> look like it was ok to have a token per local frame tree.

I was expecting that to be worked out later when we do the necessary refactoring
for gestures to work with out-of-process iframes.

More immediately, though, we need to be able to have RemoteFrames in the frame
tree for us to even be able to instantiate them and use them at all. In order to
do that, code that accesses frames in the frame tree and expects them to be
local needs to cover the possibility of encountering a RemoteFrame, even if the
behavior isn't currently correct (there are a lot of features that will be
initially broken). The thinking is that once RemoteFrames are basically usable
then we can start working through all the broken features.

Can you see a way to do this with less churn than the current change? I would be
concerned that trying to communicate tokens across processes might result in a
lot more temporary code, given that I don't know what the final code will look
like.

[jochen (gone - plz use gerrit), 2014-04-23 15:20:07]

ok

Shouldn't localFrameRoot be a method on Frame?

Can you update the CL description to state that user gesture tracking is broken
if there are remote frames in the frame tree?

[kenrb, 2014-04-24 14:07:04]

On 2014/04/23 15:20:07, jochen wrote:
> ok
> 
> Shouldn't localFrameRoot be a method on Frame?
> 

LocalFrame is more appropriate, but done.

> Can you update the CL description to state that user gesture tracking is
broken
> if there are remote frames in the frame tree?

Done.

[jochen (gone - plz use gerrit), 2014-04-24 19:05:35]

lgtm

[kenrb, 2014-04-25 13:05:37]

The CQ bit was checked by kenrb@chromium.org

[commit-bot: I haz the power, 2014-04-25 13:06:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kenrb@chromium.org/248083003/20001

[commit-bot: I haz the power, 2014-04-25 14:21:25]

Change committed as 172633