Convert all connect code to use NetworkHandler instead of NetworkLibrary

chromeos/network/network_connection_handler.cc

 // Copyright (c) 2013 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 "chromeos/network/network_connection_handler.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "chromeos/chromeos_switches.h"
@@ skipping 44 matching lines @@
     dest->SetStringWithoutPathExpansion(key, string_value);
 }
 
 bool NetworkRequiresActivation(const NetworkState* network) {
   return (network->type() == flimflam::kTypeCellular &&
           (network->activation_state() != flimflam::kActivationStateActivated ||
            network->cellular_out_of_credits()));
 }
 
 bool VPNIsConfigured(const std::string& service_path,
-                     const base::DictionaryValue& service_properties) {
-  // VPN Provider values are read from the "Provider" dictionary, not the
-  // "Provider.Type", etc keys (which are used only to set the values).
-  const base::DictionaryValue* provider_properties;
-  if (!service_properties.GetDictionaryWithoutPathExpansion(
-          flimflam::kProviderProperty, &provider_properties)) {
-    NET_LOG_ERROR("VPN Provider Dictionary not present", service_path);
-    return false;
-  }
-  std::string provider_type;
-  if (!provider_properties->GetStringWithoutPathExpansion(
-          flimflam::kTypeProperty, &provider_type)) {
-    NET_LOG_ERROR("VPN Provider Type not present", service_path);
-    return false;
-  }
+                     const std::string& provider_type,
+                     const base::DictionaryValue& provider_properties) {
   if (provider_type == flimflam::kProviderOpenVpn) {
     std::string hostname;
-    provider_properties->GetStringWithoutPathExpansion(
+    provider_properties.GetStringWithoutPathExpansion(
         flimflam::kHostProperty, &hostname);
     if (hostname.empty()) {
       NET_LOG_EVENT("OpenVPN: No hostname", service_path);
       return false;
     }
     std::string username;
-    provider_properties->GetStringWithoutPathExpansion(
+    provider_properties.GetStringWithoutPathExpansion(
         flimflam::kOpenVPNUserProperty, &username);
     if (username.empty()) {
       NET_LOG_EVENT("OpenVPN: No username", service_path);
       return false;
     }
     bool passphrase_required = false;
-    provider_properties->GetBooleanWithoutPathExpansion(
+    provider_properties.GetBooleanWithoutPathExpansion(
         flimflam::kPassphraseRequiredProperty, &passphrase_required);
-    std::string passphrase;
-    provider_properties->GetStringWithoutPathExpansion(
-        flimflam::kOpenVPNPasswordProperty, &passphrase);
-    if (passphrase_required && passphrase.empty()) {
-      NET_LOG_EVENT("OpenVPN: No passphrase", service_path);
+    if (passphrase_required) {
+      NET_LOG_EVENT("OpenVPN: Passphrase Required", service_path);
       return false;
     }
     NET_LOG_EVENT("OpenVPN Is Configured", service_path);
   } else {
     bool passphrase_required = false;
     std::string passphrase;
-    provider_properties->GetBooleanWithoutPathExpansion(
+    provider_properties.GetBooleanWithoutPathExpansion(
         flimflam::kL2tpIpsecPskRequiredProperty, &passphrase_required);
-    provider_properties->GetStringWithoutPathExpansion(
-        flimflam::kL2tpIpsecPskProperty, &passphrase);
-    if (passphrase_required && passphrase.empty())
+    if (passphrase_required) {
+      NET_LOG_EVENT("VPN: Passphrase Required", service_path);
       return false;
+    }
     NET_LOG_EVENT("VPN Is Configured", service_path);
   }
   return true;
 }
 
 }  // namespace
 
 const char NetworkConnectionHandler::kErrorNotFound[] = "not-found";
 const char NetworkConnectionHandler::kErrorConnected[] = "connected";
 const char NetworkConnectionHandler::kErrorConnecting[] = "connecting";
 const char NetworkConnectionHandler::kErrorNotConnected[] = "not-connected";
 const char NetworkConnectionHandler::kErrorPassphraseRequired[] =
     "passphrase-required";
 const char NetworkConnectionHandler::kErrorActivationRequired[] =
     "activation-required";
 const char NetworkConnectionHandler::kErrorCertificateRequired[] =
     "certificate-required";
 const char NetworkConnectionHandler::kErrorConfigurationRequired[] =
     "configuration-required";
 const char NetworkConnectionHandler::kErrorAuthenticationRequired[] =
     "authentication-required";
 const char NetworkConnectionHandler::kErrorShillError[] = "shill-error";
 const char NetworkConnectionHandler::kErrorConnectFailed[] = "connect-failed";
-const char NetworkConnectionHandler::kErrorDisconnectFailed[] =
-    "disconnect-failed";
-const char NetworkConnectionHandler::kErrorMissingProviderType[] =
-    "missing-provider-type";
+const char NetworkConnectionHandler::kErrorMissingProvider[] =
+    "missing-provider";
 const char NetworkConnectionHandler::kErrorUnknown[] = "unknown-error";
 
 struct NetworkConnectionHandler::ConnectRequest {
   ConnectRequest(const std::string& service_path,
                  const base::Closure& success,
                  const network_handler::ErrorCallback& error)
       : service_path(service_path),
         connect_state(CONNECT_REQUESTED),
         success_callback(success),
         error_callback(error) {
@@ skipping 44 matching lines @@
   }
   if (network_state_handler) {
     network_state_handler_ = network_state_handler;
     network_state_handler_->AddObserver(this, FROM_HERE);
   }
   network_configuration_handler_ = network_configuration_handler;
 }
 
 void NetworkConnectionHandler::LoggedInStateChanged(
     LoginState::LoggedInState state) {
-  NET_LOG_EVENT("NewNetworkConnectionHandler",
-                CommandLine::ForCurrentProcess()->HasSwitch(
-                    chromeos::switches::kUseNewNetworkConnectionHandler) ?
-                "enabled" : "disabled");
   if (state == LoginState::LOGGED_IN_ACTIVE) {
     logged_in_ = true;
     NET_LOG_EVENT("Logged In", "");
   }
 }
 
 void NetworkConnectionHandler::OnCertificatesLoaded(
     const net::CertificateList& cert_list,
     bool initial_load) {
   certificates_loaded_ = true;
@@ skipping 89 matching lines @@
     InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
     return;
   }
   if (!network->IsConnectedState()) {
     InvokeErrorCallback(service_path, error_callback, kErrorNotConnected);
     return;
   }
   CallShillDisconnect(service_path, success_callback, error_callback);
 }
 
+void NetworkConnectionHandler::ActivateNetwork(
+    const std::string& service_path,
+    const std::string& carrier,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback) {
+  NET_LOG_USER("DisconnectNetwork", service_path);
+  const NetworkState* network =
+      network_state_handler_->GetNetworkState(service_path);
+  if (!network) {
+    InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
+    return;
+  }
+  CallShillActivate(service_path, carrier, success_callback, error_callback);
+}
+
 bool NetworkConnectionHandler::HasConnectingNetwork(
     const std::string& service_path) {
   return pending_requests_.count(service_path) != 0;
 }
 
 void NetworkConnectionHandler::NetworkListChanged() {
   CheckAllPendingRequests();
 }
 
 void NetworkConnectionHandler::NetworkPropertiesUpdated(
@@ skipping 45 matching lines @@
   // If 'passphrase_required' is still true, then the 'Passphrase' property
   // has not been set to a minimum length value.
   bool passphrase_required = false;
   service_properties.GetBooleanWithoutPathExpansion(
       flimflam::kPassphraseRequiredProperty, &passphrase_required);
   if (passphrase_required) {
     ErrorCallbackForPendingRequest(service_path, kErrorPassphraseRequired);
     return;
   }
 
-  // VPN requires a host and username to be set.
-  if (type == flimflam::kTypeVPN &&
-      !VPNIsConfigured(service_path, service_properties)) {
-    ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
-    return;
+  // Get VPN provider type and host (required for configuration) and ensure
+  // that required VPN non-cert properties are set.
+  std::string vpn_provider_type, vpn_provider_host;
+  if (type == flimflam::kTypeVPN) {
+    // VPN Provider values are read from the "Provider" dictionary, not the
+    // "Provider.Type", etc keys (which are used only to set the values).
+    const base::DictionaryValue* provider_properties;
+    if (service_properties.GetDictionaryWithoutPathExpansion(
+            flimflam::kProviderProperty, &provider_properties)) {
+      provider_properties->GetStringWithoutPathExpansion(
+          flimflam::kTypeProperty, &vpn_provider_type);
+      provider_properties->GetStringWithoutPathExpansion(
+          flimflam::kHostProperty, &vpn_provider_host);
+    }
+    if (vpn_provider_type.empty() || vpn_provider_host.empty()) {
+      ErrorCallbackForPendingRequest(service_path, kErrorMissingProvider);
+      return;
+    }
+    // VPN requires a host and username to be set.
+    if (!VPNIsConfigured(
+            service_path, vpn_provider_type, *provider_properties)) {
+      NET_LOG_ERROR("VPN Not Configured", service_path);
+      ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
+      return;
+    }
   }
 
-  // Check certificate properties in kUIDataProperty.
+  // These will be set if they need to be configured, otherwise they will
+  // be left empty and the properties will not be set.
+  std::string pkcs11_id, tpm_slot, tpm_pin;
+
+  // Check certificate properties in kUIDataProperty if configured.
+  // Note: Wifi/VPNConfigView set these properties explicitly.
   scoped_ptr<NetworkUIData> ui_data =
       ManagedNetworkConfigurationHandler::GetUIData(service_properties);
   if (ui_data && ui_data->certificate_type() == CLIENT_CERT_TYPE_PATTERN) {
     // User must be logged in to connect to a network requiring a certificate.
     if (!logged_in_ || !cert_loader_) {
       ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
       return;
     }
+
     // If certificates have not been loaded yet, queue the connect request.
     if (!certificates_loaded_) {
       ConnectRequest* request = pending_request(service_path);
       DCHECK(request);
       NET_LOG_EVENT("Connect Request Queued", service_path);
       queued_connect_.reset(new ConnectRequest(
           service_path, request->success_callback, request->error_callback));
       pending_requests_.erase(service_path);
       return;
     }
 
-    // Ensure the certificate is available.
-    std::string pkcs11_id;
+    // Ensure the certificate is available and configured.
     if (!CertificateIsConfigured(ui_data.get(), &pkcs11_id)) {
       ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
       return;
     }
+  }
 
-    // The network may not be 'Connectable' because the certificate data is
-    // not set up, so configure tpm slot/pin and pkcs11_id before connecting.
-    // TODO(stevenjb): Remove this code once NetworkConfigurationHandler
-    // handles this.
+  // The network may not be 'Connectable' because the TPM properties are
+  // not set up, so configure tpm slot/pin before connecting.
+  if (cert_loader_) {
+    tpm_slot = cert_loader_->tpm_token_slot();
+    tpm_pin = cert_loader_->tpm_user_pin();
+  }
+
+  base::DictionaryValue config_properties;
+
+  if (type == flimflam::kTypeVPN) {
+    if (vpn_provider_type == flimflam::kProviderOpenVpn) {
+      if (!pkcs11_id.empty()) {
+        config_properties.SetStringWithoutPathExpansion(
+            flimflam::kOpenVPNClientCertIdProperty, pkcs11_id);
+      }
+      if (!tpm_pin.empty()) {
+        config_properties.SetStringWithoutPathExpansion(
+            flimflam::kOpenVPNPinProperty, tpm_pin);
+      }
+    } else {
+      if (!pkcs11_id.empty()) {
+        config_properties.SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecClientCertIdProperty, pkcs11_id);
+      }
+      if (!tpm_slot.empty()) {
+        config_properties.SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecClientCertSlotProperty, tpm_slot);
+      }
+      if (!tpm_pin.empty()) {
+        config_properties.SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecPinProperty, tpm_pin);
+      }
+    }
+  } else if (type == flimflam::kTypeWifi) {
+    if (!pkcs11_id.empty()) {
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kEapCertIdProperty, pkcs11_id);
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kEapKeyIdProperty, pkcs11_id);
+    }
+    if (!tpm_pin.empty()) {
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kEapPinProperty, tpm_pin);
+    }
+  }
+
+  if (!config_properties.empty()) {
     NET_LOG_EVENT("Configuring Network", service_path);
-    const std::string& tpm_slot = cert_loader_->tpm_token_slot();
-    const std::string& tpm_pin = cert_loader_->tpm_user_pin();
-    base::DictionaryValue config_properties;
+
     // Set configuration properties required by Shill to identify the network.
     config_properties.SetStringWithoutPathExpansion(
         flimflam::kTypeProperty, type);
     CopyStringFromDictionary(service_properties, flimflam::kNameProperty,
                              &config_properties);
-    CopyStringFromDictionary(service_properties, flimflam::kSecurityProperty,
-                             &config_properties);
     CopyStringFromDictionary(service_properties, flimflam::kGuidProperty,
                              &config_properties);
+    if (type == flimflam::kTypeVPN) {
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kProviderTypeProperty, vpn_provider_type);
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kProviderHostProperty, vpn_provider_host);
+    } else if (type == flimflam::kTypeWifi) {
+      CopyStringFromDictionary(service_properties, flimflam::kSecurityProperty,
+                               &config_properties);
+    }
 
