Changed wifi arcs to mobile bars for Tether network.

Added ability to disassociate Tether and Wi-Fi networks, and now Wi-Fi and Tether networks are associated as soon as the network becomes connectable.

Adding stevenjb@ for network_icon.cc.

BUG=672263

Review-Url: https://codereview.chromium.org/2819303002
Cr-Commit-Position: refs/heads/master@{#469430}

ash/BUILD.gn

@@ -1186,6 +1186,7 @@ source_set("common_unittests") {
     "system/ime/tray_ime_chromeos_unittest.cc",
     "system/ime_menu/ime_menu_tray_unittest.cc",
     "system/media_security/multi_profile_media_tray_item_unittest.cc",
+    "system/network/network_icon_unittest.cc",
     "system/network/sms_observer_unittest.cc",
     "system/network/tray_network_unittest.cc",
     "system/network/vpn_list_unittest.cc",

ash/system/network/network_icon.cc

@@ -16,6 +16,7 @@
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/portal_detector/network_portal_detector.h"
+#include "chromeos/network/tether_constants.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
@@ -79,7 +80,9 @@ struct Badges {
 // class used for maintaining a map of network state and images.
 class NetworkIconImpl {
  public:
-  NetworkIconImpl(const std::string& path, IconType icon_type);
+  NetworkIconImpl(const std::string& path,
+                  IconType icon_type,
+                  const std::string& network_type);
 
   // Determines whether or not the associated network might be dirty and if so
   // updates and generates the icon. Does nothing if network no longer exists.
@@ -421,13 +424,31 @@ class SignalStrengthImageSource : public gfx::CanvasImageSource {
 // Utilities for extracting icon images.
 
 ImageType ImageTypeForNetworkType(const std::string& type) {
-  if (type == shill::kTypeWifi)
+  if (type == shill::kTypeWifi) {
     return ARCS;
-  else if (type == shill::kTypeCellular || type == shill::kTypeWimax)
+  } else if (type == shill::kTypeCellular || type == shill::kTypeWimax ||
+             type == chromeos::kTypeTether) {
     return BARS;
+  }
   return NONE;
 }
 
+// Returns the network type, performing a check to see if Wi-Fi networks
+// have an associated Tether network. Used to display the correct icon.
+std::string GetEffectiveNetworkType(const NetworkState* network,
+                                    IconType icon_type) {
+  if (icon_type == ICON_TYPE_TRAY && network->type() == shill::kTypeWifi &&
+      !network->tether_guid().empty()) {
+    return chromeos::kTypeTether;
+  }
+
+  return network->type();
+}
+
+ImageType ImageTypeForNetwork(const NetworkState* network, IconType icon_type) {
+  return ImageTypeForNetworkType(GetEffectiveNetworkType(network, icon_type));
+}
+
 gfx::ImageSkia GetImageForIndex(ImageType image_type,
                                 IconType icon_type,
                                 int index) {
@@ -533,9 +554,10 @@ gfx::ImageSkia GetIcon(const NetworkState* network,
     DCHECK_NE(ICON_TYPE_TRAY, icon_type);
     return gfx::CreateVectorIcon(kNetworkEthernetIcon,
                                  GetDefaultColorForIconType(ICON_TYPE_LIST));
-  } else if (network->Matches(NetworkTypePattern::Wireless())) {
+  } else if (network->Matches(NetworkTypePattern::Wireless()) ||
+             network->Matches(NetworkTypePattern::Tether())) {
     DCHECK(strength_index > 0);
-    return GetImageForIndex(ImageTypeForNetworkType(network->type()), icon_type,
+    return GetImageForIndex(ImageTypeForNetwork(network, icon_type), icon_type,
                             strength_index);
   } else if (network->Matches(NetworkTypePattern::VPN())) {
     DCHECK_NE(ICON_TYPE_TRAY, icon_type);
@@ -587,13 +609,15 @@ gfx::ImageSkia GetConnectingImage(IconType icon_type,
 //------------------------------------------------------------------------------
 // NetworkIconImpl
 
-NetworkIconImpl::NetworkIconImpl(const std::string& path, IconType icon_type)
+NetworkIconImpl::NetworkIconImpl(const std::string& path,
+                                 IconType icon_type,
+                                 const std::string& network_type)
     : network_path_(path),
       icon_type_(icon_type),
       strength_index_(-1),
       behind_captive_portal_(false) {
   // Default image
-  image_ = GetBasicImage(false, icon_type, shill::kTypeWifi);
+  image_ = GetBasicImage(false, icon_type, network_type);
 }
 
 void NetworkIconImpl::Update(const NetworkState* network) {
@@ -609,7 +633,8 @@ void NetworkIconImpl::Update(const NetworkState* network) {
 
   dirty |= UpdatePortalState(network);
 
-  if (network->Matches(NetworkTypePattern::Wireless())) {
+  if (network->Matches(NetworkTypePattern::Wireless()) ||
+      network->Matches(NetworkTypePattern::Tether())) {
     dirty |= UpdateWirelessStrengthIndex(network);
   }
 
@@ -733,7 +758,8 @@ NetworkIconImpl* FindAndUpdateImageImpl(const NetworkState* network,
   NetworkIconImpl* icon;
   NetworkIconMap::iterator iter = icon_map->find(network->path());
   if (iter == icon_map->end()) {
-    icon = new NetworkIconImpl(network->path(), icon_type);
+    icon = new NetworkIconImpl(network->path(), icon_type,
+                               GetEffectiveNetworkType(network, icon_type));
     icon_map->insert(std::make_pair(network->path(), icon));
   } else {
     icon = iter->second;
@@ -752,11 +778,13 @@ NetworkIconImpl* FindAndUpdateImageImpl(const NetworkState* network,
 gfx::ImageSkia GetImageForNetwork(const NetworkState* network,
                                   IconType icon_type) {
   DCHECK(network);
+  const std::string network_type = GetEffectiveNetworkType(network, icon_type);
+
   if (!network->visible())
-    return GetBasicImage(false, icon_type, network->type());
+    return GetBasicImage(false /* is_connected */, icon_type, network_type);
 
   if (network->IsConnectingState())
-    return GetConnectingImage(icon_type, network->type());
+    return GetConnectingImage(icon_type, network_type);
 
   NetworkIconImpl* icon = FindAndUpdateImageImpl(network, icon_type);
   return icon->image();

ash/system/network/network_icon_unittest.cc

+// Copyright 2017 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 "ash/system/network/network_icon.h"
+
+#include "ash/test/ash_test_base.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/tether_constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+#include "ui/gfx/image/image_unittest_util.h"
+
+namespace ash {
+
+namespace network_icon {
+
+class NetworkIconTest : public test::AshTestBase {
+ public:
+  NetworkIconTest() {}
+  ~NetworkIconTest() override {}
+
+  void SetUp() override {
+    test::AshTestBase::SetUp();
+    chromeos::NetworkHandler::Initialize();
+
+    tether_network =
+        base::MakeUnique<chromeos::NetworkState>("tetherNetworkPath");
+    tether_network->set_type(chromeos::kTypeTether);
+
+    wifi_network = base::MakeUnique<chromeos::NetworkState>("wifiServicePath");
+    wifi_network->set_type(shill::kTypeWifi);
+
+    cellular_network =
+        base::MakeUnique<chromeos::NetworkState>("cellularServicePath");
+    cellular_network->set_type(shill::kTypeCellular);
+
+    wifi_tether_network =
+        base::MakeUnique<chromeos::NetworkState>("wifiTetherServicePath");
+    wifi_tether_network->set_type(shill::kTypeWifi);
+    wifi_tether_network.get()->set_tether_guid("tetherNetworkGuid");
+  }
+
+  void TearDown() override {
+    PurgeNetworkIconCache();
+    chromeos::NetworkHandler::Shutdown();
+    test::AshTestBase::TearDown();
+  }
+
+  gfx::Image ImageForNetwork(chromeos::NetworkState* network) {
+    gfx::ImageSkia image_skia = GetImageForNetwork(network, icon_type);
+    return gfx::Image(image_skia);
+  }
+
+  // The icon for a Tether network should be the same as one for a cellular
+  // network. The icon for a Tether network should be different from one for a
+  // Wi-Fi network. The icon for a cellular network should be different from one
+  // for a Wi-Fi network. The icon for a Tether network should be the same as
+  // one for a Wi-Fi network with an associated Tether guid.
+  void GetAndCompareImagesByNetworkType() {
+    gfx::Image tether_image = ImageForNetwork(tether_network.get());
+    gfx::Image wifi_image = ImageForNetwork(wifi_network.get());
+    gfx::Image cellular_image = ImageForNetwork(cellular_network.get());
+    gfx::Image wifi_tether_image = ImageForNetwork(wifi_tether_network.get());
+
+    EXPECT_FALSE(gfx::test::AreImagesEqual(tether_image, wifi_image));
+    EXPECT_FALSE(gfx::test::AreImagesEqual(cellular_image, wifi_image));
+    EXPECT_TRUE(gfx::test::AreImagesEqual(tether_image, cellular_image));
+
+    EXPECT_TRUE(gfx::test::AreImagesEqual(tether_image, wifi_tether_image));
+  }
+
+  IconType icon_type = ICON_TYPE_TRAY;
+
+  std::unique_ptr<chromeos::NetworkState> tether_network;
+  std::unique_ptr<chromeos::NetworkState> wifi_network;
+  std::unique_ptr<chromeos::NetworkState> cellular_network;
+  // A network whose type is shill::kTypeWifi, but which is associated with
+  // a Tether network via its Tether network ID.
+  std::unique_ptr<chromeos::NetworkState> wifi_tether_network;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NetworkIconTest);
+};
+
+// This tests that the correct icons are being generated for the correct
+// networks by pairwise comparison of three different network types, verifying
+// that the Tether and cellular icon are the same, Tether and Wi-Fi icons are
+// different, and cellular and Wi-Fi icons are different. Additionaly, it
+// verifies that the Tether network and Wi-Fi network with associated Tether
+// guid are treated the same for purposes of icon display
+TEST_F(NetworkIconTest, CompareImagesByNetworkType_NotVisible) {
+  GetAndCompareImagesByNetworkType();
+}
+
+TEST_F(NetworkIconTest, CompareImagesByNetworkType_Connecting) {
+  tether_network->set_visible(true);
+  tether_network->set_connection_state(shill::kStateAssociation);
+
+  wifi_network->set_visible(true);
+  wifi_network->set_connection_state(shill::kStateAssociation);
+
+  cellular_network->set_visible(true);
+  cellular_network->set_connection_state(shill::kStateAssociation);
+
+  wifi_tether_network->set_visible(true);
+  wifi_tether_network->set_connection_state(shill::kStateAssociation);
+
+  GetAndCompareImagesByNetworkType();
+}
+
+TEST_F(NetworkIconTest, CompareImagesByNetworkType_Connected) {
+  tether_network->set_visible(true);
+  tether_network->set_connection_state(shill::kStateOnline);
+
+  wifi_network->set_visible(true);
+  wifi_network->set_connection_state(shill::kStateOnline);
+
+  cellular_network->set_visible(true);
+  cellular_network->set_connection_state(shill::kStateOnline);
+
+  wifi_tether_network->set_visible(true);
+  wifi_tether_network->set_connection_state(shill::kStateOnline);
+
+  GetAndCompareImagesByNetworkType();
+}
+
+}  // namespace network_icon
+
+}  // namespace ash

chromeos/components/tether/fake_wifi_hotspot_connector.cc

@@ -14,7 +14,8 @@ namespace tether {
 
 FakeWifiHotspotConnector::FakeWifiHotspotConnector(
     NetworkStateHandler* network_state_handler)
-    : WifiHotspotConnector(network_state_handler, nullptr) {}
+    : WifiHotspotConnector(network_state_handler,
+                           nullptr /* network_connect */) {}
 
 FakeWifiHotspotConnector::~FakeWifiHotspotConnector() {}
 
@@ -27,9 +28,11 @@ void FakeWifiHotspotConnector::CallMostRecentCallback(
 void FakeWifiHotspotConnector::ConnectToWifiHotspot(
     const std::string& ssid,
     const std::string& password,
+    const std::string& tether_network_guid,
     const WifiHotspotConnector::WifiConnectionCallback& callback) {
   most_recent_ssid_ = ssid;
   most_recent_password_ = password;
+  most_recent_tether_network_guid_ = tether_network_guid;
   most_recent_callback_ = callback;
 }
 

chromeos/components/tether/fake_wifi_hotspot_connector.h

@@ -27,15 +27,21 @@ class FakeWifiHotspotConnector : public WifiHotspotConnector {
 
   std::string most_recent_password() { return most_recent_password_; }
 
+  std::string most_recent_tether_network_guid() {
+    return most_recent_tether_network_guid_;
+  }
+
   // WifiHotspotConnector:
   void ConnectToWifiHotspot(
       const std::string& ssid,
       const std::string& password,
+      const std::string& tether_network_guid,
       const WifiHotspotConnector::WifiConnectionCallback& callback) override;
 
  private:
   std::string most_recent_ssid_;
   std::string most_recent_password_;
+  std::string most_recent_tether_network_guid_;
   WifiHotspotConnector::WifiConnectionCallback most_recent_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeWifiHotspotConnector);

chromeos/components/tether/tether_connector.cc

@@ -139,7 +139,7 @@ void TetherConnector::OnSuccessfulConnectTetheringResponse(
   connect_tethering_operation_.reset();
 
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      ssid_copy, password_copy,
+      ssid_copy, password_copy, active_host_->GetTetherNetworkGuid(),
       base::Bind(&TetherConnector::OnWifiConnection,
                  weak_ptr_factory_.GetWeakPtr(), remote_device_id));
 }
@@ -255,23 +255,6 @@ void TetherConnector::OnWifiConnection(const std::string& device_id,
     return;
   }
 
-  bool successful_association =
-      network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
-          device_id, wifi_network_guid);
-  if (successful_association) {
-    PA_LOG(INFO) << "Successfully connected to host device with ID "
-                 << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(device_id)
-                 << ". Tether network ID: \"" << device_id
-                 << "\", Wi-Fi network ID: \"" << wifi_network_guid << "\"";
-  } else {
-    PA_LOG(WARNING) << "Successfully connected to host device with ID "
-                    << cryptauth::RemoteDevice::TruncateDeviceIdForLogs(
-                           device_id)
-                    << ", but failed to associate tether network with ID \""
-                    << device_id << "\" to Wi-Fi network with ID \""
-                    << wifi_network_guid << "\".";
-  }
-
   SetConnectionSucceeded(device_id, wifi_network_guid);
 }
 

chromeos/components/tether/tether_connector_unittest.cc

@@ -181,19 +181,6 @@ class TetherConnectorTest : public NetworkStateTest {
     fake_wifi_hotspot_connector_->CallMostRecentCallback(kWifiNetworkGuid);
   }
 
-  void VerifyTetherAndWifiNetworkAssociation(
-      const std::string& tether_network_guid) {
-    const NetworkState* tether_network_state =
-        network_state_handler()->GetNetworkStateFromGuid(tether_network_guid);
-    EXPECT_TRUE(tether_network_state);
-    EXPECT_EQ(kWifiNetworkGuid, tether_network_state->tether_guid());
-
-    const NetworkState* wifi_network_state =
-        network_state_handler()->GetNetworkStateFromGuid(kWifiNetworkGuid);
-    EXPECT_TRUE(wifi_network_state);
-    EXPECT_EQ(tether_network_guid, wifi_network_state->tether_guid());
-  }
-
   void SuccessCallback() { result_ = kSuccessResult; }
 
   void ErrorCallback(const std::string& error_name) { result_ = error_name; }
@@ -329,6 +316,8 @@ TEST_F(TetherConnectorTest, TestConnectingToWifiFails) {
   // connect.
   EXPECT_EQ(kSsid, fake_wifi_hotspot_connector_->most_recent_ssid());
   EXPECT_EQ(kPassword, fake_wifi_hotspot_connector_->most_recent_password());
+  EXPECT_EQ(fake_active_host_->GetTetherNetworkGuid(),
+            fake_wifi_hotspot_connector_->most_recent_tether_network_guid());
   fake_wifi_hotspot_connector_->CallMostRecentCallback("");
 
   // The failure should have resulted in the host being disconnected.
@@ -389,10 +378,11 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
   // Wi-Fi network.
   EXPECT_EQ(kSsid, fake_wifi_hotspot_connector_->most_recent_ssid());
   EXPECT_EQ(kPassword, fake_wifi_hotspot_connector_->most_recent_password());
+  EXPECT_EQ(fake_active_host_->GetTetherNetworkGuid(),
+            fake_wifi_hotspot_connector_->most_recent_tether_network_guid());
   SuccessfullyJoinWifiNetwork();
 
-  // The active host should now be connected, and the tether and Wi-Fi networks
-  // should be associated.
+  // The active host should now be connected.
   EXPECT_EQ(ActiveHost::ActiveHostStatus::CONNECTED,
             fake_active_host_->GetActiveHostStatus());
   EXPECT_EQ(test_devices_[0].GetDeviceId(),
@@ -400,8 +390,7 @@ TEST_F(TetherConnectorTest, TestSuccessfulConnection) {
   EXPECT_EQ(GetTetherNetworkGuid(test_devices_[0].GetDeviceId()),
             fake_active_host_->GetTetherNetworkGuid());
   EXPECT_EQ(kWifiNetworkGuid, fake_active_host_->GetWifiNetworkGuid());
-  VerifyTetherAndWifiNetworkAssociation(
-      GetTetherNetworkGuid(test_devices_[0].GetDeviceId()));
+
   EXPECT_EQ(kSuccessResult, GetResultAndReset());
 }
 
@@ -465,6 +454,8 @@ TEST_F(TetherConnectorTest,
   // No connection should have been started.
   EXPECT_TRUE(fake_wifi_hotspot_connector_->most_recent_ssid().empty());
   EXPECT_TRUE(fake_wifi_hotspot_connector_->most_recent_password().empty());
+  EXPECT_TRUE(
+      fake_wifi_hotspot_connector_->most_recent_tether_network_guid().empty());
 
   // The second operation replies successfully, and this response should
   // result in a Wi-Fi connection attempt.
@@ -472,6 +463,8 @@ TEST_F(TetherConnectorTest,
       kSsid, kPassword);
   EXPECT_EQ(kSsid, fake_wifi_hotspot_connector_->most_recent_ssid());
   EXPECT_EQ(kPassword, fake_wifi_hotspot_connector_->most_recent_password());
+  EXPECT_EQ(fake_active_host_->GetTetherNetworkGuid(),
+            fake_wifi_hotspot_connector_->most_recent_tether_network_guid());
 }
 
 TEST_F(TetherConnectorTest,
@@ -491,6 +484,8 @@ TEST_F(TetherConnectorTest,
             fake_active_host_->GetActiveHostStatus());
   EXPECT_EQ(kSsid, fake_wifi_hotspot_connector_->most_recent_ssid());
   EXPECT_EQ(kPassword, fake_wifi_hotspot_connector_->most_recent_password());
+  EXPECT_EQ(fake_active_host_->GetTetherNetworkGuid(),
+            fake_wifi_hotspot_connector_->most_recent_tether_network_guid());
 
   // While the connection to the Wi-Fi network is in progress, start a new
   // connection attempt.

chromeos/components/tether/wifi_hotspot_connector.cc

@@ -36,6 +36,7 @@ WifiHotspotConnector::~WifiHotspotConnector() {
 void WifiHotspotConnector::ConnectToWifiHotspot(
     const std::string& ssid,
     const std::string& password,
+    const std::string& tether_network_guid,
     const WifiConnectionCallback& callback) {
   DCHECK(!ssid.empty());
   // Note: |password| can be empty in some cases.
@@ -44,14 +45,29 @@ void WifiHotspotConnector::ConnectToWifiHotspot(
     DCHECK(timer_->IsRunning());
 
     // If another connection attempt was underway but had not yet completed,
-    // call the callback, passing an empty string to signal that the connection
-    // did not complete successfully.
+    // disassociate that network from the Tether network and call the callback,
+    // passing an empty string to signal that the connection did not complete
+    // successfully.
+    bool successful_disassociation =
+        network_state_handler_->DisassociateTetherNetworkStateFromWifiNetwork(
+            tether_network_guid_);
+    if (successful_disassociation) {
+      PA_LOG(INFO) << "Wifi network with ID " << wifi_network_guid_
+                   << " successfully disassociated from Tether network with ID "
+                   << tether_network_guid_ << ".";
+    } else {
+      PA_LOG(INFO) << "Wifi network with ID " << wifi_network_guid_
+                   << " failed to disassociate from Tether network with ID "
+                   << tether_network_guid_ << ".";
+    }
+
     InvokeWifiConnectionCallback(std::string());
   }
 
   ssid_ = ssid;
   password_ = password;
-  wifi_guid_ = base::GenerateGUID();
+  tether_network_guid_ = tether_network_guid;
+  wifi_network_guid_ = base::GenerateGUID();
   callback_ = callback;
   timer_->Start(FROM_HERE,
                 base::TimeDelta::FromSeconds(kConnectionTimeoutSeconds),
@@ -65,21 +81,41 @@ void WifiHotspotConnector::ConnectToWifiHotspot(
 
 void WifiHotspotConnector::NetworkPropertiesUpdated(
     const NetworkState* network) {
-  if (network->guid() != wifi_guid_) {
+  if (network->guid() != wifi_network_guid_) {
     // If a different network has been connected, return early and wait for the
-    // network with ID |wifi_guid_| is updated.
+    // network with ID |wifi_network_guid_| is updated.
     return;
   }
 
   if (network->IsConnectedState()) {
     // If a connection occurred, notify observers and exit early.
-    InvokeWifiConnectionCallback(wifi_guid_);
+    InvokeWifiConnectionCallback(wifi_network_guid_);
     return;
   }
 
   if (network->connectable()) {
-    // If the network is now connectable, initiate a connection to it.
-    network_connect_->ConnectToNetworkId(wifi_guid_);
+    // If the network is now connectable, associate it with a Tether network
+    // ASAP so that the correct icon will be displayed in the tray while the
+    // network is connecting.
+    bool successful_association =
+        network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
+            tether_network_guid_, wifi_network_guid_);
+    if (successful_association) {
+      PA_LOG(INFO) << "Wifi network with ID " << wifi_network_guid_
+                   << " is connectable, and successfully associated "
+                      "with Tether network. Tether network ID: \""
+                   << tether_network_guid_ << "\", Wi-Fi network ID: \""
+                   << wifi_network_guid_ << "\"";
+    } else {
+      PA_LOG(INFO) << "Wifi network with ID " << wifi_network_guid_
+                   << " is connectable, but failed to associate tether network "
+                      "with ID \""
+                   << tether_network_guid_ << "\" to Wi-Fi network with ID: \""
+                   << wifi_network_guid_ << "\"";
+    }
+
+    // Initiate a connection to the network.
+    network_connect_->ConnectToNetworkId(wifi_network_guid_);
   }
 }
 
@@ -87,17 +123,17 @@ void WifiHotspotConnector::InvokeWifiConnectionCallback(
     const std::string& wifi_guid) {
   DCHECK(!callback_.is_null());
 
-  // |wifi_guid| may be a reference to |wifi_guid_|, so make a copy of it first
-  // before clearing it below.
-  std::string wifi_guid_copy = wifi_guid;
+  // |wifi_guid| may be a reference to |wifi_network_guid_|, so make a copy of
+  // it first before clearing it below.
+  std::string wifi_network_guid_copy = wifi_guid;
 
   ssid_.clear();
   password_.clear();
-  wifi_guid_.clear();
+  wifi_network_guid_.clear();
 
   timer_->Stop();
 
-  callback_.Run(wifi_guid_copy);
+  callback_.Run(wifi_network_guid_copy);
   callback_.Reset();
 }
 
@@ -107,12 +143,13 @@ base::DictionaryValue WifiHotspotConnector::CreateWifiPropertyDictionary(
   PA_LOG(INFO) << "Creating network configuration. "
                << "SSID: " << ssid << ", "
                << "Password: " << password << ", "
-               << "Wi-Fi network GUID: " << wifi_guid_;
+               << "Wi-Fi network GUID: " << wifi_network_guid_;
 
   base::DictionaryValue properties;
 
   shill_property_util::SetSSID(ssid, &properties);
-  properties.SetStringWithoutPathExpansion(shill::kGuidProperty, wifi_guid_);
+  properties.SetStringWithoutPathExpansion(shill::kGuidProperty,
+                                           wifi_network_guid_);
   properties.SetBooleanWithoutPathExpansion(shill::kAutoConnectProperty, false);
   properties.SetStringWithoutPathExpansion(shill::kTypeProperty,
                                            shill::kTypeWifi);

chromeos/components/tether/wifi_hotspot_connector.h

@@ -11,6 +11,7 @@
 #include "base/observer_list.h"
 #include "base/timer/timer.h"
 #include "base/values.h"
+#include "chromeos/components/tether/active_host.h"
 #include "chromeos/network/network_state_handler_observer.h"
 
 namespace chromeos {
@@ -39,6 +40,7 @@ class WifiHotspotConnector : public NetworkStateHandlerObserver {
   // will begin.
   virtual void ConnectToWifiHotspot(const std::string& ssid,
                                     const std::string& password,
+                                    const std::string& tether_network_guid,
                                     const WifiConnectionCallback& callback);
 
   // NetworkStateHandlerObserver:
@@ -65,7 +67,8 @@ class WifiHotspotConnector : public NetworkStateHandlerObserver {
 
   std::string ssid_;
   std::string password_;
-  std::string wifi_guid_;
+  std::string tether_network_guid_;
+  std::string wifi_network_guid_;
   WifiConnectionCallback callback_;
 
   base::WeakPtrFactory<WifiHotspotConnector> weak_ptr_factory_;

chromeos/components/tether/wifi_hotspot_connector_unittest.cc

@@ -36,6 +36,9 @@ const char kPassword[] = "password";
 
 const char kOtherWifiServiceGuid[] = "otherWifiServiceGuid";
 
+const char kTetherNetworkGuid[] = "tetherNetworkGuid";
+const char kTetherNetworkGuid2[] = "tetherNetworkGuid2";
+
 std::string CreateConfigurationJsonString(const std::string& guid) {
   std::stringstream ss;
   ss << "{"
@@ -116,10 +119,23 @@ class WifiHotspotConnectorTest : public NetworkStateTest {
 
     DBusThreadManager::Initialize();
     NetworkStateTest::SetUp();
+    network_state_handler()->SetTetherTechnologyState(
+        NetworkStateHandler::TechnologyState::TECHNOLOGY_ENABLED);
+
     SetUpShillState();
 
     test_network_connect_ = base::WrapUnique(new TestNetworkConnect(this));
 
+    network_state_handler()->AddTetherNetworkState(
+        kTetherNetworkGuid, "tetherNetworkName" /* name */,
+        "tetherNetworkCarrier" /* carrier */, 100 /* full battery */,
+        100 /* full signal strength */, false /* has_connected_to_host */);
+
+    network_state_handler()->AddTetherNetworkState(
+        kTetherNetworkGuid2, "tetherNetworkName2" /* name */,
+        "tetherNetworkCarrier2" /* carrier */, 100 /* full battery */,
+        100 /* full signal strength */, false /* has_connected_to_host */);
+
     wifi_hotspot_connector_ = base::WrapUnique(new WifiHotspotConnector(
         network_state_handler(), test_network_connect_.get()));
 
@@ -201,6 +217,26 @@ class WifiHotspotConnectorTest : public NetworkStateTest {
     return wifi_guid;
   }
 
+  void VerifyTetherAndWifiNetworkAssociation(const std::string& wifi_guid,
+                                             const std::string& tether_guid) {
+    const NetworkState* wifi_network_state =
+        network_state_handler()->GetNetworkStateFromGuid(wifi_guid);
+    ASSERT_TRUE(wifi_network_state);
+    EXPECT_EQ(tether_guid, wifi_network_state->tether_guid());
+
+    const NetworkState* tether_network_state =
+        network_state_handler()->GetNetworkStateFromGuid(tether_guid);
+    ASSERT_TRUE(tether_network_state);
+    EXPECT_EQ(wifi_guid, tether_network_state->tether_guid());
+  }
+
+  void VerifyNetworkNotAssociated(const std::string& guid) {
+    const NetworkState* network_state =
+        network_state_handler()->GetNetworkStateFromGuid(guid);
+    ASSERT_TRUE(network_state);
+    EXPECT_TRUE(network_state->tether_guid().empty());
+  }
+
   void WifiConnectionCallback(const std::string& wifi_guid) {
     connection_callback_responses_.push_back(wifi_guid);
   }
@@ -221,7 +257,7 @@ class WifiHotspotConnectorTest : public NetworkStateTest {
 
 TEST_F(WifiHotspotConnectorTest, TestConnect_NetworkDoesNotBecomeConnectable) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      std::string(kSsid), std::string(kPassword),
+      std::string(kSsid), std::string(kPassword), kTetherNetworkGuid,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -241,7 +277,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_NetworkDoesNotBecomeConnectable) {
 
 TEST_F(WifiHotspotConnectorTest, TestConnect_AnotherNetworkBecomesConnectable) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      std::string(kSsid), std::string(kPassword),
+      std::string(kSsid), std::string(kPassword), kTetherNetworkGuid,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -252,6 +288,11 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_AnotherNetworkBecomesConnectable) {
   // Another network becomes connectable. This should not cause the connection
   // to start.
   NotifyConnectable(other_wifi_service_path_);
+  VerifyNetworkNotAssociated(wifi_guid);
+  std::string other_wifi_guid = network_state_handler()
+                                    ->GetNetworkState(other_wifi_service_path_)
+                                    ->guid();
+  VerifyNetworkNotAssociated(other_wifi_guid);
   EXPECT_EQ("", test_network_connect_->network_id_to_connect());
 
   // Timeout timer fires.
@@ -263,7 +304,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_AnotherNetworkBecomesConnectable) {
 
 TEST_F(WifiHotspotConnectorTest, TestConnect_CannotConnectToNetwork) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      std::string(kSsid), std::string(kPassword),
+      std::string(kSsid), std::string(kPassword), kTetherNetworkGuid,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -273,6 +314,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_CannotConnectToNetwork) {
 
   // Network becomes connectable.
   NotifyConnectable(test_network_connect_->last_service_path_created());
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid, kTetherNetworkGuid);
   EXPECT_EQ(wifi_guid, test_network_connect_->network_id_to_connect());
 
   // Network connection does not occur.
@@ -286,7 +328,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_CannotConnectToNetwork) {
 
 TEST_F(WifiHotspotConnectorTest, TestConnect_Success) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      std::string(kSsid), std::string(kPassword),
+      std::string(kSsid), std::string(kPassword), kTetherNetworkGuid,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -296,6 +338,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_Success) {
 
   // Network becomes connectable.
   NotifyConnectable(test_network_connect_->last_service_path_created());
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid, kTetherNetworkGuid);
   EXPECT_EQ(wifi_guid, test_network_connect_->network_id_to_connect());
   EXPECT_EQ(0u, connection_callback_responses_.size());
 
@@ -308,7 +351,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_Success) {
 
 TEST_F(WifiHotspotConnectorTest, TestConnect_Success_EmptyPassword) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      std::string(kSsid), "",
+      std::string(kSsid), "" /* password */, kTetherNetworkGuid,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -317,6 +360,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_Success_EmptyPassword) {
 
   // Network becomes connectable.
   NotifyConnectable(test_network_connect_->last_service_path_created());
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid, kTetherNetworkGuid);
   EXPECT_EQ(wifi_guid, test_network_connect_->network_id_to_connect());
   EXPECT_EQ(0u, connection_callback_responses_.size());
 
@@ -330,7 +374,7 @@ TEST_F(WifiHotspotConnectorTest, TestConnect_Success_EmptyPassword) {
 TEST_F(WifiHotspotConnectorTest,
        TestConnect_SecondConnectionWhileWaitingForFirstToBecomeConnectable) {
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      "ssid1", "password1",
+      "ssid1", "password1", "tetherNetworkGuid1",
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -343,7 +387,7 @@ TEST_F(WifiHotspotConnectorTest,
   // Before network becomes connectable, start the new connection.
   EXPECT_EQ(0u, connection_callback_responses_.size());
   wifi_hotspot_connector_->ConnectToWifiHotspot(
-      "ssid2", "password2",
+      "ssid2", "password2", kTetherNetworkGuid2,
       base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
                  base::Unretained(this)));
 
@@ -352,6 +396,7 @@ TEST_F(WifiHotspotConnectorTest,
   std::string service_path2 =
       test_network_connect_->last_service_path_created();
   EXPECT_FALSE(service_path2.empty());
+  VerifyNetworkNotAssociated(wifi_guid1);
 
   EXPECT_NE(service_path1, service_path2);
 
@@ -368,6 +413,59 @@ TEST_F(WifiHotspotConnectorTest,
 
   // Second network becomes connectable.
   NotifyConnectable(service_path2);
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid2, kTetherNetworkGuid2);
+  EXPECT_EQ(wifi_guid2, test_network_connect_->network_id_to_connect());
+  EXPECT_EQ(1u, connection_callback_responses_.size());
+
+  // Connection to network successful.
+  NotifyConnected(service_path2);
+  EXPECT_EQ(2u, connection_callback_responses_.size());
+  EXPECT_EQ(wifi_guid2, connection_callback_responses_[1]);
+  VerifyTimerStopped();
+}
+
+TEST_F(WifiHotspotConnectorTest,
+       TestConnect_SecondConnectionWhileWaitingForFirstToConnect) {
+  wifi_hotspot_connector_->ConnectToWifiHotspot(
+      "ssid1", "password1", kTetherNetworkGuid,
+      base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
+                 base::Unretained(this)));
+
+  std::string wifi_guid1 = VerifyLastConfiguration("ssid1", "password1");
+  EXPECT_FALSE(wifi_guid1.empty());
+  std::string service_path1 =
+      test_network_connect_->last_service_path_created();
+  EXPECT_FALSE(service_path1.empty());
+
+  // First network becomes connectable.
+  NotifyConnectable(service_path1);
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid1, kTetherNetworkGuid);
+
+  // After network becomes connectable, request a connection to second network.
+  wifi_hotspot_connector_->ConnectToWifiHotspot(
+      "ssid2", "password2", kTetherNetworkGuid2,
+      base::Bind(&WifiHotspotConnectorTest::WifiConnectionCallback,
+                 base::Unretained(this)));
+
+  // The first Tether and Wi-Fi networks should no longer be associated.
+  VerifyNetworkNotAssociated(kTetherNetworkGuid);
+  VerifyNetworkNotAssociated(wifi_guid1);
+
+  // The original connection attempt should have gotten a "" response.
+  EXPECT_EQ(1u, connection_callback_responses_.size());
+  EXPECT_EQ("", connection_callback_responses_[0]);
+
+  std::string wifi_guid2 = VerifyLastConfiguration("ssid2", "password2");
+  EXPECT_FALSE(wifi_guid2.empty());
+  std::string service_path2 =
+      test_network_connect_->last_service_path_created();
+  EXPECT_FALSE(service_path2.empty());
+
+  EXPECT_NE(service_path1, service_path2);
+
+  // Second network becomes connectable.
+  NotifyConnectable(service_path2);
+  VerifyTetherAndWifiNetworkAssociation(wifi_guid2, kTetherNetworkGuid2);
   EXPECT_EQ(wifi_guid2, test_network_connect_->network_id_to_connect());
   EXPECT_EQ(1u, connection_callback_responses_.size());
 

chromeos/network/managed_state.h

@@ -14,6 +14,12 @@
 #include "base/macros.h"
 #include "chromeos/chromeos_export.h"
 
+namespace ash {
+namespace network_icon {
+class NetworkIconTest;
+}  // namespace network_icon
+}  // namesapce ash
+
 namespace base {
 class Value;
 class DictionaryValue;
@@ -113,6 +119,7 @@ class CHROMEOS_EXPORT ManagedState {
  private:
   friend class NetworkChangeNotifierChromeosUpdateTest;
   friend class NetworkStateHandler;
+  friend class ash::network_icon::NetworkIconTest;
 
   ManagedType managed_type_;
 

chromeos/network/network_state_handler.cc

@@ -594,13 +594,44 @@ bool NetworkStateHandler::RemoveTetherNetworkState(const std::string& guid) {
 
       tether_network_list_.erase(iter);
       NotifyNetworkListChanged();
+
       return true;
     }
   }
-
   return false;
 }
 
+bool NetworkStateHandler::DisassociateTetherNetworkStateFromWifiNetwork(
+    const std::string& tether_network_guid) {
+  NetworkState* tether_network =
+      GetModifiableNetworkStateFromGuid(tether_network_guid);
+
+  if (!tether_network) {
+    NET_LOG(ERROR) << "DisassociateTetherNetworkStateWithWifiNetwork(): Tether "
+                   << "network with ID " << tether_network_guid
+                   << " not registered; could not remove association.";
+    return false;
+  }
+
+  std::string wifi_network_guid = tether_network->tether_guid();
+  NetworkState* wifi_network =
+      GetModifiableNetworkStateFromGuid(wifi_network_guid);
+
+  if (!wifi_network) {
+    NET_LOG(ERROR) << "DisassociateTetherNetworkStateWithWifiNetwork(): Wi-Fi "
+                   << "network with ID " << wifi_network_guid
+                   << " not registered; could not remove association.";
+    return false;
+  }
+
+  wifi_network->set_tether_guid(std::string());
+  tether_network->set_tether_guid(std::string());
+
+  NotifyNetworkListChanged();
+
+  return true;
+}
+
 bool NetworkStateHandler::AssociateTetherNetworkStateWithWifiNetwork(
     const std::string& tether_network_guid,
     const std::string& wifi_network_guid) {

chromeos/network/network_state_handler.h

@@ -240,6 +240,12 @@ class CHROMEOS_EXPORT NetworkStateHandler
   // function does nothing. Returns whether the network was actually removed.
   bool RemoveTetherNetworkState(const std::string& guid);
 
+  // Disassociates the Tether network specified by |tether_network_guid| from
+  // its associated Wi-Fi network. Returns whether the networkd were
+  // successfully disassociated.
+  bool DisassociateTetherNetworkStateFromWifiNetwork(
+      const std::string& tether_network_guid);
+
   // Inform NetworkStateHandler that the provided Tether network with the
   // provided guid |tether_network_guid| is associated with the Wi-Fi network
   // with the provided guid |wifi_network_guid|. This Wi-Fi network can now be

chromeos/network/network_state_handler_unittest.cc

@@ -818,6 +818,49 @@ TEST_F(NetworkStateHandlerTest, TetherNetworkStateAssociation) {
   ASSERT_TRUE(wifi_network->tether_guid().empty());
 }
 
+TEST_F(NetworkStateHandlerTest, TetherNetworkStateDisassociation) {
+  network_state_handler_->SetTetherTechnologyState(
+      NetworkStateHandler::TECHNOLOGY_ENABLED);
+
+  const std::string profile = "/profile/profile1";
+  const std::string wifi_path = "/service/wifi_with_guid";
+  AddService(wifi_path, kWifiGuid1, kWifiName1, shill::kTypeWifi,
+             shill::kStateOnline);
+  profile_test_->AddProfile(profile, "" /* userhash */);
+  EXPECT_TRUE(profile_test_->AddService(profile, wifi_path));
+  UpdateManagerProperties();
+
+  EXPECT_EQ(1u, test_observer_->network_list_changed_count());
+
+  network_state_handler_->AddTetherNetworkState(
+      kTetherGuid1, kTetherName1, kTetherCarrier1, kTetherBatteryPercentage1,
+      kTetherSignalStrength1, kTetherHasConnectedToHost1);
+
+  EXPECT_EQ(2u, test_observer_->network_list_changed_count());
+
+  EXPECT_TRUE(
+      network_state_handler_->AssociateTetherNetworkStateWithWifiNetwork(
+          kTetherGuid1, kWifiGuid1));
+
+  EXPECT_EQ(3u, test_observer_->network_list_changed_count());
+
+  const NetworkState* wifi_network =
+      network_state_handler_->GetNetworkStateFromGuid(kWifiGuid1);
+  EXPECT_EQ(kTetherGuid1, wifi_network->tether_guid());
+
+  const NetworkState* tether_network =
+      network_state_handler_->GetNetworkStateFromGuid(kTetherGuid1);
+  EXPECT_EQ(kWifiGuid1, tether_network->tether_guid());
+
+  network_state_handler_->DisassociateTetherNetworkStateFromWifiNetwork(
+      kTetherGuid1);
+
+  ASSERT_TRUE(wifi_network->tether_guid().empty());
+  ASSERT_TRUE(tether_network->tether_guid().empty());
+
+  EXPECT_EQ(4u, test_observer_->network_list_changed_count());
+}
+
 TEST_F(NetworkStateHandlerTest, TetherNetworkStateAssociationWifiRemoved) {
   network_state_handler_->SetTetherTechnologyState(
       NetworkStateHandler::TECHNOLOGY_ENABLED);