Convert all connect code to use NetworkHandler instead of NetworkLibrary

ash/system/chromeos/network/network_connect.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 "ash/system/chromeos/network/network_connect.h"
 
 #include "ash/shell.h"
 #include "ash/system/chromeos/network/network_observer.h"
 #include "ash/system/chromeos/network/network_state_notifier.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/system_tray_notifier.h"
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "chromeos/network/device_state.h"
+#include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_connection_handler.h"
+#include "chromeos/network/network_event_log.h"
+#include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "grit/ash_strings.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using chromeos::DeviceState;
+using chromeos::NetworkConfigurationHandler;
 using chromeos::NetworkConnectionHandler;
 using chromeos::NetworkHandler;
+using chromeos::NetworkProfile;
+using chromeos::NetworkProfileHandler;
 using chromeos::NetworkState;
 
 namespace ash {
 
 namespace {
 
+bool IsDirectActivatedCarrier(const std::string& carrier) {

[pneubeck (no reviews), 2013/08/06 15:45:19]

nit: maybe it's worth explaining what "DirectActivated" means.

[stevenjb, 2013/08/06 20:23:55]

Done.

+  if (carrier == shill::kCarrierSprint)
+    return true;
+  return false;
+}
+
 void OnConnectFailed(const std::string& service_path,
+                     gfx::NativeWindow owning_window,
                      const std::string& error_name,
                      scoped_ptr<base::DictionaryValue> error_data) {
-  VLOG(1) << "Connect Failed for " << service_path << ": " << error_name;
+  NET_LOG_ERROR("Connect Failed: " + error_name, service_path);
   if (error_name == NetworkConnectionHandler::kErrorPassphraseRequired) {
     // TODO(stevenjb): Possibly add inline UI to handle passphrase entry here.
     ash::Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
         service_path);
     return;
   }
+  if (error_name == NetworkConnectionHandler::kErrorCertificateRequired) {
+    ash::Shell::GetInstance()->system_tray_delegate()->EnrollOrConfigureNetwork(

[pneubeck (no reviews), 2013/08/06 15:45:19]

make this conditional on owning_window!=NULL and log an error otherwise.

[stevenjb, 2013/08/06 20:23:55]

It's not conditional on owning_window != NULL. The delegate should handle that
case by attaching the window to the default browser.

+        service_path, owning_window);
+    return;
+  }
   if (error_name == NetworkConnectionHandler::kErrorActivationRequired ||

[pneubeck (no reviews), 2013/08/06 15:45:19]

kErrorActivationRequired should lead to
network_connect::ActivateCellular(service_path);
and replace lines 104-109.

(lines 104-109 duplicate NetworkRequiresActivation in
network_connection_handler.cc)

[stevenjb, 2013/08/06 20:23:55]

Yes... that looks sound. Done.

-      error_name == NetworkConnectionHandler::kErrorCertificateRequired ||
       error_name == NetworkConnectionHandler::kErrorConfigurationRequired ||
       error_name == NetworkConnectionHandler::kErrorAuthenticationRequired) {
     ash::Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
         service_path);
     return;
   }
   if (error_name == NetworkConnectionHandler::kErrorConnected ||

[pneubeck (no reviews), 2013/08/06 15:45:19]

merge all IFs with the same statement.

[stevenjb, 2013/08/06 20:23:55]

I merged PassphraseRequired with Config/AuthReq and removed the TODO - the jury
is still out as to whether or not we will ever inline simple passphrase or other
configuration.

       error_name == NetworkConnectionHandler::kErrorConnecting) {
     ash::Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
         service_path);
     return;
   }
   // Shill does not always provide a helpful error. In this case, show the
   // configuration UI and a notification. See crbug.com/217033 for an example.
   if (error_name == NetworkConnectionHandler::kErrorConnectFailed) {
     ash::Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
         service_path);
   }
   ash::Shell::GetInstance()->system_tray_notifier()->network_state_notifier()->
       ShowNetworkConnectError(error_name, service_path);
 }
 
 void OnConnectSucceeded(const std::string& service_path) {
-  VLOG(1) << "Connect Succeeded for " << service_path;
+  NET_LOG_USER("Connect Succeeded", service_path);
   ash::Shell::GetInstance()->system_tray_notifier()->NotifyClearNetworkMessage(
       NetworkObserver::ERROR_CONNECT_FAILED);
 }
 
