Cleanup shill_stub_helper, GetSharedProfilePath -> ShillProfileClient

Also replaces IsStubPortalledWifiEnabled() with more robust logic.

BUG=345033
R=pneubeck@chromium.org

Review URL: https://codereview.chromium.org/183033004

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

12 files changed, 51 insertions, 72 deletions

diff --git a/chromeos/dbus/fake_shill_service_client.cc b/chromeos/dbus/fake_shill_service_client.cc
index c968d80e..8987c18 100644
--- a/chromeos/dbus/fake_shill_service_client.cc
+++ b/chromeos/dbus/fake_shill_service_client.cc
@@ -13,6 +13,7 @@
 #include "base/values.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/shill_device_client.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_property_changed_observer.h"
 #include "chromeos/dbus/shill_stub_helper.h"
@@ -337,6 +338,9 @@ void FakeShillServiceClient::AddServiceWithIPConfig(
     bool add_to_watch_list) {
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
       AddManagerService(service_path, add_to_visible_list, add_to_watch_list);
+  std::string device_path =
+      DBusThreadManager::Get()->GetShillDeviceClient()->GetTestInterface()->
+      GetDevicePathForType(type);
 
   base::DictionaryValue* properties =
       GetModifiableServiceProperties(service_path, true);
@@ -347,8 +351,7 @@ void FakeShillServiceClient::AddServiceWithIPConfig(
       base::Value::CreateStringValue(name));
   properties->SetWithoutPathExpansion(
       shill::kDeviceProperty,
-      base::Value::CreateStringValue(
-          shill_stub_helper::DevicePathForType(type)));
+      base::Value::CreateStringValue(device_path));
   properties->SetWithoutPathExpansion(
       shill::kTypeProperty,
       base::Value::CreateStringValue(type));