-    if (type == flimflam::kTypeVPN) {
-      // VPN Provider values are read from the "Provider" dictionary, not the
-      // "Provider.Type", etc keys (which are used only to set the values).
-      std::string provider_type;
-      const base::DictionaryValue* provider_properties;
-      if (service_properties.GetDictionaryWithoutPathExpansion(
-              flimflam::kProviderProperty, &provider_properties)) {
-        provider_properties->GetStringWithoutPathExpansion(
-            flimflam::kTypeProperty, &provider_type);
-      }
-      if (provider_type.empty()) {
-        ErrorCallbackForPendingRequest(service_path, kErrorMissingProviderType);
-        return;
-      }
-      if (provider_type == flimflam::kProviderOpenVpn) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kOpenVPNClientCertSlotProperty, tpm_slot);
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kOpenVPNPinProperty, tpm_pin);
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kOpenVPNClientCertIdProperty, pkcs11_id);
-      } else {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecClientCertSlotProperty, tpm_slot);
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecPinProperty, tpm_pin);
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecClientCertIdProperty, pkcs11_id);
-      }
-    } else if (type == flimflam::kTypeWifi) {
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapPinProperty, cert_loader_->tpm_user_pin());
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapCertIdProperty, pkcs11_id);
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapKeyIdProperty, pkcs11_id);
-    }
     network_configuration_handler_->SetProperties(
         service_path,
         config_properties,
         base::Bind(&NetworkConnectionHandler::CallShillConnect,
                    AsWeakPtr(), service_path),
         base::Bind(&NetworkConnectionHandler::HandleConfigurationFailure,
                    AsWeakPtr(), service_path));
    return;
   }
 
