[Views] Make MenuButton track its desired state while pressed

[Views] Make MenuButton track its desired state while pressed

Make MenuButton track its desried state while it's pressed so that, upon
release, it returns to the proper state (instead of assuming STATE_NORMAL).

BUG=431481
Committed: https://crrev.com/f7efba8fd39ab70ffd77abc5bceadb66656e553d
Cr-Commit-Position: refs/heads/master@{#313105}

[Devlin, 2015-01-23 22:47:39]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-01-23 22:47:43]

Patchset #1 (id:20001) has been deleted

[Devlin, 2015-01-23 22:47:48]

Patchset #1 (id:40001) has been deleted

[Devlin, 2015-01-23 22:49:32]

rdevlin.cronin@chromium.org changed reviewers:
+ sky@chromium.org

[Devlin, 2015-01-23 22:49:33]

Scott, please take a look when you can. :)

[sky, 2015-01-24 00:00:36]

https://codereview.chromium.org/873513002/diff/60001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right):

https://codereview.chromium.org/873513002/diff/60001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:873: if
(!content_window_ || !host_)
This has me worried, and at the least is worth a comment. Why do we need to
check for no host here when no where else in this class do we do it?

[sky, 2015-01-24 00:00:42]

Everything else looks good.

[Devlin, 2015-01-24 00:11:48]

https://codereview.chromium.org/873513002/diff/60001/ui/views/widget/desktop_...
File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right):

https://codereview.chromium.org/873513002/diff/60001/ui/views/widget/desktop_...
ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:873: if
(!content_window_ || !host_)
On 2015/01/24 00:00:36, sky wrote:
> This has me worried, and at the least is worth a comment. Why do we need to
> check for no host here when no where else in this class do we do it?

Yeah, I'm not real keen on it either.
Reason:
- OnHostClosed() resets() |host_|, and then sets |content_window_| to null.
- |host_|'s destructor works its way around to calling OnWidgetDestroying.
- OnWidgetDestroying is a path for release MenuButton::PressedLocks.
- Releasing the last pressed lock checks if the mouse is hovered, which checks
if IsMouseEventsEnabled() (leads to crash).

In short, checking if mouse events are enabled during a widget shutdown crashes.
 I was wondering about other possible solutions (mark a widget as destroying in
views::Widget, and don't check it if it is? [Seems like a bandaid, not a
solution.]  Set content_window_ to null before resetting host_? [Tried that one;
it fails because destruction/reset order is hyper-sensitive]).

This seemed like the safest solution, because it's common to check something for
nullness before using it (on line 876), but I'd love your opinion.

[sky, 2015-01-26 15:48:36]

I can't think of anything better. Add a comment and LGTM

[Devlin, 2015-01-26 17:29:35]

Patchset #2 (id:80001) has been deleted

[Devlin, 2015-01-26 19:50:59]

On 2015/01/26 15:48:36, sky wrote:
> I can't think of anything better. Add a comment and LGTM

Done.

[Devlin, 2015-01-26 19:51:02]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2015-01-26 19:52:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/873513002/100001

[commit-bot: I haz the power, 2015-01-26 19:57:16]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2015-01-26 19:58:30]

Patchset 2 (id:??) landed as
https://crrev.com/f7efba8fd39ab70ffd77abc5bceadb66656e553d
Cr-Commit-Position: refs/heads/master@{#313105}