Make the member |is_cursor_visible_| in DesktopWindowTreeHostWin static.

Make the member |is_cursor_visible_| in DesktopWindowTreeHostWin static.

On Windows we can have multiple root windows and the implementation
of ::ShowCursor() is based on a counter, so making this member static
ensures that ::ShowCursor() is always called exactly once whenever the
cursor visibility state changes. This prevents an incorrectly hidden
(or incorrectly shown) cursor because the number of calls to
::ShowCursor() is no longer dependent on the number of instantiated
root windows at the time when a request to change the cursor visibility
is made.

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

[tdanderson, 2014-01-31 21:26:27]

Can you please take a look? If this approach looks good to you, I think the fix
is simple enough that it would be a suitable merge candidate for M32 and M33.

[sky, 2014-02-03 20:57:21]

https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_root_window_host_win.cc (right):

https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_root_window_host_win.cc:558: if
(is_cursor_visible_ == show)
What if the destructor is hit and iss_cursor_visible_ is false?

[tdanderson, 2014-02-03 22:41:01]

https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_root_window_host_win.cc (right):

https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_root_window_host_win.cc:558: if
(is_cursor_visible_ == show)
On 2014/02/03 20:57:21, sky wrote:
> What if the destructor is hit and iss_cursor_visible_ is false?

Is your concern that I'm not resetting this member to its initial value (true)
in the situation where the last remaining DWTHW instance is destroyed? Isn't
that only going to happen when the application is exiting? (Actually, I think
this is a concern for tests - we will probably need to do this so that the
member is reset to true in between test cases since root windows may not persist
between tests.)

Please let me know if my understanding of your comment is correct.

[sky, 2014-02-04 00:47:44]

Lets say you have two DRWHWs, A and B.
OnCursorVisibiiltyChanged(false) on B is invoked, which hides the
cursor. B is destroyed. Isn't the cursor stuck hidden?

On Mon, Feb 3, 2014 at 2:41 PM,  <tdanderson@chromium.org> wrote:
>
>
https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
> File ui/views/widget/desktop_aura/desktop_root_window_host_win.cc
> (right):
>
>
https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
> ui/views/widget/desktop_aura/desktop_root_window_host_win.cc:558: if
> (is_cursor_visible_ == show)
> On 2014/02/03 20:57:21, sky wrote:
>>
>> What if the destructor is hit and iss_cursor_visible_ is false?
>
>
> Is your concern that I'm not resetting this member to its initial value
> (true) in the situation where the last remaining DWTHW instance is
> destroyed? Isn't that only going to happen when the application is
> exiting? (Actually, I think this is a concern for tests - we will
> probably need to do this so that the member is reset to true in between
> test cases since root windows may not persist between tests.)
>
> Please let me know if my understanding of your comment is correct.
>
> https://codereview.chromium.org/151563004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tdanderson, 2014-02-04 19:00:58]

On 2014/02/04 00:47:44, sky wrote:
> Lets say you have two DRWHWs, A and B.
> OnCursorVisibiiltyChanged(false) on B is invoked, which hides the
> cursor. B is destroyed. Isn't the cursor stuck hidden?

I don't believe so because |cursor_manager_| and
|native_cursor_manager_| are static members of
DesktopNativeWidgetAura, and DesktopNativeCursorManager stores
a list of all root windows. Requests to show the cursor will
call OnCursorVisibilityChanged(true) on each of these root windows,
so OnCursorVisibilityChanged(true) will be called for A. Since
the static |is_cursor_visible_| is false, ::ShowCursor(true) will be 
called and the cursor will be shown.


> On Mon, Feb 3, 2014 at 2:41 PM,  <mailto:tdanderson@chromium.org> wrote:
> >
> >
>
https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
> > File ui/views/widget/desktop_aura/desktop_root_window_host_win.cc
> > (right):
> >
> >
>
https://codereview.chromium.org/151563004/diff/40001/ui/views/widget/desktop_...
> > ui/views/widget/desktop_aura/desktop_root_window_host_win.cc:558: if
> > (is_cursor_visible_ == show)
> > On 2014/02/03 20:57:21, sky wrote:
> >>
> >> What if the destructor is hit and iss_cursor_visible_ is false?
> >
> >
> > Is your concern that I'm not resetting this member to its initial value
> > (true) in the situation where the last remaining DWTHW instance is
> > destroyed? Isn't that only going to happen when the application is
> > exiting? (Actually, I think this is a concern for tests - we will
> > probably need to do this so that the member is reset to true in between
> > test cases since root windows may not persist between tests.)
> >
> > Please let me know if my understanding of your comment is correct.
> >
> > https://codereview.chromium.org/151563004/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sky, 2014-02-04 19:02:35]

Ok, LGTM

[tdanderson, 2014-02-04 19:12:58]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-02-04 19:15:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tdanderson@chromium.org/151563004/40001

[commit-bot: I haz the power, 2014-02-04 21:14:39]

Retried try job too often on mac_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[tdanderson, 2014-02-04 22:22:27]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-02-04 22:22:35]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-02-04 22:32:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tdanderson@chromium.org/151563004/40001

[commit-bot: I haz the power, 2014-02-05 01:10:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tdanderson@chromium.org/151563004/40001

[tdanderson, 2014-02-05 03:20:01]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-02-05 03:20:06]

The CQ bit was checked by tdanderson@chromium.org

[tdanderson, 2014-02-05 17:52:31]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-02-05 17:52:39]

The CQ bit was checked by tdanderson@chromium.org

[tdanderson, 2014-02-06 01:16:52]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-02-06 01:16:58]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-02-06 01:23:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tdanderson@chromium.org/151563004/40001

[commit-bot: I haz the power, 2014-02-06 09:29:49]

Change committed as 249310