Cellular: Make icon consistent in System Tray + Settings

Bug: 795078
Change-Id: I2acd51764c405958aae549d3e80ce5bea12a3867
Reviewed-on: https://chromium-review.googlesource.com/827598
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524281}

ash/system/network/network_icon.cc

@@ -389,9 +389,12 @@ Badge ConnectingVpnBadge(double animation, IconType icon_type) {
 }
 
 int StrengthIndex(int strength) {
+  if (strength == 0)
+    return 0;
   // Return an index in the range [1, kNumNetworkImages - 1].
-  const float findex = (static_cast<float>(strength) / 100.0f) *
-                       nextafter(static_cast<float>(kNumNetworkImages - 1), 0);
+  // This logic is equivalent to cr_network_icon.js:strengthToIndex_().
+  const float findex = (static_cast<float>(strength - 1) / 100.0f) *
+                       static_cast<float>(kNumNetworkImages - 1);
   int index = 1 + static_cast<int>(findex);
   index = std::max(std::min(index, kNumNetworkImages - 1), 1);
   return index;
@@ -434,7 +437,6 @@ gfx::ImageSkia GetIcon(const NetworkState* network,
     return gfx::CreateVectorIcon(kNetworkEthernetIcon,
                                  GetDefaultColorForIconType(ICON_TYPE_LIST));
   } else if (network->Matches(NetworkTypePattern::Wireless())) {
-    DCHECK(strength_index > 0);
     return GetImageForIndex(ImageTypeForNetwork(network, icon_type), icon_type,
                             strength_index);
   } else if (network->Matches(NetworkTypePattern::VPN())) {
@@ -595,7 +597,9 @@ void NetworkIconImpl::GetBadges(const NetworkState* network, Badges* badges) {
   } else if (type == shill::kTypeWimax) {
     technology_badge_ = {&kNetworkBadgeTechnology4gIcon, icon_color};
   } else if (type == shill::kTypeCellular) {
-    if (network->roaming() == shill::kRoamingStateRoaming) {
+    // technology_badge_ is set in UpdateCellularState.
+    if (network->IsConnectedState() &&
+        network->roaming() == shill::kRoamingStateRoaming) {
       // For networks that are always in roaming don't show roaming badge.
       const DeviceState* device =
           NetworkHandler::Get()->network_state_handler()->GetDeviceState(
@@ -607,12 +611,11 @@ void NetworkIconImpl::GetBadges(const NetworkState* network, Badges* badges) {
       }
     }
   }
-  if (!network->IsConnectingState()) {
+  // Only show technology, VPN, and captive portal badges when connected.
+  if (network->IsConnectedState()) {
     badges->top_left = technology_badge_;
     badges->bottom_left = vpn_badge_;
-  }
-
-  if (behind_captive_portal_) {
+    if (behind_captive_portal_)
       badges->bottom_right = {&kNetworkBadgeCaptivePortalIcon, icon_color};
   }
 }
@@ -768,8 +771,8 @@ void SignalStrengthImageSource::DrawBars(gfx::Canvas* canvas) {
     flags.setColor(color_);
     // As a percentage of the bg triangle, the length of one of the short
     // sides of the fg triangle, indexed by signal strength.
-    static constexpr float kTriangleSidePercents[] = {0.f, 0.5f, 0.625f, 0.75f,
-                                                      1.f};
+    static constexpr float kTriangleSidePercents[] = {0.f, 0.375f, 0.5833f,
+                                                      0.75f, 1.f};
     canvas->DrawPath(make_triangle(kTriangleSidePercents[signal_strength_] *
                                    kFullTriangleSide),
                      flags);
@@ -1019,6 +1022,8 @@ SignalStrength GetSignalStrengthForNetwork(
   // attempted to be split evenly from |kNumNetworkImages| - 1. Remainders go
   // first to the lowest bucket and then the second lowest bucket.
   const int index = StrengthIndex(network->signal_strength());
+  if (index == 0)
+    return SignalStrength::NONE;
   const int seperations = kNumNetworkImages - 1;
   const int bucket_size = seperations / 3;
 

ash/system/network/network_icon.h

@@ -33,7 +33,7 @@ enum IconType {
 enum ImageType { ARCS, BARS, NONE };
 
 // Strength of a wireless signal.
-enum class SignalStrength { WEAK, MEDIUM, STRONG, NOT_WIRELESS };
+enum class SignalStrength { NONE, WEAK, MEDIUM, STRONG, NOT_WIRELESS };
 
 // Depicts a given signal strength using arcs (e.g. for WiFi connections) or
 // bars (e.g. for cell connections).

ash/system/network/network_icon_unittest.cc

@@ -217,20 +217,20 @@ TEST_F(NetworkIconTest, NetworkSignalStrength) {
             GetSignalStrengthForNetwork(ethernet_network_.get()));
   EXPECT_NE(ss::NOT_WIRELESS, GetSignalStrengthForNetwork(wifi_network_.get()));
 
-  // Signal strength is divided into three categories: weak, medium and strong.
-  // They are meant to match the number of sections in the wifi icon. The wifi
-  // icon currently has four levels; signals [0, 100] are mapped to [1, 4].
-  // There are only three signal strengths so icons that were mapped to 2 are
-  // also considered weak.
+  // Signal strength is divided into four categories: none, weak, medium and
+  // strong. They are meant to match the number of sections in the wifi icon.
+  // The wifi icon currently has four levels; signals [0, 100] are mapped to [1,
+  // 4]. There are only three signal strengths so icons that were mapped to 2
+  // are also considered weak.
   wifi_network_->set_signal_strength(0);
-  EXPECT_EQ(ss::WEAK, GetSignalStrengthForNetwork(wifi_network_.get()));
-  wifi_network_->set_signal_strength(49);
-  EXPECT_EQ(ss::WEAK, GetSignalStrengthForNetwork(wifi_network_.get()));
+  EXPECT_EQ(ss::NONE, GetSignalStrengthForNetwork(wifi_network_.get()));
   wifi_network_->set_signal_strength(50);
-  EXPECT_EQ(ss::MEDIUM, GetSignalStrengthForNetwork(wifi_network_.get()));
-  wifi_network_->set_signal_strength(74);
+  EXPECT_EQ(ss::WEAK, GetSignalStrengthForNetwork(wifi_network_.get()));
+  wifi_network_->set_signal_strength(51);
   EXPECT_EQ(ss::MEDIUM, GetSignalStrengthForNetwork(wifi_network_.get()));
   wifi_network_->set_signal_strength(75);
+  EXPECT_EQ(ss::MEDIUM, GetSignalStrengthForNetwork(wifi_network_.get()));
+  wifi_network_->set_signal_strength(76);
   EXPECT_EQ(ss::STRONG, GetSignalStrengthForNetwork(wifi_network_.get()));
   wifi_network_->set_signal_strength(100);
   EXPECT_EQ(ss::STRONG, GetSignalStrengthForNetwork(wifi_network_.get()));

ash/system/network/tray_network.cc

@@ -128,6 +128,7 @@ class NetworkTrayView : public TrayItemView,
       // to |connected_network|.
       base::string16 signal_strength_string;
       switch (network_icon::GetSignalStrengthForNetwork(connected_network)) {
+        case SignalStrength::NONE:
         case SignalStrength::NOT_WIRELESS:
           break;
         case SignalStrength::WEAK:
@@ -142,9 +143,6 @@ class NetworkTrayView : public TrayItemView,
           signal_strength_string = l10n_util::GetStringUTF16(
               IDS_ASH_STATUS_TRAY_NETWORK_SIGNAL_STRONG);
           break;
-        default:
-          NOTREACHED();
-          break;
       }
 
       if (!signal_strength_string.empty()) {