Cache network connecting bitmaps.

chrome/browser/chromeos/status/network_menu.cc

 // Copyright (c) 2011 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 "chrome/browser/chromeos/status/network_menu.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/stringprintf.h"
@@ skipping 62 matching lines @@
 };
 // static
 const int NetworkMenu::kBarsImagesVLowData[kNumBarsImages] = {
   IDR_STATUSBAR_NETWORK_BARS1_RED,
   IDR_STATUSBAR_NETWORK_BARS2_RED,
   IDR_STATUSBAR_NETWORK_BARS3_RED,
   IDR_STATUSBAR_NETWORK_BARS4_RED,
 };
 */
 
+// static
+const int NetworkMenu::kNumAnimatingImages = 10;
+
+// static
+SkBitmap* NetworkMenu::kAnimatingImages = new SkBitmap[kNumAnimatingImages];

[stevenjb, 2011/02/28 20:39:05]

Any reason to use 'new' here instead of putting this into a local namespace and
using 'SkBitmap kAnimatingImages[kAnimatingImages]' ?

[Charlie Lee, 2011/02/28 21:26:33]

Done.

+// static
+SkBitmap* NetworkMenu::kAnimatingImagesBlack =
+    new SkBitmap[kNumAnimatingImages];
+
 NetworkMenu::NetworkMenu()
     : min_width_(-1) {
   use_settings_ui_ = !CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableTabbedOptions);
   network_menu_.reset(NetworkMenuUI::CreateMenu2(this));
 }
 
 NetworkMenu::~NetworkMenu() {
 }
 
@@ skipping 255 matching lines @@
 }
 
 void NetworkMenu::UpdateMenu() {
   refreshing_menu_ = true;
   InitMenuItems();
   network_menu_->Rebuild();
   refreshing_menu_ = false;
 }
 
 // static
