Provide Shill Error to failure notification

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_state_notifier.h"
 #include "ash/system/system_notifier.h"
 #include "ash/system/tray/system_tray_delegate.h"
@@ skipping 40 matching lines @@
 const char kErrorInProgress[] = "org.chromium.flimflam.Error.InProgress";
 
 // Returns true for carriers that can be activated through Shill instead of
 // through a WebUI dialog.
 bool IsDirectActivatedCarrier(const std::string& carrier) {
   if (carrier == shill::kCarrierSprint)
     return true;
   return false;
 }
 
-void ShowErrorNotification(const std::string& error,
+void ShowErrorNotification(const std::string& error_name,
+                           const std::string& shill_error,
                            const std::string& service_path) {
   Shell::GetInstance()->system_tray_notifier()->network_state_notifier()->
-      ShowNetworkConnectError(error, service_path);
+      ShowNetworkConnectError(error_name, shill_error, service_path);
 }
 
 void OnConnectFailed(const std::string& service_path,
                      gfx::NativeWindow owning_window,
                      const std::string& error_name,
                      scoped_ptr<base::DictionaryValue> error_data) {
   NET_LOG_ERROR("Connect Failed: " + error_name, service_path);
 
   if (!ash::Shell::HasInstance())
     return;
@@ skipping 22 matching lines @@
     return;
   }
 
   if (error_name == NetworkConnectionHandler::kErrorConnected ||
       error_name == NetworkConnectionHandler::kErrorConnecting) {
     network_connect::ShowNetworkSettings(service_path);
     return;
   }
 
   // ConnectFailed or unknown error; show a notification.
-  ShowErrorNotification(error_name, service_path);
+  std::string shill_error;
+  error_data.get()->GetString(
+      chromeos::network_handler::kErrorDetail, &shill_error);
+  ShowErrorNotification(error_name, shill_error, service_path);
 
   // Show a configure dialog for ConnectFailed errors.
   if (error_name != flimflam::kErrorConnectFailed)
     return;
 
   // If Shill reports an InProgress error, don't try to configure the network.
   std::string dbus_error_name;
   error_data.get()->GetString(
       chromeos::network_handler::kDbusErrorName, &dbus_error_name);
   if (dbus_error_name == kErrorInProgress)
@@ skipping 28 matching lines @@
       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);
-  ShowErrorNotification(network_connect::kErrorActivateFailed, service_path);
+  ShowErrorNotification(
+      network_connect::kErrorActivateFailed, "", 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", "");
-  ShowErrorNotification(NetworkConnectionHandler::kErrorConfigureFailed, "");
+  ShowErrorNotification(
+      NetworkConnectionHandler::kErrorConfigureFailed, "", "");

[armansito, 2013/09/14 01:13:12]

You may want to always check the contents of |error_data| instead of assigning
an empty string, as shill might have set the error.

[stevenjb, 2013/09/16 16:38:07]

This still needs a re-factoring / cleanup pass, but right now if 'error_name' ==
"kErrorConfigureFailed", 'shill_error' will be ignored.

[armansito, 2013/09/16 19:24:00]

sgtm

 }
 
 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;
   const gfx::NativeWindow owning_window = NULL;
   CallConnectToNetwork(service_path, check_error_state, owning_window);
 }
 
 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);
   ShowErrorNotification(
-      NetworkConnectionHandler::kErrorConfigureFailed, service_path);
+      NetworkConnectionHandler::kErrorConfigureFailed, "", 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()) {
     std::string value_str;
     if (iter.value().GetAsString(&value_str) && value_str.empty())
       properties_to_clear->push_back(iter.key());
@@ skipping 204 matching lines @@
 void ConfigureNetworkAndConnect(const std::string& service_path,
                                 const base::DictionaryValue& properties,
                                 bool shared) {
   NET_LOG_USER("ConfigureNetworkAndConnect", service_path);
 
   scoped_ptr<base::DictionaryValue> properties_to_set(properties.DeepCopy());
 
   std::string profile_path;
   if (!GetNetworkProfilePath(shared, &profile_path)) {
     ShowErrorNotification(
-        NetworkConnectionHandler::kErrorConfigureFailed, service_path);
+        NetworkConnectionHandler::kErrorConfigureFailed, "", service_path);
     return;
   }
   NetworkHandler::Get()->network_configuration_handler()->SetNetworkProfile(
       service_path, profile_path,
       base::Bind(&ConfigureSetProfileSucceeded,
                  service_path, base::Passed(&properties_to_set)),
       base::Bind(&SetPropertiesFailed,
                  "SetProfile: " + profile_path, service_path));
 }
 
 void CreateConfigurationAndConnect(base::DictionaryValue* properties,
                                    bool shared) {
   NET_LOG_USER("CreateConfigurationAndConnect", "");
   std::string profile_path;
   if (!GetNetworkProfilePath(shared, &profile_path)) {
-    ShowErrorNotification(NetworkConnectionHandler::kErrorConfigureFailed, "");
+    ShowErrorNotification(
+        NetworkConnectionHandler::kErrorConfigureFailed, "", "");
     return;
   }
   properties->SetStringWithoutPathExpansion(
       flimflam::kProfileProperty, profile_path);
   NetworkHandler::Get()->network_configuration_handler()->CreateConfiguration(
       *properties,
       base::Bind(&OnConfigureSucceeded),
       base::Bind(&OnConfigureFailed));
 }
 
@@ skipping 68 matching lines @@
 
 void ShowNetworkSettings(const std::string& service_path) {
   if (!ash::Shell::HasInstance())
     return;
   ash::Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
       service_path);
 }
 
 }  // network_connect
 }  // ash