Generalize the main thread event queue into a common event queue.

Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012, 599152
Committed: https://crrev.com/cb9341c9f5539577a82aae52e19bd789148001bc
Cr-Commit-Position: refs/heads/master@{#407826}

[dtapuska, 2016-07-21 16:31:14]

Description was changed from

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152
==========

to

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152
==========

[dtapuska, 2016-07-21 16:31:15]

dtapuska@chromium.org changed reviewers:
+ tdresser@chromium.org

[dtapuska, 2016-07-21 16:31:29]

How is this?

[dtapuska, 2016-07-21 16:31:49]

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

[commit-bot: I haz the power, 2016-07-21 16:32:10]

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-07-21 18:09:44]

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

[commit-bot: I haz the power, 2016-07-21 18:09:45]

Dry run: This issue passed the CQ dry run.

[tdresser, 2016-07-22 19:13:42]

I wouldn't mind taking another pass once these nits have been fixed, but LGTM if
you're in a rush.

https://codereview.chromium.org/2170913002/diff/1/content/common/input/event_...
File content/common/input/event_with_latency_info.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/common/input/event_...
content/common/input/event_with_latency_info.h:41: const ui::LatencyInfo& l);
e and l -> event and latency_info

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
File content/common/input/web_input_event_queue.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
content/common/input/web_input_event_queue.h:23: void Queue(std::unique_ptr<T>&&
event) {
We're supposed to avoid passing rvalue references, if I recall correctly.

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
content/common/input/web_input_event_queue.h:25: last_event_iter !=
queue_.rend(); ++last_event_iter) {
Range based for?

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
File content/renderer/input/main_thread_event_queue.cc (right):

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.cc:64: bool isTouch =
blink::WebInputEvent::isTouchEventType(event->type);
is_touch

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
File content/renderer/input/main_thread_event_queue.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.h:9: #include
"base/memory/ref_counted.h"
Not used (yet).

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.h:36: std::deque<uint32_t>
eventsToAck_;
Indicate that this is touch specific.

[dtapuska, 2016-07-22 20:14:08]

https://codereview.chromium.org/2170913002/diff/1/content/common/input/event_...
File content/common/input/event_with_latency_info.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/common/input/event_...
content/common/input/event_with_latency_info.h:41: const ui::LatencyInfo& l);
On 2016/07/22 19:13:41, tdresser wrote:
> e and l -> event and latency_info

Done.

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
File content/common/input/web_input_event_queue.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
content/common/input/web_input_event_queue.h:23: void Queue(std::unique_ptr<T>&&
event) {
On 2016/07/22 19:13:41, tdresser wrote:
> We're supposed to avoid passing rvalue references, if I recall correctly.

Done.

https://codereview.chromium.org/2170913002/diff/1/content/common/input/web_in...
content/common/input/web_input_event_queue.h:25: last_event_iter !=
queue_.rend(); ++last_event_iter) {
On 2016/07/22 19:13:41, tdresser wrote:
> Range based for?

This is a reverse iterator; reverse range iterators don't exist unless you
construct an intermediate until C++14 I believe.

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
File content/renderer/input/main_thread_event_queue.cc (right):

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.cc:64: bool isTouch =
blink::WebInputEvent::isTouchEventType(event->type);
On 2016/07/22 19:13:41, tdresser wrote:
> is_touch

Done.

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
File content/renderer/input/main_thread_event_queue.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.h:9: #include
"base/memory/ref_counted.h"
On 2016/07/22 19:13:41, tdresser wrote:
> Not used (yet).

Done.

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.h:36: std::deque<uint32_t>
eventsToAck_;
On 2016/07/22 19:13:41, tdresser wrote:
> Indicate that this is touch specific.

I think that is a specialization just in the instance of this right now (and is
really hidden because it only works on touch and mouse wheel) and won't
necessarily be true for mouse moves that get coalesced.
I'd expect eventsToAck_ to have a list of a series of 0's for a bunch of mouse
moves. Ideally we could just adjust the IPC and really return a list here.

But I think we could change all of this around with historical points and pass
the list of coalesced events at the event time.

[tdresser, 2016-07-25 14:01:41]

LGTM, thanks.

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
File content/renderer/input/main_thread_event_queue.h (right):

https://codereview.chromium.org/2170913002/diff/1/content/renderer/input/main...
content/renderer/input/main_thread_event_queue.h:36: std::deque<uint32_t>
eventsToAck_;
On 2016/07/22 20:14:08, dtapuska wrote:
> On 2016/07/22 19:13:41, tdresser wrote:
> > Indicate that this is touch specific.
> 
> I think that is a specialization just in the instance of this right now (and
is
> really hidden because it only works on touch and mouse wheel) and won't
> necessarily be true for mouse moves that get coalesced.
> I'd expect eventsToAck_ to have a list of a series of 0's for a bunch of mouse
> moves. Ideally we could just adjust the IPC and really return a list here.
> 
> But I think we could change all of this around with historical points and pass
> the list of coalesced events at the event time.
> 

I guess that's reasonable. Maybe add a comment indicating how this is populated
for different input modalities?

https://codereview.chromium.org/2170913002/diff/40001/content/renderer/input/...
File content/renderer/input/main_thread_event_queue.cc (right):

https://codereview.chromium.org/2170913002/diff/40001/content/renderer/input/...
content/renderer/input/main_thread_event_queue.cc:64: bool is_touch =
blink::WebInputEvent::isTouchEventType(event->type);
Can we have a bool for is_wheel, just to make this read a bit more
symmetrically?

[dtapuska, 2016-07-25 20:05:23]

dtapuska@chromium.org changed reviewers:
+ rbyers@chromium.org

[dtapuska, 2016-07-25 20:05:24]

Rick can you check out WebInputEvent.h?

https://codereview.chromium.org/2170913002/diff/40001/content/renderer/input/...
File content/renderer/input/main_thread_event_queue.cc (right):

https://codereview.chromium.org/2170913002/diff/40001/content/renderer/input/...
content/renderer/input/main_thread_event_queue.cc:64: bool is_touch =
blink::WebInputEvent::isTouchEventType(event->type);
On 2016/07/25 14:01:40, tdresser wrote:
> Can we have a bool for is_wheel, just to make this read a bit more
> symmetrically? 

Done.

[Rick Byers, 2016-07-26 04:37:12]

public/ LGTM

[dtapuska, 2016-07-26 13:25:23]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-07-26 13:25:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2170913002/#ps60001
(title: "Fix nits")

[commit-bot: I haz the power, 2016-07-26 13:25:37]

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-07-26 16:28:07]

Description was changed from

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152
==========

to

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152
==========

[commit-bot: I haz the power, 2016-07-26 16:28:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-26 16:30:25]

Description was changed from

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152
==========

to

==========
Generalize the main thread event queue into a common event queue.

Teach the WebInputEventQueue about ScopedWebInputEvents so we can
use a single queue instead of one per event class.

This reduces code duplication and allows for another patch to then take
advantage of this for all events.

BUG=624012,599152

Committed: https://crrev.com/cb9341c9f5539577a82aae52e19bd789148001bc
Cr-Commit-Position: refs/heads/master@{#407826}
==========

[commit-bot: I haz the power, 2016-07-26 16:30:26]

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