Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(65)

Issue 2269483002: Set the coordinates of the synthetic touch event correctly in SyntheticGestureTargetAura (Closed)

Created:
1 year, 5 months ago by lanwei
Modified:
1 year, 4 months ago
Reviewers:
bokan, dsansome, tdresser
CC:
chromium-reviews, jam, darin-cc_chromium.org, dtapuska+chromiumwatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Set the coordinates of the synthetic touch event correctly in SyntheticGestureTargetAura. In ConvertLocationToTarget function, the offset added to the event location is not scaled, so we should apply the screen scale factor to the event location after it has been transformed to the target. Then we can find the right target to send the event using the correct coordinates. BUG=634343 Committed: https://crrev.com/34bff62e925142091ce98722c7bd447f51b3794b Cr-Commit-Position: refs/heads/master@{#415119}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Add test #

Total comments: 20

Patch Set 3 #

Total comments: 3

Patch Set 4 : Scale the position later #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -2 lines) Patch
M content/browser/renderer_host/input/synthetic_gesture_target_aura.cc View 1 2 3 2 chunks +9 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/VirtualTestSuites View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/events/synthetic-events/tap-on-scaled-screen.html View 1 2 1 chunk +44 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/scalefactor150/fast/events/synthetic-events/README.txt View 1 2 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 67 (52 generated)
tdresser
Can we write a test for this? https://codereview.chromium.org/2269483002/diff/1/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (right): https://codereview.chromium.org/2269483002/diff/1/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#newcode45 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:45: touch_with_latency, &events, ...
1 year, 5 months ago (2016-08-22 12:52:39 UTC) #9
lanwei
https://codereview.chromium.org/2269483002/diff/1/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (right): https://codereview.chromium.org/2269483002/diff/1/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#newcode45 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:45: touch_with_latency, &events, LOCAL_COORDINATES); On 2016/08/22 12:52:39, tdresser wrote: > ...
1 year, 5 months ago (2016-08-24 11:56:06 UTC) #22
tdresser
This LGTM, but I'd appreciate a second pair of eyes confirming that this approach to ...
1 year, 5 months ago (2016-08-24 12:27:49 UTC) #24
bokan
Nit in description: Avoid transform the synthetic... -> Avoid transforming the synthetic... https://codereview.chromium.org/2269483002/diff/60001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc ...
1 year, 5 months ago (2016-08-24 15:56:57 UTC) #27
lanwei
https://codereview.chromium.org/2269483002/diff/60001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (right): https://codereview.chromium.org/2269483002/diff/60001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#newcode41 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:41: touch_with_latency.event.touches[i].radiusY *= device_scale_factor_;; On 2016/08/24 12:27:48, tdresser wrote: > ...
1 year, 5 months ago (2016-08-25 02:28:56 UTC) #42
bokan
https://codereview.chromium.org/2269483002/diff/60001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (right): https://codereview.chromium.org/2269483002/diff/60001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#newcode45 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:45: touch_with_latency, &events, SCREEN_COORDINATES); On 2016/08/25 02:28:55, lanwei wrote: > ...
1 year, 5 months ago (2016-08-25 12:36:52 UTC) #43
lanwei
https://codereview.chromium.org/2269483002/diff/140001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (left): https://codereview.chromium.org/2269483002/diff/140001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#oldcode38 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:38: touch_with_latency.event.touches[i].position.y *= device_scale_factor_; On 2016/08/25 12:36:51, bokan wrote: > ...
1 year, 5 months ago (2016-08-25 15:19:57 UTC) #44
bokan
https://codereview.chromium.org/2269483002/diff/140001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc (left): https://codereview.chromium.org/2269483002/diff/140001/content/browser/renderer_host/input/synthetic_gesture_target_aura.cc#oldcode38 content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:38: touch_with_latency.event.touches[i].position.y *= device_scale_factor_; On 2016/08/25 15:19:57, lanwei wrote: > ...
1 year, 5 months ago (2016-08-25 18:53:08 UTC) #45
lanwei
1 year, 4 months ago (2016-08-26 23:39:58 UTC) #54
bokan
This looks better, thanks! LGTM
1 year, 4 months ago (2016-08-29 12:59:19 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2269483002/180001
1 year, 4 months ago (2016-08-29 13:33:56 UTC) #58
dsansome
(I unset the commit bit on this change to try fix a stuck CQ - ...
1 year, 4 months ago (2016-08-30 03:53:59 UTC) #61
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2269483002/180001
1 year, 4 months ago (2016-08-30 03:57:09 UTC) #63
commit-bot: I haz the power
Committed patchset #4 (id:180001)
1 year, 4 months ago (2016-08-30 04:51:46 UTC) #65
commit-bot: I haz the power
1 year, 4 months ago (2016-08-30 04:53:31 UTC) #67
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/34bff62e925142091ce98722c7bd447f51b3794b
Cr-Commit-Position: refs/heads/master@{#415119}

Powered by Google App Engine
This is Rietveld 408576698