ash uber tray: Add entry for power and date.

ash uber tray: Add entry for power and date.

BUG=110130
TEST=none

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

[sadrul, 2012-03-01 22:36:47]

This doesn't really look as good as the mocks. But it's functional.

[Ben Goodger (Google), 2012-03-01 23:58:30]

lgtm

http://codereview.chromium.org/9562008/diff/18/ash/system/power/tray_power_da...
File ash/system/power/tray_power_date.h (right):

http://codereview.chromium.org/9562008/diff/18/ash/system/power/tray_power_da...
ash/system/power/tray_power_date.h:28: // Overridden from SystemTrayItem
.

http://codereview.chromium.org/9562008/diff/18/chrome/browser/chromeos/dbus/p...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/9562008/diff/18/chrome/browser/chromeos/dbus/p...
chrome/browser/chromeos/dbus/power_manager_client.h:13: #if defined(USE_AURA)
USE_ASH, here and elsewhere

[sadrul, 2012-03-02 01:59:57]

http://codereview.chromium.org/9562008/diff/18/ash/system/power/tray_power_da...
File ash/system/power/tray_power_date.h (right):

http://codereview.chromium.org/9562008/diff/18/ash/system/power/tray_power_da...
ash/system/power/tray_power_date.h:28: // Overridden from SystemTrayItem
On 2012/03/01 23:58:30, Ben Goodger (Google) wrote:
> .

Done.

http://codereview.chromium.org/9562008/diff/18/chrome/browser/chromeos/dbus/p...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/9562008/diff/18/chrome/browser/chromeos/dbus/p...
chrome/browser/chromeos/dbus/power_manager_client.h:13: #if defined(USE_AURA)
On 2012/03/01 23:58:30, Ben Goodger (Google) wrote:
> USE_ASH, here and elsewhere

Done.

[sadrul, 2012-03-02 04:54:35]

+stevenjb for chromeos/dbus/OWNERS

[stevenjb, 2012-03-02 18:18:26]

lgtm

http://codereview.chromium.org/9562008/diff/12001/ash/shell.h
File ash/shell.h (right):

http://codereview.chromium.org/9562008/diff/12001/ash/shell.h#newcode289
ash/shell.h:289: // These controllers are not owner by the shell.
nit: s/owner/owned/

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
File ash/system/power/tray_power_date.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:52: position);
nit: date_string on separate line, and aligned with first arg, or all three args
on second line.

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:54: position.getEndIndex());
nit: aligin 2nd arg with 1st, or both on second line

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:95: base::kDropAmPm));
nit: alignment

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:120: &DateView::UpdateText);
nit: arg alignment

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:256: tray::DateView::DATE));
nit: alignment

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:265: SkColorSetARGB(255, 240, 240, 240)));
nit: alignment

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray.cc
File ash/system/tray/system_tray.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:80: 5, 0, 3));
nit: alignment

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:82: SkColorSetARGB(127, 0, 0, 0)));
nit: alignment

[sadrul, 2012-03-02 18:52:42]

http://codereview.chromium.org/9562008/diff/12001/ash/shell.h
File ash/shell.h (right):

http://codereview.chromium.org/9562008/diff/12001/ash/shell.h#newcode289
ash/shell.h:289: // These controllers are not owner by the shell.
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: s/owner/owned/

Done.

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
File ash/system/power/tray_power_date.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:52: position);
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: date_string on separate line, and aligned with first arg, or all three
args
> on second line.

Done. (style-guide allows both)

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:54: position.getEndIndex());
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: aligin 2nd arg with 1st, or both on second line

ditto

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:95: base::kDropAmPm));
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: alignment

Done.

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:256: tray::DateView::DATE));
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: alignment

Done.

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:265: SkColorSetARGB(255, 240, 240, 240)));
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: alignment

Done.

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray.cc
File ash/system/tray/system_tray.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:80: 5, 0, 3));
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: alignment

Done.

http://codereview.chromium.org/9562008/diff/12001/ash/system/tray/system_tray...
ash/system/tray/system_tray.cc:82: SkColorSetARGB(127, 0, 0, 0)));
On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> nit: alignment

Done.

[stevenjb, 2012-03-02 19:31:46]

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
File ash/system/power/tray_power_date.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:52: position);
On 2012/03/02 18:52:42, sadrul wrote:
> On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> > nit: date_string on separate line, and aligned with first arg, or all three
> args
> > on second line.
> 
> Done. (style-guide allows both)
The chromium style guide says "put each argument on a separate line unless
everything fits on one line":
https://sites.google.com/a/chromium.org/dev/developers/coding-style#TOC-Code-...
(This came up recently on a review of my own :) I forgot that even all args on
the second line is against the style guide, although I see that a lot)

[sadrul, 2012-03-02 19:34:54]

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
File ash/system/power/tray_power_date.cc (right):

http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
ash/system/power/tray_power_date.cc:52: position);
On 2012/03/02 19:31:46, stevenjb (chromium) wrote:
> On 2012/03/02 18:52:42, sadrul wrote:
> > On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> > > nit: date_string on separate line, and aligned with first arg, or all
three
> > args
> > > on second line.
> > 
> > Done. (style-guide allows both)
> The chromium style guide says "put each argument on a separate line unless
> everything fits on one line":
>
https://sites.google.com/a/chromium.org/dev/developers/coding-style#TOC-Code-...
> (This came up recently on a review of my own :) I forgot that even all args on
> the second line is against the style guide, although I see that a lot)

This is for function definitions. The next bullet: "When calling a function,
however, feel free to collapse things to take less space.  Do whatever seems
most readable" :)

[stevenjb, 2012-03-02 19:48:16]

On 2012/03/02 19:34:54, sadrul wrote:
>
http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
> File ash/system/power/tray_power_date.cc (right):
> 
>
http://codereview.chromium.org/9562008/diff/12001/ash/system/power/tray_power...
> ash/system/power/tray_power_date.cc:52: position);
> On 2012/03/02 19:31:46, stevenjb (chromium) wrote:
> > On 2012/03/02 18:52:42, sadrul wrote:
> > > On 2012/03/02 18:18:26, stevenjb (chromium) wrote:
> > > > nit: date_string on separate line, and aligned with first arg, or all
> three
> > > args
> > > > on second line.
> > > 
> > > Done. (style-guide allows both)
> > The chromium style guide says "put each argument on a separate line unless
> > everything fits on one line":
> >
>
https://sites.google.com/a/chromium.org/dev/developers/coding-style#TOC-Code-...
> > (This came up recently on a review of my own :) I forgot that even all args
on
> > the second line is against the style guide, although I see that a lot)
> 
> This is for function definitions. The next bullet: "When calling a function,
> however, feel free to collapse things to take less space.  Do whatever seems
> most readable" :)

Ugh, sorry, you're right, although I feel like I've had that feedback in
function calls too. Kind of wish those were consistent. I'll try to keep that in
mind though, thanks.