diff --git a/chromeos/dbus/shill_profile_client.cc b/chromeos/dbus/shill_profile_client.cc
index 6fb0a1b..25f48b6 100644
--- a/chromeos/dbus/shill_profile_client.cc
+++ b/chromeos/dbus/shill_profile_client.cc
@@ -20,6 +20,8 @@ namespace chromeos {
 
 namespace {
 
+const char kSharedProfilePath[] = "/profile/default";
+
 class ShillProfileClientImpl : public ShillProfileClient {
  public:
   ShillProfileClientImpl();
@@ -138,4 +140,9 @@ ShillProfileClient* ShillProfileClient::Create() {
   return new ShillProfileClientImpl();
 }
 
+// static
+std::string ShillProfileClient::GetSharedProfilePath() {
+  return std::string(kSharedProfilePath);
+}
+
 }  // namespace chromeos
diff --git a/chromeos/dbus/shill_profile_client.h b/chromeos/dbus/shill_profile_client.h
index e6d3885..b237fb0 100644
--- a/chromeos/dbus/shill_profile_client.h
+++ b/chromeos/dbus/shill_profile_client.h
@@ -76,6 +76,9 @@ class CHROMEOS_EXPORT ShillProfileClient : public DBusClient {
   // For normal usage, access the singleton via DBusThreadManager::Get().
   static ShillProfileClient* Create();
 
+  // Returns the shared profile path.
+  static std::string GetSharedProfilePath();
+
   // Adds a property changed |observer| for the profile at |profile_path|.
   virtual void AddPropertyChangedObserver(
       const dbus::ObjectPath& profile_path,
diff --git a/chromeos/dbus/shill_stub_helper.cc b/chromeos/dbus/shill_stub_helper.cc
index fe1e0d0..9f2dcda 100644
--- a/chromeos/dbus/shill_stub_helper.cc
+++ b/chromeos/dbus/shill_stub_helper.cc
@@ -20,19 +20,11 @@ namespace shill_stub_helper {
 
 namespace {
 
-const char kDevicePathEthernet[] = "/device/eth1";
-const char kDevicePathWifi[] = "/device/wifi1";
-const char kDevicePathCellular[] = "/device/cellular1";
-const char kDevicePathWimax[] = "/device/wimax1";
-
-const char kStubPortalledWifiName[] = "Portalled Wifi";
-const char kStubPortalledWifiPath[] = "portalled_wifi";
-
-void UpdatePortalledWifiState() {
+void UpdatePortalledWifiState(const std::string& service_path) {
   ShillServiceClient::TestInterface* services =
       DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
 
-  services->SetServiceProperty(kStubPortalledWifiPath,
+  services->SetServiceProperty(service_path,
                                shill::kStateProperty,
                                base::StringValue(shill::kStatePortal));
 }
@@ -57,16 +49,6 @@ bool IsStubNetworkTypeEnabled(const std::string& network_type) {
 
 }  // namespace
 
-const char kSharedProfilePath[] = "/profile/default";
-
-bool IsStubPortalledWifiEnabled(const std::string& path) {
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-           chromeos::switches::kEnableStubPortalledWifi)) {
-    return false;
-  }
-  return path == kStubPortalledWifiPath;
-}
-
 void SetupDefaultEnvironment() {
   ShillServiceClient::TestInterface* services =
       DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
@@ -85,19 +67,19 @@ void SetupDefaultEnvironment() {
   manager->AddTechnology(shill::kTypeCellular, true);
   manager->AddTechnology(shill::kTypeWimax, true);
 
-  profiles->AddProfile(kSharedProfilePath, std::string());
+  std::string shared_profile = ShillProfileClient::GetSharedProfilePath();
+  profiles->AddProfile(shared_profile, std::string());
 
-  devices->AddDevice(
-      kDevicePathEthernet, shill::kTypeEthernet, "stub_eth_device1");
-  devices->AddDevice(kDevicePathWifi, shill::kTypeWifi, "stub_wifi_device1");
+  devices->AddDevice("/device/eth1", shill::kTypeEthernet, "stub_eth_device1");
+  devices->AddDevice("/device/wifi1", shill::kTypeWifi, "stub_wifi_device1");
 
   devices->AddDevice(
-      kDevicePathCellular, shill::kTypeCellular, "stub_cellular_device1");
-  devices->SetDeviceProperty(kDevicePathCellular,
+      "/device/cellular1", shill::kTypeCellular, "stub_cellular_device1");
+  devices->SetDeviceProperty("/device/cellular1",
                              shill::kCarrierProperty,
                              base::StringValue(shill::kCarrierSprint));
 
-  devices->AddDevice(kDevicePathWimax, shill::kTypeWimax, "stub_wimax_device1");
+  devices->AddDevice("/device/wimax1", shill::kTypeWimax, "stub_wimax_device1");
 
   const bool add_to_visible = true;
   const bool add_to_watchlist = true;
@@ -108,7 +90,7 @@ void SetupDefaultEnvironment() {
                          shill::kTypeEthernet,
                          shill::kStateOnline,
                          add_to_visible, add_to_watchlist);
-    profiles->AddService(kSharedProfilePath, "eth1");
+    profiles->AddService(shared_profile, "eth1");
   }
 
   // Wifi
@@ -122,7 +104,7 @@ void SetupDefaultEnvironment() {
   services->SetServiceProperty("wifi1",
                                shill::kSecurityProperty,
                                base::StringValue(shill::kSecurityWep));
-  profiles->AddService(kSharedProfilePath, "wifi1");
+  profiles->AddService(shared_profile, "wifi1");
 
   services->AddService("wifi2",
                        "wifi2_PSK",
@@ -135,23 +117,26 @@ void SetupDefaultEnvironment() {
   base::FundamentalValue strength_value(80);
   services->SetServiceProperty(
       "wifi2", shill::kSignalStrengthProperty, strength_value);
-  profiles->AddService(kSharedProfilePath, "wifi2");
+  profiles->AddService(shared_profile, "wifi2");
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           chromeos::switches::kEnableStubPortalledWifi)) {
-    services->AddService(kStubPortalledWifiPath,
-                         kStubPortalledWifiName,
+    const std::string kPortalledWifiPath = "portalled_wifi";
+    services->AddService(kPortalledWifiPath,
+                         "Portalled Wifi",
                          shill::kTypeWifi,
                          shill::kStatePortal,
                          add_to_visible, add_to_watchlist);
-    services->SetServiceProperty(kStubPortalledWifiPath,
+    services->SetServiceProperty(kPortalledWifiPath,
                                  shill::kSecurityProperty,
                                  base::StringValue(shill::kSecurityNone));
-    services->SetConnectBehavior(kStubPortalledWifiPath,
-                                 base::Bind(&UpdatePortalledWifiState));
-    services->SetServiceProperty(kStubPortalledWifiPath,
+    services->SetConnectBehavior(kPortalledWifiPath,
+                                 base::Bind(&UpdatePortalledWifiState,
+                                            kPortalledWifiPath));
+    services->SetServiceProperty(kPortalledWifiPath,
                                  shill::kConnectableProperty,
                                  base::FundamentalValue(true));
+    profiles->AddService(shared_profile, kPortalledWifiPath);
   }
 
   // Wimax
@@ -200,7 +185,7 @@ void SetupDefaultEnvironment() {
                        add_to_visible, add_to_watchlist);
   services->SetServiceProperty(
       "vpn1", shill::kProviderProperty, provider_properties);
-  profiles->AddService(kSharedProfilePath, "vpn1");
+  profiles->AddService(shared_profile, "vpn1");
 
   services->AddService("vpn2",
                        "vpn2",
@@ -213,17 +198,5 @@ void SetupDefaultEnvironment() {
   manager->SortManagerServices();
 }
 
-std::string DevicePathForType(const std::string& type) {
-  if (type == shill::kTypeEthernet)
-    return kDevicePathEthernet;
-  if (type == shill::kTypeWifi)
-    return kDevicePathWifi;
-  if (type == shill::kTypeCellular)
-    return kDevicePathCellular;
-  if (type == shill::kTypeWimax)
-    return kDevicePathWimax;
-  return "";
-}
-
 }  // namespace shill_stub_helper
 }  // namespace chromeos
diff --git a/chromeos/dbus/shill_stub_helper.h b/chromeos/dbus/shill_stub_helper.h
index b599014..dce7991 100644
--- a/chromeos/dbus/shill_stub_helper.h
+++ b/chromeos/dbus/shill_stub_helper.h
@@ -12,18 +12,10 @@
 namespace chromeos {
 namespace shill_stub_helper {
 
-CHROMEOS_EXPORT extern const char kSharedProfilePath[];
-
 // Add default devices, services and profiles. This works only if
 // DBusThreadManager was initialized with the FakeShill*Clients.
 void SetupDefaultEnvironment();
 
-std::string DevicePathForType(const std::string& type);
-
-// Returns true when stub portalled wifi is enabled and it's service
-// path equals to |path|.
-CHROMEOS_EXPORT bool IsStubPortalledWifiEnabled(const std::string& path);
-
 }  // namespace shill_stub_helper
 }  // namespace chromeos
 
diff --git a/chromeos/network/favorite_state.cc b/chromeos/network/favorite_state.cc
index f1a2584..f43d332 100644
--- a/chromeos/network/favorite_state.cc
+++ b/chromeos/network/favorite_state.cc
@@ -69,7 +69,7 @@ bool FavoriteState::PropertyChanged(const std::string& key,
 
 bool FavoriteState::IsPrivate() const {
   return !profile_path_.empty() &&
-         profile_path_ != NetworkProfileHandler::kSharedProfilePath;
+      profile_path_ != NetworkProfileHandler::GetSharedProfilePath();
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/managed_network_configuration_handler_unittest.cc b/chromeos/network/managed_network_configuration_handler_unittest.cc
index 2fe454d..2a81bcc 100644
--- a/chromeos/network/managed_network_configuration_handler_unittest.cc
+++ b/chromeos/network/managed_network_configuration_handler_unittest.cc
@@ -44,7 +44,6 @@ std::string ValueToString(const base::Value* value) {
   return str.str();
 }
 
-const char kSharedProfilePath[] = "/profile/default";
 const char kUser1[] = "user1";
 const char kUser1ProfilePath[] = "/profile/user1/shill";
 
@@ -208,7 +207,8 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {
     network_profile_handler_->
         AddProfileForTest(NetworkProfile(kUser1ProfilePath, kUser1));
     network_profile_handler_->
-        AddProfileForTest(NetworkProfile(kSharedProfilePath, std::string()));
+        AddProfileForTest(NetworkProfile(
+            NetworkProfileHandler::GetSharedProfilePath(), std::string()));
   }
 
   void SetUpEntry(const std::string& path_to_shill_json,
diff --git a/chromeos/network/network_configuration_handler_unittest.cc b/chromeos/network/network_configuration_handler_unittest.cc
index d1bd4be..03fabc4 100644
--- a/chromeos/network/network_configuration_handler_unittest.cc
+++ b/chromeos/network/network_configuration_handler_unittest.cc
@@ -12,8 +12,8 @@
 #include "chromeos/dbus/mock_shill_manager_client.h"
 #include "chromeos/dbus/mock_shill_profile_client.h"
 #include "chromeos/dbus/mock_shill_service_client.h"
-#include "chromeos/dbus/shill_stub_helper.h"
 #include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_handler_observer.h"
@@ -600,7 +600,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubCreateConfiguration) {
   properties.SetStringWithoutPathExpansion(
       shill::kStateProperty, shill::kStateIdle);
   properties.SetStringWithoutPathExpansion(
-      shill::kProfileProperty, shill_stub_helper::kSharedProfilePath);
+      shill::kProfileProperty, NetworkProfileHandler::GetSharedProfilePath());
 
   network_configuration_handler_->CreateConfiguration(
       properties,
@@ -620,7 +620,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubCreateConfiguration) {
   std::string actual_profile;
   EXPECT_TRUE(GetServiceStringProperty(
       create_service_path_, shill::kProfileProperty, &actual_profile));
-  EXPECT_EQ(shill_stub_helper::kSharedProfilePath, actual_profile);
+  EXPECT_EQ(NetworkProfileHandler::GetSharedProfilePath(), actual_profile);
 }
 
 }  // namespace chromeos
diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index decccfd..0c829cd 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -5,12 +5,10 @@
 #include "chromeos/network/network_connection_handler.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/strings/string_number_conversions.h"
 #include "chromeos/cert_loader.h"
-#include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_service_client.h"
@@ -92,11 +90,12 @@ std::string GetDefaultUserProfilePath(const NetworkState* network) {
       !LoginState::Get()->IsUserAuthenticated() ||
       (network && network->type() == shill::kTypeWifi &&
        network->security() == shill::kSecurityNone)) {
-    return NetworkProfileHandler::kSharedProfilePath;
+    return NetworkProfileHandler::GetSharedProfilePath();
   }
   const NetworkProfile* profile  =
       NetworkHandler::Get()->network_profile_handler()->GetDefaultUserProfile();
-  return profile ? profile->path : NetworkProfileHandler::kSharedProfilePath;
+  return profile ? profile->path
+                 : NetworkProfileHandler::GetSharedProfilePath();
 }
 
 }  // namespace
diff --git a/chromeos/network/network_profile_handler.cc b/chromeos/network/network_profile_handler.cc
index 19a47b6..f4f381e 100644
--- a/chromeos/network/network_profile_handler.cc
+++ b/chromeos/network/network_profile_handler.cc
@@ -56,7 +56,9 @@ class ProfilePathEquals {
 }  // namespace
 
 // static
-const char NetworkProfileHandler::kSharedProfilePath[] = "/profile/default";
+std::string NetworkProfileHandler::GetSharedProfilePath() {
+  return ShillProfileClient::GetSharedProfilePath();
+}
 
 void NetworkProfileHandler::AddObserver(NetworkProfileObserver* observer) {
   observers_.AddObserver(observer);
diff --git a/chromeos/network/network_profile_handler.h b/chromeos/network/network_profile_handler.h
index 15f52289..15e4e63 100644
--- a/chromeos/network/network_profile_handler.h
+++ b/chromeos/network/network_profile_handler.h
@@ -57,7 +57,7 @@ class CHROMEOS_EXPORT NetworkProfileHandler
   // userhash.
   const NetworkProfile* GetDefaultUserProfile() const;
 
-  static const char kSharedProfilePath[];
+  static std::string GetSharedProfilePath();
 
  protected:
   friend class ClientCertResolverTest;
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index ae779de..c448df0 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -236,7 +236,7 @@ bool NetworkState::IsConnectingState() const {
 
 bool NetworkState::IsPrivate() const {
   return !profile_path_.empty() &&
-      profile_path_ != NetworkProfileHandler::kSharedProfilePath;
+      profile_path_ != NetworkProfileHandler::GetSharedProfilePath();
 }
 
 std::string NetworkState::GetDnsServersAsString() const {