Move low battery notification to Message Center.

ash/system/chromeos/power/tray_power_unittest.cc

Index: ash/system/chromeos/power/tray_power_unittest.cc
diff --git a/ash/system/chromeos/power/tray_power_unittest.cc b/ash/system/chromeos/power/tray_power_unittest.cc
index 23777f7e8a28691dec9214bc14f3b33fd934ab20..def1cfeca6bb80a94dd65f36e2b0bb45a0a27189 100644
--- a/ash/system/chromeos/power/tray_power_unittest.cc
+++ b/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,9 @@ class TrayPowerTest : public test::AshTestBase {
     return tray_power_->MaybeShowUsbChargerNotification();
   }
 
-  bool UpdateNotificationState(const PowerSupplyProperties& proto) {
+  void UpdateNotificationState(const PowerSupplyProperties& proto) {
     PowerStatus::Get()->SetProtoForTesting(proto);
-    return tray_power_->UpdateNotificationState();
+    tray_power_->OnPowerStatusChanged();
   }
 
   void SetUsbChargerConnected(bool connected) {
@@ -141,14 +152,18 @@ 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));
+  UpdateNotificationState(no_battery);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());

[Mr4D (OOO till 08-26), 2015/03/17 17:04:37]

Instead of having here lots of EXPECT_EQ's - couldn't you pass
UpdateNotificationState(<property>, <expected add>, <expected remove>) and check
that the function incremented accordingly? That way it seems easier to follow
than it is now.

[michaelpg, 2015/03/17 18:32:46]

Will do, good idea.

[michaelpg, 2015/03/17 23:10:39]

Done. Added SCOPED_TRACE so the failures will at least point to the right line.
This is easier to follow and IMO a lot cleaner, although it's 5 lines instead of
4 in most cases.

+  EXPECT_EQ(0, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // No notification when calculating remaining battery time.
   PowerSupplyProperties calculating = DefaultPowerSupplyProperties();
   calculating.set_is_calculating_battery_time(true);
-  EXPECT_FALSE(UpdateNotificationState(calculating));
+  UpdateNotificationState(calculating);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(0, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // No notification when charging.
   PowerSupplyProperties charging = DefaultPowerSupplyProperties();
@@ -156,66 +171,86 @@ TEST_F(TrayPowerTest, UpdateNotificationState) {
       power_manager::PowerSupplyProperties_ExternalPower_AC);
   charging.set_battery_state(
       power_manager::PowerSupplyProperties_BatteryState_CHARGING);
-  EXPECT_FALSE(UpdateNotificationState(charging));
+  UpdateNotificationState(charging);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(0, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // 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));
+  UpdateNotificationState(low);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(0, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // 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));
+  UpdateNotificationState(low);
   EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  EXPECT_EQ(1, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // It should persist at lower values.
   low.set_battery_time_to_empty_sec(TrayPower::kLowPowerMinutes * 60 - 20);
-  EXPECT_FALSE(UpdateNotificationState(low));
+  UpdateNotificationState(low);
   EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  EXPECT_EQ(1, message_center()->add_count());
+  EXPECT_EQ(0, message_center()->remove_count());
 
   // 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));
+  UpdateNotificationState(critical);
   EXPECT_EQ(TrayPower::NOTIFICATION_CRITICAL, notification_state());
+  EXPECT_EQ(2, message_center()->add_count());
+  EXPECT_EQ(1, message_center()->remove_count());
 
   // 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));
+  UpdateNotificationState(safe);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(2, message_center()->add_count());
+  EXPECT_EQ(2, message_center()->remove_count());
 
   // 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));
+  UpdateNotificationState(low_usb);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(3, message_center()->add_count());
+  EXPECT_EQ(2, message_center()->remove_count());
 
   low_usb.set_battery_percent(TrayPower::kLowPowerPercentage + 0.49);
-  EXPECT_TRUE(UpdateNotificationState(low_usb));
+  UpdateNotificationState(low_usb);
   EXPECT_EQ(TrayPower::NOTIFICATION_LOW_POWER, notification_state());
+  EXPECT_EQ(4, message_center()->add_count());
+  EXPECT_EQ(2, message_center()->remove_count());
 
   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));
+  UpdateNotificationState(critical_usb);
   EXPECT_EQ(TrayPower::NOTIFICATION_CRITICAL, notification_state());
+  EXPECT_EQ(5, message_center()->add_count());
+  EXPECT_EQ(3, message_center()->remove_count());
 
   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));
+  UpdateNotificationState(safe_usb);
   EXPECT_EQ(TrayPower::NOTIFICATION_NONE, notification_state());
+  EXPECT_EQ(5, message_center()->add_count());
+  EXPECT_EQ(4, message_center()->remove_count());
 }
 
 }  // namespace ash