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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by lanwei
Modified:
1 year 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 
Commit queue not available (can’t edit this change).

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

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b