Fix cursor getting invisible after moving a window.

Fix cursor getting invisible after moving a window.

The platform cursor value was not correctly set when cursor was unlocked.
This CL fixes it and adds unittests.

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

[oshima, 2012-09-20 20:19:03]

lgtm with nits

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager.h
File ash/wm/cursor_manager.h (right):

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager.h#new...
ash/wm/cursor_manager.h:28: gfx::NativeCursor current_cursor();
GetCurrentCursor()

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager_unitt...
File ash/wm/cursor_manager_unittest.cc (right):

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager_unitt...
ash/wm/cursor_manager_unittest.cc:5: #include "ash/wm/cursor_manager.h"
new line

[mazda, 2012-09-20 20:29:31]

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager.h
File ash/wm/cursor_manager.h (right):

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager.h#new...
ash/wm/cursor_manager.h:28: gfx::NativeCursor current_cursor();
On 2012/09/20 20:19:03, oshima wrote:
> GetCurrentCursor()

Done.

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager_unitt...
File ash/wm/cursor_manager_unittest.cc (right):

http://codereview.chromium.org/10965020/diff/1001/ash/wm/cursor_manager_unitt...
ash/wm/cursor_manager_unittest.cc:5: #include "ash/wm/cursor_manager.h"
On 2012/09/20 20:19:03, oshima wrote:
> new line

Done.

[mazda, 2012-09-20 20:30:11]

+sky
Could you do an OWNERS review for ash?

[sky, 2012-09-20 20:46:27]

LGTM

http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
File ash/ash.gyp (right):

http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
sort

http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
File ash/wm/cursor_manager.h (right):

http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
Can you put this in a separate file so it isn't cluttering up CursorManager.

[mazda, 2012-09-20 21:13:40]

http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
File ash/ash.gyp (right):

http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
On 2012/09/20 20:46:27, sky wrote:
> sort

It's already sorted. :-)

http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
File ash/wm/cursor_manager.h (right):

http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
On 2012/09/20 20:46:27, sky wrote:
> Can you put this in a separate file so it isn't cluttering up CursorManager.
Changed as follows:
- only declare TestApi here.
- moved the implementation to cursor_manager_unittest.cc

[sky, 2012-09-20 21:43:42]

On Thu, Sep 20, 2012 at 2:13 PM,  <mazda@chromium.org> wrote:
>
> http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
> File ash/ash.gyp (right):
>
> http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
> ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
> On 2012/09/20 20:46:27, sky wrote:
>>
>> sort
>
>
> It's already sorted. :-)
>
>
> http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
> File ash/wm/cursor_manager.h (right):
>
>
http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
> ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
> On 2012/09/20 20:46:27, sky wrote:
>>
>> Can you put this in a separate file so it isn't cluttering up
>
> CursorManager.
> Changed as follows:
> - only declare TestApi here.
> - moved the implementation to cursor_manager_unittest.cc
>
> http://codereview.chromium.org/10965020/

I would rather see it in its own header.

  -Scott

[mazda, 2012-09-20 22:45:17]

On 2012/09/20 21:43:42, sky wrote:
> On Thu, Sep 20, 2012 at 2:13 PM,  <mailto:mazda@chromium.org> wrote:
> >
> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
> > File ash/ash.gyp (right):
> >
> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
> > ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
> > On 2012/09/20 20:46:27, sky wrote:
> >>
> >> sort
> >
> >
> > It's already sorted. :-)
> >
> >
> > http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
> > File ash/wm/cursor_manager.h (right):
> >
> >
>
http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
> > ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
> > On 2012/09/20 20:46:27, sky wrote:
> >>
> >> Can you put this in a separate file so it isn't cluttering up
> >
> > CursorManager.
> > Changed as follows:
> > - only declare TestApi here.
> > - moved the implementation to cursor_manager_unittest.cc
> >
> > http://codereview.chromium.org/10965020/
> 
> I would rather see it in its own header.
> 
>   -Scott

