chromeos: Update power status code.

This updates ash's power-status code to deal with updated
PowerSupplyProperties sent by the power manager where:

a) the battery time-to-full estimate is set when the battery
   is charging and time-to-empty is set when the battery is
   discharging (previously, time-to-full was set whenever a
   charger was connected)

b) negative battery time estimates are used in situations
   where the current is close to zero, which would result in
   a huge estimate

It also updates ash to not display estimates above one day
and to not show an estimate when the battery is discharging
while a charger is connected.

Finally, it makes some minor improvements to battery-time
accessibility strings.

BUG=252948,254173,chrome-os-partner:20343

Review URL: https://chromiumcodereview.appspot.com/18324014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210762 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 167 insertions, 66 deletions

diff --git a/ash/ash_strings.grd b/ash/ash_strings.grd
index a49e9bc..1265019 100644
--- a/ash/ash_strings.grd
+++ b/ash/ash_strings.grd
@@ -449,14 +449,14 @@ Press Shift + Alt to switch.
       <message name="IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE" desc="The label in the tray dialog to indicate that battery charging is unreliable.">
         Low-power charger
       </message>
-      <message name="IDS_ASH_STATUS_TRAY_BATTERY_FULL_CHARGE_ACCESSIBLE" desc="The message used by accessibility to show battery is full and charging.">
-        Battery is full and charging.
+      <message name="IDS_ASH_STATUS_TRAY_BATTERY_FULL_CHARGE_ACCESSIBLE" desc="The message used by accessibility to show battery is fully charged.">
+        Battery is full.
       </message>
-      <message name="IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_ACCESSIBLE" desc="The message used by accessibility to show battery is calculating its time.">
-        Battery is <ph name="percentage">$1<ex>56</ex></ph>% full
+      <message name="IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_ACCESSIBLE" desc="The message used by accessibility to show battery is discharging.">
+        Battery is <ph name="percentage">$1<ex>56</ex></ph>% full.
       </message>
-      <message name="IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_CHARGING_ACCESSIBLE" desc="The message used by accessibility to show battery is calculating its time.">
-        Battery is <ph name="percentage">$1<ex>56</ex></ph>% full and charging
+      <message name="IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_CHARGING_ACCESSIBLE" desc="The message used by accessibility to show battery is being charged.">
+        Battery is <ph name="percentage">$1<ex>56</ex></ph>% full and charging.
       </message>
       <message name="IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING_ACCESSIBLE" desc="The message used by accessibility to show battery is calculating its time in short message.">
         Calculating battery time.
diff --git a/ash/system/chromeos/power/power_status.cc b/ash/system/chromeos/power/power_status.cc
index 7982aa02..a9f4dbf 100644
--- a/ash/system/chromeos/power/power_status.cc
+++ b/ash/system/chromeos/power/power_status.cc
@@ -76,6 +76,8 @@ const int kNumPowerImages = 15;
 
 }  // namespace
 
