Implemented nicer battery status

chrome/browser/chromeos/status/power_menu_button.cc

Index: chrome/browser/chromeos/status/power_menu_button.cc
diff --git a/chrome/browser/chromeos/status/power_menu_button.cc b/chrome/browser/chromeos/status/power_menu_button.cc
index eb1fcf4210f71dc570a8c6a01301f328b0eabdec..2a7ed0733dee087a2a9fc77e2b96d60a118f11b9 100644
--- a/chrome/browser/chromeos/status/power_menu_button.cc
+++ b/chrome/browser/chromeos/status/power_menu_button.cc
@@ -15,6 +15,8 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/canvas_skia.h"
+#include "ui/gfx/font.h"
 #include "views/controls/menu/menu_item_view.h"
 #include "views/controls/menu/submenu_view.h"
 #include "views/widget/widget.h"
@@ -28,25 +30,219 @@ enum {
   POWER_NO_BATTERY,
 };
 
+enum ImageType {
+  DISCHARGING,
+  CHARGING,
+  BOLT
+};
+
+enum ImageSize {
+  SMALL,
+  LARGE
+};
+
+// Initialize time deltas to large values so they will be replaced when set
+// to small values.
+const int64 kInitialMS = 0x7fffffff;
+// Width and height of small images.
+const int kSmallImageWidth = 26, kSmallImageHeight = 24;
+// Width and height of large images.
+const int kLargeImageWidth = 57, kLargeImageHeight = 35;
+// Number of different power states.
+const int kNumPowerImages = 20;
+
+// Constants for status displayed when user clicks button.
+// Padding around status.
+const int kPadLeftX = 10, kPadRightX = 10, kPadY = 5;
+// Padding between battery and text.
+const int kBatteryPadX = 10;
+// Spacing between lines of text.
+const int kEstimateSpacing = 3;
+// Color of text embedded within battery.
+const SkColor kPercentageColor = 0xFF333333;
+// Used for embossing text.
+const SkColor kPercentageShadowColor = 0x80ffffff;
+// Status text/
+const SkColor kEstimateColor = SK_ColorBLACK;
+// Size of percentage w/in battery.
+const int kBatteryFontSizeDelta = 3;
+
+// Battery images come from two collections (small and large). In each there
+// are |kNumPowerImages| battery states for both on and off charge, followed
+// by the missing battery image and the unknown image.
+// They are layed out like this:
+// Discharging Charging Bolt
+// |           |        +
+// ||          ||        +
+// ...
+// |||||       |||||        +
+// ||X||       ||?||
+SkBitmap GetImage(ImageSize size, ImageType type, int index) {
+  int image_width, image_height, image_index;
+
+  if (size == SMALL) {
+    image_index = IDR_STATUSBAR_BATTERY_SMALL_ALL;
+    image_width = kSmallImageWidth;
+    image_height = kSmallImageHeight;
+  } else {
+    image_index = IDR_STATUSBAR_BATTERY_LARGE_ALL;
+    image_width = kLargeImageWidth;
+    image_height = kLargeImageHeight;
+  }
+  SkBitmap* all_images =
+      ResourceBundle::GetSharedInstance().GetBitmapNamed(image_index);
+  SkBitmap image;

[achuithb, 2011/08/09 18:58:07]

Maybe move this to right before extractSubset?

[DaveMoore, 2011/08/12 21:03:11]

Done.

+  SkIRect subset =
+      SkIRect::MakeXYWH(
+          static_cast<int>(type) * image_width,
+          index * image_height,
+          image_width,
+          image_height);
+
+  all_images->extractSubset(&image, subset);
+  return image;
+}
+
+SkBitmap GetMissingImage(ImageSize size) {
+  return GetImage(size, DISCHARGING, kNumPowerImages);
+}
+
+SkBitmap GetUnknownImage(ImageSize size) {
+  return GetImage(size, CHARGING, kNumPowerImages);
+}
+
 }  // namespace
 
 namespace chromeos {
 
 using base::TimeDelta;
 
+class PowerMenuButton::StatusView : public View {
+ public:
+  explicit StatusView(PowerMenuButton* menu_button)
+      : menu_button_(menu_button) {
+    estimate_font_ =
+        ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::BaseFont);
+    percentage_font_ =
+        estimate_font_.DeriveFont(kBatteryFontSizeDelta, gfx::Font::BOLD);
+  }
+
+  gfx::Size GetPreferredSize() {
+    int estimate_w, estimate_h, charging_w, charging_h;
+    string16 estimate_text = menu_button_->GetBatteryIsChargedText();
+    string16 charging_text = l10n_util::GetStringUTF16(
+        menu_button_->line_power_on_ ?
+            IDS_STATUSBAR_BATTERY_CHARGING :
+            IDS_STATUSBAR_BATTERY_DISCHARGING);
+    gfx::CanvasSkia::SizeStringInt(
+        estimate_text, estimate_font_, &estimate_w, &estimate_h, 0);
+    gfx::CanvasSkia::SizeStringInt(
+        charging_text, estimate_font_, &charging_w, &charging_h, 0);
+    gfx::Size size = gfx::Size(
+        kPadLeftX + kLargeImageWidth + kBatteryPadX + kPadRightX +
+            std::max(charging_w, estimate_w),
+        (2 * kPadY) +
+            std::max(kLargeImageHeight,
+                     kEstimateSpacing + (2 * estimate_font_.GetHeight())));
+    return size;
+  }
+
+  void Update() {
+    PreferredSizeChanged();
+    // Force a paint even if the size didn't change.
+    SchedulePaint();
+  }
+
+ protected:
+  void OnPaint(gfx::Canvas* canvas) {
+    SkBitmap image;
+
+    bool draw_percentage_text = false;
+    if (!CrosLibrary::Get()->EnsureLoaded()) {
+      image = GetUnknownImage(LARGE);
+    } else if (!menu_button_->battery_is_present_) {
+      image = GetMissingImage(LARGE);
+    } else {
+      image = GetImage(
+          LARGE,
+          menu_button_->line_power_on_ ? CHARGING : DISCHARGING,
+          menu_button_->battery_index_);
+      if (menu_button_->battery_percentage_ < 100 ||
+          !menu_button_->line_power_on_) {
+        draw_percentage_text = true;
+      }
+    }
+    int image_x = kPadLeftX, image_y = (height() - image.height()) / 2;
+

[achuithb, 2011/08/09 18:58:07]

Shouldn't this newline be after DrawBitmapInt?

[DaveMoore, 2011/08/12 21:03:11]

Done.

+    canvas->DrawBitmapInt(image, image_x, image_y);
+    if (draw_percentage_text) {
+      string16 text = UTF8ToUTF16(StringPrintf(
+          "%d%%",
+          static_cast<int>(menu_button_->battery_percentage_)));
+      int text_h = percentage_font_.GetHeight();
+      int text_y = ((height() - text_h) / 2);
+      canvas->DrawStringInt(
+          text, percentage_font_, kPercentageShadowColor,
+          image_x, text_y + 1, image.width(), text_h,
+          gfx::Canvas::TEXT_ALIGN_CENTER | gfx::Canvas::NO_ELLIPSIS);
+      canvas->DrawStringInt(
+          text, percentage_font_, kPercentageColor,
+          image_x, text_y, image.width(), text_h,
+          gfx::Canvas::TEXT_ALIGN_CENTER | gfx::Canvas::NO_ELLIPSIS);
+      if (menu_button_->line_power_on_) {
+        image = GetImage(LARGE, BOLT, menu_button_->battery_index_);
+        canvas->DrawBitmapInt(image, image_x, image_y);
+      }
+    }
+    string16 charging_text = l10n_util::GetStringUTF16(
+        menu_button_->line_power_on_ ?
+            IDS_STATUSBAR_BATTERY_CHARGING :
+            IDS_STATUSBAR_BATTERY_DISCHARGING);
+    string16 estimate_text = menu_button_->GetBatteryIsChargedText();
+    int text_h = estimate_font_.GetHeight();
+    int text_x = image_x + kLargeImageWidth + kBatteryPadX;
+    int charging_y = (height() - (kEstimateSpacing + (2 * text_h))) / 2;
+    int estimate_y = charging_y + text_h + kEstimateSpacing;
+    canvas->DrawStringInt(
+        charging_text, estimate_font_, kEstimateColor,
+        text_x, charging_y, width() - text_x, text_h,
+        gfx::Canvas::TEXT_ALIGN_LEFT);
+    canvas->DrawStringInt(
+        estimate_text, estimate_font_, kEstimateColor,
+        text_x, estimate_y, width() - text_x, text_h,
+        gfx::Canvas::TEXT_ALIGN_LEFT);
+  }
+
+  bool OnMousePressed(const views::MouseEvent& event) {
+    return true;
+  }
+
+  void OnMouseReleased(const views::MouseEvent& event) {
+    if (event.IsLeftMouseButton()) {
+      DCHECK(menu_button_->menu_);
+      menu_button_->menu_->Cancel();
+    }
+  }
+
+ private:
+  PowerMenuButton* menu_button_;
+  gfx::Font percentage_font_;
+  gfx::Font estimate_font_;
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // PowerMenuButton
 
-// static
-const int PowerMenuButton::kNumPowerImages = 19;
-
 PowerMenuButton::PowerMenuButton(StatusAreaHost* host)
     : StatusAreaButton(host, this),
       battery_is_present_(false),
       line_power_on_(false),
-      battery_fully_charged_(false),
       battery_percentage_(0.0),
-      icon_id_(-1) {
+      battery_index_(-1),
+      battery_time_to_full_(TimeDelta::FromMicroseconds(kInitialMS)),
+      battery_time_to_empty_(TimeDelta::FromMicroseconds(kInitialMS)),
+      status_(NULL),
+      menu_(NULL) {
   UpdateIconAndLabelInfo();
   CrosLibrary::Get()->GetPowerLibrary()->AddObserver(this);
 }
@@ -58,19 +254,7 @@ PowerMenuButton::~PowerMenuButton() {
 // PowerMenuButton, views::MenuDelegate implementation:
 
 std::wstring PowerMenuButton::GetLabel(int id) const {
-  string16 label;
-  switch (id) {
-    case POWER_BATTERY_PERCENTAGE_ITEM:
-      label = GetBatteryPercentageText();
-      break;
-    case POWER_BATTERY_IS_CHARGED_ITEM:
-      label = GetBatteryIsChargedText();
-      break;
-    default:
-      NOTREACHED();
-  }
-
-  return UTF16ToWide(label);
+  return std::wstring();

[achuithb, 2011/08/09 18:58:07]

What is GetLabel used for?

 }
 
 bool PowerMenuButton::IsCommandEnabled(int id) const {
@@ -85,7 +269,7 @@ string16 PowerMenuButton::GetBatteryPercentageText() const {
 
 string16 PowerMenuButton::GetBatteryIsChargedText() const {
   // The second item shows the battery is charged if it is.
-  if (battery_fully_charged_)
+  if (battery_percentage_ >= 100 && line_power_on_)
     return l10n_util::GetStringUTF16(IDS_STATUSBAR_BATTERY_IS_CHARGED);
 
   // If battery is in an intermediate charge state, show how much time left.
@@ -130,7 +314,18 @@ void PowerMenuButton::OnLocaleChanged() {
 // PowerMenuButton, views::ViewMenuDelegate implementation:
 
 void PowerMenuButton::RunMenu(views::View* source, const gfx::Point& pt) {
-  UpdateMenu();
+  menu_ = new views::MenuItemView(this);
+  views::MenuItemView* submenu =
+      menu_->AppendMenuItem(
+          POWER_BATTERY_PERCENTAGE_ITEM,
+          std::wstring(),
+          views::MenuItemView::NORMAL);
+  status_ = new StatusView(this);
+  submenu->AddChildView(status_);
+  menu_->CreateSubmenu()->set_resize_open_menu(true);
+  menu_->SetMargins(0, 0);
+  submenu->SetMargins(0, 0);
+  menu_->ChildrenChanged();
 
   gfx::Point screen_location;
   views::View::ConvertPointToScreen(source, &screen_location);
@@ -141,6 +336,9 @@ void PowerMenuButton::RunMenu(views::View* source, const gfx::Point& pt) {
       bounds,
       views::MenuItemView::TOPRIGHT,
       true);
+  delete menu_;
+  status_ = NULL;

[achuithb, 2011/08/09 18:58:07]

Is status_ leaked or does AddChildView transfer ownership to the submenu?

Also, why create MenuItemView and StatusView on the heap? Why not make these
stack variables and get rid of the class variables menu_ and status_. Am I
missing something?

[DaveMoore, 2011/08/12 21:03:11]

AddChildView transfers ownership.
That aren't stack variables because they're accessed from within the status
view.

On 2011/08/09 18:58:07, achuith.bhandarkar wrote:

+  menu_ = NULL;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -162,87 +360,47 @@ void PowerMenuButton::UpdateIconAndLabelInfo() {
   if (cros_loaded) {
     battery_is_present_ = cros->battery_is_present();
     line_power_on_ = cros->line_power_on();
-    battery_fully_charged_ = cros->battery_fully_charged();
-    battery_percentage_ = cros->battery_percentage();
+
     // If fully charged, always show 100% even if internal number is a bit less.
-    // Note: we always call cros->battery_percentage() for test predictability.
-    if (battery_fully_charged_)
+    if (cros->battery_fully_charged()) {
+      // We always call cros->battery_percentage() for test predictability.
+      cros->battery_percentage();
       battery_percentage_ = 100.0;
-    // Map 1-100 to 0-100, so 1% shows up as 0%, and 100% remains 100%.
-    static const double k = 100.0/99;
-    battery_percentage_ = (battery_percentage_ > 1.0 ?
-        battery_percentage_ : 1.0) * k - k;
+    } else {
+      battery_percentage_ = cros->battery_percentage();
+    }
 
     UpdateBatteryTime(&battery_time_to_full_, cros->battery_time_to_full());
     UpdateBatteryTime(&battery_time_to_empty_, cros->battery_time_to_empty());
   }
 
   if (!cros_loaded) {
-    icon_id_ = IDR_STATUSBAR_BATTERY_UNKNOWN;
+    battery_index_ = -1;
+    SetIcon(GetUnknownImage(SMALL));
   } else if (!battery_is_present_) {
-    icon_id_ = IDR_STATUSBAR_BATTERY_MISSING;
-  } else if (line_power_on_ && battery_fully_charged_) {
-    icon_id_ = IDR_STATUSBAR_BATTERY_CHARGED;
+    battery_index_ = -1;
+    SetIcon(GetMissingImage(SMALL));
   } else {
-    // Get the power image depending on battery percentage. Percentage is
-    // from 0 to 100, so we need to convert that to 0 to kNumPowerImages - 1.
-    // NOTE: Use an array rather than just calculating a resource number to
-    // avoid creating implicit ordering dependencies on the resource values.
-    static const int kChargingImages[kNumPowerImages] = {
-      IDR_STATUSBAR_BATTERY_CHARGING_1,
-      IDR_STATUSBAR_BATTERY_CHARGING_2,
-      IDR_STATUSBAR_BATTERY_CHARGING_3,
-      IDR_STATUSBAR_BATTERY_CHARGING_4,
-      IDR_STATUSBAR_BATTERY_CHARGING_5,
-      IDR_STATUSBAR_BATTERY_CHARGING_6,
-      IDR_STATUSBAR_BATTERY_CHARGING_7,
-      IDR_STATUSBAR_BATTERY_CHARGING_8,
-      IDR_STATUSBAR_BATTERY_CHARGING_9,
-      IDR_STATUSBAR_BATTERY_CHARGING_10,
-      IDR_STATUSBAR_BATTERY_CHARGING_11,
-      IDR_STATUSBAR_BATTERY_CHARGING_12,
-      IDR_STATUSBAR_BATTERY_CHARGING_13,
-      IDR_STATUSBAR_BATTERY_CHARGING_14,
-      IDR_STATUSBAR_BATTERY_CHARGING_15,
-      IDR_STATUSBAR_BATTERY_CHARGING_16,
-      IDR_STATUSBAR_BATTERY_CHARGING_17,
-      IDR_STATUSBAR_BATTERY_CHARGING_18,
-      IDR_STATUSBAR_BATTERY_CHARGING_19,
-    };
-    static const int kDischargingImages[kNumPowerImages] = {
-      IDR_STATUSBAR_BATTERY_DISCHARGING_1,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_2,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_3,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_4,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_5,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_6,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_7,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_8,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_9,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_10,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_11,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_12,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_13,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_14,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_15,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_16,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_17,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_18,
-      IDR_STATUSBAR_BATTERY_DISCHARGING_19,
-    };
-
-    int index = static_cast<int>(battery_percentage_ / 100.0 *
-                nextafter(static_cast<float>(kNumPowerImages), 0));
-    index = std::max(std::min(index, kNumPowerImages - 1), 0);
-    icon_id_ = line_power_on_ ?
-        kChargingImages[index] : kDischargingImages[index];
+    // Preserve the fully charged icon for 100% only.
+    if (battery_percentage_ >= 100) {
+      battery_index_ = kNumPowerImages - 1;
+    } else {
+      battery_index_ =
+          static_cast<int>(battery_percentage_ / 100.0 *
+              nextafter(static_cast<float>(kNumPowerImages - 1), 0));
+      battery_index_ =
+          std::max(std::min(battery_index_, kNumPowerImages - 2), 0);
+    }
+    SetIcon(GetImage(
+        SMALL, line_power_on_ ? CHARGING : DISCHARGING, battery_index_));
   }
 
-  SetIcon(*ResourceBundle::GetSharedInstance().GetBitmapNamed(icon_id_));
-  SetTooltipText(GetLabel(POWER_BATTERY_PERCENTAGE_ITEM));
-  SetAccessibleName(GetBatteryPercentageText());
-  UpdateMenu();
+  percentage_text_ = GetBatteryPercentageText();
+  SetTooltipText(UTF16ToWide(percentage_text_));
+  SetAccessibleName(percentage_text_);
   SchedulePaint();
+  if (status_)
+    status_->Update();
 }
 
 void PowerMenuButton::UpdateBatteryTime(TimeDelta* previous,
@@ -252,29 +410,13 @@ void PowerMenuButton::UpdateBatteryTime(TimeDelta* previous,
   const TimeDelta diff = current - *previous;
   // If the diff is small and positive, ignore it in favor of
   // keeping time monotonically decreasing.
-  if (diff < kMinDiff || diff > kMaxDiff)
+  // If previous is 0, then it either was never set (initial condition)
+  // or got down to 0

[achuithb, 2011/08/09 18:58:07]

period at end of comment.

[DaveMoore, 2011/08/12 21:03:11]

Done.

+  if (*previous == TimeDelta::FromMicroseconds(kInitialMS) ||
+      diff < kMinDiff ||
+      diff > kMaxDiff) {
     *previous = current;
-}
-
-void PowerMenuButton::UpdateMenu() {
-  if (!menu_.get())
-    menu_.reset(new views::MenuItemView(this));
-
-  // Remove existing items.
-  const int old_count = menu_->CreateSubmenu()->child_count();
-  for (int i = 0; i < old_count; ++i)
-    menu_->RemoveMenuItemAt(0);
-
-  if (battery_is_present_) {
-    // Create menu items whose text will be supplied by GetLabel().
-    menu_->AppendDelegateMenuItem(POWER_BATTERY_PERCENTAGE_ITEM);
-    menu_->AppendDelegateMenuItem(POWER_BATTERY_IS_CHARGED_ITEM);
-  } else {
-    menu_->AppendMenuItemWithLabel(
-        POWER_NO_BATTERY,
-        UTF16ToWide(l10n_util::GetStringUTF16(IDS_STATUSBAR_NO_BATTERY)));
   }
-  menu_->ChildrenChanged();
 }
 
 }  // namespace chromeos