Merge 220596 "Differentiate between 'connect-failed' and 'bad-pa..."

> Differentiate between 'connect-failed' and 'bad-passphrase'
> 
> Now that all connection requests are going through
> NetworkConnectionHandler we can start to improve the connect flow
> logic.
> 
> 'connect-failed' can indicate a lot of things; we shouldn't assume that
> configuration is required when that Error state is set.
> 
> BUG=278559
> R=pneubeck@chromium.org
> 
> Review URL: https://codereview.chromium.org/22867045

TBR=stevenjb@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/branches/1599/src@221734 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 45 insertions, 40 deletions

diff --git a/ash/system/chromeos/network/network_connect.cc b/ash/system/chromeos/network/network_connect.cc
index 688c910..d733a01 100644
--- a/ash/system/chromeos/network/network_connect.cc
+++ b/ash/system/chromeos/network/network_connect.cc
@@ -66,7 +66,8 @@ void OnConnectFailed(const std::string& service_path,
   if (error_name == NetworkConnectionHandler::kErrorConnectCanceled)
     return;
 
-  if (error_name == NetworkConnectionHandler::kErrorPassphraseRequired ||
+  if (error_name == flimflam::kErrorBadPassphrase ||
+      error_name == NetworkConnectionHandler::kErrorPassphraseRequired ||
       error_name == NetworkConnectionHandler::kErrorConfigurationRequired ||
       error_name == NetworkConnectionHandler::kErrorAuthenticationRequired) {
     ash::Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
@@ -96,7 +97,7 @@ void OnConnectFailed(const std::string& service_path,
   ShowErrorNotification(error_name, service_path);
 
   // Show a configure dialog for ConnectFailed errors.
-  if (error_name != NetworkConnectionHandler::kErrorConnectFailed)
+  if (error_name != flimflam::kErrorConnectFailed)
     return;
 
   // If Shill reports an InProgress error, don't try to configure the network.
@@ -137,11 +138,10 @@ void CallConnectToNetwork(const std::string& service_path,
 }
 
 void OnActivateFailed(const std::string& service_path,
-                     const std::string& error_name,
+                      const std::string& error_name,
                       scoped_ptr<base::DictionaryValue> error_data) {
   NET_LOG_ERROR("Unable to activate network", service_path);
-  ShowErrorNotification(
-      NetworkConnectionHandler::kErrorActivateFailed, service_path);
+  ShowErrorNotification(network_connect::kErrorActivateFailed, service_path);
 }
 
 void OnActivateSucceeded(const std::string& service_path) {
@@ -246,6 +246,8 @@ void ConfigureSetProfileSucceeded(
 
 namespace network_connect {
 
+const char kErrorActivateFailed[] = "activate-failed";
+
 void ConnectToNetwork(const std::string& service_path,
                       gfx::NativeWindow owning_window) {
   const bool check_error_state = true;
diff --git a/ash/system/chromeos/network/network_connect.h b/ash/system/chromeos/network/network_connect.h
index 65c430d..06656ce 100644
--- a/ash/system/chromeos/network/network_connect.h
+++ b/ash/system/chromeos/network/network_connect.h
@@ -18,6 +18,8 @@ class DictionaryValue;
 namespace ash {
 namespace network_connect {
 
+ASH_EXPORT extern const char kErrorActivateFailed[];
+
 // Requests a network connection and handles any errors and notifications.
 // |owning_window| is used to parent any UI on failure (e.g. for certificate
 // enrollment). If NULL, the default window will be used.
diff --git a/ash/system/chromeos/network/network_state_notifier.cc b/ash/system/chromeos/network/network_state_notifier.cc
index 1c971a3..47940d5 100644
--- a/ash/system/chromeos/network/network_state_notifier.cc
+++ b/ash/system/chromeos/network/network_state_notifier.cc
@@ -35,7 +35,7 @@ string16 GetConnectErrorString(const std::string& error_name) {
   if (error_name == NetworkConnectionHandler::kErrorConfigureFailed)
     return l10n_util::GetStringUTF16(
         IDS_CHROMEOS_NETWORK_ERROR_CONFIGURE_FAILED);
-  if (error_name == NetworkConnectionHandler::kErrorActivateFailed)
+  if (error_name == ash::network_connect::kErrorActivateFailed)
     return l10n_util::GetStringUTF16(
         IDS_CHROMEOS_NETWORK_ERROR_ACTIVATION_FAILED);
   return string16();
@@ -71,7 +71,7 @@ void NetworkStateNotifier::NetworkListChanged() {
   // case a connect attempt fails because a network is no longer visible.
   if (!connect_failed_network_.empty()) {
     ShowNetworkConnectError(
-        NetworkConnectionHandler::kErrorConnectFailed, connect_failed_network_);
+        flimflam::kErrorConnectFailed, connect_failed_network_);
   }
 }
 
@@ -92,7 +92,7 @@ void NetworkStateNotifier::NetworkPropertiesUpdated(
   // property has been set.
   if (network->path() == connect_failed_network_ && !network->error().empty()) {
     ShowNetworkConnectError(
-        NetworkConnectionHandler::kErrorConnectFailed, connect_failed_network_);
+        flimflam::kErrorConnectFailed, connect_failed_network_);
   }
   // Trigger "Out of credits" notification if the cellular network is the most
   // recent default network (i.e. we have not switched to another network).
@@ -141,7 +141,7 @@ void NetworkStateNotifier::ShowNetworkConnectError(
     const std::string& service_path) {
   const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
       GetNetworkState(service_path);
-  if (error_name == NetworkConnectionHandler::kErrorConnectFailed &&
+  if (error_name == flimflam::kErrorConnectFailed &&
       service_path != connect_failed_network_) {
     // Shill may not have set the Error property yet. First request an update
     // and wait for either the update to complete or the network list to be
diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index c144253..b13db37 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -118,16 +118,10 @@ const char NetworkConnectionHandler::kErrorConfigurationRequired[] =
 const char NetworkConnectionHandler::kErrorAuthenticationRequired[] =
     "authentication-required";
 const char NetworkConnectionHandler::kErrorShillError[] = "shill-error";
-const char NetworkConnectionHandler::kErrorConnectFailed[] = "connect-failed";
 const char NetworkConnectionHandler::kErrorConfigureFailed[] =
     "configure-failed";
-const char NetworkConnectionHandler::kErrorActivateFailed[] =
-    "activate-failed";
-const char NetworkConnectionHandler::kErrorMissingProvider[] =
-    "missing-provider";
 const char NetworkConnectionHandler::kErrorConnectCanceled[] =
     "connect-canceled";
-const char NetworkConnectionHandler::kErrorUnknown[] = "unknown-error";
 
 struct NetworkConnectionHandler::ConnectRequest {
   ConnectRequest(const std::string& service_path,
@@ -254,14 +248,8 @@ void NetworkConnectionHandler::ConnectToNetwork(
 
     if (check_error_state) {
       const std::string& error = network->error();
-      if (error == flimflam::kErrorConnectFailed) {
-        InvokeErrorCallback(
-            service_path, error_callback, kErrorPassphraseRequired);
-        return;
-      }
       if (error == flimflam::kErrorBadPassphrase) {
-        InvokeErrorCallback(
-            service_path, error_callback, kErrorPassphraseRequired);
+        InvokeErrorCallback(service_path, error_callback, error);
         return;
       }
       if (IsAuthenticationError(error)) {
@@ -401,7 +389,7 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
           flimflam::kHostProperty, &vpn_provider_host);
     }
     if (vpn_provider_type.empty() || vpn_provider_host.empty()) {
-      ErrorCallbackForPendingRequest(service_path, kErrorMissingProvider);
+      ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
       return;
     }
     // VPN requires a host and username to be set.
@@ -574,7 +562,7 @@ void NetworkConnectionHandler::HandleShillConnectFailure(
   network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
   network_handler::ShillErrorCallbackFunction(
-      kErrorConnectFailed, service_path, error_callback,
+      flimflam::kErrorConnectFailed, service_path, error_callback,
       dbus_error_name, dbus_error_message);
 }
 
@@ -614,7 +602,7 @@ void NetworkConnectionHandler::CheckPendingRequest(
     error_name = kErrorConnectCanceled;
     error_detail = "";
   } else {
-    error_name = kErrorConnectFailed;
+    error_name = flimflam::kErrorConnectFailed;
     error_detail = network->error();
     if (error_detail.empty()) {
       if (network->connection_state() == flimflam::kStateFailure)
diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h
index 31fcfc5..02028db 100644
--- a/chromeos/network/network_connection_handler.h
+++ b/chromeos/network/network_connection_handler.h
@@ -48,21 +48,40 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
       public base::SupportsWeakPtr<NetworkConnectionHandler> {
  public:
   // Constants for |error_name| from |error_callback| for Connect.
+
+  //  No network matching |service_path| is found (hidden networks must be
+  //  configured before connecting).
   static const char kErrorNotFound[];
+
+  // Already connected to the network.
   static const char kErrorConnected[];
+
+  // Already connecting to the network.
   static const char kErrorConnecting[];
+
+  // The passphrase is missing or invalid.
   static const char kErrorPassphraseRequired[];
+
   static const char kErrorActivationRequired[];
+
+  // The network requires a cert and none exists.
   static const char kErrorCertificateRequired[];
+
+  // The network had an authentication error, indicating that additional or
+  // different authentication information is required.
   static const char kErrorAuthenticationRequired[];
+
+  // Additional configuration is required.
   static const char kErrorConfigurationRequired[];
-  static const char kErrorShillError[];
-  static const char kErrorConnectFailed[];
+
+  // Configuration failed during the configure stage of the connect flow.
   static const char kErrorConfigureFailed[];
-  static const char kErrorActivateFailed[];
-  static const char kErrorMissingProvider[];
+
+  // For Disconnect or Activate, an unexpected DBus or Shill error occurred.
+  static const char kErrorShillError[];
+
+  // A new network connect request canceled this one.
   static const char kErrorConnectCanceled[];
-  static const char kErrorUnknown[];
 
   // Constants for |error_name| from |error_callback| for Disconnect.
   static const char kErrorNotConnected[];
@@ -71,16 +90,10 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
 
   // ConnectToNetwork() will start an asynchronous connection attempt.
   // On success, |success_callback| will be called.
-  // On failure, |error_callback| will be called with |error_name| one of:
-  //  kErrorNotFound if no network matching |service_path| is found
-  //    (hidden networks must be configured before connecting).
-  //  kErrorConnected if already connected to the network.
-  //  kErrorConnecting if already connecting to the network.
-  //  kErrorCertificateRequired if the network requires a cert and none exists.
-  //  kErrorPassphraseRequired if passphrase only is missing or incorrect.
-  //  kErrorAuthenticationRequired if other authentication is required.
-  //  kErrorConfigurationRequired if additional configuration is required.
-  //  kErrorShillError if a DBus or Shill error occurred.
+  // On failure, |error_callback| will be called with |error_name| one of the
+  //   constants defined above, or flimflam::kErrorConnectFailed or
+  //   flimflam::kErrorBadPassphrase if the Shill Error property (from a
+  //   previous connect attempt) was set to one of those.
   // |error_message| will contain an additional error string for debugging.
   // If |check_error_state| is true, the current state of the network is
   //  checked for errors, otherwise current state is ignored (e.g. for recently