cros: Show notification when low-power charger connected

cros: Show notification when low-power charger connected

Encourage the user to plug in the official charger.

Also extract FakeMessageCenter into a message_center_test_support target
so I can use it for a unit test.

BUG=249852
TEST=ash_unittests TrayPowerTest.*

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

[James Cook, 2013-06-19 17:28:39]

derat, PTAL at ash/*

mukai, PTAL at ui/message_center/*

[Jun Mukai, 2013-06-19 17:43:31]

a few nitpicks

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:209: message_center::MessageCenter*
message_center)
you don't need "meesage_center::" since you declare using.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:443: notification.reset(new
Notification(
nit: just scoped_ptr<Notification> notification(new Notification(...)) would be
better

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:450: kNotificationId,  // extension_id
The extension_id and display_source can be empty. Those fields are actually used
for the context menu, especially for the menu item of "disable notifications
from <source>".  If they are empty, that menu item is just hidden, which seems
the preferred situation.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:452: NULL));
probably better to call notification->SetSystemPriority(), then the notification
is promoted to the system priority.

[James Cook, 2013-06-19 18:00:08]

Updated - please take another look.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:209: message_center::MessageCenter*
message_center)
On 2013/06/19 17:43:31, Jun Mukai wrote:
> you don't need "meesage_center::" since you declare using.

Done.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:443: notification.reset(new
Notification(
On 2013/06/19 17:43:31, Jun Mukai wrote:
> nit: just scoped_ptr<Notification> notification(new Notification(...)) would
be
> better

Done.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:450: kNotificationId,  // extension_id
On 2013/06/19 17:43:31, Jun Mukai wrote:
> The extension_id and display_source can be empty. Those fields are actually
used
> for the context menu, especially for the menu item of "disable notifications
> from <source>".  If they are empty, that menu item is just hidden, which seems
> the preferred situation.

Done. I might want to provide a way to turn off this notification, but we can do
that in a different CL if we decided we want it.

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:452: NULL));
On 2013/06/19 17:43:31, Jun Mukai wrote:
> probably better to call notification->SetSystemPriority(), then the
notification
> is promoted to the system priority.

I don't think I want this to be system priority -- it looks like that requires
the user to click to close the notification and I would like the notification to
close itself automatically.  Should I use RichNotificationData() to set the
priority to something other than DEFAULT_PRIORITY?

[Daniel Erat, 2013-06-19 19:48:37]

LGTM

I have a slight preference for this being turned off until
http://crosbug.com/20320 is tracked down and fixed (let me know if you have time
to help on that one :-P), but since it adds a new notification it can probably
go in before then without adding too much confusion to bug reports from users.

[Jun Mukai, 2013-06-19 21:01:56]

lgtm

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/17157007/diff/1/ash/system/chromeos/power/tra...
ash/system/chromeos/power/tray_power.cc:452: NULL));
On 2013/06/19 18:00:08, James Cook (Chromium) wrote:
> On 2013/06/19 17:43:31, Jun Mukai wrote:
> > probably better to call notification->SetSystemPriority(), then the
> notification
> > is promoted to the system priority.
> 
> I don't think I want this to be system priority -- it looks like that requires
> the user to click to close the notification and I would like the notification
to
> close itself automatically.  Should I use RichNotificationData() to set the
> priority to something other than DEFAULT_PRIORITY?

Then, that's fine. system priority was added to notify special important things
and that's why it doesn't have timeout, and I just thought low-battery alert
would be important as such.
You can specify HIGH or MAX priority instead, then timeout will get longer to 25
secs instead of default 7 secs, but it's up to you.

[James Cook, 2013-06-19 22:30:09]

> Then, that's fine. system priority was added to notify special important
things
> and that's why it doesn't have timeout, and I just thought low-battery alert
> would be important as such.
> You can specify HIGH or MAX priority instead, then timeout will get longer to
25
> secs instead of default 7 secs, but it's up to you.

I talked to Alex - we're going to start with 7 seconds. If that's not enough
I'll increase to HIGH or MAX priority in a later CL.

[commit-bot: I haz the power, 2013-06-20 01:51:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamescook@chromium.org/17157007/7001

[commit-bot: I haz the power, 2013-06-20 09:19:13]

Change committed as 207374

[James Cook, 2013-06-20 15:54:08]

On 2013/06/20 09:19:13, I haz the power (commit-bot) wrote:
> Change committed as 207374

Broke Win Aura compile:
http://build.chromium.org/p/chromium.win/builders/Win%20Aura%20Builder/builds...