aura: Fix Chrome OS status area browser tests.

aura: Fix Chrome OS status area browser tests.

The tests for StatusAreaView, PowerMenuButton, and
CapsLockMenuButton were crashing when they tried to get the
StatusAreaView from chromeos::BrowserView, which isn't used
in Aura builds.  This change makes them instead get the
status area from the Aura shell.

BUG=103501, 103495, 104666
TEST=ran tests

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

[stevenjb, 2011-11-16 23:38:31]

lgtm

[stevenjb, 2011-11-17 00:25:55]

Oh, did you mean to exclude power_menu_button_browsertest also?

Thanks for doing this by the way, I should have caught these.

[Daniel Erat, 2011-11-17 00:28:46]

On 2011/11/17 00:25:55, Steven Bennetts wrote:
> Oh, did you mean to exclude power_menu_button_browsertest also?
> 
> Thanks for doing this by the way, I should have caught these.

Actually, I updated the change's description in anticipation of the new version
I just uploaded, which fixes the StatusAreaView and PowerMenuButton tests
instead of just removing the status area test.  Another look?

[stevenjb, 2011-11-17 19:24:49]

lgtm with nit

http://codereview.chromium.org/8591003/diff/6001/ui/aura_shell/shelf_layout_c...
File ui/aura_shell/shelf_layout_controller.h (right):

http://codereview.chromium.org/8591003/diff/6001/ui/aura_shell/shelf_layout_c...
ui/aura_shell/shelf_layout_controller.h:30: views::Widget* status() { return
status_; }
nit: can we rename this status_widget_ (or status_area_widget_ if we want to be
really explicit), since "status" is a bit overloaded.

[Daniel Erat, 2011-11-17 20:51:01]

http://codereview.chromium.org/8591003/diff/6001/ui/aura_shell/shelf_layout_c...
File ui/aura_shell/shelf_layout_controller.h (right):

http://codereview.chromium.org/8591003/diff/6001/ui/aura_shell/shelf_layout_c...
ui/aura_shell/shelf_layout_controller.h:30: views::Widget* status() { return
status_; }
On 2011/11/17 19:24:49, Steven Bennetts wrote:
> nit: can we rename this status_widget_ (or status_area_widget_ if we want to
be
> really explicit), since "status" is a bit overloaded.

Sure, this was bugging me too.  Done.

[Daniel Erat, 2011-11-18 02:33:54]

@#@! OWNERS file.  Scott, can you take a look for ui/aura_shell?

[Daniel Erat, 2011-11-18 16:47:36]

Ben, mind taking a look?

[Daniel Erat, 2011-11-18 17:28:17]

Scott took a similar approach in http://codereview.chromium.org/8600001/.  I'll
wait for his change to go in and update this one to use his code.

[Ben Goodger (Google), 2011-11-18 20:31:17]

LGTM, one comment:

http://codereview.chromium.org/8591003/diff/12001/ui/aura_shell/shell.h
File ui/aura_shell/shell.h (right):

http://codereview.chromium.org/8591003/diff/12001/ui/aura_shell/shell.h#newco...
ui/aura_shell/shell.h:74: views::Widget* GetStatusAreaWidget();
Add ForTesting() to the method name, so that it's clear this isn't a general
purpose getter.

[Daniel Erat, 2011-11-19 01:49:11]

Hmm.  Was hoping to finally get this in today, but it looks like Scott's change
was just reverted.

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/power_menu_button_browsertest.cc (right):

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/power_menu_button_browsertest.cc:35: return
const_cast<PowerMenuButton*>(
This is somewhat gross.  Scott, what's the plan for GetStatusArea()?  If it's
just for tests, perhaps it could be made to return a non-const view and be
renamed to GetStatusAreaForTest().

[sky, 2011-11-19 23:26:52]

GAH. You're right, I was reverted. The patch is cycling through the bots again,
hopefully it sticks this time.

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/power_menu_button_browsertest.cc (right):

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/power_menu_button_browsertest.cc:35: return
const_cast<PowerMenuButton*>(
On 2011/11/19 01:49:11, Daniel Erat wrote:
> This is somewhat gross.  Scott, what's the plan for GetStatusArea()?  If it's
> just for tests, perhaps it could be made to return a non-const view and be
> renamed to GetStatusAreaForTest().

I was hoping we could keep it const as most tests shouldn't need to modify it.
Does this test really need it to be non-const? But, as you say, the method is
really for testing so if it needs to be non-const go ahead and change it.

[Daniel Erat, 2011-11-21 16:45:51]

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
File chrome/browser/chromeos/status/power_menu_button_browsertest.cc (right):

http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
chrome/browser/chromeos/status/power_menu_button_browsertest.cc:35: return
const_cast<PowerMenuButton*>(
On 2011/11/19 23:26:52, sky wrote:
> On 2011/11/19 01:49:11, Daniel Erat wrote:
> > This is somewhat gross.  Scott, what's the plan for GetStatusArea()?  If
it's
> > just for tests, perhaps it could be made to return a non-const view and be
> > renamed to GetStatusAreaForTest().
> 
> I was hoping we could keep it const as most tests shouldn't need to modify it.
> Does this test really need it to be non-const? But, as you say, the method is
> really for testing so if it needs to be non-const go ahead and change it. 

I'm happy leaving it const if you're okay with the const_cast here.  The tests
in this file look like they're checking what the PowerMenuButton does when it's
told that the power status has changed, so I don't think they can be adapted to
use a const button.

[sky, 2011-11-21 16:59:07]

On 2011/11/21 16:45:51, Daniel Erat wrote:
>
http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
> File chrome/browser/chromeos/status/power_menu_button_browsertest.cc (right):
> 
>
http://codereview.chromium.org/8591003/diff/16001/chrome/browser/chromeos/sta...
> chrome/browser/chromeos/status/power_menu_button_browsertest.cc:35: return
> const_cast<PowerMenuButton*>(
> On 2011/11/19 23:26:52, sky wrote:
> > On 2011/11/19 01:49:11, Daniel Erat wrote:
> > > This is somewhat gross.  Scott, what's the plan for GetStatusArea()?  If
> it's
> > > just for tests, perhaps it could be made to return a non-const view and be
> > > renamed to GetStatusAreaForTest().
> > 
> > I was hoping we could keep it const as most tests shouldn't need to modify
it.
> > Does this test really need it to be non-const? But, as you say, the method
is
> > really for testing so if it needs to be non-const go ahead and change it. 
> 
> I'm happy leaving it const if you're okay with the const_cast here.  The tests
> in this file look like they're checking what the PowerMenuButton does when
it's
> told that the power status has changed, so I don't think they can be adapted
to
> use a const button.

I don't like the const cast here either, and as you say this test needs a
non-const object. So, in that case, change the function I added to non-const so
tests like this can work.

  -Scott

[Daniel Erat, 2011-11-21 17:37:49]

Another look?

[sky, 2011-11-21 17:44:46]

LGTM

[commit-bot: I haz the power, 2011-11-21 17:49:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/8591003/23001

[commit-bot: I haz the power, 2011-11-21 19:26:17]

Try job failure for 8591003-23001 (retry) on mac_rel for step "ui_tests"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-11-21 20:13:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/8591003/23001

[commit-bot: I haz the power, 2011-11-21 20:31:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/8591003/23001