Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(384)

Issue 19624005: Add InputEvent and EventPacket types for batched input delivery (Closed)

Created:
7 years, 5 months ago by jdduke (slow)
Modified:
7 years, 3 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Add InputEvent and EventPacket types for batched input delivery With the BufferedInputRouter, browser input events will be batched into packets before being sent to the renderer. Add a common InputEvent type that provides id'ed carriage of either WebInputEvents or general IPC input messages. Also add a composite EventPacket type for batched InputEvent transport and dispatch. BUG=245499 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221905

Patch Set 1 #

Patch Set 2 : Similarity fix #

Patch Set 3 : Rebase #

Patch Set 4 : Rebase #

Total comments: 2

Patch Set 5 : Separate InputEvent types for events based on WebKit::WebInputEvent #

Total comments: 6

Patch Set 6 : Comments and cleanup. #

Patch Set 7 : Out-of-line destructors ftw. #

Patch Set 8 : Cleanup #

Patch Set 9 : Cleanup #

Patch Set 10 : Cleanup #

Total comments: 5

Patch Set 11 : InputEvent with payload #

Total comments: 4

Patch Set 12 : Fix comments #

Patch Set 13 : Code review #

Total comments: 20

Patch Set 14 : Code review++ #

Patch Set 15 : Include recent cleanup #

Total comments: 6

Patch Set 16 : Code review and rebase #

Patch Set 17 : Fix tests #

Patch Set 18 : Clang format #