+const int PowerStatus::kMaxBatteryTimeToDisplaySec = 24 * 60 * 60;
+
 // static
 void PowerStatus::Initialize() {
   CHECK(!g_power_status);
@@ -100,6 +102,22 @@ PowerStatus* PowerStatus::Get() {
   return g_power_status;
 }
 
+// static
+bool PowerStatus::ShouldDisplayBatteryTime(const base::TimeDelta& time) {
+  return time >= base::TimeDelta::FromMinutes(1) &&
+      time.InSeconds() <= kMaxBatteryTimeToDisplaySec;
+}
+
+// static
+void PowerStatus::SplitTimeIntoHoursAndMinutes(const base::TimeDelta& time,
+                                               int* hours,
+                                               int* minutes) {
+  DCHECK(hours);
+  DCHECK(minutes);
+  *hours = time.InHours();
+  *minutes = (time - base::TimeDelta::FromHours(*hours)).InMinutes();
+}
+
 void PowerStatus::AddObserver(Observer* observer) {
   DCHECK(observer);
   observers_.AddObserver(observer);
@@ -125,6 +143,16 @@ bool PowerStatus::IsBatteryFull() const {
       power_manager::PowerSupplyProperties_BatteryState_FULL;
 }
 
+bool PowerStatus::IsBatteryCharging() const {
+  return proto_.battery_state() ==
+      power_manager::PowerSupplyProperties_BatteryState_CHARGING;
+}
+
+bool PowerStatus::IsBatteryDischargingOnLinePower() const {
+  return IsLinePowerConnected() && proto_.battery_state() ==
+      power_manager::PowerSupplyProperties_BatteryState_DISCHARGING;
+}
+
 double PowerStatus::GetBatteryPercent() const {
   return proto_.battery_percent();
 }
@@ -174,10 +202,13 @@ gfx::ImageSkia PowerStatus::GetBatteryImage(IconSet icon_set) const {
         IDR_AURA_UBER_TRAY_POWER_SMALL_DARK : IDR_AURA_UBER_TRAY_POWER_SMALL);
   }
 
-  // Get the horizontal offset in the battery icon array image.
-  int offset = (IsUsbChargerConnected() || !IsLinePowerConnected()) ? 0 : 1;
+  // Get the horizontal offset in the battery icon array image. The USB /
+  // "unreliable charging" image has a single column of icons; the other
+  // image contains a "battery" column on the left and a "line power"
+  // column on the right.
+  int offset = IsUsbChargerConnected() ? 0 : (IsLinePowerConnected() ? 1 : 0);
 
-  // Get the icon index in the battery icon array image.
+  // Get the vertical offset corresponding to the current battery level.
   int index = -1;
   if (GetBatteryPercent() >= 100.0) {
     index = kNumPowerImages - 1;
@@ -197,40 +228,39 @@ gfx::ImageSkia PowerStatus::GetBatteryImage(IconSet icon_set) const {
 
 base::string16 PowerStatus::GetAccessibleNameString() const {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  if (IsLinePowerConnected() && IsBatteryFull()) {
+  if (IsBatteryFull()) {
     return rb.GetLocalizedString(
         IDS_ASH_STATUS_TRAY_BATTERY_FULL_CHARGE_ACCESSIBLE);
   }
+
   base::string16 battery_percentage_accessible = l10n_util::GetStringFUTF16(
-      IsLinePowerConnected() ?
+      IsBatteryCharging() ?
       IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_CHARGING_ACCESSIBLE :
       IDS_ASH_STATUS_TRAY_BATTERY_PERCENT_ACCESSIBLE,
       base::IntToString16(GetRoundedBatteryPercent()));
   base::string16 battery_time_accessible = base::string16();
+  const base::TimeDelta time = IsBatteryCharging() ? GetBatteryTimeToFull() :
+      GetBatteryTimeToEmpty();
+
   if (IsUsbChargerConnected()) {
     battery_time_accessible = rb.GetLocalizedString(
         IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE_ACCESSIBLE);
-  } else {
-    if (IsBatteryTimeBeingCalculated()) {
-      battery_time_accessible = rb.GetLocalizedString(
-          IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING_ACCESSIBLE);
-    } else {
-      base::TimeDelta time = IsLinePowerConnected() ? GetBatteryTimeToFull() :
-          GetBatteryTimeToEmpty();
-      int hour = time.InHours();
-      int min = (time - base::TimeDelta::FromHours(hour)).InMinutes();
-      if (hour || min) {
-        base::string16 minute = min < 10 ?
-            ASCIIToUTF16("0") + base::IntToString16(min) :
-            base::IntToString16(min);
-        battery_time_accessible =
-            l10n_util::GetStringFUTF16(
-                IsLinePowerConnected() ?
-                IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL_ACCESSIBLE :
-                IDS_ASH_STATUS_TRAY_BATTERY_TIME_LEFT_ACCESSIBLE,
-                GetBatteryTimeAccessibilityString(hour, min));
-      }
-    }
+  } else if (IsBatteryTimeBeingCalculated()) {
+    battery_time_accessible = rb.GetLocalizedString(
+        IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING_ACCESSIBLE);
+  } else if (ShouldDisplayBatteryTime(time) &&
+             !IsBatteryDischargingOnLinePower()) {
+    int hour = 0, min = 0;
+    PowerStatus::SplitTimeIntoHoursAndMinutes(time, &hour, &min);
+    base::string16 minute = min < 10 ?
+        ASCIIToUTF16("0") + base::IntToString16(min) :
+        base::IntToString16(min);
+    battery_time_accessible =
+        l10n_util::GetStringFUTF16(
+            IsBatteryCharging() ?
+            IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL_ACCESSIBLE :
+            IDS_ASH_STATUS_TRAY_BATTERY_TIME_LEFT_ACCESSIBLE,
+            GetBatteryTimeAccessibilityString(hour, min));
   }
   return battery_time_accessible.empty() ?
       battery_percentage_accessible :
diff --git a/ash/system/chromeos/power/power_status.h b/ash/system/chromeos/power/power_status.h
index 4eba247..a5cf3ae 100644
--- a/ash/system/chromeos/power/power_status.h
+++ b/ash/system/chromeos/power/power_status.h
@@ -39,7 +39,10 @@ class ASH_EXPORT PowerStatus : public chromeos::PowerManagerClient::Observer {
     virtual ~Observer() {}
   };
 
-  virtual ~PowerStatus();
+  // Maximum battery time-to-full or time-to-empty that should be displayed
+  // in the UI. If the current is close to zero, battery time estimates can
+  // get very large; avoid displaying these large numbers.
+  static const int kMaxBatteryTimeToDisplaySec;
 
   // Sets the global instance. Must be called before any calls to Get().
   static void Initialize();
@@ -53,6 +56,16 @@ class ASH_EXPORT PowerStatus : public chromeos::PowerManagerClient::Observer {
   // Gets the global instance. Initialize must be called first.
   static PowerStatus* Get();
 
+  // Returns true if |time|, a time returned by GetBatteryTimeToEmpty() or
+  // GetBatteryTimeToFull(), should be displayed in the UI.
+  // Less-than-a-minute or very large values aren't displayed.
+  static bool ShouldDisplayBatteryTime(const base::TimeDelta& time);
+
+  // Copies the hour and minute components of |time| to |hours| and |minutes|.
+  static void SplitTimeIntoHoursAndMinutes(const base::TimeDelta& time,
+                                           int* hours,
+                                           int* minutes);
+
   // Adds or removes an observer.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
@@ -63,9 +76,21 @@ class ASH_EXPORT PowerStatus : public chromeos::PowerManagerClient::Observer {
   // Returns true if a battery is present.
   bool IsBatteryPresent() const;
 
-  // Returns true if the battery is full.
+  // Returns true if the battery is full. This also implies that a charger
+  // is connected.
   bool IsBatteryFull() const;
 
+  // Returns true if the battery is charging. Note that this implies that a
+  // charger is connected but the converse is not necessarily true: the
+  // battery may be discharging even while a (perhaps low-power) charger is
+  // connected. Use Is*Connected() to test for the presence of a charger
+  // and also see IsBatteryDischargingOnLinePower().
+  bool IsBatteryCharging() const;
+
+  // Returns true if the battery is discharging (or neither charging nor
+  // discharging while not being full) while line power is connected.
+  bool IsBatteryDischargingOnLinePower() const;
+
   // Returns the battery's remaining charge as a value in the range [0.0,
   // 100.0].
   double GetBatteryPercent() const;
@@ -106,6 +131,7 @@ class ASH_EXPORT PowerStatus : public chromeos::PowerManagerClient::Observer {
 
  protected:
   PowerStatus();
+  virtual ~PowerStatus();
 
  private:
   // Overriden from PowerManagerClient::Observer.
diff --git a/ash/system/chromeos/power/power_status_unittest.cc b/ash/system/chromeos/power/power_status_unittest.cc
index 831dd3dad..69dcdb0 100644
--- a/ash/system/chromeos/power/power_status_unittest.cc
+++ b/ash/system/chromeos/power/power_status_unittest.cc
@@ -66,7 +66,7 @@ class PowerStatusTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(PowerStatusTest);
 };
 
-TEST_F(PowerStatusTest, PowerStatusInitializeAndUpdate) {
+TEST_F(PowerStatusTest, InitializeAndUpdate) {
   // Test that the initial power supply state should be acquired after
   // PowerStatus is instantiated. This depends on
   // PowerManagerClientStubImpl, which responds to power status update
@@ -82,5 +82,54 @@ TEST_F(PowerStatusTest, PowerStatusInitializeAndUpdate) {
   EXPECT_EQ(2, test_observer_->power_changed_count());
 }
 
+TEST_F(PowerStatusTest, ShouldDisplayBatteryTime) {
+  EXPECT_FALSE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(-1)));
+  EXPECT_FALSE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(0)));
+  EXPECT_FALSE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(59)));
+  EXPECT_TRUE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(60)));
+  EXPECT_TRUE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(600)));
+  EXPECT_TRUE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(3600)));
+  EXPECT_TRUE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(
+          PowerStatus::kMaxBatteryTimeToDisplaySec)));
+  EXPECT_FALSE(PowerStatus::ShouldDisplayBatteryTime(
+      base::TimeDelta::FromSeconds(
+          PowerStatus::kMaxBatteryTimeToDisplaySec + 1)));
+}
+
+TEST_F(PowerStatusTest, SplitTimeIntoHoursAndMinutes) {
+  int hours = 0, minutes = 0;
+  PowerStatus::SplitTimeIntoHoursAndMinutes(
+      base::TimeDelta::FromSeconds(0), &hours, &minutes);
+  EXPECT_EQ(0, hours);
+  EXPECT_EQ(0, minutes);
+
+  PowerStatus::SplitTimeIntoHoursAndMinutes(
+      base::TimeDelta::FromSeconds(60), &hours, &minutes);
+  EXPECT_EQ(0, hours);
+  EXPECT_EQ(1, minutes);
+
+  PowerStatus::SplitTimeIntoHoursAndMinutes(
+      base::TimeDelta::FromSeconds(3600), &hours, &minutes);
+  EXPECT_EQ(1, hours);
+  EXPECT_EQ(0, minutes);
+
+  PowerStatus::SplitTimeIntoHoursAndMinutes(
+      base::TimeDelta::FromSeconds(3600 + 60), &hours, &minutes);
+  EXPECT_EQ(1, hours);
+  EXPECT_EQ(1, minutes);
+
+  PowerStatus::SplitTimeIntoHoursAndMinutes(
+      base::TimeDelta::FromSeconds(7 * 3600 + 23 * 60), &hours, &minutes);
+  EXPECT_EQ(7, hours);
+  EXPECT_EQ(23, minutes);
+}
+
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/system/chromeos/power/power_status_view.cc b/ash/system/chromeos/power/power_status_view.cc
index 4c88784..a38d315 100644
--- a/ash/system/chromeos/power/power_status_view.cc
+++ b/ash/system/chromeos/power/power_status_view.cc
@@ -115,7 +115,7 @@ void PowerStatusView::UpdateTextForDefaultView() {
   base::string16 battery_percentage;
   base::string16 battery_time_status;
 
-  if (status.IsLinePowerConnected() && status.IsBatteryFull()) {
+  if (status.IsBatteryFull()) {
     battery_time_status =
         rb.GetLocalizedString(IDS_ASH_STATUS_TRAY_BATTERY_FULL);
   } else {
@@ -125,27 +125,26 @@ void PowerStatusView::UpdateTextForDefaultView() {
     if (status.IsUsbChargerConnected()) {
       battery_time_status = rb.GetLocalizedString(
           IDS_ASH_STATUS_TRAY_BATTERY_CHARGING_UNRELIABLE);
+    } else if (status.IsBatteryTimeBeingCalculated()) {
+      battery_time_status =
+          rb.GetLocalizedString(IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING);
     } else {
-      if (status.IsBatteryTimeBeingCalculated()) {
+      base::TimeDelta time = status.IsBatteryCharging() ?
+          status.GetBatteryTimeToFull() : status.GetBatteryTimeToEmpty();
+      if (PowerStatus::ShouldDisplayBatteryTime(time) &&
+          !status.IsBatteryDischargingOnLinePower()) {
+        int hour = 0, min = 0;
+        PowerStatus::SplitTimeIntoHoursAndMinutes(time, &hour, &min);
+        base::string16 minute = min < 10 ?
+            ASCIIToUTF16("0") + base::IntToString16(min) :
+            base::IntToString16(min);
         battery_time_status =
-            rb.GetLocalizedString(IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING);
-      } else {
-        base::TimeDelta time = status.IsLinePowerConnected() ?
-            status.GetBatteryTimeToFull() : status.GetBatteryTimeToEmpty();
-        int hour = time.InHours();
-        int min = (time - base::TimeDelta::FromHours(hour)).InMinutes();
-        if (hour || min) {
-          base::string16 minute = min < 10 ?
-              ASCIIToUTF16("0") + base::IntToString16(min) :
-              base::IntToString16(min);
-          battery_time_status =
-              l10n_util::GetStringFUTF16(
-                  status.IsLinePowerConnected() ?
-                  IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL_SHORT :
-                  IDS_ASH_STATUS_TRAY_BATTERY_TIME_LEFT_SHORT,
-                  base::IntToString16(hour),
-                  minute);
-        }
+            l10n_util::GetStringFUTF16(
+                status.IsBatteryCharging() ?
+                IDS_ASH_STATUS_TRAY_BATTERY_TIME_UNTIL_FULL_SHORT :
+                IDS_ASH_STATUS_TRAY_BATTERY_TIME_LEFT_SHORT,
+                base::IntToString16(hour),
+                minute);
       }
     }
     battery_percentage = battery_time_status.empty() ?
@@ -159,7 +158,7 @@ void PowerStatusView::UpdateTextForDefaultView() {
 
 void PowerStatusView::UpdateTextForNotificationView() {
   const PowerStatus& status = *PowerStatus::Get();
-  if (status.IsLinePowerConnected() && status.IsBatteryFull()) {
+  if (status.IsBatteryFull()) {
     status_label_->SetText(
         ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
             IDS_ASH_STATUS_TRAY_BATTERY_FULL));
@@ -170,14 +169,8 @@ void PowerStatusView::UpdateTextForNotificationView() {
             base::IntToString16(status.GetRoundedBatteryPercent())));
   }
 
-  int hour = 0;
-  int min = 0;
-  if (!status.IsBatteryTimeBeingCalculated()) {
-    base::TimeDelta time = status.IsLinePowerConnected() ?
-        status.GetBatteryTimeToFull() : status.GetBatteryTimeToEmpty();
-    hour = time.InHours();
-    min = (time - base::TimeDelta::FromHours(hour)).InMinutes();
-  }
+  const base::TimeDelta time = status.IsBatteryCharging() ?
+      status.GetBatteryTimeToFull() : status.GetBatteryTimeToEmpty();
 
   if (status.IsUsbChargerConnected()) {
     time_label_->SetText(
@@ -187,8 +180,11 @@ void PowerStatusView::UpdateTextForNotificationView() {
     time_label_->SetText(
         ui::ResourceBundle::GetSharedInstance().GetLocalizedString(
             IDS_ASH_STATUS_TRAY_BATTERY_CALCULATING));
-  } else if (hour || min) {
-    if (status.IsLinePowerConnected()) {
+  } 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,