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

Issue 1190013002: Pass MotionEvent tool type to Blink. (Closed)

Created:
5 years, 6 months ago by USE eero AT chromium.org
Modified:
5 years, 4 months ago
Reviewers:
jdduke (slow), sadrul
CC:
chromium-reviews, jdduke+watch_chromium.org, tdresser+watch_chromium.org, Rick Byers, mustaq
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Pass MotionEvent tool type to Blink. This maps MotionEvent tool type to PointerEvent pointerType and passes that to WebTouchPoints. This CL is a part of a patch series: 1. https://codereview.chromium.org/1253183005/ for new WebPointerProperties fields 2. https://codereview.chromium.org/1260693003/ for eventSender web pointer property support 3. https://codereview.chromium.org/1192463008/ for handling pointer type in Blink event handlers 4. https://codereview.chromium.org/1190013002/ (this) BUG=514360 Committed: https://crrev.com/3c4c729b54ab44ef38c2b3bddeb19dc5e2c0de7e Cr-Commit-Position: refs/heads/master@{#342992}

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : MotionEventWeb #

Total comments: 2

Patch Set 4 : Pass pointer_index #

Total comments: 3

Patch Set 5 : Really pass pointer_index #

Total comments: 2

Patch Set 6 : NOTREACHED message #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -2 lines) Patch
M content/browser/renderer_host/input/motion_event_web.cc View 1 2 2 chunks +17 lines, -2 lines 0 comments Download
M ui/events/blink/blink_event_util.cc View 1 2 3 4 5 1 chunk +20 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (7 generated)
USE eero AT chromium.org
PTAL
5 years, 6 months ago (2015-06-17 14:03:52 UTC) #2
USE eero AT chromium.org
PTAL. Blink side CLs landed (and have finally rolled into DEPS).
5 years, 4 months ago (2015-08-07 17:35:12 UTC) #5
jdduke (slow)
https://codereview.chromium.org/1190013002/diff/40001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/40001/ui/events/blink/blink_event_util.cc#newcode76 ui/events/blink/blink_event_util.cc:76: switch (event.GetToolType()) { Don't we need a pointer_index here?
5 years, 4 months ago (2015-08-07 18:06:34 UTC) #6
USE eero AT chromium.org
https://codereview.chromium.org/1190013002/diff/40001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/40001/ui/events/blink/blink_event_util.cc#newcode76 ui/events/blink/blink_event_util.cc:76: switch (event.GetToolType()) { On 2015/08/07 18:06:34, jdduke wrote: > ...
5 years, 4 months ago (2015-08-10 12:28:58 UTC) #7
jdduke (slow)
lgtm. It'd be nice if we had some test coverage here but it appears we ...
5 years, 4 months ago (2015-08-10 20:54:59 UTC) #8
jdduke (slow)
Sigh, I guess a test might have caught this? https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc#newcode76 ui/events/blink/blink_event_util.cc:76: ...
5 years, 4 months ago (2015-08-10 20:55:41 UTC) #9
USE eero AT chromium.org
https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc#newcode76 ui/events/blink/blink_event_util.cc:76: switch (event.GetToolType()) { On 2015/08/10 20:55:41, jdduke wrote: > ...
5 years, 4 months ago (2015-08-10 22:43:37 UTC) #10
USE eero AT chromium.org
https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc#newcode76 ui/events/blink/blink_event_util.cc:76: switch (event.GetToolType()) { On 2015/08/10 22:43:37, e_hakkinen wrote: > ...
5 years, 4 months ago (2015-08-11 14:37:08 UTC) #11
jdduke (slow)
On 2015/08/11 14:37:08, e_hakkinen wrote: > https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc > File ui/events/blink/blink_event_util.cc (right): > > https://codereview.chromium.org/1190013002/diff/60001/ui/events/blink/blink_event_util.cc#newcode76 > ...
5 years, 4 months ago (2015-08-11 14:57:38 UTC) #12
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1190013002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1190013002/80001
5 years, 4 months ago (2015-08-11 15:20:14 UTC) #14
USE eero AT chromium.org
@sadrul: I think that I need your approval for ui/events/blink/blink_event_util.cc.
5 years, 4 months ago (2015-08-11 15:40:04 UTC) #15
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-11 17:33:16 UTC) #17
sadrul
lgtm https://codereview.chromium.org/1190013002/diff/80001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/80001/ui/events/blink/blink_event_util.cc#newcode88 ui/events/blink/blink_event_util.cc:88: NOTREACHED(); NOTREACHED() << "Invalid MotionEvent::ToolType = " << ...
5 years, 4 months ago (2015-08-11 19:33:35 UTC) #18
USE eero AT chromium.org
https://codereview.chromium.org/1190013002/diff/80001/ui/events/blink/blink_event_util.cc File ui/events/blink/blink_event_util.cc (right): https://codereview.chromium.org/1190013002/diff/80001/ui/events/blink/blink_event_util.cc#newcode88 ui/events/blink/blink_event_util.cc:88: NOTREACHED(); On 2015/08/11 19:33:34, sadrul wrote: > NOTREACHED() << ...
5 years, 4 months ago (2015-08-12 07:50:11 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1190013002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1190013002/100001
5 years, 4 months ago (2015-08-12 07:50:21 UTC) #22
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 4 months ago (2015-08-12 08:46:54 UTC) #23
commit-bot: I haz the power
5 years, 4 months ago (2015-08-12 08:47:43 UTC) #24
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/3c4c729b54ab44ef38c2b3bddeb19dc5e2c0de7e
Cr-Commit-Position: refs/heads/master@{#342992}

Powered by Google App Engine
This is Rietveld 408576698