-  // Otherwise, we need to configure the network.
-  ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
+  // Otherwise, we probably still need to configure the network since
+  // 'Connectable' is false. If |check_error_state| is true, signal an
+  // error, otherwise attempt to connect to possibly gain additional error
+  // state from Shill (or in case 'Connectable' is improperly unset).
+  if (check_error_state)
+    ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
+  else
+    CallShillConnect(service_path);
 }
 
 void NetworkConnectionHandler::CallShillConnect(
     const std::string& service_path) {
   NET_LOG_EVENT("Sending Connect Request to Shill", service_path);
   DBusThreadManager::Get()->GetShillServiceClient()->Connect(
       dbus::ObjectPath(service_path),
       base::Bind(&NetworkConnectionHandler::HandleShillConnectSuccess,
                  AsWeakPtr(), service_path),
       base::Bind(&NetworkConnectionHandler::HandleShillConnectFailure,
@@ skipping 125 matching lines @@
 void NetworkConnectionHandler::CallShillDisconnect(
     const std::string& service_path,
     const base::Closure& success_callback,
     const network_handler::ErrorCallback& error_callback) {
   NET_LOG_USER("Disconnect Request", service_path);
   DBusThreadManager::Get()->GetShillServiceClient()->Disconnect(
       dbus::ObjectPath(service_path),
       base::Bind(&NetworkConnectionHandler::HandleShillDisconnectSuccess,
                  AsWeakPtr(), service_path, success_callback),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
-                 kErrorDisconnectFailed, service_path, error_callback));
+                 kErrorShillError, service_path, error_callback));
 }
 
 void NetworkConnectionHandler::HandleShillDisconnectSuccess(
     const std::string& service_path,
     const base::Closure& success_callback) {
   NET_LOG_EVENT("Disconnect Request Sent", service_path);
   if (!success_callback.is_null())
     success_callback.Run();
 }
 
