Fix the issue of cursor's device scale factor when using monitors with differnt device scale factor.

Fix the issue of cursor's device scale factor when using monitors with differnt device scale factor.

The cursor's device scale factor is correctly set when cursor warps to another
monitor. The original code was added for handling the case where moves to
another monitor without warping when testing on desktop. But this caused adverse
effects in some cases, so just deleting the code would be fine.

This CL also adds regression tests for the following cases
- cursor warps without dragging anything
- cursor warps while dragging a window
- cursor warps while dragging a tab

TODO: Add a test case where cursor warps when a chromium menu is open (http://crbug.com/154377).
This CL depends on http://codereview.chromium.org/11066045/.

BUG=154183
TEST=MouseCursorEventFilterTest.CursorDeviceScaleFactor, WorkspaceWindowResizerTest.CursorDeviceScaleFactor, DifferentDeviceScaleFactorDisplayTabDragControllerTest.CursorDeviceScaleFactor

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162625

[oshima, 2012-10-05 00:02:22]

can you add a test for this?

[mazda, 2012-10-05 21:14:37]

Added 3 regression tests.
Please take another look.

[oshima, 2012-10-05 21:20:59]

lgtm with nits

https://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

https://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1082: };
can you add COMPILE_ASSERT to make sure they have same array size?

https://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1097:
p.x, p.y, base::Bind(&CursorDeviceScaleFactorStep<N+1>,
N + 1

[mazda, 2012-10-05 21:34:43]

http://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/t...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

http://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/t...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1082: };
On 2012/10/05 21:20:59, oshima wrote:
> can you add COMPILE_ASSERT to make sure they have same array size?

Done.

http://codereview.chromium.org/11033038/diff/4/chrome/browser/ui/views/tabs/t...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1097:
p.x, p.y, base::Bind(&CursorDeviceScaleFactorStep<N+1>,
On 2012/10/05 21:20:59, oshima wrote:
> N + 1

Done.

[mazda, 2012-10-05 21:35:52]

+sky
Could you do an OWNERS review for chrome, ash and ui/aura?

[oshima, 2012-10-10 04:41:17]

FYI: CusroDeviceScaleFactor is failing and looks legit.

[mazda, 2012-10-11 19:07:54]

I fixed the tests failures. Please take another look.
In order to fix the tests, I added changes as follows.

- ui_controls_ash.cc
Removed the code for the case there's a capture window as this seems incorrect.
For example, when a menu is opened, the location of mouse move events is not
relative to the root window where the menu is shown but relative to the root
window on which the mouse cursor exist.
When passive grab is taken, the location of mouse events should be relative to
the root window for which passive grab is taken. My change does not consider
this case but it looks there's no test that relies on this behavior. I added a
TODO comment to support this case.

- ui_controls_x11.cc
Convert DIP point into pixel as RootWindow::PostNativeEvent expect that.

- snap_resizer.cc
Fixed an array index out of bounds bug reported by ASAN :)

[mazda, 2012-10-16 17:32:25]

Oshima-san,
Could you review the parts added in Patch Set 7 (ui_controls_ash.cc,
ui_controls_x11.cc, and snap_resizer.cc) again?
The details of the changes are written in my previous message.

[oshima, 2012-10-17 18:47:17]

lgtm

http://codereview.chromium.org/11033038/diff/21006/ash/ui_controls_ash.cc
File ash/ui_controls_ash.cc (right):

http://codereview.chromium.org/11033038/diff/21006/ash/ui_controls_ash.cc#new...
ash/ui_controls_ash.cc:39: ui_controls::UIControlsAura*
GetUIControlsAt(gfx::Point* point) {
point_in_screen while you're at it.

[mazda, 2012-10-17 19:13:30]

Scott,
Could you do an OWNERS review?

http://codereview.chromium.org/11033038/diff/21006/ash/ui_controls_ash.cc
File ash/ui_controls_ash.cc (right):

http://codereview.chromium.org/11033038/diff/21006/ash/ui_controls_ash.cc#new...
ash/ui_controls_ash.cc:39: ui_controls::UIControlsAura*
GetUIControlsAt(gfx::Point* point) {
On 2012/10/17 18:47:17, oshima wrote:
> point_in_screen while you're at it.

Done.

[sky, 2012-10-17 21:29:42]

http://codereview.chromium.org/11033038/diff/33001/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

http://codereview.chromium.org/11033038/diff/33001/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1171:
template <int N>
Ugh. Why do we need the templates here? Can't we just pass in the array and
index? Or convert to a std::vector<gfx::Point> and pass that around?

[mazda, 2012-10-17 23:44:28]

Please take another look.

http://codereview.chromium.org/11033038/diff/33001/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

http://codereview.chromium.org/11033038/diff/33001/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1171:
template <int N>
On 2012/10/17 21:29:42, sky wrote:
> Ugh. Why do we need the templates here? Can't we just pass in the array and
> index? Or convert to a std::vector<gfx::Point> and pass that around?

Rewrote it without using template.

[sky, 2012-10-18 00:13:37]

LGTM

http://codereview.chromium.org/11033038/diff/37003/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

http://codereview.chromium.org/11033038/diff/37003/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1208: //
enough that it attaches to browser2.
there is no browser2 here.

[mazda, 2012-10-18 00:26:16]

http://codereview.chromium.org/11033038/diff/37003/chrome/browser/ui/views/ta...
File chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
(right):

http://codereview.chromium.org/11033038/diff/37003/chrome/browser/ui/views/ta...
chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc:1208: //
enough that it attaches to browser2.
On 2012/10/18 00:13:37, sky wrote:
> there is no browser2 here.

Fixed the comment. Thanks.

[commit-bot: I haz the power, 2012-10-18 00:26:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/11033038/31016

[commit-bot: I haz the power, 2012-10-18 02:46:27]

Change committed as 162625