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

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

Created:
1 year, 2 months ago by lanwei
Modified:
1 year, 2 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
Trybot results:  chromium_presubmit   win_chromium_rel_ng   win_chromium_rel_ng   linux_chromium_rel_ng   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_rel_ng   linux_chromium_asan_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_clobber_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   linux_android_rel_ng   mac_chromium_rel_ng   android_clang_dbg_recipe   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   mac_chromium_rel_ng   android_clang_dbg_recipe   linux_android_rel_ng   cast_shell_android   android_arm64_dbg_recipe   android_compile_dbg   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   linux_chromium_rel_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_clobber_rel_ng 

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, 2 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, 2 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, 2 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, 2 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, 2 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, 2 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, 2 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, 2 months ago (2016-08-25 18:53:08 UTC) #45
lanwei
1 year, 2 months ago (2016-08-26 23:39:58 UTC) #54
bokan
This looks better, thanks! LGTM
1 year, 2 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, 2 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, 2 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, 2 months ago (2016-08-30 03:57:09 UTC) #63
commit-bot: I haz the power
Committed patchset #4 (id:180001)
1 year, 2 months ago (2016-08-30 04:51:46 UTC) #65
commit-bot: I haz the power
1 year, 2 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 efc10ee0f