Add NetworkConnectionHandler class

This moves NetworkConfigurationHandler::Connect -> 
NetworkConnectionHandler::ConnectToNetwork and adds error handling
and certificate checking.

Depends on https://codereview.chromium.org/14522013/

Test with --use-new-network-configuration-handlers

BUG=235243

Review URL: https://chromiumcodereview.appspot.com/14566009

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

5 files changed, 115 insertions, 100 deletions

diff --git a/chromeos/dbus/shill_manager_client.h b/chromeos/dbus/shill_manager_client.h
index 231da2c..e9fca9c 100644
--- a/chromeos/dbus/shill_manager_client.h
+++ b/chromeos/dbus/shill_manager_client.h
@@ -42,13 +42,6 @@ class CHROMEOS_EXPORT ShillManagerClient {
     virtual void AddDevice(const std::string& device_path) = 0;
     virtual void RemoveDevice(const std::string& device_path) = 0;
     virtual void ClearDevices() = 0;
-    virtual void AddService(const std::string& service_path,
-                            bool add_to_watch_list) = 0;
-    virtual void AddServiceAtIndex(const std::string& service_path,
-                                   size_t index,
-                                   bool add_to_watch_list) = 0;
-    virtual void RemoveService(const std::string& service_path) = 0;
-    virtual void ClearServices() = 0;
     virtual void AddTechnology(const std::string& type, bool enabled) = 0;
     virtual void RemoveTechnology(const std::string& type) = 0;
     virtual void SetTechnologyInitializing(const std::string& type,
@@ -65,6 +58,18 @@ class CHROMEOS_EXPORT ShillManagerClient {
     // Used to reset all properties; does not notify observers.
     virtual void ClearProperties() = 0;
 
+    // Move an existing service to a different index, e.g. to simulate the
+    // result of a successful connect.
+    virtual void MoveServiceToIndex(const std::string& service_path,
+                                    size_t index,
+                                    bool add_to_watch_list) = 0;
+
+    // Add/Remove/ClearService should only be called from ShillServiceClient.
+    virtual void AddManagerService(const std::string& service_path,
+                                   bool add_to_watch_list) = 0;
+    virtual void RemoveManagerService(const std::string& service_path) = 0;
+    virtual void ClearManagerServices() = 0;
+
    protected:
     virtual ~TestInterface() {}
   };
diff --git a/chromeos/dbus/shill_manager_client_stub.cc b/chromeos/dbus/shill_manager_client_stub.cc
index cd406c3..6e20a6a 100644
--- a/chromeos/dbus/shill_manager_client_stub.cc
+++ b/chromeos/dbus/shill_manager_client_stub.cc
@@ -39,7 +39,7 @@ struct ValueEquals {
 }  // namespace
 
 ShillManagerClientStub::ShillManagerClientStub()
-: weak_ptr_factory_(this) {
+    : weak_ptr_factory_(this) {
   SetDefaultProperties();
 }
 
@@ -180,9 +180,6 @@ void ShillManagerClientStub::ConfigureService(
     const base::DictionaryValue& properties,
     const ObjectPathCallback& callback,
     const ErrorCallback& error_callback) {
-  if (callback.is_null())
-    return;
-
   ShillServiceClient::TestInterface* service_client =
       DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface();
 
@@ -190,10 +187,13 @@ void ShillManagerClientStub::ConfigureService(
   std::string type;
   if (!properties.GetString(flimflam::kGuidProperty, &guid) ||
       !properties.GetString(flimflam::kTypeProperty, &type)) {
+    LOG(ERROR) << "ConfigureService requies GUID and Type to be defined";
     // If the properties aren't filled out completely, then just return an empty
     // object path.
-    MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(callback, dbus::ObjectPath()));
+    if (!callback.is_null()) {
+      MessageLoop::current()->PostTask(
+          FROM_HERE, base::Bind(callback, dbus::ObjectPath()));
+    }
     return;
   }
 
@@ -231,8 +231,10 @@ void ShillManagerClientStub::ConfigureService(
       DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface();
   profile_test->AddService(service_path);
 
-  MessageLoop::current()->PostTask(
-      FROM_HERE, base::Bind(callback, dbus::ObjectPath(service_path)));
+  if (!callback.is_null()) {
+    MessageLoop::current()->PostTask(
+        FROM_HERE, base::Bind(callback, dbus::ObjectPath(service_path)));
+  }
 }
 
 void ShillManagerClientStub::ConfigureServiceForProfile(
@@ -320,53 +322,6 @@ void ShillManagerClientStub::ClearDevices() {
   CallNotifyObserversPropertyChanged(flimflam::kDevicesProperty, 0);
 }
 
-void ShillManagerClientStub::ClearServices() {
-  GetListProperty(flimflam::kServicesProperty)->Clear();
-  GetListProperty(flimflam::kServiceWatchListProperty)->Clear();
-  CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
-  CallNotifyObserversPropertyChanged(flimflam::kServiceWatchListProperty, 0);
-}
-
-void ShillManagerClientStub::AddService(const std::string& service_path,
-                                        bool add_to_watch_list) {
-  if (GetListProperty(flimflam::kServicesProperty)->AppendIfNotPresent(
-      base::Value::CreateStringValue(service_path))) {
-    CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
-  }
-  if (add_to_watch_list)
-    AddServiceToWatchList(service_path);
-}
-
-void ShillManagerClientStub::AddServiceAtIndex(const std::string& service_path,
-                                               size_t index,
-                                               bool add_to_watch_list) {
-  base::StringValue path_value(service_path);
-  base::ListValue* service_list =
-      GetListProperty(flimflam::kServicesProperty);
-  base::ListValue::iterator iter =
-      std::find_if(service_list->begin(), service_list->end(),
-                   ValueEquals(&path_value));
-  if (iter != service_list->end())
-    service_list->Erase(iter, NULL);
-  service_list->Insert(index, path_value.DeepCopy());
-  CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
-  if (add_to_watch_list)
-    AddServiceToWatchList(service_path);
-}
-
-void ShillManagerClientStub::RemoveService(const std::string& service_path) {
-  base::StringValue service_path_value(service_path);
-  if (GetListProperty(flimflam::kServicesProperty)->Remove(
-      service_path_value, NULL)) {
-    CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
-  }
-  if (GetListProperty(flimflam::kServiceWatchListProperty)->Remove(
-      service_path_value, NULL)) {
-    CallNotifyObserversPropertyChanged(
-        flimflam::kServiceWatchListProperty, 0);
-  }
-}
-
 void ShillManagerClientStub::AddTechnology(const std::string& type,
                                            bool enabled) {
   if (GetListProperty(flimflam::kAvailableTechnologiesProperty)->
@@ -417,6 +372,57 @@ void ShillManagerClientStub::ClearProperties() {
   stub_properties_.Clear();
 }
 
+void ShillManagerClientStub::MoveServiceToIndex(
+    const std::string& service_path,
+    size_t index,
+    bool add_to_watch_list) {
+  base::StringValue path_value(service_path);
+  base::ListValue* service_list = GetListProperty(flimflam::kServicesProperty);
+  base::ListValue::iterator iter =
+      std::find_if(service_list->begin(), service_list->end(),
+                   ValueEquals(&path_value));
+  if (iter == service_list->end()) {
+    LOG(ERROR) << "Service not found to move: " << service_path;
+    return;
+  }
+  service_list->Erase(iter, NULL);
+  service_list->Insert(index, path_value.DeepCopy());
+  CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+  if (add_to_watch_list)
+    AddServiceToWatchList(service_path);
+}
+
+void ShillManagerClientStub::AddManagerService(const std::string& service_path,
+                                               bool add_to_watch_list) {
+  if (GetListProperty(flimflam::kServicesProperty)->AppendIfNotPresent(
+      base::Value::CreateStringValue(service_path))) {
+    CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+  }
+  if (add_to_watch_list)
+    AddServiceToWatchList(service_path);
+}
+
+void ShillManagerClientStub::RemoveManagerService(
+    const std::string& service_path) {
+  base::StringValue service_path_value(service_path);
+  if (GetListProperty(flimflam::kServicesProperty)->Remove(
+      service_path_value, NULL)) {
+    CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+  }
+  if (GetListProperty(flimflam::kServiceWatchListProperty)->Remove(
+      service_path_value, NULL)) {
+    CallNotifyObserversPropertyChanged(
+        flimflam::kServiceWatchListProperty, 0);
+  }
+}
+
+void ShillManagerClientStub::ClearManagerServices() {
+  GetListProperty(flimflam::kServicesProperty)->Clear();
+  GetListProperty(flimflam::kServiceWatchListProperty)->Clear();
+  CallNotifyObserversPropertyChanged(flimflam::kServicesProperty, 0);
+  CallNotifyObserversPropertyChanged(flimflam::kServiceWatchListProperty, 0);
+}
+
 void ShillManagerClientStub::AddGeoNetwork(
     const std::string& technology,
     const base::DictionaryValue& network) {
@@ -439,12 +445,13 @@ void ShillManagerClientStub::AddProfile(const std::string& profile_path) {
 
 void ShillManagerClientStub::AddServiceToWatchList(
     const std::string& service_path) {
-  if (GetListProperty(
-      flimflam::kServiceWatchListProperty)->AppendIfNotPresent(
-          base::Value::CreateStringValue(service_path))) {
-    CallNotifyObserversPropertyChanged(
-        flimflam::kServiceWatchListProperty, 0);
-  }
+  // Remove and insert the service, moving it to the front of the watch list.
+  GetListProperty(flimflam::kServiceWatchListProperty)->Remove(
+      base::StringValue(service_path), NULL);
+  GetListProperty(flimflam::kServiceWatchListProperty)->Insert(
+      0, base::Value::CreateStringValue(service_path));
+  CallNotifyObserversPropertyChanged(
+      flimflam::kServiceWatchListProperty, 0);
 }
 
 void ShillManagerClientStub::SetDefaultProperties() {
@@ -479,12 +486,6 @@ void ShillManagerClientStub::PassStubGeoNetworks(
 void ShillManagerClientStub::CallNotifyObserversPropertyChanged(
     const std::string& property,
     int delay_ms) {
-  // Don't actually delay unless we're interactive.
-  if (!CommandLine::ForCurrentProcess()->HasSwitch(
-      chromeos::switches::kEnableStubInteractive)) {
-    delay_ms = 0;
-  }
-
   // Avoid unnecessary delayed task if we have no observers (e.g. during
   // initial setup).
   if (observer_list_.size() == 0)
diff --git a/chromeos/dbus/shill_manager_client_stub.h b/chromeos/dbus/shill_manager_client_stub.h
index d1ff8de..53863a2 100644
--- a/chromeos/dbus/shill_manager_client_stub.h
+++ b/chromeos/dbus/shill_manager_client_stub.h
@@ -13,9 +13,9 @@
 
 namespace chromeos {
 
-// A stub implementation of ShillManagerClient.
-// Implemented: Stub devices and services for NetworkStateManager tests.
-// Implemented: Stub cellular device entry for SMS tests.
+// A stub implementation of ShillManagerClient. This works in close coordination
+// with ShillServiceClientStub. ShillDeviceClientStub, and
+// ShillProfileClientStub, and is not intended to be used independently.
 class ShillManagerClientStub : public ShillManagerClient,
                                public ShillManagerClient::TestInterface {
  public:
@@ -78,17 +78,9 @@ class ShillManagerClientStub : public ShillManagerClient,
   virtual ShillManagerClient::TestInterface* GetTestInterface() OVERRIDE;
 
   // ShillManagerClient::TestInterface overrides.
-
   virtual void AddDevice(const std::string& device_path) OVERRIDE;
   virtual void RemoveDevice(const std::string& device_path) OVERRIDE;
   virtual void ClearDevices() OVERRIDE;
-  virtual void ClearServices() OVERRIDE;
-  virtual void AddService(const std::string& service_path,
-                          bool add_to_watch_list) OVERRIDE;
-  virtual void AddServiceAtIndex(const std::string& service_path,
-                                 size_t index,
-                                 bool add_to_watch_list) OVERRIDE;
-  virtual void RemoveService(const std::string& service_path) OVERRIDE;
   virtual void AddTechnology(const std::string& type, bool enabled) OVERRIDE;
   virtual void RemoveTechnology(const std::string& type) OVERRIDE;
   virtual void SetTechnologyInitializing(const std::string& type,
@@ -97,6 +89,13 @@ class ShillManagerClientStub : public ShillManagerClient,
                              const base::DictionaryValue& network) OVERRIDE;
   virtual void AddProfile(const std::string& profile_path) OVERRIDE;
   virtual void ClearProperties() OVERRIDE;
+  virtual void MoveServiceToIndex(const std::string& service_path,
+                                  size_t index,
+                                  bool add_to_watch_list) OVERRIDE;
+  virtual void AddManagerService(const std::string& service_path,
+                                 bool add_to_watch_list) OVERRIDE;
+  virtual void RemoveManagerService(const std::string& service_path) OVERRIDE;
+  virtual void ClearManagerServices() OVERRIDE;
 
  private:
   void AddServiceToWatchList(const std::string& service_path);
diff --git a/chromeos/dbus/shill_service_client_stub.cc b/chromeos/dbus/shill_service_client_stub.cc
index 677f9d6..434c4ce 100644
--- a/chromeos/dbus/shill_service_client_stub.cc
+++ b/chromeos/dbus/shill_service_client_stub.cc
@@ -112,8 +112,7 @@ void ShillServiceClientStub::SetProperty(const dbus::ObjectPath& service_path,
     if (value.GetAsString(&state) && state == flimflam::kStateOnline) {
       ShillManagerClient* manager_client =
           DBusThreadManager::Get()->GetShillManagerClient();
-      manager_client->GetTestInterface()->RemoveService(service_path.value());
-      manager_client->GetTestInterface()->AddServiceAtIndex(
+      manager_client->GetTestInterface()->MoveServiceToIndex(
           service_path.value(), 0, true);
     }
   }
@@ -189,13 +188,18 @@ void ShillServiceClientStub::Connect(const dbus::ObjectPath& service_path,
     error_callback.Run("Error.InvalidService", "Invalid Service");
     return;
   }
-  // Set Associating
-  base::StringValue associating_value(flimflam::kStateAssociation);
-  SetServiceProperty(service_path.value(),
-                     flimflam::kStateProperty,
-                     associating_value);
+  base::TimeDelta delay;
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kEnableStubInteractive)) {
+    // Set Associating
+    base::StringValue associating_value(flimflam::kStateAssociation);
+    SetServiceProperty(service_path.value(),
+                       flimflam::kStateProperty,
+                       associating_value);
+    const int kConnectDelaySeconds = 5;
+    delay = base::TimeDelta::FromSeconds(kConnectDelaySeconds);
+  }
   // Set Online after a delay
-  const int kConnectDelaySeconds = 5;
   base::StringValue online_value(flimflam::kStateOnline);
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
@@ -206,7 +210,7 @@ void ShillServiceClientStub::Connect(const dbus::ObjectPath& service_path,
                  online_value,
                  base::Bind(&base::DoNothing),
                  error_callback),
-                 base::TimeDelta::FromSeconds(kConnectDelaySeconds));
+      delay);
   callback.Run();
 }
 
@@ -218,8 +222,13 @@ void ShillServiceClientStub::Disconnect(const dbus::ObjectPath& service_path,
     error_callback.Run("Error.InvalidService", "Invalid Service");
     return;
   }
+  base::TimeDelta delay;
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kEnableStubInteractive)) {
+    const int kConnectDelaySeconds = 2;
+    delay = base::TimeDelta::FromSeconds(kConnectDelaySeconds);
+  }
   // Set Idle after a delay
-  const int kConnectDelaySeconds = 2;
   base::StringValue idle_value(flimflam::kStateIdle);
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
@@ -230,7 +239,7 @@ void ShillServiceClientStub::Disconnect(const dbus::ObjectPath& service_path,
                  idle_value,
                  base::Bind(&base::DoNothing),
                  error_callback),
-                 base::TimeDelta::FromSeconds(kConnectDelaySeconds));
+      delay);
   callback.Run();
 }
 
@@ -290,7 +299,7 @@ void ShillServiceClientStub::AddServiceWithIPConfig(
     const std::string& ipconfig_path,
     bool add_to_watch_list) {
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
-      AddService(service_path, add_to_watch_list);
+      AddManagerService(service_path, add_to_watch_list);
 
   base::DictionaryValue* properties =
       GetModifiableServiceProperties(service_path);
@@ -314,7 +323,7 @@ void ShillServiceClientStub::AddServiceWithIPConfig(
 
 void ShillServiceClientStub::RemoveService(const std::string& service_path) {
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
-      RemoveService(service_path);
+      RemoveManagerService(service_path);
 
   stub_services_.RemoveWithoutPathExpansion(service_path, NULL);
 }
@@ -336,7 +345,7 @@ const base::DictionaryValue* ShillServiceClientStub::GetServiceProperties(
 
 void ShillServiceClientStub::ClearServices() {
   DBusThreadManager::Get()->GetShillManagerClient()->GetTestInterface()->
-      ClearServices();
+      ClearManagerServices();
 
   stub_services_.Clear();
 }
diff --git a/chromeos/dbus/shill_service_client_stub.h b/chromeos/dbus/shill_service_client_stub.h
index 2e9bfdd..e30a5f3 100644
--- a/chromeos/dbus/shill_service_client_stub.h
+++ b/chromeos/dbus/shill_service_client_stub.h
@@ -15,7 +15,8 @@
 
 namespace chromeos {
 
-// A stub implementation of ShillServiceClient.
+// A stub implementation of ShillServiceClient. This works in close coordination
+// with ShillManagerClientStub and is not intended to be used independently.
 class ShillServiceClientStub : public ShillServiceClient,
                                public ShillServiceClient::TestInterface {
  public: