Use platform's device scale factor for cursor

Use platform's device scale factor for cursor instead of gfx::Display because gfx::Display's DSF may differ from
platform's one when UI scaling is used.

BUG=361672
TEST=covered by unit test

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265592

R=sky@chromium.org, tdanderson@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265747

[tdanderson, 2014-04-12 01:59:03]

Is this up for review or still a WIP?

[oshima, 2014-04-12 02:25:37]

On 2014/04/12 01:59:03, tdanderson wrote:
> Is this up for review or still a WIP?

Not yet, although if you have non nit comments, you're welcome.

[oshima, 2014-04-21 19:39:46]

This is ready. PTAL.

https://codereview.chromium.org/226293005/diff/190001/ash/display/screen_ash.cc
File ash/display/screen_ash.cc (left):

https://codereview.chromium.org/226293005/diff/190001/ash/display/screen_ash....
ash/display/screen_ash.cc:237: return gfx::Display(1);
This was wrong. If the root window is gone, this should return primary, which is
consistent with the behavior during shutdown.

[tdanderson, 2014-04-22 17:28:09]

LGTM with a few questions inline:

https://codereview.chromium.org/226293005/diff/190001/ash/test/cursor_manager...
File ash/test/cursor_manager_test_api.h (right):

https://codereview.chromium.org/226293005/diff/190001/ash/test/cursor_manager...
ash/test/cursor_manager_test_api.h:28: gfx::Display::Rotation
GetCurrentCursorRotation() const;
Is the |scale_| member in CursorLoader tested anywhere?

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.cc
File ash/wm/image_cursors.cc (right):

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.cc...
ash/wm/image_cursors.cc:97: cursor_loader_->set_scale(scale_factor);
Would set_rotation() be needed here too?

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.h
File ash/wm/image_cursors.h (left):

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.h#...
ash/wm/image_cursors.h:42: void SetScale(float scale);
Was this dead code? I don't see any calls to SetScale() removed in this patch.

[oshima, 2014-04-23 01:47:11]

sky -> chrome/browser, ui/base changes

https://codereview.chromium.org/226293005/diff/190001/ash/test/cursor_manager...
File ash/test/cursor_manager_test_api.h (right):

https://codereview.chromium.org/226293005/diff/190001/ash/test/cursor_manager...
ash/test/cursor_manager_test_api.h:28: gfx::Display::Rotation
GetCurrentCursorRotation() const;
On 2014/04/22 17:28:10, tdanderson wrote:
> Is the |scale_| member in CursorLoader tested anywhere?

They're indirectly tested in

ui/base/cursor/cursor_loader_x11_unittest.cc

and

ash/wm/ash_native_cursor_manager_unittest.cc

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.cc
File ash/wm/image_cursors.cc (right):

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.cc...
ash/wm/image_cursors.cc:97: cursor_loader_->set_scale(scale_factor);
On 2014/04/22 17:28:10, tdanderson wrote:
> Would set_rotation() be needed here too?

This was actually redundant (They're updated after if block).

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.h
File ash/wm/image_cursors.h (left):

https://codereview.chromium.org/226293005/diff/190001/ash/wm/image_cursors.h#...
ash/wm/image_cursors.h:42: void SetScale(float scale);
On 2014/04/22 17:28:10, tdanderson wrote:
> Was this dead code? I don't see any calls to SetScale() removed in this patch.

Yes, it was.

[sky, 2014-04-23 02:53:05]

LGTM

https://codereview.chromium.org/226293005/diff/210001/ui/base/cursor/cursor_l...
File ui/base/cursor/cursor_loader_x11.cc (right):

https://codereview.chromium.org/226293005/diff/210001/ui/base/cursor/cursor_l...
ui/base/cursor/cursor_loader_x11.cc:235: rotation() == gfx::Display::ROTATE_0) {
nit: fit on one line if you can.

[oshima, 2014-04-23 05:57:46]

https://codereview.chromium.org/226293005/diff/210001/ui/base/cursor/cursor_l...
File ui/base/cursor/cursor_loader_x11.cc (right):

https://codereview.chromium.org/226293005/diff/210001/ui/base/cursor/cursor_l...
ui/base/cursor/cursor_loader_x11.cc:235: rotation() == gfx::Display::ROTATE_0) {
On 2014/04/23 02:53:05, sky wrote:
> nit: fit on one line if you can.

Done.

[oshima, 2014-04-23 05:57:50]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-04-23 05:58:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/226293005/230001

[commit-bot: I haz the power, 2014-04-23 09:03:33]

Change committed as 265592

[pfeldman, 2014-04-23 13:21:01]

A revert of this CL has been created in
https://codereview.chromium.org/249303002/ by pfeldman@chromium.org.

The reason for reverting is: There are no resize cursors in DevTools on Linux
Aura anymore..

[oshima, 2014-04-23 17:08:58]

On 2014/04/23 13:21:01, pfeldman wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/249303002/ by mailto:pfeldman@chromium.org.
> 
> The reason for reverting is: There are no resize cursors in DevTools on Linux
> Aura anymore..

I didn't understand the reason. Did this break something in linux aura?

[oshima, 2014-04-23 21:59:10]

On 2014/04/23 17:08:58, oshima wrote:
> On 2014/04/23 13:21:01, pfeldman wrote:
> > A revert of this CL has been created in
> > https://codereview.chromium.org/249303002/ by mailto:pfeldman@chromium.org.
> > 
> > The reason for reverting is: There are no resize cursors in DevTools on
Linux
> > Aura anymore..
> 
> I didn't understand the reason. Did this break something in linux aura?

I tested patch set 5 on linux_aura on goobuntu and confirmed that cursor is
visible.

[oshima, 2014-04-23 22:00:29]

Committed patchset #5 manually as r265747 (presubmit successful).