+// If |check_error_state| is true, error state for the network is checked,
+//  otherwise any current error state is ignored (e.g. for recently configured
+//  networks or repeat connect attempts).
+void CallConnectToNetwork(const std::string& service_path,
+                          bool check_error_state,
+                          gfx::NativeWindow owning_window) {
+  NET_LOG_USER("ConnectToNetwork", service_path);
+
+  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->

[pneubeck (no reviews), 2013/08/06 15:45:19]

if you did the change proposed above, this could be replaced by handling
NetworkConnectionHandler::kErrorNotFound in OnConnectFailed.

[stevenjb, 2013/08/06 20:23:55]

Actually, removing this will trigger a similar log entry and show a notification
which is really what we want, so removed.

+      GetNetworkState(service_path);
+  if (!network) {
+    NET_LOG_USER("ConnectToNetwork: Not Found", service_path);
+    return;
+  }
+  if (network->type() == flimflam::kTypeCellular &&
+      (network->activation_state() != flimflam::kActivationStateActivated ||
+       network->cellular_out_of_credits())) {
+    network_connect::ActivateCellular(service_path);
+    return;
+  }
+
+  ash::Shell::GetInstance()->system_tray_notifier()->NotifyClearNetworkMessage(
+      NetworkObserver::ERROR_CONNECT_FAILED);
+  NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
+      service_path,
+      base::Bind(&OnConnectSucceeded, service_path),
+      base::Bind(&OnConnectFailed, service_path, owning_window),
+      check_error_state);
+}
+
+void OnActivateFailed(const std::string& service_path,
+                     const std::string& error_name,
+                      scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR("Unable to activate network", service_path);
+}
+
+void OnActivateSucceeded(const std::string& service_path) {
+  NET_LOG_USER("Activation Succeeded", service_path);
+}
+
+void OnConfigureFailed(const std::string& error_name,
+                       scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR("Unable to configure network", "");
+}
+
+void OnConfigureSucceeded(const std::string& service_path) {
+  NET_LOG_USER("Configure Succeeded", service_path);
+  // After configuring a network, ignore any (possibly stale) error state.
+  const bool check_error_state = false;
+  CallConnectToNetwork(service_path, check_error_state, NULL);
+}
+
+void SetPropertiesFailed(const std::string& desc,
+                         const std::string& service_path,
+                         const std::string& config_error_name,
+                         scoped_ptr<base::DictionaryValue> error_data) {
+  NET_LOG_ERROR(desc + ": Failed: " + config_error_name, service_path);
+}
+
+void SetPropertiesToClear(base::DictionaryValue* properties_to_set,
+                          std::vector<std::string>* properties_to_clear) {
+  // Move empty string properties to properties_to_clear.
+  for (base::DictionaryValue::Iterator iter(*properties_to_set);
+       !iter.IsAtEnd(); iter.Advance()) {
+    if (iter.value().GetType() == base::Value::TYPE_STRING) {

[pneubeck (no reviews), 2013/08/06 15:45:19]

how about:

std::string value_str;
if (iter.value().GetAsString(&value_str) && value_str.empty())
  properties_to_clear->push_back(iter.key());

[stevenjb, 2013/08/06 20:23:55]

Done.

+      std::string value_str;
+      iter.value().GetAsString(&value_str);
+      if (value_str.empty())
+        properties_to_clear->push_back(iter.key());
+    }
+  }
+  // Remove cleared properties from properties_to_set.
+  for (std::vector<std::string>::iterator iter = properties_to_clear->begin();
+       iter != properties_to_clear->end(); ++iter) {
+    properties_to_set->RemoveWithoutPathExpansion(*iter, NULL);
+  }
+}
+
+void ClearPropertiesAndConnect(
+    const std::string& service_path,
+    const std::vector<std::string>& properties_to_clear) {
+  NET_LOG_USER("ClearPropertiesAndConnect", service_path);
+  // After configuring a network, ignore any (possibly stale) error state.
+  const bool check_error_state = false;
+  if (properties_to_clear.empty()) {
+    CallConnectToNetwork(service_path, check_error_state, NULL);

[pneubeck (no reviews), 2013/08/06 15:45:19]

either document that parent=NULL is a valid argument at the definition of
CallConnectToNetwork/OnConnectFailed and handle it in OnConnectFailed.
or provide a valid parent window here.

Similarly it should be mentioned if parent=NULL is a valid arg to
ConnectToNetwork.

[stevenjb, 2013/08/06 20:23:55]

Done.

+  } else {
+    NetworkHandler::Get()->network_configuration_handler()->ClearProperties(

[pneubeck (no reviews), 2013/08/06 15:45:19]

NetworkConfigurationHandler::ClearProperties could early return if the list is
empty.

[stevenjb, 2013/08/06 20:23:55]

Done.

+        service_path,
+        properties_to_clear,
+        base::Bind(&CallConnectToNetwork,
+                   service_path, check_error_state,
+                   static_cast<gfx::NativeWindow>(NULL)),
+        base::Bind(&SetPropertiesFailed, "ClearProperties", service_path));
+  }
+}
+
 }  // namespace
 
 namespace network_connect {
 
-void ConnectToNetwork(const std::string& service_path) {
-  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
-      GetNetworkState(service_path);
-  if (!network)
+void ConnectToNetwork(const std::string& service_path,
+                      gfx::NativeWindow owning_window) {
+  const bool check_error_state = true;
+  CallConnectToNetwork(service_path, check_error_state, owning_window);
+}
+
+void ActivateCellular(const std::string& service_path) {
+  NET_LOG_USER("ActivateCellular", service_path);
+  const DeviceState* cellular_device =
+      NetworkHandler::Get()->network_state_handler()->
+      GetDeviceStateByType(flimflam::kTypeCellular);
+  if (!cellular_device) {
+    NET_LOG_ERROR("ActivateCellular with no Device", service_path);
     return;
-  ash::Shell::GetInstance()->system_tray_notifier()->NotifyClearNetworkMessage(
-      NetworkObserver::ERROR_CONNECT_FAILED);
-  NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
+  }
+  if (!IsDirectActivatedCarrier(cellular_device->carrier())) {
+    // For non direct activation, show the mobile setup dialog which can be
+    // used to activate the network.
+    ash::Shell::GetInstance()->system_tray_delegate()->ShowMobileSetup(
+        service_path);
+    return;
+  }
+  const NetworkState* cellular =
+      NetworkHandler::Get()->network_state_handler()->
+      FirstNetworkByType(flimflam::kTypeCellular);
+  if (!cellular) {

[pneubeck (no reviews), 2013/08/06 15:45:19]

what if cellular->path() != service_path ?
why not instead GetNetworkState(service_path) and check that the type is
kTypeCellular.

[stevenjb, 2013/08/06 20:23:55]

Yeah, this was converted a little too literally. Done.

+    NET_LOG_ERROR("ActivateCellular with no Service", service_path);
+    return;
+  }
+  if (cellular->activation_state() == flimflam::kActivationStateActivated) {
+    NET_LOG_ERROR("ActivateCellular for activated service", service_path);
+    return;
+  }
+
+  NetworkHandler::Get()->network_connection_handler()->ActivateNetwork(
       service_path,
-      base::Bind(&OnConnectSucceeded, service_path),
-      base::Bind(&OnConnectFailed, service_path),
-      true /* check_error_state */);
+      "",  // carrier
+      base::Bind(&OnActivateSucceeded, service_path),
+      base::Bind(&OnActivateFailed, service_path));
+}
+
+void ConfigureNetworkAndConnect(const std::string& service_path,
+                                const base::DictionaryValue& properties) {
+  NET_LOG_USER("ConfigureNetworkAndConnect", service_path);
+  scoped_ptr<base::DictionaryValue> properties_to_set(properties.DeepCopy());
+  std::vector<std::string> properties_to_clear;
+  SetPropertiesToClear(properties_to_set.get(), &properties_to_clear);
+  if (properties_to_set->empty()) {
+    ClearPropertiesAndConnect(service_path,
+                              properties_to_clear);
+  } else {
+    NetworkHandler::Get()->network_configuration_handler()->SetProperties(

[pneubeck (no reviews), 2013/08/06 15:45:19]

NetworkConfigurationHandler::SetProperties could early return if the dict is
empty.

[stevenjb, 2013/08/06 20:23:55]

Done.

+        service_path,
+        *properties_to_set,
+        base::Bind(&ClearPropertiesAndConnect,
+                   service_path,
+                   properties_to_clear),
+        base::Bind(&SetPropertiesFailed, "SetProperties", service_path));
+  }
+}
+
+void CreateConfigurationAndConnect(base::DictionaryValue* properties,
+                                   bool shared) {

[pneubeck (no reviews), 2013/08/06 15:45:19]

|shared| is not used.

[stevenjb, 2013/08/06 20:23:55]

Oops. Fixed this on the calling side, removed 'shared'.

+  NET_LOG_USER("CreateConfigurationAndConnect", "");
+  const NetworkProfile* profile =

[pneubeck (no reviews), 2013/08/06 15:45:19]

enclose this in a 'if (!shared)'

log error if GetDefaultUserProfile() returns NULL.
but ensure that shared profile is not used if shared=false.

[stevenjb, 2013/08/06 20:23:55]

Done.

+      NetworkHandler::Get()->network_profile_handler()->
+      GetDefaultUserProfile();
+  if (profile) {
+    properties->SetStringWithoutPathExpansion(
+        flimflam::kProfileProperty, profile->path);

[pneubeck (no reviews), 2013/08/06 15:45:19]

set a 'std::string profile_path' in the if clauses and call
 properties->SetStringWithoutPathExpansion(
   flimflam::kProfileProperty, profile_path);
afterwards.

[stevenjb, 2013/08/06 20:23:55]

Done.

+  } else {
+    properties->SetStringWithoutPathExpansion(
+        flimflam::kProfileProperty,
+        NetworkProfileHandler::kSharedProfilePath);
+  }
+  NetworkHandler::Get()->network_configuration_handler()->CreateConfiguration(
+      *properties,
+      base::Bind(&OnConfigureSucceeded),
+      base::Bind(&OnConfigureFailed));
 }
 
 string16 ErrorString(const std::string& error) {
   if (error.empty())
     return string16();
   if (error == flimflam::kErrorOutOfRange)
     return l10n_util::GetStringUTF16(IDS_CHROMEOS_NETWORK_ERROR_OUT_OF_RANGE);
   if (error == flimflam::kErrorPinMissing)
     return l10n_util::GetStringUTF16(IDS_CHROMEOS_NETWORK_ERROR_PIN_MISSING);
   if (error == flimflam::kErrorDhcpFailed)
@@ skipping 53 matching lines @@
   if (StringToLowerASCII(error) ==
       StringToLowerASCII(std::string(flimflam::kUnknownString))) {
     return l10n_util::GetStringUTF16(IDS_CHROMEOS_NETWORK_ERROR_UNKNOWN);
   }
   return l10n_util::GetStringFUTF16(IDS_NETWORK_UNRECOGNIZED_ERROR,
                                     UTF8ToUTF16(error));
 }
 
 }  // network_connect
 }  // ash