-SkBitmap NetworkMenu::IconForNetworkStrength(const WifiNetwork* wifi,
-                                             bool black) {
+SkBitmap* NetworkMenu::IconForNetworkStrength(const WifiNetwork* wifi,
+                                              bool black) {

[stevenjb, 2011/02/28 20:39:05]

Since the result is never allowed to be NULL, and since we do not test for NULL
where we use the result, I would return a const SkBitmap& instead of a *.

[Charlie Lee, 2011/02/28 21:26:33]

Done.

   DCHECK(wifi);
   if (wifi->strength() == 0) {
-    return *ResourceBundle::GetSharedInstance().GetBitmapNamed(
+    return ResourceBundle::GetSharedInstance().GetBitmapNamed(
         black ? IDR_STATUSBAR_NETWORK_BARS0_BLACK :
                 IDR_STATUSBAR_NETWORK_BARS0);
   }
   int index = static_cast<int>(wifi->strength() / 100.0 *
       nextafter(static_cast<float>(kNumBarsImages), 0));
   index = std::max(std::min(index, kNumBarsImages - 1), 0);
   const int* images = black ? kBarsImagesBlack : kBarsImages;
-  return *ResourceBundle::GetSharedInstance().GetBitmapNamed(images[index]);
+  return ResourceBundle::GetSharedInstance().GetBitmapNamed(images[index]);
 }
 
 // static
-SkBitmap NetworkMenu::IconForNetworkStrength(const CellularNetwork* cellular,
-                                             bool black) {
+SkBitmap* NetworkMenu::IconForNetworkStrength(const CellularNetwork* cellular,
+                                              bool black) {

[stevenjb, 2011/02/28 20:39:05]

ditto

[Charlie Lee, 2011/02/28 21:26:33]

Done.

   DCHECK(cellular);
   // If no data, then we show 0 bars.
   if (cellular->strength() == 0 ||
       cellular->GetDataLeft() == CellularNetwork::DATA_NONE) {
-    return *ResourceBundle::GetSharedInstance().GetBitmapNamed(
+    return ResourceBundle::GetSharedInstance().GetBitmapNamed(
         black ? IDR_STATUSBAR_NETWORK_BARS0_BLACK :
                 IDR_STATUSBAR_NETWORK_BARS0);
   }
   int index = static_cast<int>(cellular->strength() / 100.0 *
       nextafter(static_cast<float>(kNumBarsImages), 0));
   index = std::max(std::min(index, kNumBarsImages - 1), 0);
   const int* images = black ? kBarsImagesBlack : kBarsImages;
-  return *ResourceBundle::GetSharedInstance().GetBitmapNamed(images[index]);
+  return ResourceBundle::GetSharedInstance().GetBitmapNamed(images[index]);
 }
 
 // static
-SkBitmap NetworkMenu::IconForNetworkConnecting(double animation_value,
-                                               bool black) {
+SkBitmap* NetworkMenu::IconForNetworkConnecting(double animation_value,
+                                                bool black) {

[stevenjb, 2011/02/28 20:39:05]

ditto

[Charlie Lee, 2011/02/28 21:26:33]

Done.

   // Draw animation of bars icon fading in and out.
   // We are fading between 0 bars and a third of the opacity of 4 bars.
   // Use the current value of the animation to calculate the alpha value
   // of how transparent the icon is.
-  ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-  return SkBitmapOperations::CreateBlendedBitmap(
-      *rb.GetBitmapNamed(black ? IDR_STATUSBAR_NETWORK_BARS0_BLACK :
-                                 IDR_STATUSBAR_NETWORK_BARS0),
-      *rb.GetBitmapNamed(black ? IDR_STATUSBAR_NETWORK_BARS4_BLACK :
-                                 IDR_STATUSBAR_NETWORK_BARS4),
-      animation_value / 3);
+
+  int index = static_cast<int>(animation_value *
+      nextafter(static_cast<float>(kNumAnimatingImages), 0));
+  index = std::max(std::min(index, kNumAnimatingImages - 1), 0);
+
+  SkBitmap* images = black ? kAnimatingImagesBlack : kAnimatingImages;
+  // Lazily cache images.
+  if (images[index].empty()) {
+    // Divide index (0-9) by 9 (assume kNumAnimatingImages==10) to get (0.0-1.0)
+    // Then we take a third of that for the alpha value.
+    double alpha = static_cast<double>(index) / (kNumAnimatingImages - 1) / 3;
+    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    images[index] = SkBitmapOperations::CreateBlendedBitmap(
+        *rb.GetBitmapNamed(black ? IDR_STATUSBAR_NETWORK_BARS0_BLACK :
+                                   IDR_STATUSBAR_NETWORK_BARS0),
+        *rb.GetBitmapNamed(black ? IDR_STATUSBAR_NETWORK_BARS4_BLACK :
+                                   IDR_STATUSBAR_NETWORK_BARS4),
+        alpha);
+  }
+  return &images[index];
 }
 
 // static
-SkBitmap NetworkMenu::BadgeForNetworkTechnology(
+SkBitmap* NetworkMenu::BadgeForNetworkTechnology(

[stevenjb, 2011/02/28 20:39:05]

const SkBitmap*

[Charlie Lee, 2011/02/28 21:26:33]

Done.

     const CellularNetwork* cellular) {
   if (!cellular)
-    return SkBitmap();
+    return NULL;
 
   int id = -1;
   switch (cellular->network_technology()) {
     case NETWORK_TECHNOLOGY_EVDO:
       switch (cellular->GetDataLeft()) {
         case CellularNetwork::DATA_NONE:
           id = IDR_STATUSBAR_NETWORK_3G_ERROR;
           break;
         case CellularNetwork::DATA_VERY_LOW:
         case CellularNetwork::DATA_LOW:
@@ skipping 42 matching lines @@
     case NETWORK_TECHNOLOGY_LTE:
       id = IDR_STATUSBAR_NETWORK_LTE;
       break;
     case NETWORK_TECHNOLOGY_LTE_ADVANCED:
       id = IDR_STATUSBAR_NETWORK_LTE_ADVANCED;
       break;
     case NETWORK_TECHNOLOGY_UNKNOWN:
       break;
   }
   if (id == -1)
-    return SkBitmap();
+    return NULL;
   else
-    return *ResourceBundle::GetSharedInstance().GetBitmapNamed(id);
+    return ResourceBundle::GetSharedInstance().GetBitmapNamed(id);
 }
 
 // static
-SkBitmap NetworkMenu::IconForDisplay(SkBitmap icon, SkBitmap badge) {
+SkBitmap NetworkMenu::IconForDisplay(SkBitmap* icon, SkBitmap* badge) {

[stevenjb, 2011/02/28 20:39:05]

Do we want to copy the SkBitmap here, or return a reference?
SkBitmap* icon should be const SkBitmap&, since we dereference it without
checking.
const SkBitmap* badge.

Alternately, if you prefer both icon and badge to be const*, there should be a
test or a CHECK wherever the values are dereferenced.

[Charlie Lee, 2011/02/28 21:26:33]

Since we are not storing the newly created SkBitmap anywhere, it makes sense to
just return a copy, right?

On 2011/02/28 20:39:05, Steven Bennetts wrote:

[stevenjb, 2011/02/28 23:02:37]

Ah, yes, my bad, I didn't notice that this was creating a new bitmap.

On 2011/02/28 21:26:33, Charlie Lee wrote:

+  if (badge == NULL)
+    return *icon;
+
   // Draw badge at (14,14).
   static const int kBadgeX = 14;
   static const int kBadgeY = 14;
 
-  gfx::CanvasSkia canvas(icon.width(), icon.height(), false);
-  canvas.DrawBitmapInt(icon, 0, 0);
-  if (!badge.empty())
-    canvas.DrawBitmapInt(badge, kBadgeX, kBadgeY);
+  gfx::CanvasSkia canvas(icon->width(), icon->height(), false);
+  canvas.DrawBitmapInt(*icon, 0, 0);
+  canvas.DrawBitmapInt(*badge, kBadgeX, kBadgeY);
   return canvas.ExtractBitmap();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkMenu, views::ViewMenuDelegate implementation:
 
 void NetworkMenu::RunMenu(views::View* source, const gfx::Point& pt) {
   refreshing_menu_ = true;
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   cros->RequestWifiScan();
@@ skipping 40 matching lines @@
     bool ethernet_connecting = cros->ethernet_connecting();
 
     if (ethernet_connecting) {
       label = l10n_util::GetStringFUTF16(
           IDS_STATUSBAR_NETWORK_DEVICE_STATUS,
           l10n_util::GetStringUTF16(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET),
           l10n_util::GetStringUTF16(IDS_STATUSBAR_NETWORK_DEVICE_CONNECTING));
     } else {
       label = l10n_util::GetStringUTF16(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET);
     }
-    SkBitmap icon = *rb.GetBitmapNamed(IDR_STATUSBAR_WIRED_BLACK);
-    SkBitmap badge = ethernet_connecting || ethernet_connected ?
-        SkBitmap() : *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_DISCONNECTED);
+    SkBitmap* icon = rb.GetBitmapNamed(IDR_STATUSBAR_WIRED_BLACK);
+    SkBitmap* badge = ethernet_connecting || ethernet_connected ?
+        NULL : rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_DISCONNECTED);
     int flag = FLAG_ETHERNET;
     if (ethernet_connecting || ethernet_connected)
       flag |= FLAG_ASSOCIATED;
     menu_items_.push_back(
         MenuItem(ui::MenuModel::TYPE_COMMAND, label,
                  IconForDisplay(icon, badge), std::string(), flag));
   }
 
   // Wifi Networks
   bool wifi_available = cros->wifi_available();
@@ skipping 15 matching lines @@
       }
 
       // First add a separator if necessary.
       if (!separator_added) {
         separator_added = true;
         if (!menu_items_.empty()) {  // Don't add if first menu item.
           menu_items_.push_back(MenuItem());  // Separator
         }
       }
 
-      SkBitmap icon = IconForNetworkStrength(wifi_networks[i], true);
-      SkBitmap badge = wifi_networks[i]->encrypted() ?
-          *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_SECURE) : SkBitmap();
+      SkBitmap* icon = IconForNetworkStrength(wifi_networks[i], true);
+      SkBitmap* badge = wifi_networks[i]->encrypted() ?
+          rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_SECURE) : NULL;
       int flag = FLAG_WIFI;
       if (!wifi_networks[i]->connectable())
         flag |= FLAG_DISABLED;
       if (active_wifi
           && wifi_networks[i]->service_path() == active_wifi->service_path())
         flag |= FLAG_ASSOCIATED;
       menu_items_.push_back(
           MenuItem(ui::MenuModel::TYPE_COMMAND, label,
                    IconForDisplay(icon, badge),
                    wifi_networks[i]->service_path(), flag));
@@ skipping 37 matching lines @@
       }
 
       // First add a separator if necessary.
       if (!separator_added) {
         separator_added = true;
         if (!menu_items_.empty()) {  // Don't add if first menu item.
           menu_items_.push_back(MenuItem());  // Separator
         }
       }
 
-      SkBitmap icon = IconForNetworkStrength(cell_networks[i], true);
-      SkBitmap badge = BadgeForNetworkTechnology(cell_networks[i]);
+      SkBitmap* icon = IconForNetworkStrength(cell_networks[i], true);
+      SkBitmap* badge = BadgeForNetworkTechnology(cell_networks[i]);
       int flag = FLAG_CELLULAR;
       if (!cell_networks[i]->connectable())
         flag |= FLAG_DISABLED;
       bool isActive = active_cellular &&
           cell_networks[i]->service_path() == active_cellular->service_path() &&
           (cell_networks[i]->connecting() || cell_networks[i]->connected());
       if (isActive)
         flag |= FLAG_ASSOCIATED;
       menu_items_.push_back(
           MenuItem(ui::MenuModel::TYPE_COMMAND, label,
@@ skipping 26 matching lines @@
     menu_items_.push_back(MenuItem(ui::MenuModel::TYPE_COMMAND, label,
         SkBitmap(), std::string(), FLAG_DISABLED));
   }
 
   // Add network.
   if (wifi_available && wifi_enabled) {
     menu_items_.push_back(MenuItem());  // Separator
     menu_items_.push_back(MenuItem(
         ui::MenuModel::TYPE_COMMAND,
         l10n_util::GetStringUTF16(IDS_OPTIONS_SETTINGS_OTHER_NETWORKS),
-        IconForDisplay(*rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS0_BLACK),
-                       SkBitmap()),
+        *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS0_BLACK),
         std::string(), FLAG_OTHER_NETWORK));
   }
 
   // Enable / disable wireless.
   if (wifi_available || cellular_available) {
     menu_items_.push_back(MenuItem());  // Separator
 
     if (wifi_available) {
       // Add 'Scanning...'
       if (cros->wifi_scanning()) {
@@ skipping 90 matching lines @@
                                       chrome::kInternetOptionsSubPage,
                                       chromeos::TYPE_WIFI);
       browser->ShowOptionsTab(page);
     }
   } else {
     ShowNetworkConfigView(new NetworkConfigView());
   }
 }
 
 }  // namespace chromeos