+// Activate
+
+void NetworkConnectionHandler::CallShillActivate(

[gauravsh, 2013/08/07 23:33:37]

Talking to Arman, it looks like this would be a better fit for a separate
NetworkActivationHandler which lets us make activation related shill calls and
manage activation state. (The WIP MobileActivator change I have needs this and
CompleteCellularActivation as well).

Since connection and activation are very different things, I am inclined to
agree with that as well.

[stevenjb, 2013/08/08 01:02:51]

That sounds reasonable to me, although I am hesitant to add that to this CL
also. Would you be OK with adding a TODO to move this later?

+    const std::string& service_path,
+    const std::string& carrier,
+    const base::Closure& success_callback,
+    const network_handler::ErrorCallback& error_callback) {
+  NET_LOG_USER("Activate Request", service_path + ": '" + carrier + "'");
+  DBusThreadManager::Get()->GetShillServiceClient()->ActivateCellularModem(
+      dbus::ObjectPath(service_path),
+      carrier,
+      base::Bind(&NetworkConnectionHandler::HandleShillActivateSuccess,
+                 AsWeakPtr(), service_path, success_callback),
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 kErrorShillError, service_path, error_callback));
+}
+
+void NetworkConnectionHandler::HandleShillActivateSuccess(
+    const std::string& service_path,
+    const base::Closure& success_callback) {
+  NET_LOG_EVENT("Activate Request Sent", service_path);
+  if (!success_callback.is_null())
+    success_callback.Run();
+}
+
 }  // namespace chromeos