Are you suggesting to use friend class instead of nested class for private
member access?
Isn't it a convention to declare a nested class named TestApi that provides
accessors to private members for tests?

[sky, 2012-09-20 22:52:35]

On Thu, Sep 20, 2012 at 3:45 PM,  <mazda@chromium.org> wrote:
> On 2012/09/20 21:43:42, sky wrote:
>
>> On Thu, Sep 20, 2012 at 2:13 PM,  <mailto:mazda@chromium.org> wrote:
>> >
>> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
>> > File ash/ash.gyp (right):
>> >
>> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
>> > ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
>> > On 2012/09/20 20:46:27, sky wrote:
>> >>
>> >> sort
>> >
>> >
>> > It's already sorted. :-)
>> >
>> >
>> >
>> > http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
>> > File ash/wm/cursor_manager.h (right):
>> >
>> >
>
>
>
http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
>>
>> > ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
>> > On 2012/09/20 20:46:27, sky wrote:
>> >>
>> >> Can you put this in a separate file so it isn't cluttering up
>> >
>> > CursorManager.
>> > Changed as follows:
>> > - only declare TestApi here.
>> > - moved the implementation to cursor_manager_unittest.cc
>> >
>> > http://codereview.chromium.org/10965020/
>
>
>> I would rather see it in its own header.
>
>
>>    -Scott
>
>
> Are you suggesting to use friend class instead of nested class for private
> member access?

Yes.

> Isn't it a convention to declare a nested class named TestApi that provides
> accessors to private members for tests?

IMO that clutters the header.

  -Scott

[mazda, 2012-09-20 23:32:49]

On 2012/09/20 22:52:35, sky wrote:
> On Thu, Sep 20, 2012 at 3:45 PM,  <mailto:mazda@chromium.org> wrote:
> > On 2012/09/20 21:43:42, sky wrote:
> >
> >> On Thu, Sep 20, 2012 at 2:13 PM,  <mailto:mazda@chromium.org> wrote:
> >> >
> >> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp
> >> > File ash/ash.gyp (right):
> >> >
> >> > http://codereview.chromium.org/10965020/diff/4002/ash/ash.gyp#newcode508
> >> > ash/ash.gyp:508: 'wm/cursor_manager_unittest.cc',
> >> > On 2012/09/20 20:46:27, sky wrote:
> >> >>
> >> >> sort
> >> >
> >> >
> >> > It's already sorted. :-)
> >> >
> >> >
> >> >
> >> > http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h
> >> > File ash/wm/cursor_manager.h (right):
> >> >
> >> >
> >
> >
> >
>
http://codereview.chromium.org/10965020/diff/4002/ash/wm/cursor_manager.h#new...
> >>
> >> > ash/wm/cursor_manager.h:24: class ASH_EXPORT TestApi {
> >> > On 2012/09/20 20:46:27, sky wrote:
> >> >>
> >> >> Can you put this in a separate file so it isn't cluttering up
> >> >
> >> > CursorManager.
> >> > Changed as follows:
> >> > - only declare TestApi here.
> >> > - moved the implementation to cursor_manager_unittest.cc
> >> >
> >> > http://codereview.chromium.org/10965020/
> >
> >
> >> I would rather see it in its own header.
> >
> >
> >>    -Scott
> >
> >
> > Are you suggesting to use friend class instead of nested class for private
> > member access?
> 
> Yes.
> 
> > Isn't it a convention to declare a nested class named TestApi that provides
> > accessors to private members for tests?
> 
> IMO that clutters the header.
> 
>   -Scott

Thank you for the explanation. I moved the test api to other files.

[sky, 2012-09-20 23:47:01]

LGTM

[commit-bot: I haz the power, 2012-09-21 01:35:01]

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

[commit-bot: I haz the power, 2012-09-21 04:21:15]

Change committed as 157924