Close any open menus on activation changes.

Close any open menus on activation changes.

BUG=131027
TEST=Close all open windows, right click on application icon, trigger lock screen. Menu is closed.


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

[flackr, 2012-06-25 22:15:44]

Hi,

Please take a look. The problem seems to be that we only called
Widget::OnNativeWidgetActivationChanged if there was an active window which
isn't the case when you close all browser windows.

Thanks!

[sky, 2012-06-25 23:34:38]

https://chromiumcodereview.appspot.com/10668037/diff/1/ash/wm/activation_cont...
File ash/wm/activation_controller.cc (right):

https://chromiumcodereview.appspot.com/10668037/diff/1/ash/wm/activation_cont...
ash/wm/activation_controller.cc:290: views::MenuController::GetActiveInstance();
I don't like this dependency here. I would rather see MenuController get aura
specific code to deal with this. That could be an ActivationChangeObserver
implementation.

[flackr, 2012-06-26 13:17:16]

https://chromiumcodereview.appspot.com/10668037/diff/1/ash/wm/activation_cont...
File ash/wm/activation_controller.cc (right):

https://chromiumcodereview.appspot.com/10668037/diff/1/ash/wm/activation_cont...
ash/wm/activation_controller.cc:290: views::MenuController::GetActiveInstance();
On 2012/06/25 23:34:38, sky wrote:
> I don't like this dependency here. I would rather see MenuController get aura
> specific code to deal with this. That could be an ActivationChangeObserver
> implementation.

Sounds good to me. Done.

[sky, 2012-06-26 16:19:35]

Since this has broken once already, can you add a test for coverage of this?

[flackr, 2012-06-27 18:41:31]

On 2012/06/26 16:19:35, sky wrote:
> Since this has broken once already, can you add a test for coverage of this?

Sure, sorry this is taking a while. An integration / UI test seems necessary
since a unit test wouldn't have caught any of these problems and only tests the
way it's implemented. I was going to add a screen locker test for this and found
out that they were mistakenly removed in
https://chromiumcodereview.appspot.com/9664072/.

[sky, 2012-06-27 20:58:53]

I'm LGTMing this now. You can straighten out the test separately, just make sure
we get coverage some how.

[commit-bot: I haz the power, 2012-06-27 21:11:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/10668037/7004

[commit-bot: I haz the power, 2012-06-28 00:36:47]

Change committed as 144625