Input event filtering and compositor thread setup.

Input event filtering and compositor thread setup.

There are two new classes: InputEventFilter and CompositorThread.
The first sets up a MessageFilter designed to route input events
to the compositor thread.  CompositorThread sets up the compositor
thread and it has a InputEventFilter.

When we pass an event to the CompositorThread, we are actually
passing it to a WebCompositor, which can respond asynchronously
via WebCompositorClient to tell us whether it handled the event or
wants us to punt it up to the WebWidget.  It can also tell us that
it did not handle the event and that we should not bother sending
it to the WebWidget.

InputEventFilter contains all of the interesting thread marshalling
code.  CompositorThread has the WebCompositor hookup.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104258

[darin (slow to review), 2011-10-05 21:04:34]

John, it'd be great if you could review this patch.  There are two new classes: 
InputEventFilter and CompositorThread.  The first sets up a MessageFilter
designed to route input events to the compositor thread.  CompositorThread sets
up the compositor thread and it has a InputEventFilter.

When we pass an event to the CompositorThread, we are actually passing it to a
WebCompositor, which can respond asynchronously via WebCompositorClient to tell
us whether it handled the event or wants us to punt it up to the WebWidget.  It
can also tell us that it did not handle the event and that we should not bother
sending it to the WebWidget.

InputEventFilter contains all of the interesting thread marshalling code. 
CompositorThread has the WebCompositor hookup.

There is a unit test for InputEventFilter.

[jamesr, 2011-10-05 21:16:43]

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/input_...
File content/renderer/gpu/input_event_filter.h (right):

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/input_...
content/renderer/gpu/input_event_filter.h:88: std::set<int> routes_;
on linux i had to add #include <set> to this file to make it compile. maybe this
is picked up implicitly on windows somehow?

[jamesr, 2011-10-05 21:34:42]

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
File content/renderer/gpu/compositor_thread.cc (right):

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
content/renderer/gpu/compositor_thread.cc:28: ~CompositorWrapper() {
one more nit: i think this should be marked virtual (it is actually virtual
anyway because the d'tor of WebKit::WebCompositorClient is declared virtual, but
good to be explicit about it)

[jam, 2011-10-05 21:41:49]

lgtm

it would be helpful to move more of the message you wrote into the cl
description so that it shows up in the svn log

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
File content/renderer/gpu/compositor_thread.cc (right):

http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
content/renderer/gpu/compositor_thread.cc:110: DCHECK_EQ(routing_id,
wrapper->routing_id());
nit: do you think this dcheck will ever trip up? are you that worried about
developers messing up two functions above?

http://codereview.chromium.org/8089002/diff/36001/content/renderer/render_thr...
File content/renderer/render_thread.cc (right):

http://codereview.chromium.org/8089002/diff/36001/content/renderer/render_thr...
content/renderer/render_thread.cc:449:
AddFilter(compositor_thread_->GetMessageFilter());
nit: I'm curious why you chose to make RenderThread know that  CompositorThread
has a filter, instead of having CT add/remove its filter when it's
created/destructed?

[darin (slow to review), 2011-10-05 22:13:57]

On 2011/10/05 21:34:42, jamesr wrote:
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
> File content/renderer/gpu/compositor_thread.cc (right):
> 
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
> content/renderer/gpu/compositor_thread.cc:28: ~CompositorWrapper() {
> one more nit: i think this should be marked virtual (it is actually virtual
> anyway because the d'tor of WebKit::WebCompositorClient is declared virtual,
but
> good to be explicit about it)

Done.

[darin (slow to review), 2011-10-05 22:14:08]

On 2011/10/05 21:16:43, jamesr wrote:
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/input_...
> File content/renderer/gpu/input_event_filter.h (right):
> 
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/input_...
> content/renderer/gpu/input_event_filter.h:88: std::set<int> routes_;
> on linux i had to add #include <set> to this file to make it compile. maybe
this
> is picked up implicitly on windows somehow?

Done.

[darin (slow to review), 2011-10-05 22:23:38]

On 2011/10/05 21:41:49, John Abd-El-Malek wrote:
> lgtm
> 
> it would be helpful to move more of the message you wrote into the cl
> description so that it shows up in the svn log

Will do.

> 
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
> File content/renderer/gpu/compositor_thread.cc (right):
> 
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/gpu/compos...
> content/renderer/gpu/compositor_thread.cc:110: DCHECK_EQ(routing_id,
> wrapper->routing_id());
> nit: do you think this dcheck will ever trip up? are you that worried about
> developers messing up two functions above?

Yeah, that's gratuitous.  I'll remove it.


http://codereview.chromium.org/8089002/diff/36001/content/renderer/render_thr...
> File content/renderer/render_thread.cc (right):
> 
>
http://codereview.chromium.org/8089002/diff/36001/content/renderer/render_thr...
> content/renderer/render_thread.cc:449:
> AddFilter(compositor_thread_->GetMessageFilter());
> nit: I'm curious why you chose to make RenderThread know that 
CompositorThread
> has a filter, instead of having CT add/remove its filter when it's
> created/destructed?

I didn't want the CompositorThread to depend directly on RenderThread, which
would be necessary if it had to call AddFilter itself.

[darin (slow to review), 2011-10-05 22:25:16]

On 2011/10/05 22:23:38, darin wrote:
> I didn't want the CompositorThread to depend directly on RenderThread, which
> would be necessary if it had to call AddFilter itself.

^^^ Reason:  I was thinking about creating a CompositorThread unit test.  I
can't quite do that yet, because I need a way to mock out
WebCompositor::fromIdentifer.