Patch Set 19 : Win export fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1012 lines, -3 lines) Patch
A + content/common/input/OWNERS View 5 1 chunk +1 line, -1 line 0 comments Download
A content/common/input/event_packet.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +49 lines, -0 lines 0 comments Download
A content/common/input/event_packet.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +22 lines, -0 lines 0 comments Download
A content/common/input/input_event.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +60 lines, -0 lines 0 comments Download
A content/common/input/input_event.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +28 lines, -0 lines 0 comments Download
A content/common/input/input_event_disposition.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +41 lines, -0 lines 0 comments Download
A content/common/input/input_event_disposition.cc View 1 2 3 4 5 1 chunk +67 lines, -0 lines 0 comments Download
A content/common/input/input_param_traits.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +81 lines, -0 lines 0 comments Download
A content/common/input/input_param_traits.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +174 lines, -0 lines 0 comments Download
A content/common/input/input_param_traits_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +211 lines, -0 lines 0 comments Download
A content/common/input/ipc_input_event_payload.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +40 lines, -0 lines 0 comments Download
A content/common/input/ipc_input_event_payload.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +35 lines, -0 lines 0 comments Download
A content/common/input/web_input_event_payload.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +67 lines, -0 lines 0 comments Download
A content/common/input/web_input_event_payload.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +98 lines, -0 lines 0 comments Download
M content/common/input_messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +24 lines, -1 line 0 comments Download
M content/content_common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +12 lines, -0 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +1 line, -1 line 0 comments Download
M content/port/common/input_event_ack_state.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 30 (0 generated)
jdduke (slow)
PTAL. This is the first patch pulled from the megapatch at https://codereview.chromium.org/19220002/. Both browser and ...
7 years, 5 months ago (2013-07-24 16:14:45 UTC) #1
nduca
Cray idea: break up into add input event patch and then a packet one. The ...
7 years, 4 months ago (2013-08-12 23:31:58 UTC) #2
nduca
https://codereview.chromium.org/19624005/diff/10001/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/10001/content/common/input/input_event.h#newcode28 content/common/input/input_event.h:28: IPC::Message message; also why's it pickled? I would have ...
7 years, 4 months ago (2013-08-12 23:32:40 UTC) #3
jdduke (slow)
https://codereview.chromium.org/19624005/diff/10001/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/10001/content/common/input/input_event.h#newcode28 content/common/input/input_event.h:28: IPC::Message message; On 2013/08/12 23:32:40, nduca wrote: > also ...
7 years, 4 months ago (2013-08-12 23:45:27 UTC) #4
jdduke (slow)
"insteading", while a typo, sounds like it would make a great addition to the English ...
7 years, 4 months ago (2013-08-12 23:48:15 UTC) #5
jdduke (slow)
PTAL. InputEvent is no longer a glorified container for an IPC::Message. This should make it ...
7 years, 4 months ago (2013-08-14 20:24:23 UTC) #6
nduca
https://codereview.chromium.org/19624005/diff/18001/content/common/input/generic_event.h File content/common/input/generic_event.h (right): https://codereview.chromium.org/19624005/diff/18001/content/common/input/generic_event.h#newcode16 content/common/input/generic_event.h:16: class CONTENT_EXPORT GenericEvent : public InputEvent { GenericInputEvent since ...
7 years, 4 months ago (2013-08-15 01:22:46 UTC) #7
jdduke (slow)
https://codereview.chromium.org/19624005/diff/18001/content/common/input/generic_event.h File content/common/input/generic_event.h (right): https://codereview.chromium.org/19624005/diff/18001/content/common/input/generic_event.h#newcode16 content/common/input/generic_event.h:16: class CONTENT_EXPORT GenericEvent : public InputEvent { On 2013/08/15 ...
7 years, 4 months ago (2013-08-15 15:23:44 UTC) #8
jdduke (slow)
PTAL. Bringing in aelias@ as he's a bit more familiar with the BufferedInputRouter code, and ...
7 years, 4 months ago (2013-08-16 00:12:20 UTC) #9
nduca
https://codereview.chromium.org/19624005/diff/39001/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/39001/content/common/input/input_event.h#newcode18 content/common/input/input_event.h:18: GENERIC, In your explanation to me of this, you ...
7 years, 3 months ago (2013-08-27 18:45:35 UTC) #10
nduca
stale comments https://codereview.chromium.org/19624005/diff/39001/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/39001/content/common/input/input_event.h#newcode23 content/common/input/input_event.h:23: enum AckType { DispositionRequirement https://codereview.chromium.org/19624005/diff/39001/content/common/input/input_event.h#newcode26 content/common/input/input_event.h:26: ACK_CAN_CREATE_FOLLOWUP_EVENTS ...
7 years, 3 months ago (2013-08-27 22:16:07 UTC) #11
jdduke (slow)
nduca@, please see patch set #10 delta from crrev.com/19220002 for relevant changes to the core ...
7 years, 3 months ago (2013-08-30 22:34:18 UTC) #12
nduca
lgtm thank you
7 years, 3 months ago (2013-09-03 21:48:57 UTC) #13
jdduke (slow)
palmer@: content/common/input_messages.h for security? Thanks.
7 years, 3 months ago (2013-09-03 22:28:51 UTC) #14
aelias_OOO_until_Jul13
lgtm with nits https://codereview.chromium.org/19624005/diff/49001/content/common/input/input_param_traits.cc File content/common/input/input_param_traits.cc (right): https://codereview.chromium.org/19624005/diff/49001/content/common/input/input_param_traits.cc#newcode91 content/common/input/input_param_traits.cc:91: l->append("))"); Nit: unbalanced parens https://codereview.chromium.org/19624005/diff/49001/content/common/input/input_param_traits_unittest.cc File ...
7 years, 3 months ago (2013-09-03 22:31:09 UTC) #15
jdduke (slow)
jamesr@, this is the first patch to land for the BufferedInputRouter, for which the tracking ...
7 years, 3 months ago (2013-09-03 22:52:14 UTC) #16
jamesr
Is there a higher-level description or design doc explaining these types and how they interact?
7 years, 3 months ago (2013-09-03 23:03:22 UTC) #17
palmer
https://codereview.chromium.org/19624005/diff/50018/content/common/input/event_packet.h File content/common/input/event_packet.h (right): https://codereview.chromium.org/19624005/diff/50018/content/common/input/event_packet.h#newcode25 content/common/input/event_packet.h:25: // |event| should be non-NULL and valid. Should, or ...
7 years, 3 months ago (2013-09-03 23:12:47 UTC) #18
jdduke (slow)
On 2013/09/03 23:03:22, jamesr wrote: > Is there a higher-level description or design doc explaining ...
7 years, 3 months ago (2013-09-03 23:19:38 UTC) #19
jdduke (slow)
https://codereview.chromium.org/19624005/diff/50018/content/common/input/event_packet.h File content/common/input/event_packet.h (right): https://codereview.chromium.org/19624005/diff/50018/content/common/input/event_packet.h#newcode30 content/common/input/event_packet.h:30: InputEvents::const_iterator begin() const { return events.begin(); } On 2013/09/03 ...
7 years, 3 months ago (2013-09-04 17:53:24 UTC) #20
jdduke (slow)
https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_param_traits.h File content/common/input/input_param_traits.h (right): https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_param_traits.h#newcode39 content/common/input/input_param_traits.h:39: param_type temp(new P()); On 2013/09/04 17:53:24, jdduke wrote: > ...
7 years, 3 months ago (2013-09-05 00:01:21 UTC) #21
palmer
https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_event.h#newcode28 content/common/input/input_event.h:28: // |id| should be non-zero and |payload| should be ...
7 years, 3 months ago (2013-09-05 01:20:15 UTC) #22
palmer
LGTM with some questions. https://codereview.chromium.org/19624005/diff/106001/content/common/input/input_param_traits.cc File content/common/input/input_param_traits.cc (right): https://codereview.chromium.org/19624005/diff/106001/content/common/input/input_param_traits.cc#newcode103 content/common/input/input_param_traits.cc:103: break; Does it make sense ...
7 years, 3 months ago (2013-09-05 18:56:29 UTC) #23
jdduke (slow)
Awesome, thanks for review palmer@. https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_event.h File content/common/input/input_event.h (right): https://codereview.chromium.org/19624005/diff/50018/content/common/input/input_event.h#newcode28 content/common/input/input_event.h:28: // |id| should be ...
7 years, 3 months ago (2013-09-05 19:31:48 UTC) #24
jdduke (slow)
jam@: PTAL for owners, particularly content/*.gypi and content/port. Thanks!
7 years, 3 months ago (2013-09-06 18:29:01 UTC) #25
jam
On 2013/09/06 18:29:01, jdduke wrote: > jam@: PTAL for owners, particularly content/*.gypi and content/port. Thanks! ...
7 years, 3 months ago (2013-09-06 21:22:34 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/19624005/120001
7 years, 3 months ago (2013-09-06 21:47:24 UTC) #27
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 3 months ago (2013-09-06 23:29:31 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jdduke@chromium.org/19624005/140001
7 years, 3 months ago (2013-09-06 23:38:52 UTC) #29
commit-bot: I haz the power
7 years, 3 months ago (2013-09-07 07:07:54 UTC) #30
Message was sent while issue was closed.
Change committed as 221905

Powered by Google App Engine
This is Rietveld 408576698