Move low battery notification to Message Center.

The low battery notification is a TrayNotificationView. This CL makes it
into a message center notification instead, giving it updated icons.

This fixes issues caused by using the outdated tray notification style.

A new BatteryNotification class is added to show, hide and update
the MessageCenter Notification. TrayPower::PowerNotificationView is
removed. PowerStatusView and PowerStatusViewTest are simplified.

BUG=163120,371364
R=skuhne@chromium.org, oshima@chromium.org

Review URL: https://codereview.chromium.org/1014753003

Cr-Commit-Position: refs/heads/master@{#321521}

ash/ash.gyp

@@ -299,6 +299,8 @@
       'system/chromeos/network/vpn_delegate.h',
       'system/chromeos/network/vpn_list_view.cc',
       'system/chromeos/network/vpn_list_view.h',
+      'system/chromeos/power/battery_notification.cc',
+      'system/chromeos/power/battery_notification.h',
       'system/chromeos/power/power_event_observer.cc',
       'system/chromeos/power/power_event_observer.h',
       'system/chromeos/power/power_status.cc',

ash/resources/ash_resources.grd

@@ -105,6 +105,9 @@
 
       <!-- ChromeOS specific icons -->
       <if expr="chromeos">
+        <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_BATTERY_CRITICAL" file="cros/notification/notification_battery_critical_icon.png" />
+        <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_BATTERY_FLUCTUATING" file="cros/notification/notification_battery_fluctuating_icon.png" />
+        <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_BATTERY_LOW" file="cros/notification/notification_battery_low_icon.png" />
         <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_BLUETOOTH" file="cros/notification/notification_bluetooth_icon.png" />
         <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_DISPLAY" file="cros/notification/display_notification_icon.png" />
         <structure type="chrome_scaled_image" name="IDR_AURA_NOTIFICATION_LOW_POWER_CHARGER" file="cros/notification/notification_low_power_charger.png" />

ash/system/chromeos/power/battery_notification.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/chromeos/power/battery_notification.h"
+
+#include "ash/system/chromeos/power/power_status.h"
+#include "ash/system/system_notifier.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
+#include "grit/ash_resources.h"
+#include "grit/ash_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/l10n/time_format.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification.h"
+
+using message_center::MessageCenter;
+using message_center::Notification;
+
+namespace ash {
+
+namespace {
+
+const char kBatteryNotificationId[] = "battery";
+
+gfx::Image& GetBatteryImage(TrayPower::NotificationState notification_state) {
+  int resource_id;
+  if (PowerStatus::Get()->IsUsbChargerConnected()) {
+    resource_id = IDR_AURA_NOTIFICATION_BATTERY_FLUCTUATING;
+  } else if (notification_state == TrayPower::NOTIFICATION_LOW_POWER) {
+    resource_id = IDR_AURA_NOTIFICATION_BATTERY_LOW;
+  } else if (notification_state == TrayPower::NOTIFICATION_CRITICAL) {
+    resource_id = IDR_AURA_NOTIFICATION_BATTERY_CRITICAL;
+  } else {
+    NOTREACHED();
+    resource_id = 0;
+  }
+
+  return ui::ResourceBundle::GetSharedInstance().GetImageNamed(resource_id);
+}
+
+scoped_ptr<Notification> CreateNotification(
+    TrayPower::NotificationState notification_state) {
+  const PowerStatus& status = *PowerStatus::Get();
+
+  base::string16 message = l10n_util::GetStringFUTF16(
+      IDS_ASH_STATUS_TRAY_BATTERY_PERCENT,
+      base::IntToString16(status.GetRoundedBatteryPercent()));
+
+  const base::TimeDelta time = status.IsBatteryCharging()
+                                   ? status.GetBatteryTimeToFull()
+                                   : status.GetBatteryTimeToEmpty();
+  base::string16 time_message;
+  if (status.IsUsbChargerConnected()) {
+    time_message = l10n_util::GetStringUTF16(
+        IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE);
+  } else if (PowerStatus::ShouldDisplayBatteryTime(time) &&
+             !status.IsBatteryDischargingOnLinePower()) {
+    int hour = 0, min = 0;
+    PowerStatus::SplitTimeIntoHoursAndMinutes(time, &hour, &min);
+    if (status.IsBatteryCharging()) {
+      time_message = l10n_util::GetStringFUTF16(
+          IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL,
+          base::IntToString16(hour), base::IntToString16(min));
+    } else {
+      // This is a low battery warning prompting the user in minutes.
+      time_message = ui::TimeFormat::Simple(
+          ui::TimeFormat::FORMAT_REMAINING, ui::TimeFormat::LENGTH_LONG,
+          base::TimeDelta::FromMinutes(hour * 60 + min));
+    }
+  }
+
+  if (!time_message.empty())
+    message = message + base::ASCIIToUTF16("\n") + time_message;
+
+  scoped_ptr<Notification> notification(new Notification(
+      message_center::NOTIFICATION_TYPE_SIMPLE, kBatteryNotificationId,
+      base::string16(), message, GetBatteryImage(notification_state),
+      base::string16(),
+      message_center::NotifierId(message_center::NotifierId::SYSTEM_COMPONENT,
+                                 system_notifier::kNotifierBattery),
+      message_center::RichNotificationData(), NULL));
+  notification->SetSystemPriority();
+  return notification;
+}
+
+}  // namespace
+
+BatteryNotification::BatteryNotification(
+    MessageCenter* message_center,
+    TrayPower::NotificationState notification_state)
+    : message_center_(message_center) {
+  message_center_->AddNotification(
+      CreateNotification(notification_state).Pass());
+}
+
+BatteryNotification::~BatteryNotification() {
+  if (message_center_->FindVisibleNotificationById(kBatteryNotificationId))
+    message_center_->RemoveNotification(kBatteryNotificationId, false);
+}
+
+void BatteryNotification::Update(
+    TrayPower::NotificationState notification_state) {
+  if (message_center_->FindVisibleNotificationById(kBatteryNotificationId)) {
+    message_center_->UpdateNotification(
+        kBatteryNotificationId, CreateNotification(notification_state).Pass());
+  }
+}
+
+}  // namespace ash

ash/system/chromeos/power/battery_notification.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_SYSTEM_CHROMEOS_POWER_BATTERY_NOTIFICATION_H_
+#define ASH_SYSTEM_CHROMEOS_POWER_BATTERY_NOTIFICATION_H_
+
+#include "ash/ash_export.h"
+#include "ash/system/chromeos/power/tray_power.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace message_center {
+class MessageCenter;
+}
+
+namespace ash {
+
+// Class for showing and hiding a MessageCenter low battery notification.
+class ASH_EXPORT BatteryNotification {
+ public:
+  BatteryNotification(message_center::MessageCenter* message_center,
+                      TrayPower::NotificationState notification_state);
+  ~BatteryNotification();
+
+  // Updates the notification if it still exists.
+  void Update(TrayPower::NotificationState notification_state);
+
+ private:
+  message_center::MessageCenter* message_center_;
+
+  DISALLOW_COPY_AND_ASSIGN(BatteryNotification);
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_CHROMEOS_POWER_BATTERY_NOTIFICATION_H_

ash/system/chromeos/power/power_status_view.cc

@@ -26,26 +26,14 @@ namespace ash {
 // Padding between battery status text and battery icon on default view.
 const int kPaddingBetweenBatteryStatusAndIcon = 3;
 
-PowerStatusView::PowerStatusView(ViewType view_type,
-                                 bool default_view_right_align)
+PowerStatusView::PowerStatusView(bool default_view_right_align)
     : default_view_right_align_(default_view_right_align),
-      status_label_(NULL),
-      time_label_(NULL),
-      time_status_label_(NULL),
-      percentage_label_(NULL),
-      icon_(NULL),
-      view_type_(view_type) {
+      time_status_label_(new views::Label),
+      percentage_label_(new views::Label),
+      icon_(NULL) {
   PowerStatus::Get()->AddObserver(this);
-  if (view_type == VIEW_DEFAULT) {
-    time_status_label_ = new views::Label;
-    percentage_label_ = new views::Label;
-    percentage_label_->SetEnabledColor(kHeaderTextColorNormal);
-    LayoutDefaultView();
-  } else {
-    status_label_ = new views::Label;
-    time_label_ = new views::Label;
-    LayoutNotificationView();
-  }
+  percentage_label_->SetEnabledColor(kHeaderTextColorNormal);
+  LayoutView();
   OnPowerStatusChanged();
 }
 
@@ -54,8 +42,7 @@ PowerStatusView::~PowerStatusView() {
 }
 
 void PowerStatusView::OnPowerStatusChanged() {
-  view_type_ == VIEW_DEFAULT ?
-      UpdateTextForDefaultView() : UpdateTextForNotificationView();
+  UpdateText();
 
   if (icon_) {
     icon_->SetImage(
@@ -64,7 +51,7 @@ void PowerStatusView::OnPowerStatusChanged() {
   }
 }
 
-void PowerStatusView::LayoutDefaultView() {
+void PowerStatusView::LayoutView() {
   if (default_view_right_align_) {
     views::BoxLayout* layout =
         new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0,
@@ -91,17 +78,7 @@ void PowerStatusView::LayoutDefaultView() {
   }
 }
 
-void PowerStatusView::LayoutNotificationView() {
-  SetLayoutManager(
-      new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 1));
-  status_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  AddChildView(status_label_);
-
-  time_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  AddChildView(time_label_);
-}
-
-void PowerStatusView::UpdateTextForDefaultView() {
+void PowerStatusView::UpdateText() {
   const PowerStatus& status = *PowerStatus::Get();
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   base::string16 battery_percentage;
@@ -148,51 +125,6 @@ void PowerStatusView::UpdateTextForDefaultView() {
   time_status_label_->SetText(battery_time_status);
 }
 
-void PowerStatusView::UpdateTextForNotificationView() {
-  const PowerStatus& status = *PowerStatus::Get();
-  if (status.IsBatteryFull()) {
-    status_label_->SetText(
-        ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
-            IDS_ASH_STATUS_TRAY_BATTERY_FULL));
-  } else {
-    status_label_->SetText(
-        l10n_util::GetStringFUTF16(
-            IDS_ASH_STATUS_TRAY_BATTERY_PERCENT,
-            base::IntToString16(status.GetRoundedBatteryPercent())));
-  }
-
-  const base::TimeDelta time = status.IsBatteryCharging() ?
-      status.GetBatteryTimeToFull() : status.GetBatteryTimeToEmpty();
-
-  if (status.IsUsbChargerConnected()) {
-    time_label_->SetText(
-        ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
-            IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE));
-  } else if (status.IsBatteryTimeBeingCalculated()) {
-    time_label_->SetText(
-        ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
-            IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING));
-  } else if (PowerStatus::ShouldDisplayBatteryTime(time) &&
-             !status.IsBatteryDischargingOnLinePower()) {
-    int hour = 0, min = 0;
-    PowerStatus::SplitTimeIntoHoursAndMinutes(time, &hour, &min);
-    if (status.IsBatteryCharging()) {
-      time_label_->SetText(
-          l10n_util::GetStringFUTF16(
-              IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL,
-              base::IntToString16(hour),
-              base::IntToString16(min)));
-    } else {
-      // This is a low battery warning prompting the user in minutes.
-      time_label_->SetText(ui::TimeFormat::Simple(
-          ui::TimeFormat::FORMAT_REMAINING, ui::TimeFormat::LENGTH_LONG,
-          base::TimeDelta::FromMinutes(hour * 60 + min)));
-    }
-  } else {
-    time_label_->SetText(base::string16());
-  }
-}
-
 void PowerStatusView::ChildPreferredSizeChanged(views::View* child) {
   PreferredSizeChanged();
 }

ash/system/chromeos/power/power_status_view.h

@@ -19,12 +19,7 @@ namespace ash {
 class ASH_EXPORT PowerStatusView : public views::View,
                                    public PowerStatus::Observer {
  public:
-  enum ViewType {
-    VIEW_DEFAULT,
-    VIEW_NOTIFICATION
-  };
-
-  PowerStatusView(ViewType view_type, bool default_view_right_align);
+  PowerStatusView(bool default_view_right_align);
   ~PowerStatusView() override;
 
   // Overridden from views::View.
@@ -36,13 +31,10 @@ class ASH_EXPORT PowerStatusView : public views::View,
   void OnPowerStatusChanged() override;
 
  private:
-  friend class PowerStatusDefaultViewTest;
-  friend class PowerStatusNotificationViewTest;
+  friend class PowerStatusViewTest;
 
-  void LayoutDefaultView();
-  void LayoutNotificationView();
-  void UpdateTextForDefaultView();
-  void UpdateTextForNotificationView();
+  void LayoutView();
+  void UpdateText();
 
   // Overridden from views::View.
   void ChildPreferredSizeChanged(views::View* child) override;
@@ -51,19 +43,12 @@ class ASH_EXPORT PowerStatusView : public views::View,
   // if true; otherwise, layout the UI items on the left side.
   bool default_view_right_align_;
 
-  // Labels used only for VIEW_NOTIFICATION.
-  views::Label* status_label_;
-  views::Label* time_label_;
-
-  // Labels used only for VIEW_DEFAULT.
   views::Label* time_status_label_;
   views::Label* percentage_label_;
 
   // Battery status indicator icon.
   views::ImageView* icon_;
 
-  ViewType view_type_;
-
   DISALLOW_COPY_AND_ASSIGN(PowerStatusView);
 };
 

ash/system/chromeos/power/power_status_view_unittest.cc

@@ -24,7 +24,7 @@ class PowerStatusViewTest : public test::AshTestBase {
   // Overridden from testing::Test:
   void SetUp() override {
     test::AshTestBase::SetUp();
-    view_.reset(new PowerStatusView(GetViewType(), false));
+    view_.reset(new PowerStatusView(false));
   }
 
   void TearDown() override {
@@ -33,69 +33,30 @@ class PowerStatusViewTest : public test::AshTestBase {
   }
 
  protected:
-  virtual PowerStatusView::ViewType GetViewType() = 0;
-  PowerStatusView* view() { return view_.get(); }
-
   void UpdatePowerStatus(const power_manager::PowerSupplyProperties& proto) {
     PowerStatus::Get()->SetProtoForTesting(proto);
     view_->OnPowerStatusChanged();
   }
 
- private:
-  scoped_ptr<PowerStatusView> view_;
-
-  DISALLOW_COPY_AND_ASSIGN(PowerStatusViewTest);
-};
-
-class PowerStatusDefaultViewTest : public PowerStatusViewTest {
- public:
-  PowerStatusDefaultViewTest() {}
-  ~PowerStatusDefaultViewTest() override {}
-
- protected:
-  PowerStatusView::ViewType GetViewType() override {
-    return PowerStatusView::VIEW_DEFAULT;
-  }
-
-  bool IsPercentageVisible() {
-    return view()->percentage_label_->visible();
+  bool IsPercentageVisible() const {
+    return view_->percentage_label_->visible();
   }
 
-  bool IsTimeStatusVisible() {
-    return view()->time_status_label_->visible();
+  bool IsTimeStatusVisible() const {
+    return view_->time_status_label_->visible();
   }
 
-  base::string16 RemainingTimeInView() {
-    return view()->time_status_label_->text();
+  base::string16 RemainingTimeInView() const {
+    return view_->time_status_label_->text();
   }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(PowerStatusDefaultViewTest);
-};
-
-class PowerStatusNotificationViewTest : public PowerStatusViewTest {
- public:
-  PowerStatusNotificationViewTest() {}
-  ~PowerStatusNotificationViewTest() override {}
-
- protected:
-  PowerStatusView::ViewType GetViewType() override {
-    return PowerStatusView::VIEW_NOTIFICATION;
-  }
-
-  base::string16 StatusInView() {
-    return view()->status_label_->text();
-  }
-
-  base::string16 RemainingTimeInView() {
-    return view()->time_label_->text();
-  }
+  scoped_ptr<PowerStatusView> view_;
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(PowerStatusNotificationViewTest);
+  DISALLOW_COPY_AND_ASSIGN(PowerStatusViewTest);
 };
 
-TEST_F(PowerStatusDefaultViewTest, Basic) {
+TEST_F(PowerStatusViewTest, Basic) {
   EXPECT_FALSE(IsPercentageVisible());
   EXPECT_TRUE(IsTimeStatusVisible());
 
@@ -154,61 +115,4 @@ TEST_F(PowerStatusDefaultViewTest, Basic) {
   EXPECT_FALSE(IsTimeStatusVisible());
 }
 
-TEST_F(PowerStatusNotificationViewTest, Basic) {
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_BATTERY_FULL),
-            StatusInView());
-  EXPECT_TRUE(RemainingTimeInView().empty());
-
-  // Disconnect the power.
-  PowerSupplyProperties prop;
-  prop.set_external_power(PowerSupplyProperties::DISCONNECTED);
-  prop.set_battery_state(PowerSupplyProperties::DISCHARGING);
-  prop.set_battery_percent(99.0);
-  prop.set_battery_time_to_empty_sec(125);
-  prop.set_is_calculating_battery_time(true);
-  UpdatePowerStatus(prop);
-
-  EXPECT_NE(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_BATTERY_FULL),
-            StatusInView());
-  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING),
-            RemainingTimeInView());
-
-  prop.set_is_calculating_battery_time(false);
-  UpdatePowerStatus(prop);
-  // Low power warning has to be calculated by ui::TimeFormat, but ignore
-  // seconds.
-  EXPECT_EQ(ui::TimeFormat::Simple(ui::TimeFormat::FORMAT_REMAINING,
-                                   ui::TimeFormat::LENGTH_LONG,
-                                   base::TimeDelta::FromMinutes(2)),
-            RemainingTimeInView());
-
-  prop.set_external_power(PowerSupplyProperties::AC);
-  prop.set_battery_state(PowerSupplyProperties::CHARGING);
-  prop.set_battery_time_to_full_sec(120);
-  UpdatePowerStatus(prop);
-  EXPECT_NE(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_BATTERY_FULL),
-            StatusInView());
-  // Charging time is somehow using another format?
-  EXPECT_NE(ui::TimeFormat::Simple(ui::TimeFormat::FORMAT_REMAINING,
-                                   ui::TimeFormat::LENGTH_LONG,
-                                   base::TimeDelta::FromMinutes(2)),
-            RemainingTimeInView());
-
-  // Unreliable connection.
-  prop.set_external_power(PowerSupplyProperties::USB);
-  UpdatePowerStatus(prop);
-  EXPECT_EQ(
-      l10n_util::GetStringUTF16(
-          IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE),
-      RemainingTimeInView());
-
-  // Tricky -- connected to non-USB but still discharging. Not likely happening
-  // on production though.
-  prop.set_external_power(PowerSupplyProperties::AC);
-  prop.set_battery_state(PowerSupplyProperties::DISCHARGING);
-  prop.set_battery_time_to_full_sec(120);
-  UpdatePowerStatus(prop);
-  EXPECT_TRUE(RemainingTimeInView().empty());
-}
-
 }  // namespace ash

ash/system/chromeos/power/tray_power.cc

@@ -7,32 +7,23 @@
 #include "ash/accessibility_delegate.h"
 #include "ash/ash_switches.h"
 #include "ash/shell.h"
-#include "ash/system/chromeos/power/power_status_view.h"
+#include "ash/system/chromeos/power/battery_notification.h"
 #include "ash/system/date/date_view.h"
 #include "ash/system/system_notifier.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/tray_constants.h"
-#include "ash/system/tray/tray_notification_view.h"
 #include "ash/system/tray/tray_utils.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
-#include "third_party/icu/source/i18n/unicode/fieldpos.h"
-#include "third_party/icu/source/i18n/unicode/fmtable.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/notification.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/image_view.h"
-#include "ui/views/controls/label.h"
-#include "ui/views/layout/box_layout.h"
-#include "ui/views/layout/fill_layout.h"
-#include "ui/views/layout/grid_layout.h"
 #include "ui/views/view.h"
-#include "ui/views/widget/widget.h"
 
 using message_center::MessageCenter;
 using message_center::Notification;
@@ -75,29 +66,8 @@ class PowerTrayView : public views::ImageView {
   DISALLOW_COPY_AND_ASSIGN(PowerTrayView);
 };
 
-class PowerNotificationView : public TrayNotificationView {
- public:
-  explicit PowerNotificationView(TrayPower* owner)
-      : TrayNotificationView(owner, 0) {
-    power_status_view_ =
-        new PowerStatusView(PowerStatusView::VIEW_NOTIFICATION, true);
-    InitView(power_status_view_);
-  }
-
-  void UpdateStatus() {
-    SetIconImage(PowerStatus::Get()->GetBatteryImage(PowerStatus::ICON_DARK));
-  }
-
- private:
-  PowerStatusView* power_status_view_;
-
-  DISALLOW_COPY_AND_ASSIGN(PowerNotificationView);
-};
-
 }  // namespace tray
 
-using tray::PowerNotificationView;
-
 const int TrayPower::kCriticalMinutes = 5;
 const int TrayPower::kLowPowerMinutes = 15;
 const int TrayPower::kNoWarningMinutes = 30;
@@ -109,7 +79,6 @@ TrayPower::TrayPower(SystemTray* system_tray, MessageCenter* message_center)
     : SystemTrayItem(system_tray),
       message_center_(message_center),
       power_tray_(NULL),
-      notification_view_(NULL),
       notification_state_(NOTIFICATION_NONE),
       usb_charger_was_connected_(false),
       line_power_was_connected_(false) {
@@ -136,17 +105,6 @@ views::View* TrayPower::CreateDefaultView(user::LoginStatus status) {
   return NULL;
 }
 
-views::View* TrayPower::CreateNotificationView(user::LoginStatus status) {
-  CHECK(notification_view_ == NULL);
-  if (!PowerStatus::Get()->IsBatteryPresent())
-    return NULL;
-
-  notification_view_ = new PowerNotificationView(this);
-  notification_view_->UpdateStatus();
-
-  return notification_view_;
-}
-
 void TrayPower::DestroyTrayView() {
   power_tray_ = NULL;
 }
@@ -154,10 +112,6 @@ void TrayPower::DestroyTrayView() {
 void TrayPower::DestroyDefaultView() {
 }
 
-void TrayPower::DestroyNotificationView() {
-  notification_view_ = NULL;
-}
-
 void TrayPower::UpdateAfterLoginStatusChange(user::LoginStatus status) {
 }
 
@@ -169,8 +123,6 @@ void TrayPower::OnPowerStatusChanged() {
   bool battery_alert = UpdateNotificationState();
   if (power_tray_)
     power_tray_->UpdateStatus(battery_alert);
-  if (notification_view_)
-    notification_view_->UpdateStatus();
 
   // Factory testing may place the battery into unusual states.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -179,10 +131,18 @@ void TrayPower::OnPowerStatusChanged() {
 
   MaybeShowUsbChargerNotification();
 
-  if (battery_alert)
-    ShowNotificationView();
-  else if (notification_state_ == NOTIFICATION_NONE)
-    HideNotificationView();
+  if (battery_alert) {
+    // Remove any existing notification so it's dismissed before adding a new
+    // one. Otherwise we might update a "low battery" notification to "critical"
+    // without it being shown again.
+    battery_notification_.reset();
+    battery_notification_.reset(
+        new BatteryNotification(message_center_, notification_state_));
+  } else if (notification_state_ == NOTIFICATION_NONE) {
+    battery_notification_.reset();
+  } else if (battery_notification_.get()) {
+    battery_notification_->Update(notification_state_);
+  }
 
   usb_charger_was_connected_ = PowerStatus::Get()->IsUsbChargerConnected();
   line_power_was_connected_ = PowerStatus::Get()->IsLinePowerConnected();

ash/system/chromeos/power/tray_power.h

@@ -20,15 +20,16 @@ class MessageCenter;
 }
 
 namespace ash {
+
+class BatteryNotification;
+
 namespace tray {
-class PowerNotificationView;
 class PowerTrayView;
 }
 
 class ASH_EXPORT TrayPower : public SystemTrayItem,
                              public PowerStatus::Observer {
  public:
-  // Visible for testing.
   enum NotificationState {
     NOTIFICATION_NONE,
 
@@ -70,10 +71,8 @@ class ASH_EXPORT TrayPower : public SystemTrayItem,
   // Overridden from SystemTrayItem.
   views::View* CreateTrayView(user::LoginStatus status) override;
   views::View* CreateDefaultView(user::LoginStatus status) override;
-  views::View* CreateNotificationView(user::LoginStatus status) override;
   void DestroyTrayView() override;
   void DestroyDefaultView() override;
-  void DestroyNotificationView() override;
   void UpdateAfterLoginStatusChange(user::LoginStatus status) override;
   void UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) override;
 
@@ -91,7 +90,7 @@ class ASH_EXPORT TrayPower : public SystemTrayItem,
 
   message_center::MessageCenter* message_center_;  // Not owned.
   tray::PowerTrayView* power_tray_;
-  tray::PowerNotificationView* notification_view_;
+  scoped_ptr<BatteryNotification> battery_notification_;
   NotificationState notification_state_;
 
   // Was a USB charger connected the last time OnPowerStatusChanged() was

ash/system/chromeos/power/tray_power_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "ash/system/chromeos/power/tray_power.h"
 
+#include <map>
+#include <string>
+
 #include "ash/ash_switches.h"
 #include "ash/test/ash_test_base.h"
 #include "base/memory/scoped_ptr.h"
@@ -26,14 +29,22 @@ class MockMessageCenter : public message_center::FakeMessageCenter {
   // message_center::FakeMessageCenter overrides:
   void AddNotification(scoped_ptr<Notification> notification) override {
     add_count_++;
+    notifications_[notification->id()] = notification.get();
   }
   void RemoveNotification(const std::string& id, bool by_user) override {
     remove_count_++;
+    notifications_.erase(id);
+  }
+
+  Notification* FindVisibleNotificationById(const std::string& id) override {
+    auto it = notifications_.find(id);
+    return it == notifications_.end() ? NULL : it->second;
   }
 
  private:
   int add_count_;
   int remove_count_;
+  std::map<std::string, Notification*> notifications_;
 
   DISALLOW_COPY_AND_ASSIGN(MockMessageCenter);
 };
@@ -72,9 +83,18 @@ class TrayPowerTest : public test::AshTestBase {
     return tray_power_->MaybeShowUsbChargerNotification();
   }
 
-  bool UpdateNotificationState(const PowerSupplyProperties& proto) {
+  void UpdateNotificationState(const PowerSupplyProperties& proto,
+                               TrayPower::NotificationState expected_state,
+                               bool expected_add,
+                               bool expected_remove) {
+    int prev_add = message_center_->add_count();
+    int prev_remove = message_center_->remove_count();
     PowerStatus::Get()->SetProtoForTesting(proto);
-    return tray_power_->UpdateNotificationState();
+    tray_power_->OnPowerStatusChanged();
+    EXPECT_EQ(expected_state, notification_state());
+    EXPECT_EQ(expected_add, message_center_->add_count() == prev_add + 1);
+    EXPECT_EQ(expected_remove,
+              message_center_->remove_count() == prev_remove + 1);
   }
 
   void SetUsbChargerConnected(bool connected) {
@@ -141,14 +161,20 @@ TEST_F(TrayPowerTest, UpdateNotificationState) {
       power_manager::PowerSupplyProperties_ExternalPower_AC);
   no_battery.set_battery_state(
       power_manager::PowerSupplyProperties_BatteryState_NOT_PRESENT);
-  EXPECT_FALSE(UpdateNotificationState(no_battery));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("No notifications when no battery present");
+    UpdateNotificationState(no_battery, TrayPower::NOTIFICATION_NONE, false,
+                            false);
+  }
 
   // No notification when calculating remaining battery time.
   PowerSupplyProperties calculating = DefaultPowerSupplyProperties();
   calculating.set_is_calculating_battery_time(true);
-  EXPECT_FALSE(UpdateNotificationState(calculating));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("No notification when calculating remaining battery time");
+    UpdateNotificationState(calculating, TrayPower::NOTIFICATION_NONE, false,
+                            false);
+  }
 
   // No notification when charging.
   PowerSupplyProperties charging = DefaultPowerSupplyProperties();
@@ -156,66 +182,93 @@ TEST_F(TrayPowerTest, UpdateNotificationState) {
       power_manager::PowerSupplyProperties_ExternalPower_AC);
   charging.set_battery_state(
       power_manager::PowerSupplyProperties_BatteryState_CHARGING);
-  EXPECT_FALSE(UpdateNotificationState(charging));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("No notification when charging");
+    UpdateNotificationState(charging, TrayPower::NOTIFICATION_NONE, false,
+                            false);
+  }
 
   // When the rounded minutes-to-empty are above the threshold, no notification
   // should be shown.
   PowerSupplyProperties low = DefaultPowerSupplyProperties();
   low.set_battery_time_to_empty_sec(TrayPower::kLowPowerMinutes * 60 + 30);
-  EXPECT_FALSE(UpdateNotificationState(low));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("No notification when time to empty above threshold");
+    UpdateNotificationState(low, TrayPower::NOTIFICATION_NONE, false, false);
+  }
 
   // When the rounded value matches the threshold, the notification should
   // appear.
   low.set_battery_time_to_empty_sec(TrayPower::kLowPowerMinutes * 60 + 29);
-  EXPECT_TRUE(UpdateNotificationState(low));
-  EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  {
+    SCOPED_TRACE("Notification when time to empty matches threshold");
+    UpdateNotificationState(low, TrayPower::NOTIFICATION_LOW_POWER, true,
+                            false);
+  }
 
   // It should persist at lower values.
   low.set_battery_time_to_empty_sec(TrayPower::kLowPowerMinutes * 60 - 20);
-  EXPECT_FALSE(UpdateNotificationState(low));
-  EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  {
+    SCOPED_TRACE("Notification persists at lower values");
+    UpdateNotificationState(low, TrayPower::NOTIFICATION_LOW_POWER, false,
+                            false);
+  }
 
   // The critical low battery notification should be shown when the rounded
   // value is at the lower threshold.
   PowerSupplyProperties critical = DefaultPowerSupplyProperties();
   critical.set_battery_time_to_empty_sec(TrayPower::kCriticalMinutes * 60 + 29);
-  EXPECT_TRUE(UpdateNotificationState(critical));
-  EXPECT_EQ(TrayPower::NOTIFICATION_CRITICAL, notification_state());
+  {
+    SCOPED_TRACE("Critical notification when time to empty is critical");
+    UpdateNotificationState(critical, TrayPower::NOTIFICATION_CRITICAL, true,
+                            true);
+  }
 
   // The notification should be dismissed when the no-warning threshold is
   // reached.
   PowerSupplyProperties safe = DefaultPowerSupplyProperties();
   safe.set_battery_time_to_empty_sec(TrayPower::kNoWarningMinutes * 60 - 29);
-  EXPECT_FALSE(UpdateNotificationState(safe));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("Notification removed when battery not low");
+    UpdateNotificationState(safe, TrayPower::NOTIFICATION_NONE, false, true);
+  }
 
   // Test that rounded percentages are used when a USB charger is connected.
   PowerSupplyProperties low_usb = DefaultPowerSupplyProperties();
   low_usb.set_external_power(
       power_manager::PowerSupplyProperties_ExternalPower_USB);
   low_usb.set_battery_percent(TrayPower::kLowPowerPercentage + 0.5);
-  EXPECT_FALSE(UpdateNotificationState(low_usb));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("No notification for rounded battery percent");
+    UpdateNotificationState(low_usb, TrayPower::NOTIFICATION_NONE, true, false);
+  }
 
   low_usb.set_battery_percent(TrayPower::kLowPowerPercentage + 0.49);
-  EXPECT_TRUE(UpdateNotificationState(low_usb));
-  EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  {
+    SCOPED_TRACE("Notification for rounded low power percent");
+    UpdateNotificationState(low_usb, TrayPower::NOTIFICATION_LOW_POWER, true,
+                            false);
+  }
 
   PowerSupplyProperties critical_usb = DefaultPowerSupplyProperties();
   critical_usb.set_external_power(
       power_manager::PowerSupplyProperties_ExternalPower_USB);
   critical_usb.set_battery_percent(TrayPower::kCriticalPercentage + 0.2);
-  EXPECT_TRUE(UpdateNotificationState(critical_usb));
-  EXPECT_EQ(TrayPower::NOTIFICATION_CRITICAL, notification_state());
+  {
+    SCOPED_TRACE("Notification for rounded critical power percent");
+    UpdateNotificationState(critical_usb, TrayPower::NOTIFICATION_CRITICAL,
+                            true, true);
+  }
 
   PowerSupplyProperties safe_usb = DefaultPowerSupplyProperties();
   safe_usb.set_external_power(
       power_manager::PowerSupplyProperties_ExternalPower_USB);
   safe_usb.set_battery_percent(TrayPower::kNoWarningPercentage - 0.1);
-  EXPECT_FALSE(UpdateNotificationState(safe_usb));
-  EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  {
+    SCOPED_TRACE("Notification removed for rounded percent above threshold");
+    UpdateNotificationState(safe_usb, TrayPower::NOTIFICATION_NONE, false,
+                            true);
+  }
 }
 
 }  // namespace ash

ash/system/chromeos/settings/tray_settings.cc

@@ -64,8 +64,7 @@ class SettingsDefaultView : public ActionableView,
     }
 
     if (PowerStatus::Get()->IsBatteryPresent()) {
-      power_status_view_ = new ash::PowerStatusView(
-          ash::PowerStatusView::VIEW_DEFAULT, power_view_right_align);
+      power_status_view_ = new ash::PowerStatusView(power_view_right_align);
       AddChildView(power_status_view_);
       OnPowerStatusChanged();
     }

ash/system/system_notifier.cc

@@ -18,16 +18,16 @@ namespace {
 // See http://dev.chromium.org/chromium-os/chromiumos-design-docs/
 // system-notifications for the reasoning.
 const char* kAlwaysShownNotifierIds[] = {
-  kNotifierDisplay,
-  kNotifierDisplayError,
+    kNotifierBattery,
+    kNotifierDisplay,
+    kNotifierDisplayError,
 #if defined(OS_CHROMEOS)
-  ui::NetworkStateNotifier::kNotifierNetworkError,
+    ui::NetworkStateNotifier::kNotifierNetworkError,
 #endif
-  kNotifierPower,
-  // Note: Order doesn't matter here, so keep this in alphabetic order, don't
-  // just add your stuff at the end!
-  NULL
-};
+    kNotifierPower,
+    // Note: Order doesn't matter here, so keep this in alphabetic order, don't
+    // just add your stuff at the end!
+    NULL};
 
 const char* kAshSystemNotifiers[] = {
   kNotifierBluetooth,
@@ -66,6 +66,7 @@ bool MatchSystemNotifierId(const message_center::NotifierId& notifier_id,
 
 }  // namespace
 
+const char kNotifierBattery[] = "ash.battery";
 const char kNotifierBluetooth[] = "ash.bluetooth";
 const char kNotifierDisplay[] = "ash.display";
 const char kNotifierDisplayError[] = "ash.display.error";

ash/system/system_notifier.h

@@ -14,6 +14,7 @@ namespace ash {
 namespace system_notifier {
 
 // The list of ash system notifier IDs. Alphabetical order.
+ASH_EXPORT extern const char kNotifierBattery[];
 ASH_EXPORT extern const char kNotifierBluetooth[];
 ASH_EXPORT extern const char kNotifierDisplay[];
 ASH_EXPORT extern const char kNotifierDisplayResolutionChange[];