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

Issue 560633002: Ensure that mouse lock works correctly on Chrome ASH in Windows 8. (Closed)

Created:
6 years, 3 months ago by ananta
Modified:
6 years, 3 months ago
Reviewers:
cpu_(ooo_6.6-7.5), sky
CC:
chromium-reviews, sadrul, ben+aura_chromium.org, kalyank
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Ensure that mouse lock works correctly on Chrome ASH in Windows 8. The lock mouse operation invoked by plugins or pages hides the mouse cursor and ensures that the cursor stays within the bounds of the webpage. To ensure that the mouse stays within the bounds of the page, the SetCursorPos operation is executed by the host. In ASH, the SetCursorPos API is executed by the viewer process for historical reasons. As a result on Windows 8 there is a faint possibility that the user may move the mouse to the charms section of the OS, which causes the cursor to become visible. Fix for this is to track if the mouse changed from what was last set in the viewer process and restore it. The other change is in the RemoteWindowTreeHostWin class where the member ignore_mouse_moves_until_set_cursor_ack_ has been changed to a count from a bool flag. This is because the RemoteWindowTreeHostWin::MoveCursorToNative function can be called multiple times before the acks are received causing DCHECKs to fire on the ignore_mouse_moves_until_set_cursor_ack_ flag. BUG=398792 Committed: https://crrev.com/b306b1e61ab3178f8d5ab54a2233e748d926bd4d Cr-Commit-Position: refs/heads/master@{#294274}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Code review comments #

Total comments: 2

Patch Set 3 : Review comments #

Patch Set 4 : Build errors #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -8 lines) Patch
M ui/aura/remote_window_tree_host_win.h View 1 chunk +2 lines, -2 lines 0 comments Download
M ui/aura/remote_window_tree_host_win.cc View 1 2 3 3 chunks +4 lines, -4 lines 0 comments Download
M win8/metro_driver/chrome_app_view_ash.h View 1 chunk +3 lines, -0 lines 0 comments Download
M win8/metro_driver/chrome_app_view_ash.cc View 3 chunks +9 lines, -2 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
ananta
cpu : please review everything. sky: ui\aura changes.
6 years, 3 months ago (2014-09-09 23:12:57 UTC) #2
sky
https://codereview.chromium.org/560633002/diff/1/ui/aura/remote_window_tree_host_win.cc File ui/aura/remote_window_tree_host_win.cc (right): https://codereview.chromium.org/560633002/diff/1/ui/aura/remote_window_tree_host_win.cc#newcode433 ui/aura/remote_window_tree_host_win.cc:433: if (ignore_mouse_moves_until_set_cursor_ack_ > 0) { DCHECK?
6 years, 3 months ago (2014-09-09 23:34:19 UTC) #3
ananta
https://codereview.chromium.org/560633002/diff/1/ui/aura/remote_window_tree_host_win.cc File ui/aura/remote_window_tree_host_win.cc (right): https://codereview.chromium.org/560633002/diff/1/ui/aura/remote_window_tree_host_win.cc#newcode433 ui/aura/remote_window_tree_host_win.cc:433: if (ignore_mouse_moves_until_set_cursor_ack_ > 0) { On 2014/09/09 23:34:18, sky ...
6 years, 3 months ago (2014-09-09 23:39:18 UTC) #4
sky
LGTM https://codereview.chromium.org/560633002/diff/20001/ui/aura/remote_window_tree_host_win.cc File ui/aura/remote_window_tree_host_win.cc (right): https://codereview.chromium.org/560633002/diff/20001/ui/aura/remote_window_tree_host_win.cc#newcode433 ui/aura/remote_window_tree_host_win.cc:433: DCHECK(ignore_mouse_moves_until_set_cursor_ack_ > 0); DCHECK_GT
6 years, 3 months ago (2014-09-09 23:40:59 UTC) #5
ananta
https://codereview.chromium.org/560633002/diff/20001/ui/aura/remote_window_tree_host_win.cc File ui/aura/remote_window_tree_host_win.cc (right): https://codereview.chromium.org/560633002/diff/20001/ui/aura/remote_window_tree_host_win.cc#newcode433 ui/aura/remote_window_tree_host_win.cc:433: DCHECK(ignore_mouse_moves_until_set_cursor_ack_ > 0); On 2014/09/09 23:40:59, sky wrote: > ...
6 years, 3 months ago (2014-09-09 23:52:51 UTC) #6
cpu_(ooo_6.6-7.5)
lgtm
6 years, 3 months ago (2014-09-10 00:57:44 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ananta@chromium.org/560633002/60001
6 years, 3 months ago (2014-09-10 22:20:56 UTC) #9
commit-bot: I haz the power
Committed patchset #4 (id:60001) as f83a7d53649cc7462b32716f5a81b3e4e0e4c7c8
6 years, 3 months ago (2014-09-11 00:44:28 UTC) #10
commit-bot: I haz the power
6 years, 3 months ago (2014-09-11 00:46:31 UTC) #11
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/b306b1e61ab3178f8d5ab54a2233e748d926bd4d
Cr-Commit-Position: refs/heads/master@{#294274}

Powered by Google App Engine
This is Rietveld 408576698