chromium-os:5494 Networks with identical names not handled properly.

Changed lookups based on name to use service_path instead.
Added wifi_network() and cellular_network() to network_library and removed wifi_name, wifi_strength, celular_name, cellular_service_path, and cellular_strength to simplify API.

BUG=chromium-os:5495
TEST=Network menu tests. Set up two networks with the same name, one encrypted and one not. Confirm that all operations on each work as expected.

Review URL: http://codereview.chromium.org/3744009

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

11 files changed, 72 insertions, 66 deletions

diff --git a/chrome/browser/chromeos/cros/cros_mock.cc b/chrome/browser/chromeos/cros/cros_mock.cc
index 9761d9b..ab33dc0 100644
--- a/chrome/browser/chromeos/cros/cros_mock.cc
+++ b/chrome/browser/chromeos/cros/cros_mock.cc
@@ -315,17 +315,17 @@ void CrosMock::SetNetworkLibraryStatusAreaExpectations() {
       .Times(1)
       .WillRepeatedly((Return(false)))
       .RetiresOnSaturation();
-  EXPECT_CALL(*mock_network_library_, wifi_name())
+  EXPECT_CALL(*mock_network_library_, wifi_network())
       .Times(1)
-      .WillRepeatedly((ReturnRef(empty_string_)))
+      .WillRepeatedly((ReturnRef(wifi_network_)))
       .RetiresOnSaturation();
   EXPECT_CALL(*mock_network_library_, cellular_networks())
       .Times(1)
       .WillRepeatedly((ReturnRef(cellular_networks_)))
       .RetiresOnSaturation();
-  EXPECT_CALL(*mock_network_library_, cellular_name())
+  EXPECT_CALL(*mock_network_library_, cellular_network())
       .Times(1)
-      .WillRepeatedly((ReturnRef(empty_string_)))
+      .WillRepeatedly((ReturnRef(cellular_network_)))
       .RetiresOnSaturation();
   EXPECT_CALL(*mock_network_library_, cellular_available())
       .Times(1)
diff --git a/chrome/browser/chromeos/cros/cros_mock.h b/chrome/browser/chromeos/cros/cros_mock.h
index f38081d..c39843e 100644
--- a/chrome/browser/chromeos/cros/cros_mock.h
+++ b/chrome/browser/chromeos/cros/cros_mock.h
@@ -103,7 +103,9 @@ class CrosMock {
   MockTouchpadLibrary* mock_touchpad_library_;
 
   ImePropertyList ime_properties_;
+  WifiNetwork wifi_network_;
   WifiNetworkVector wifi_networks_;
+  CellularNetwork cellular_network_;
   CellularNetworkVector cellular_networks_;
   std::string empty_string_;
 
diff --git a/chrome/browser/chromeos/cros/mock_network_library.h b/chrome/browser/chromeos/cros/mock_network_library.h
index 543d4f6f..71922aa 100644
--- a/chrome/browser/chromeos/cros/mock_network_library.h
+++ b/chrome/browser/chromeos/cros/mock_network_library.h
@@ -22,16 +22,13 @@ class MockNetworkLibrary : public NetworkLibrary {
   MOCK_CONST_METHOD0(ethernet_network, const EthernetNetwork&(void));
   MOCK_CONST_METHOD0(ethernet_connecting, bool(void));
   MOCK_CONST_METHOD0(ethernet_connected, bool(void));
-  MOCK_CONST_METHOD0(wifi_name, const std::string&(void));
+  MOCK_CONST_METHOD0(wifi_network, const WifiNetwork&(void));
   MOCK_CONST_METHOD0(wifi_connecting, bool(void));
   MOCK_CONST_METHOD0(wifi_connected, bool(void));
-  MOCK_CONST_METHOD0(wifi_strength, int(void));
 
-  MOCK_CONST_METHOD0(cellular_name, const std::string&(void));
-  MOCK_CONST_METHOD0(cellular_service_path, const std::string&(void));
+  MOCK_CONST_METHOD0(cellular_network, const CellularNetwork&(void));
   MOCK_CONST_METHOD0(cellular_connecting, bool(void));
   MOCK_CONST_METHOD0(cellular_connected, bool(void));
-  MOCK_CONST_METHOD0(cellular_strength, int(void));
 
   MOCK_CONST_METHOD0(Connected, bool(void));
   MOCK_CONST_METHOD0(Connecting, bool(void));
diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index 099ed89..a5a53df 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -450,17 +450,13 @@ class NetworkLibraryImpl : public NetworkLibrary  {
   virtual bool ethernet_connecting() const { return ethernet_.connecting(); }
   virtual bool ethernet_connected() const { return ethernet_.connected(); }
 
-  virtual const std::string& wifi_name() const { return wifi_.name(); }
+  virtual const WifiNetwork& wifi_network() const { return wifi_; }
   virtual bool wifi_connecting() const { return wifi_.connecting(); }
   virtual bool wifi_connected() const { return wifi_.connected(); }
-  virtual int wifi_strength() const { return wifi_.strength(); }
 
-  virtual const std::string& cellular_name() const { return cellular_.name(); }
-  virtual const std::string& cellular_service_path() const {
-    return cellular_.service_path(); }
+  virtual const CellularNetwork& cellular_network() const { return cellular_; }
   virtual bool cellular_connecting() const { return cellular_.connecting(); }
   virtual bool cellular_connected() const { return cellular_.connected(); }
-  virtual int cellular_strength() const { return cellular_.strength(); }
 
   bool Connected() const {
     return ethernet_connected() || wifi_connected() || cellular_connected();
@@ -840,7 +836,8 @@ class NetworkLibraryImpl : public NetworkLibrary  {
     NetworkLibraryImpl* network = static_cast<NetworkLibraryImpl*>(object);
     DCHECK(network);
     // Store data plan for currently connected cellular network.
-    if (network->cellular_service_path().compare(modem_service_path) == 0) {
+    if (network->cellular_network().service_path()
+        .compare(modem_service_path) == 0) {
       if (dataplan != NULL) {
         network->UpdateCellularDataPlan(*dataplan);
       }
@@ -1177,17 +1174,16 @@ class NetworkLibraryStubImpl : public NetworkLibrary {
   }
   virtual bool ethernet_connecting() const { return false; }
   virtual bool ethernet_connected() const { return true; }
-  virtual const std::string& wifi_name() const { return EmptyString(); }
+  virtual const WifiNetwork& wifi_network() const {
+    return wifi_;
+  }
   virtual bool wifi_connecting() const { return false; }
   virtual bool wifi_connected() const { return false; }
-  virtual int wifi_strength() const { return 0; }
-
-  virtual const std::string& cellular_name() const { return EmptyString(); }
-  virtual const std::string& cellular_service_path() const {
-    return EmptyString(); }
+  virtual const CellularNetwork& cellular_network() const {
+    return cellular_;
+  }
   virtual bool cellular_connecting() const { return false; }
   virtual bool cellular_connected() const { return false; }
-  virtual int cellular_strength() const { return false; }
 
   bool Connected() const { return true; }
   bool Connecting() const { return false; }
@@ -1251,6 +1247,8 @@ class NetworkLibraryStubImpl : public NetworkLibrary {
  private:
   std::string ip_address_;
   EthernetNetwork ethernet_;
+  WifiNetwork wifi_;
+  CellularNetwork cellular_;
   WifiNetworkVector wifi_networks_;
   CellularNetworkVector cellular_networks_;
 };
diff --git a/chrome/browser/chromeos/cros/network_library.h b/chrome/browser/chromeos/cros/network_library.h
index 668783c..7f7a7b1 100644
--- a/chrome/browser/chromeos/cros/network_library.h
+++ b/chrome/browser/chromeos/cros/network_library.h
@@ -311,20 +311,20 @@ class NetworkLibrary {
   virtual void AddObserver(Observer* observer) = 0;
   virtual void RemoveObserver(Observer* observer) = 0;
 
+  // Return the active Ethernet network (or a default structure if inactive).
   virtual const EthernetNetwork& ethernet_network() const = 0;
   virtual bool ethernet_connecting() const = 0;
   virtual bool ethernet_connected() const = 0;
 
-  virtual const std::string& wifi_name() const = 0;
+  // Return the active Wifi network (or a default structure if none active).
+  virtual const WifiNetwork& wifi_network() const = 0;
   virtual bool wifi_connecting() const = 0;
   virtual bool wifi_connected() const = 0;
-  virtual int wifi_strength() const = 0;
 
-  virtual const std::string& cellular_name() const = 0;
-  virtual const std::string& cellular_service_path() const = 0;
+  // Return the active Cellular network (or a default structure if none active).
+  virtual const CellularNetwork& cellular_network() const = 0;
   virtual bool cellular_connecting() const = 0;
   virtual bool cellular_connected() const = 0;
-  virtual int cellular_strength() const = 0;
 
   // Return true if any network is currently connected.
   virtual bool Connected() const = 0;
diff --git a/chrome/browser/chromeos/login/network_screen.cc b/chrome/browser/chromeos/login/network_screen.cc
index d35a98e..5e305a3 100644
--- a/chrome/browser/chromeos/login/network_screen.cc
+++ b/chrome/browser/chromeos/login/network_screen.cc
@@ -161,16 +161,16 @@ void NetworkScreen::UpdateStatus(NetworkLibrary* network) {
     StopWaitingForConnection(
         l10n_util::GetStringUTF16(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET));
   } else if (network->wifi_connected()) {
-    StopWaitingForConnection(ASCIIToUTF16(network->wifi_name()));
+    StopWaitingForConnection(ASCIIToUTF16(network->wifi_network().name()));
   } else if (network->cellular_connected()) {
-    StopWaitingForConnection(ASCIIToUTF16(network->cellular_name()));
+    StopWaitingForConnection(ASCIIToUTF16(network->cellular_network().name()));
   } else if (network->ethernet_connecting()) {
     WaitForConnection(
         l10n_util::GetStringUTF16(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET));
   } else if (network->wifi_connecting()) {
-    WaitForConnection(ASCIIToUTF16(network->wifi_name()));
+    WaitForConnection(ASCIIToUTF16(network->wifi_network().name()));
   } else if (network->cellular_connecting()) {
-    WaitForConnection(ASCIIToUTF16(network->cellular_name()));
+    WaitForConnection(ASCIIToUTF16(network->cellular_network().name()));
   } else {
     StopWaitingForConnection(network_id_);
   }
diff --git a/chrome/browser/chromeos/login/network_screen_browsertest.cc b/chrome/browser/chromeos/login/network_screen_browsertest.cc
index a2a4b34..bce1a6f 100644
--- a/chrome/browser/chromeos/login/network_screen_browsertest.cc
+++ b/chrome/browser/chromeos/login/network_screen_browsertest.cc
@@ -180,9 +180,9 @@ IN_PROC_BROWSER_TEST_F(NetworkScreenTest, Wifi) {
       .WillOnce((Return(false)));
   EXPECT_CALL(*mock_network_library_, wifi_connecting())
       .WillOnce((Return(true)));
-  std::string wifi_name = "wifi";
-  EXPECT_CALL(*mock_network_library_, wifi_name())
-      .WillOnce(ReturnRef(wifi_name));
+  WifiNetwork wifi;
+  EXPECT_CALL(*mock_network_library_, wifi_network())
+      .WillOnce(ReturnRef(wifi));
 
   network_screen->NetworkChanged(mock_network_library_);
   EXPECT_FALSE(network_view->IsContinueEnabled());
@@ -221,9 +221,9 @@ IN_PROC_BROWSER_TEST_F(NetworkScreenTest, Cellular) {
       .WillOnce((Return(false)));
   EXPECT_CALL(*mock_network_library_, cellular_connecting())
       .WillOnce((Return(true)));
-  std::string cellular_name = "3G";
-  EXPECT_CALL(*mock_network_library_, cellular_name())
-      .WillOnce(ReturnRef(cellular_name));
+  CellularNetwork cellular;
+  EXPECT_CALL(*mock_network_library_, cellular_network())
+      .WillOnce(ReturnRef(cellular));
 
   network_screen->NetworkChanged(mock_network_library_);
   EXPECT_FALSE(network_view->IsContinueEnabled());
@@ -260,9 +260,9 @@ IN_PROC_BROWSER_TEST_F(NetworkScreenTest, Timeout) {
       .WillOnce((Return(false)));
   EXPECT_CALL(*mock_network_library_, wifi_connecting())
       .WillOnce((Return(true)));
-  std::string wifi_name = "wifi";
-  EXPECT_CALL(*mock_network_library_, wifi_name())
-      .WillOnce(ReturnRef(wifi_name));
+  WifiNetwork wifi;
+  EXPECT_CALL(*mock_network_library_, wifi_network())
+      .WillOnce(ReturnRef(wifi));
   EXPECT_CALL(*mock_network_library_, Connected())
       .WillOnce(Return(false));
 
diff --git a/chrome/browser/chromeos/network_list.cc b/chrome/browser/chromeos/network_list.cc
index 9780642..926df02 100644
--- a/chrome/browser/chromeos/network_list.cc
+++ b/chrome/browser/chromeos/network_list.cc
@@ -78,7 +78,7 @@ void NetworkList::NetworkChanged(chromeos::NetworkLibrary* network_lib) {
                                     ASCIIToUTF16(it->name()),
                                     *it,
                                     CellularNetwork()));
-    if (network_lib->wifi_name() == it->name()) {
+    if (network_lib->wifi_network().service_path() == it->service_path()) {
       AddNetworkIndexToList(index,
                             network_lib->wifi_connected(),
                             network_lib->wifi_connecting());
@@ -92,7 +92,7 @@ void NetworkList::NetworkChanged(chromeos::NetworkLibrary* network_lib) {
                                     ASCIIToUTF16(it->name()),
                                     WifiNetwork(),
                                     *it));
-    if (network_lib->cellular_name() == it->name()) {
+    if (network_lib->cellular_network().service_path() == it->service_path()) {
       AddNetworkIndexToList(index,
                             network_lib->cellular_connected(),
                             network_lib->cellular_connecting());
diff --git a/chrome/browser/chromeos/status/network_dropdown_button.cc b/chrome/browser/chromeos/status/network_dropdown_button.cc
index 2f83d6e..d5fd453 100644
--- a/chrome/browser/chromeos/status/network_dropdown_button.cc
+++ b/chrome/browser/chromeos/status/network_dropdown_button.cc
@@ -85,12 +85,14 @@ void NetworkDropdownButton::NetworkChanged(NetworkLibrary* cros) {
       SetText(l10n_util::GetString(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET));
     } else if (cros->wifi_connected()) {
       animation_connecting_.Stop();
-      SetIcon(IconForNetworkStrength(cros->wifi_strength(), true));
-      SetText(ASCIIToWide(cros->wifi_name()));
+      SetIcon(IconForNetworkStrength(
+          cros->wifi_network().strength(), true));
+      SetText(ASCIIToWide(cros->wifi_network().name()));
     } else if (cros->cellular_connected()) {
       animation_connecting_.Stop();
-      SetIcon(IconForNetworkStrength(cros->cellular_strength(), false));
-      SetText(ASCIIToWide(cros->cellular_name()));
+      SetIcon(IconForNetworkStrength(
+          cros->cellular_network().strength(), false));
+      SetText(ASCIIToWide(cros->cellular_network().name()));
     } else if (cros->wifi_connecting() || cros->cellular_connecting()) {
       if (!animation_connecting_.is_animating()) {
         animation_connecting_.Reset();
@@ -99,9 +101,9 @@ void NetworkDropdownButton::NetworkChanged(NetworkLibrary* cros) {
       }
 
       if (cros->wifi_connecting())
-        SetText(ASCIIToWide(cros->wifi_name()));
+        SetText(ASCIIToWide(cros->wifi_network().name()));
       else if (cros->cellular_connecting())
-        SetText(ASCIIToWide(cros->cellular_name()));
+        SetText(ASCIIToWide(cros->cellular_network().name()));
     }
 
     if (!cros->Connected() && !cros->Connecting()) {
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index 149721c..d23e7ff 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -72,7 +72,7 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
         menu_items_[index].wireless_path, &wifi);
     if (found) {
       info->network_type = kNetworkTypeWifi;
-      if (wifi.name() == cros->wifi_name()) {
+      if (wifi.service_path() == cros->wifi_network().service_path()) {
         if (cros->wifi_connected()) {
           info->status = kNetworkStatusConnected;
           info->message = l10n_util::GetStringUTF8(
@@ -117,7 +117,8 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
         menu_items_[index].wireless_path, &cellular);
     if (found) {
       info->network_type = kNetworkTypeCellular;
-      if (cellular.name() == cros->cellular_name()) {
+      if (cellular.service_path() ==
+          cros->cellular_network().service_path()) {
         if (cros->cellular_connected()) {
           info->status = kNetworkStatusConnected;
           info->message = l10n_util::GetStringUTF8(
@@ -275,7 +276,7 @@ void NetworkMenu::ActivatedAt(int index) {
       // If we are attempting to connect to a network that no longer exists,
       // display a notification.
       // TODO(stevenjb): Show notification.
-    } else if (wifi.name() == cros->wifi_name()) {
+    } else if (wifi.service_path() == cros->wifi_network().service_path()) {
       // Show the config settings for the active network.
       ShowWifi(wifi, false);
     } else {
@@ -289,7 +290,8 @@ void NetworkMenu::ActivatedAt(int index) {
       // If we are attempting to connect to a network that no longer exists,
       // display a notification.
       // TODO(stevenjb): Show notification.
-    } else if (cellular.name() == cros->cellular_name()) {
+    } else if (cellular.service_path() ==
+               cros->cellular_network().service_path()) {
       // Show the config settings for the cellular network.
       ShowCellular(cellular, false);
     } else {
@@ -405,29 +407,31 @@ void NetworkMenu::InitMenuItems() {
   SkBitmap icon = *rb.GetBitmapNamed(IDR_STATUSBAR_WIRED_BLACK);
   SkBitmap badge = ethernet_connecting || ethernet_connected ?
       SkBitmap() : *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_DISCONNECTED);
-  int flag = (ethernet_connecting || ethernet_connected) ?
-      FLAG_ETHERNET | FLAG_ASSOCIATED : FLAG_ETHERNET;
+  int flag = FLAG_ETHERNET;
+  if (ethernet_connecting || ethernet_connected)
+    flag |= FLAG_ASSOCIATED;
   menu_items_.push_back(MenuItem(menus::MenuModel::TYPE_COMMAND, label,
       IconForDisplay(icon, badge), std::string(), flag));
 
   // Wifi
   const WifiNetworkVector& wifi_networks = cros->wifi_networks();
-  const std::string& active_wifi_name = cros->wifi_name();
+  const WifiNetwork& active_wifi = cros->wifi_network();
   // Wifi networks ssids.
   for (size_t i = 0; i < wifi_networks.size(); ++i) {
     label = ASCIIToUTF16(wifi_networks[i].name());
     SkBitmap icon = IconForNetworkStrength(wifi_networks[i].strength(), true);
     SkBitmap badge = wifi_networks[i].encrypted() ?
         *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_SECURE) : SkBitmap();
-    flag = (wifi_networks[i].name() == active_wifi_name) ?
-        FLAG_WIFI | FLAG_ASSOCIATED : FLAG_WIFI;
+    flag = FLAG_WIFI;
+    if (wifi_networks[i].service_path() == active_wifi.service_path())
+      flag |= FLAG_ASSOCIATED;
     menu_items_.push_back(MenuItem(menus::MenuModel::TYPE_COMMAND, label,
         IconForDisplay(icon, badge), wifi_networks[i].service_path(), flag));
   }
 
   // Cellular
   const CellularNetworkVector& cell_networks = cros->cellular_networks();
-  const std::string& active_cellular_name = cros->cellular_name();
+  const CellularNetwork& active_cellular = cros->cellular_network();
   // Cellular networks ssids.
   for (size_t i = 0; i < cell_networks.size(); ++i) {
     label = ASCIIToUTF16(cell_networks[i].name());
@@ -435,8 +439,9 @@ void NetworkMenu::InitMenuItems() {
     // TODO(chocobo): Check cellular network 3g/edge.
     SkBitmap badge = *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_3G);
 //    SkBitmap badge = *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_EDGE);
-    flag = (cell_networks[i].name() == active_cellular_name) ?
-        FLAG_CELLULAR | FLAG_ASSOCIATED : FLAG_CELLULAR;
+    flag = FLAG_CELLULAR;
+    if (cell_networks[i].service_path() == active_cellular.service_path())
+      flag |= FLAG_ASSOCIATED;
     menu_items_.push_back(MenuItem(menus::MenuModel::TYPE_COMMAND, label,
         IconForDisplay(icon, badge), cell_networks[i].service_path(), flag));
   }
diff --git a/chrome/browser/chromeos/status/network_menu_button.cc b/chrome/browser/chromeos/status/network_menu_button.cc
index f4ff11c..c16383e 100644
--- a/chrome/browser/chromeos/status/network_menu_button.cc
+++ b/chrome/browser/chromeos/status/network_menu_button.cc
@@ -98,7 +98,7 @@ void NetworkMenuButton::NetworkChanged(NetworkLibrary* cros) {
         SetIcon(*rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS1));
       }
       std::string network_name = cros->wifi_connecting() ?
-          cros->wifi_name() : cros->cellular_name();
+          cros->wifi_network().name() : cros->cellular_network().name();
       SetTooltipText(
           l10n_util::GetStringF(IDS_STATUSBAR_NETWORK_CONNECTING_TOOLTIP,
                                 UTF8ToWide(network_name)));
@@ -114,15 +114,17 @@ void NetworkMenuButton::NetworkChanged(NetworkLibrary* cros) {
                 IDS_STATUSBAR_NETWORK_CONNECTED_TOOLTIP,
                 l10n_util::GetString(IDS_STATUSBAR_NETWORK_DEVICE_ETHERNET)));
       } else if (cros->wifi_connected()) {
-        SetIcon(IconForNetworkStrength(cros->wifi_strength(), false));
+        SetIcon(IconForNetworkStrength(
+            cros->wifi_network().strength(), false));
         SetTooltipText(l10n_util::GetStringF(
             IDS_STATUSBAR_NETWORK_CONNECTED_TOOLTIP,
-            UTF8ToWide(cros->wifi_name())));
+            UTF8ToWide(cros->wifi_network().name())));
       } else if (cros->cellular_connected()) {
-        SetIcon(IconForNetworkStrength(cros->cellular_strength(), false));
+        SetIcon(IconForNetworkStrength(
+            cros->cellular_network().strength(), false));
         SetTooltipText(l10n_util::GetStringF(
             IDS_STATUSBAR_NETWORK_CONNECTED_TOOLTIP,
-            UTF8ToWide(cros->cellular_name())));
+            UTF8ToWide(cros->cellular_network().name())));
       } else {
         SetIcon(*rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS0));
         SetTooltipText(l10n_util::GetString(