Trigger NetworkConnectionHandler callbacks on success or failure

This CL triggers waits for the network state to change before triggering
success or failure callbacks in NetworkConnectionHandler.

Notifications are triggered on success or failure without requiring the
NetworkStateHandler connecting_network() hack.

It also separates the enable flag from NetworkConfigurationHandler since
enabling that has different artifacts.

BUG=243899
For ash.gyp:
TBR=gspencer@chromium.org

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

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

22 files changed, 558 insertions, 192 deletions

diff --git a/ash/ash.gyp b/ash/ash.gyp
index c3b7609..ac771353 100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -205,6 +205,8 @@
         'system/chromeos/label_tray_view.cc',
         'system/chromeos/managed/tray_locally_managed_user.h',
         'system/chromeos/managed/tray_locally_managed_user.cc',
+        'system/chromeos/network/network_connect.cc',
+        'system/chromeos/network/network_connect.h',
         'system/chromeos/network/network_detailed_view.h',
         'system/chromeos/network/network_icon.cc',
         'system/chromeos/network/network_icon.h',
diff --git a/ash/ash_chromeos_strings.grdp b/ash/ash_chromeos_strings.grdp
index 7ecee26..ba59e49 100644
--- a/ash/ash_chromeos_strings.grdp
+++ b/ash/ash_chromeos_strings.grdp
@@ -117,6 +117,10 @@
   <message name="IDS_NETWORK_CONNECTION_ERROR_MESSAGE_WITH_DETAILS" desc="Message for network connection error notification with details">
     Failed to connect to network '<ph name="name">$1<ex>GoogleGuest</ex></ph>': <ph name="details">$2<ex>Unrecognized error</ex></ph>
   </message>
+  <message name="IDS_NETWORK_CONNECTION_ERROR_MESSAGE_WITH_SERVER_MESSAGE" desc="Message for network connection error notification with details and a server message">
+Failed to connect to '<ph name="name">$1<ex>GoogleGuest</ex></ph>': <ph name="details">$2<ex>Unrecognized error</ex></ph>
+Server message: <ph name="server_msg">$3<ex>Incorrect password</ex></ph>
+  </message>
   <message name="IDS_NETWORK_OUT_OF_CREDITS_TITLE" desc="Title for network out of data error notification">
     Network Connection Error
   </message>
diff --git a/ash/system/chromeos/network/network_connect.cc b/ash/system/chromeos/network/network_connect.cc
new file mode 100644
index 0000000..03c30be
--- /dev/null
+++ b/ash/system/chromeos/network/network_connect.cc
@@ -0,0 +1,77 @@
+// 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/values.h"
+#include "chromeos/network/network_connection_handler.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
+
+using chromeos::NetworkConnectionHandler;
+using chromeos::NetworkHandler;
+using chromeos::NetworkState;
+
+namespace ash {
+
+namespace {
+
+void OnConnectFailed(const std::string& service_path,
+                     const std::string& error_name,
+                     scoped_ptr<base::DictionaryValue> error_data) {
+  VLOG(1) << "Connect Failed for " << service_path << ": " << error_name;
+  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::kErrorActivationRequired ||
+      error_name == NetworkConnectionHandler::kErrorCertificateRequired ||
+      error_name == NetworkConnectionHandler::kErrorConfigurationRequired) {
+    ash::Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
+        service_path);
+    return;
+  }
+  if (error_name == NetworkConnectionHandler::kErrorConnected ||
+      error_name == NetworkConnectionHandler::kErrorConnecting) {
+    ash::Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
+        service_path);
+    return;
+  }
+  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;
+}
+
+}  // namespace
+
+namespace network_connect {
+
+void ConnectToNetwork(const std::string& service_path) {
+  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
+      GetNetworkState(service_path);
+  if (!network)
+    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),
+      false /* ignore_error_state */);
+}
+
+}  // network_connect
+}  // ash
diff --git a/ash/system/chromeos/network/network_connect.h b/ash/system/chromeos/network/network_connect.h
new file mode 100644
index 0000000..e2eee94
--- /dev/null
+++ b/ash/system/chromeos/network/network_connect.h
@@ -0,0 +1,21 @@
+// Copyright (c) 2012 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.
+
+#ifndef ASH_SYSTEM_CHROMEOS_NETWORK_NETWORK_CONNECT_H
+#define ASH_SYSTEM_CHROMEOS_NETWORK_NETWORK_CONNECT_H
+
+#include <string>
+
+#include "ash/ash_export.h"
+
+namespace ash {
+namespace network_connect {
+
+// Request a network connection and handle any errors and notifications.
+ASH_EXPORT void ConnectToNetwork(const std::string& service_path);
+
+}  // network_connect
+}  // ash
+
+#endif  // ASH_SYSTEM_CHROMEOS_NETWORK_NETWORK_CONNECT_H
diff --git a/ash/system/chromeos/network/network_state_list_detailed_view.cc b/ash/system/chromeos/network/network_state_list_detailed_view.cc
index a55e2a8..938662e 100644
--- a/ash/system/chromeos/network/network_state_list_detailed_view.cc
+++ b/ash/system/chromeos/network/network_state_list_detailed_view.cc
@@ -7,6 +7,7 @@
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
+#include "ash/system/chromeos/network/network_connect.h"
 #include "ash/system/chromeos/network/network_icon.h"
 #include "ash/system/chromeos/network/network_icon_animation.h"
 #include "ash/system/chromeos/network/tray_network_state_observer.h"
@@ -24,7 +25,6 @@
 #include "base/utf_string_conversions.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/device_state.h"
-#include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "grit/ash_resources.h"
@@ -40,7 +40,6 @@
 #include "ui/views/widget/widget.h"
 
 using chromeos::DeviceState;
-using chromeos::NetworkConnectionHandler;
 using chromeos::NetworkHandler;
 using chromeos::NetworkState;
 using chromeos::NetworkStateHandler;
@@ -276,7 +275,13 @@ void NetworkStateListDetailedView::OnViewClicked(views::View* sender) {
       Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
           service_path);
     } else {
-      ConnectToNetwork(service_path);
+      if (CommandLine::ForCurrentProcess()->HasSwitch(
+              chromeos::switches::kUseNewNetworkConnectionHandler)) {
+        ash::network_connect::ConnectToNetwork(service_path);
+      } else {
+        Shell::GetInstance()->system_tray_delegate()->ConnectToNetwork(
+            service_path);
+      }
     }
   }
 }
@@ -769,49 +774,6 @@ views::View* NetworkStateListDetailedView::CreateNetworkInfoView() {
   return container;
 }
 
-void NetworkStateListDetailedView::ConnectToNetwork(
-    const std::string& service_path) {
-  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
-      GetNetworkState(service_path);
-  if (!network)
-    return;
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-          chromeos::switches::kUseNewNetworkConfigurationHandlers)) {
-    const bool ignore_error_state = false;
-    NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
-        service_path,
-        base::Bind(&base::DoNothing),
-        base::Bind(&NetworkStateListDetailedView::OnConnectFailed,
-                   AsWeakPtr(), service_path),
-        ignore_error_state);
-  } else {
-    Shell::GetInstance()->system_tray_delegate()->ConnectToNetwork(
-        service_path);
-  }
-}
-
-void NetworkStateListDetailedView::OnConnectFailed(
-    const std::string& service_path,
-    const std::string& error_name,
-    scoped_ptr<base::DictionaryValue> error_data) {
-  if (error_name == NetworkConnectionHandler::kErrorNotFound)
-    return;
-  if (error_name == NetworkConnectionHandler::kErrorPassphraseRequired) {
-    // TODO(stevenjb): Add inline UI to handle passphrase entry here.
-    Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
-        service_path);
-    return;
-  }
-  if (error_name == NetworkConnectionHandler::kErrorCertificateRequired ||
-      error_name == NetworkConnectionHandler::kErrorConfigurationRequired) {
-    Shell::GetInstance()->system_tray_delegate()->ConfigureNetwork(
-        service_path);
-    return;
-  }
-  Shell::GetInstance()->system_tray_delegate()->ShowNetworkSettings(
-      service_path);
-}
-
 void NetworkStateListDetailedView::CallRequestScan() {
   VLOG(1) << "Requesting Network Scan.";
   NetworkHandler::Get()->network_state_handler()->RequestScan();
diff --git a/ash/system/chromeos/network/network_state_list_detailed_view.h b/ash/system/chromeos/network/network_state_list_detailed_view.h
index 9f689d6..6abeb72 100644
--- a/ash/system/chromeos/network/network_state_list_detailed_view.h
+++ b/ash/system/chromeos/network/network_state_list_detailed_view.h
@@ -107,14 +107,6 @@ class NetworkStateListDetailedView
   bool ResetInfoBubble();
   views::View* CreateNetworkInfoView();
 
-  // Handle click (connect) action.
-  void ConnectToNetwork(const std::string& service_path);
-
-  // Handle connect failures (new handlers only).
-  void OnConnectFailed(const std::string& service_path,
-                       const std::string& error_name,
-                       scoped_ptr<base::DictionaryValue> error_data);
-
   // Periodically request a network scan.
   void CallRequestScan();
 
diff --git a/ash/system/chromeos/network/network_state_notifier.cc b/ash/system/chromeos/network/network_state_notifier.cc
index bf4edce..a00f7bc 100644
--- a/ash/system/chromeos/network/network_state_notifier.cc
+++ b/ash/system/chromeos/network/network_state_notifier.cc
@@ -10,6 +10,7 @@
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
@@ -17,6 +18,7 @@
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using chromeos::NetworkConnectionHandler;
 using chromeos::NetworkHandler;
 using chromeos::NetworkState;
 using chromeos::NetworkStateHandler;
@@ -25,7 +27,19 @@ namespace {
 
 const int kMinTimeBetweenOutOfCreditsNotifySeconds = 10 * 60;
 
-string16 GetErrorString(const std::string& error) {
+// Error messages based on |error_name|, not network_state->error().
+string16 GetConnectErrorString(const std::string& error_name) {
+  if (error_name == NetworkConnectionHandler::kErrorNotFound)
+    return l10n_util::GetStringUTF16(IDS_CHROMEOS_NETWORK_ERROR_CONNECT_FAILED);
+  return string16();
+}
+
+// Error messages based on network_state->error().
+string16 GetErrorString(const NetworkState* network_state) {
+  DCHECK(network_state);
+  const std::string& error = network_state->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)
@@ -137,17 +151,7 @@ void NetworkStateNotifier::NetworkConnectionStateChanged(
 
   NET_LOG_EVENT("ConnectionFailure", network->path());
 
-  std::vector<string16> no_links;
-  string16 error = GetErrorString(network->error());
-  ash::Shell::GetInstance()->system_tray_notifier()->NotifySetNetworkMessage(
-      this,
-      NetworkObserver::ERROR_CONNECT_FAILED,
-      NetworkObserver::GetNetworkTypeForNetworkState(network),
-      l10n_util::GetStringUTF16(IDS_NETWORK_CONNECTION_ERROR_TITLE),
-      l10n_util::GetStringFUTF16(
-            IDS_NETWORK_CONNECTION_ERROR_MESSAGE_WITH_DETAILS,
-            UTF8ToUTF16(network->name()), error),
-      no_links);
+  ShowConnectError("", network);
 }
 
 void NetworkStateNotifier::NetworkPropertiesUpdated(
@@ -195,6 +199,42 @@ void NetworkStateNotifier::NotificationLinkClicked(
   }
 }
 
+void NetworkStateNotifier::ShowNetworkConnectError(
+    const std::string& error_name,
+    const std::string& service_path) {
+  const NetworkState* network = NetworkHandler::Get()->network_state_handler()->
+      GetNetworkState(service_path);
+  ShowConnectError(error_name, network);
+}
+
+void NetworkStateNotifier::ShowConnectError(const std::string& error_name,
+                                            const NetworkState* network) {
+  std::vector<string16> no_links;
+  string16 error = GetConnectErrorString(error_name);
+  if (error.empty() && network)
+    error = GetErrorString(network);
+  if (error.empty())
+    error = l10n_util::GetStringUTF16(IDS_CHROMEOS_NETWORK_ERROR_UNKNOWN);
+  std::string name = network ? network->name() : "";
+  string16 error_msg;
+  if (network && !network->error_details().empty()) {
+    error_msg = l10n_util::GetStringFUTF16(
+        IDS_NETWORK_CONNECTION_ERROR_MESSAGE_WITH_SERVER_MESSAGE,
+        UTF8ToUTF16(name), error, UTF8ToUTF16(network->error_details()));
+  } else {
+    error_msg = l10n_util::GetStringFUTF16(
+        IDS_NETWORK_CONNECTION_ERROR_MESSAGE_WITH_DETAILS,
+        UTF8ToUTF16(name), error);
+  }
+  ash::Shell::GetInstance()->system_tray_notifier()->NotifySetNetworkMessage(
+      this,
+      NetworkObserver::ERROR_CONNECT_FAILED,
+      NetworkObserver::GetNetworkTypeForNetworkState(network),
+      l10n_util::GetStringUTF16(IDS_NETWORK_CONNECTION_ERROR_TITLE),
+      error_msg,
+      no_links);
+}
+
 void NetworkStateNotifier::InitializeNetworks() {
   NetworkStateList network_list;
   NetworkHandler::Get()->network_state_handler()->GetNetworkList(&network_list);
diff --git a/ash/system/chromeos/network/network_state_notifier.h b/ash/system/chromeos/network/network_state_notifier.h
index 9fce0a2..89d46c3 100644
--- a/ash/system/chromeos/network/network_state_notifier.h
+++ b/ash/system/chromeos/network/network_state_notifier.h
@@ -14,10 +14,18 @@
 #include "base/time.h"
 #include "chromeos/network/network_state_handler_observer.h"
 
+namespace chromeos {
+class NetworkState;
+}
+
 namespace ash {
 
 // This class observes NetworkStateHandler and generates notifications
-// on connection failures.
+// on connection failures. NOTE: The Observer for this class only triggers
+// "Out of credits" notifications, and failures triggered from NetworkLibrary
+// calls (which sets NetworkStateHandler::connecting_network()).
+// Failures from NetworkStateListDetailedView::ConnectToNetwork are now
+// handled by the ConnectToNetwork callbacks.
 class ASH_EXPORT NetworkStateNotifier :
       public chromeos::NetworkStateHandlerObserver,
       public NetworkTrayDelegate {
@@ -38,9 +46,16 @@ class ASH_EXPORT NetworkStateNotifier :
       NetworkObserver::MessageType message_type,
       size_t link_index) OVERRIDE;
 
+  // Show a connection error notification.
+  void ShowNetworkConnectError(const std::string& error_name,
+                               const std::string& service_path);
+
  private:
   typedef std::map<std::string, std::string> CachedStateMap;
 
+  void ShowConnectError(const std::string& error_name,
+                        const chromeos::NetworkState* network);
+
   void InitializeNetworks();
 
   CachedStateMap cached_state_;
diff --git a/ash/system/chromeos/network/tray_network.cc b/ash/system/chromeos/network/tray_network.cc
index b554529..aa24989 100644
--- a/ash/system/chromeos/network/tray_network.cc
+++ b/ash/system/chromeos/network/tray_network.cc
@@ -9,6 +9,7 @@
 #include "ash/system/chromeos/network/network_icon_animation.h"
 #include "ash/system/chromeos/network/network_state_list_detailed_view.h"
 #include "ash/system/chromeos/network/network_tray_delegate.h"
+#include "ash/system/chromeos/network/tray_network_state_observer.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/system_tray_notifier.h"
@@ -19,6 +20,7 @@
 #include "ash/system/tray/tray_utils.h"
 #include "base/command_line.h"
 #include "base/utf_string_conversions.h"
+#include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "grit/ash_resources.h"
@@ -33,6 +35,7 @@
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
 
+using chromeos::NetworkConnectionHandler;
 using chromeos::NetworkHandler;
 using chromeos::NetworkState;
 using chromeos::NetworkStateHandler;
@@ -540,13 +543,20 @@ void TrayNetwork::GetNetworkStateHandlerImageAndLabel(
     gfx::ImageSkia* image,
     base::string16* label,
     bool* animating) {
-  NetworkStateHandler* handler = NetworkHandler::Get()->network_state_handler();
-  const NetworkState* connected_network = handler->ConnectedNetworkByType(
-      NetworkStateHandler::kMatchTypeNonVirtual);
-  const NetworkState* connecting_network = handler->ConnectingNetworkByType(
-      NetworkStateHandler::kMatchTypeWireless);
-  if (!connecting_network && icon_type == network_icon::ICON_TYPE_TRAY)
-    connecting_network = handler->ConnectingNetworkByType(flimflam::kTypeVPN);
+  NetworkStateHandler* state_handler =
+      NetworkHandler::Get()->network_state_handler();
+  NetworkConnectionHandler* connect_handler =
+      NetworkHandler::Get()->network_connection_handler();
+  const NetworkState* connected_network =
+      state_handler->ConnectedNetworkByType(
+          NetworkStateHandler::kMatchTypeNonVirtual);
+  const NetworkState* connecting_network =
+      state_handler->ConnectingNetworkByType(
+          NetworkStateHandler::kMatchTypeWireless);
+  if (!connecting_network && icon_type == network_icon::ICON_TYPE_TRAY) {
+    connecting_network =
+        state_handler->ConnectingNetworkByType(flimflam::kTypeVPN);
+  }
 
   const NetworkState* network;
   // If we are connecting to a network, and there is either no connected
@@ -554,7 +564,8 @@ void TrayNetwork::GetNetworkStateHandlerImageAndLabel(
   // network.
   if (connecting_network &&
       (!connected_network ||
-       handler->connecting_network() == connecting_network->path())) {
+       state_handler->connecting_network() == connecting_network->path() ||
+       connect_handler->HasConnectingNetwork(connecting_network->path()))) {
     network = connecting_network;
   } else {
     network = connected_network;
@@ -570,7 +581,7 @@ void TrayNetwork::GetNetworkStateHandlerImageAndLabel(
 
   if (!network) {
     // If no connecting network, check if we are activating a network.
-    const NetworkState* mobile_network = handler->FirstNetworkByType(
+    const NetworkState* mobile_network = state_handler->FirstNetworkByType(
         NetworkStateHandler::kMatchTypeMobile);
     if (mobile_network && (mobile_network->activation_state() ==
                            flimflam::kActivationStateActivating)) {
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 9d2a460..8997b99 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -6785,6 +6785,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
       <message name="IDS_FLAGS_CHROMEOS_USE_NEW_NETWORK_CONFIGURATION_HANDLERS_DESCRIPTION" desc="Description for the flag to enable using the new network configuration handlers.">
         Enable the new network configuration handlers which handle Shill connection requests without using NetworkLibrary.
       </message>
+      <message name="IDS_FLAGS_CHROMEOS_USE_NEW_NETWORK_CONNECTION_HANDLER_NAME" desc="Title for the flag to enable using the new network connection handler.">
+	Enables new network connection handler
+      </message>
+      <message name="IDS_FLAGS_CHROMEOS_USE_NEW_NETWORK_CONNECTION_HANDLER_DESCRIPTION" desc="Description for the flag to enable using the new network connection handler.">
+	Enable the new network connection handler.
+      </message>
       <message name="IDS_FLAGS_ASH_ENABLE_NEW_AUDIO_HANDLER_NAME" desc="Title for the flag to enable using the new audio handler.">
         Enables new audio handler
       </message>
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 2a6e9b0..3d9e0d7 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -1163,6 +1163,12 @@ const Experiment kExperiments[] = {
     kOsCrOS,
     SINGLE_VALUE_TYPE(chromeos::switches::kUseNewNetworkConfigurationHandlers),
   },
+  { "use-new-network-connection-handler",
+    IDS_FLAGS_CHROMEOS_USE_NEW_NETWORK_CONNECTION_HANDLER_NAME,
+    IDS_FLAGS_CHROMEOS_USE_NEW_NETWORK_CONNECTION_HANDLER_DESCRIPTION,
+    kOsCrOS,
+    SINGLE_VALUE_TYPE(chromeos::switches::kUseNewNetworkConnectionHandler),
+  },
   {
     "ash-enable-new-audio-handler2",
     IDS_FLAGS_ASH_ENABLE_NEW_AUDIO_HANDLER_NAME,
diff --git a/chrome/browser/chromeos/cros/native_network_constants.cc b/chrome/browser/chromeos/cros/native_network_constants.cc
index 9aa207b..337f7ce 100644
--- a/chrome/browser/chromeos/cros/native_network_constants.cc
+++ b/chrome/browser/chromeos/cros/native_network_constants.cc
@@ -16,7 +16,7 @@ const char kSharedProfilePath[] = "/profile/default";
 const char* ConnectionStateToString(ConnectionState state) {
   switch (state) {
     case STATE_UNKNOWN:
-      break;
+      return flimflam::kUnknownString;
     case STATE_IDLE:
       return flimflam::kStateIdle;
     case STATE_CARRIER:
diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index fc083d3..5dbe83e 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -639,28 +639,47 @@ void VirtualNetwork::CopyCredentialsFromRemembered(Network* remembered) {
 }
 
 bool VirtualNetwork::NeedMoreInfoToConnect() const {
-  if (server_hostname_.empty() || username_.empty() ||
-      IsUserPassphraseRequired())
+  if (server_hostname_.empty()) {
+    VLOG(1) << "server_hostname_.empty()";
     return true;
-  if (error() != ERROR_NO_ERROR)
+  }
+  if (username_.empty()) {
+    VLOG(1) << "username_.empty()";
+    return true;
+  }
+  if (IsUserPassphraseRequired()) {
+    VLOG(1) << "User Passphrase Required";
+    return true;
+  }
+  if (error() != ERROR_NO_ERROR) {
+    VLOG(1) << "Error: " << error();
     return true;
+  }
   switch (provider_type_) {
     case PROVIDER_TYPE_L2TP_IPSEC_PSK:
-      if (IsPSKPassphraseRequired())
+      if (IsPSKPassphraseRequired()) {
+        VLOG(1) << "PSK Passphrase Required";
         return true;
+      }
       break;
     case PROVIDER_TYPE_L2TP_IPSEC_USER_CERT:
       if (client_cert_id_.empty() &&
-          client_cert_type() != CLIENT_CERT_TYPE_PATTERN)
+          client_cert_type() != CLIENT_CERT_TYPE_PATTERN) {
+        VLOG(1) << "Certificate Required";
         return true;
+      }
       break;
     case PROVIDER_TYPE_OPEN_VPN:
-      if (client_cert_id_.empty())
+      if (client_cert_id_.empty()) {
+        VLOG(1) << "client_cert_id_.empty()";
         return true;
+      }
       // For now we always need additional info for OpenVPN.
       // TODO(stevenjb): Check connectable() once shill sets that state
       // properly, or define another mechanism to determine when additional
       // credentials are required.
+      VLOG(1) << "OpenVPN requires credentials, connectable: "
+              << connectable();
       return true;
       break;
     case PROVIDER_TYPE_MAX:
diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc b/chrome/browser/chromeos/login/chrome_restart_request.cc
index 57e05b2..d7b40813 100644
--- a/chrome/browser/chromeos/login/chrome_restart_request.cc
+++ b/chrome/browser/chromeos/login/chrome_restart_request.cc
@@ -171,6 +171,7 @@ std::string DeriveCommandLine(const GURL& start_url,
       chromeos::switches::kLoginProfile,
       chromeos::switches::kNaturalScrollDefault,
       chromeos::switches::kUseNewNetworkConfigurationHandlers,
+      chromeos::switches::kUseNewNetworkConnectionHandler,
       gfx::switches::kEnableBrowserTextSubpixelPositioning,
       gfx::switches::kEnableWebkitTextSubpixelPositioning,
       views::corewm::switches::kNoDropShadows,
diff --git a/chrome/browser/chromeos/options/network_connect.cc b/chrome/browser/chromeos/options/network_connect.cc
index 4ad2fd2..63c7206 100644
--- a/chrome/browser/chromeos/options/network_connect.cc
+++ b/chrome/browser/chromeos/options/network_connect.cc
@@ -6,8 +6,10 @@
 
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
+#include "ash/system/chromeos/network/network_connect.h"
 #include "ash/system/chromeos/network/network_observer.h"
 #include "ash/system/tray/system_tray_notifier.h"
+#include "base/command_line.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
@@ -15,6 +17,7 @@
 #include "chrome/browser/chromeos/options/network_config_view.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/webui/chromeos/mobile_setup_dialog.h"
+#include "chromeos/chromeos_switches.h"
 #include "grit/generated_resources.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -115,6 +118,12 @@ void ShowMobileSetup(const std::string& service_path) {
 
 ConnectResult ConnectToNetwork(const std::string& service_path,
                                gfx::NativeWindow parent_window) {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kUseNewNetworkConnectionHandler)) {
+    ash::network_connect::ConnectToNetwork(service_path);
+    return CONNECT_STARTED;
+  }
+
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   Network* network = cros->FindNetworkByPath(service_path);
   if (!network)
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index 3321b49..fb75489 100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -761,7 +761,7 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
 
   virtual void ConnectToNetwork(const std::string& network_id) OVERRIDE {
     DCHECK(!CommandLine::ForCurrentProcess()->HasSwitch(
-        chromeos::switches::kUseNewNetworkConfigurationHandlers));
+        chromeos::switches::kUseNewNetworkConnectionHandler));
     network_connect::ConnectResult result =
         network_connect::ConnectToNetwork(network_id, GetNativeWindow());
     if (result == network_connect::NETWORK_NOT_FOUND)
diff --git a/chrome/test/data/extensions/api_test/networking/test.js b/chrome/test/data/extensions/api_test/networking/test.js
index 36940fc..1f45b71 100644
--- a/chrome/test/data/extensions/api_test/networking/test.js
+++ b/chrome/test/data/extensions/api_test/networking/test.js
@@ -30,10 +30,12 @@ var privateHelpers = {
             self.onNetworkChange);
         done();
       };
-      var expectedState = expectedStates.pop();
-      assertEq(expectedState, properties.ConnectionState);
-      if (expectedStates.length == 0)
-        finishTest();
+      if (expectedStates.length > 0) {
+        var expectedState = expectedStates.pop();
+        assertEq(expectedState, properties.ConnectionState);
+        if (expectedStates.length == 0)
+          finishTest();
+      }
     };
     this.onNetworkChange = function(changes) {
       assertEq([network], changes);
diff --git a/chromeos/chromeos_switches.cc b/chromeos/chromeos_switches.cc
index 95d8376..1452a39 100644
--- a/chromeos/chromeos_switches.cc
+++ b/chromeos/chromeos_switches.cc
@@ -191,5 +191,8 @@ const char kStubCrosSettings[]              = "stub-cros-settings";
 const char kUseNewNetworkConfigurationHandlers[] =
     "use-new-network-configuration-handlers";
 
+const char kUseNewNetworkConnectionHandler[] =
+    "use-new-network-connection-handler";
+
 }  // namespace switches
 }  // namespace chromeos
diff --git a/chromeos/chromeos_switches.h b/chromeos/chromeos_switches.h
index f4fd779..d4ad7a3 100644
--- a/chromeos/chromeos_switches.h
+++ b/chromeos/chromeos_switches.h
@@ -72,6 +72,7 @@ CHROMEOS_EXPORT extern const char kSkipHWIDCheck[];
 CHROMEOS_EXPORT extern const char kSmsTestMessages[];
 CHROMEOS_EXPORT extern const char kStubCrosSettings[];
 CHROMEOS_EXPORT extern const char kUseNewNetworkConfigurationHandlers[];
+CHROMEOS_EXPORT extern const char kUseNewNetworkConnectionHandler[];
 
 }  // namespace switches
 }  // namespace chromeos
diff --git a/chromeos/dbus/shill_service_client_stub.cc b/chromeos/dbus/shill_service_client_stub.cc
index 0c0d2f93..17055c8 100644
--- a/chromeos/dbus/shill_service_client_stub.cc
+++ b/chromeos/dbus/shill_service_client_stub.cc
@@ -102,9 +102,12 @@ void ShillServiceClientStub::SetProperty(const dbus::ObjectPath& service_path,
   base::DictionaryValue* dict = NULL;
   if (!stub_services_.GetDictionaryWithoutPathExpansion(
           service_path.value(), &dict)) {
+    LOG(ERROR) << "Service not found:  " << service_path.value();
     error_callback.Run("Error.InvalidService", "Invalid Service");
     return;
   }
+  VLOG(1) << "Service.SetProperty: " << name << " = " << value
+          << " For: " << service_path.value();
   if (name == flimflam::kStateProperty) {
     // If the service went into a connected state, then move it to the top of
     // the list in the manager client.
@@ -186,19 +189,21 @@ void ShillServiceClientStub::ClearProperties(
 void ShillServiceClientStub::Connect(const dbus::ObjectPath& service_path,
                                      const base::Closure& callback,
                                      const ErrorCallback& error_callback) {
+  VLOG(1) << "ShillServiceClientStub::Connect: " << service_path.value();
   base::Value* service;
   if (!stub_services_.Get(service_path.value(), &service)) {
+    LOG(ERROR) << "Service not found:  " << service_path.value();
     error_callback.Run("Error.InvalidService", "Invalid Service");
     return;
   }
   base::TimeDelta delay;
+  // Set Associating
+  base::StringValue associating_value(flimflam::kStateAssociation);
+  SetServiceProperty(service_path.value(),
+                     flimflam::kStateProperty,
+                     associating_value);
   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);
   }
diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index 10f619d..97cc9f8 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -40,6 +40,12 @@ void InvokeErrorCallback(const std::string& service_path,
   error_callback.Run(error_name, error_data.Pass());
 }
 
+bool NetworkConnectable(const NetworkState* network) {
+  if (network->type() == flimflam::kTypeVPN)
+    return false;  // TODO(stevenjb): Shill needs to properly set Connectable.
+  return network->connectable();
+}
+
 bool NetworkMayNeedCredentials(const NetworkState* network) {
   if (network->type() == flimflam::kTypeWifi &&
       (network->security() == flimflam::kSecurity8021x ||
@@ -56,34 +62,62 @@ bool NetworkRequiresActivation(const NetworkState* network) {
            network->cellular_out_of_credits()));
 }
 
-bool VPNIsConfigured(const base::DictionaryValue& properties) {
+bool VPNIsConfigured(const std::string& service_path,
+                     const base::DictionaryValue& service_properties) {
+  const DictionaryValue* properties;
+  if (!service_properties.GetDictionary(flimflam::kProviderProperty,
+                                        &properties)) {
+    NET_LOG_ERROR("VPN Provider Dictionary not present", service_path);
+    return false;
+  }
   std::string provider_type;
   // Note: we use Value path expansion to extract Provider.Type.
-  properties.GetString(flimflam::kProviderTypeProperty, &provider_type);
+  if (!properties->GetString(flimflam::kTypeProperty, &provider_type)) {
+    NET_LOG_ERROR("VPN Provider Type not present", service_path);
+    return false;
+  }
   if (provider_type == flimflam::kProviderOpenVpn) {
     std::string hostname;
-    properties.GetString(flimflam::kProviderHostProperty, &hostname);
-    if (hostname.empty())
+    properties->GetString(flimflam::kHostProperty, &hostname);
+    if (hostname.empty()) {
+      NET_LOG_EVENT("OpenVPN: No hostname", service_path);
       return false;
+    }
     std::string username;
-    properties.GetStringWithoutPathExpansion(
+    properties->GetStringWithoutPathExpansion(
         flimflam::kOpenVPNUserProperty, &username);
-    if (username.empty())
+    if (username.empty()) {
+      NET_LOG_EVENT("OpenVPN: No username", service_path);
       return false;
+    }
+    bool passphrase_required = false;
+    properties->GetBooleanWithoutPathExpansion(
+        flimflam::kPassphraseRequiredProperty, &passphrase_required);
+    std::string passphrase;
+    properties->GetStringWithoutPathExpansion(
+        flimflam::kOpenVPNPasswordProperty, &passphrase);
+    if (passphrase_required && passphrase.empty()) {
+      NET_LOG_EVENT("OpenVPN: No passphrase", service_path);
+      return false;
+    }
     std::string client_cert_id;
-    properties.GetStringWithoutPathExpansion(
+    properties->GetStringWithoutPathExpansion(
         flimflam::kOpenVPNClientCertIdProperty, &client_cert_id);
-    if (client_cert_id.empty())
+    if (client_cert_id.empty()) {
+      NET_LOG_EVENT("OpenVPN: No cert id", service_path);
       return false;
+    }
+    NET_LOG_EVENT("OpenVPN Is Configured", service_path);
   } else {
     bool passphrase_required = false;
     std::string passphrase;
-    properties.GetBooleanWithoutPathExpansion(
+    properties->GetBooleanWithoutPathExpansion(
         flimflam::kL2tpIpsecPskRequiredProperty, &passphrase_required);
-    properties.GetStringWithoutPathExpansion(
+    properties->GetStringWithoutPathExpansion(
         flimflam::kL2tpIpsecPskProperty, &passphrase);
     if (passphrase_required && passphrase.empty())
       return false;
+    NET_LOG_EVENT("VPN Is Configured", service_path);
   }
   return true;
 }
@@ -124,24 +158,48 @@ const char NetworkConnectionHandler::kErrorCertificateRequired[] =
 const char NetworkConnectionHandler::kErrorConfigurationRequired[] =
     "configuration-required";
 const char NetworkConnectionHandler::kErrorShillError[] = "shill-error";
+const char NetworkConnectionHandler::kErrorConnectFailed[] = "connect-failed";
+const char NetworkConnectionHandler::kErrorUnknown[] = "unknown-error";
+
+struct NetworkConnectionHandler::ConnectRequest {
+  ConnectRequest(const base::Closure& success,
+                 const network_handler::ErrorCallback& error)
+      : connect_state(CONNECT_REQUESTED),
+        success_callback(success),
+        error_callback(error) {
+  }
+  enum ConnectState {
+    CONNECT_REQUESTED = 0,
+    CONNECT_STARTED = 1,
+    CONNECT_CONNECTING = 2
+  };
+  ConnectState connect_state;
+  base::Closure success_callback;
+  network_handler::ErrorCallback error_callback;
+};
 
 NetworkConnectionHandler::NetworkConnectionHandler()
     : network_state_handler_(NULL),
       network_configuration_handler_(NULL) {
-  const char* new_handlers_enabled =
+  const char* new_handler_enabled =
       CommandLine::ForCurrentProcess()->HasSwitch(
-          chromeos::switches::kUseNewNetworkConfigurationHandlers) ?
+          chromeos::switches::kUseNewNetworkConnectionHandler) ?
       "enabled" : "disabled";
-  NET_LOG_EVENT("NewNetworkConfigurationHandlers", new_handlers_enabled);
+  NET_LOG_EVENT("NewNetworkConnectionHandler", new_handler_enabled);
 }
 
 NetworkConnectionHandler::~NetworkConnectionHandler() {
+  if (network_state_handler_)
+    network_state_handler_->RemoveObserver(this);
 }
 
 void NetworkConnectionHandler::Init(
     NetworkStateHandler* network_state_handler,
     NetworkConfigurationHandler* network_configuration_handler) {
-  network_state_handler_ = network_state_handler;
+  if (network_state_handler) {
+    network_state_handler_ = network_state_handler;
+    network_state_handler_->AddObserver(this);
+  }
   network_configuration_handler_ = network_configuration_handler;
 }
 
@@ -157,12 +215,16 @@ void NetworkConnectionHandler::ConnectToNetwork(
     InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
     return;
   }
+  if (HasConnectingNetwork(service_path)) {
+    NET_LOG_USER("Connect Request While Pending", service_path);
+    InvokeErrorCallback(service_path, error_callback, kErrorConnecting);
+    return;
+  }
   if (network->IsConnectedState()) {
     InvokeErrorCallback(service_path, error_callback, kErrorConnected);
     return;
   }
-  if (network->IsConnectingState() ||
-      pending_requests_.find(service_path) != pending_requests_.end()) {
+  if (network->IsConnectingState()) {
     InvokeErrorCallback(service_path, error_callback, kErrorConnecting);
     return;
   }
@@ -194,20 +256,21 @@ void NetworkConnectionHandler::ConnectToNetwork(
   }
 
   // All synchronous checks passed, add |service_path| to connecting list.
-  pending_requests_.insert(service_path);
+  pending_requests_.insert(std::make_pair(
+      service_path, ConnectRequest(success_callback, error_callback)));
 
-  if (!network->connectable() && NetworkMayNeedCredentials(network)) {
+  if (!NetworkConnectable(network) && NetworkMayNeedCredentials(network)) {
     // Request additional properties to check.
     network_configuration_handler_->GetProperties(
         network->path(),
         base::Bind(&NetworkConnectionHandler::VerifyConfiguredAndConnect,
-                   AsWeakPtr(), success_callback, error_callback),
+                   AsWeakPtr()),
         base::Bind(&NetworkConnectionHandler::HandleConfigurationFailure,
-                   AsWeakPtr(), network->path(), error_callback));
+                   AsWeakPtr(), network->path()));
     return;
   }
   // All checks passed, send connect request.
-  CallShillConnect(service_path, success_callback, error_callback);
+  CallShillConnect(service_path);
 }
 
 void NetworkConnectionHandler::DisconnectNetwork(
@@ -228,50 +291,51 @@ void NetworkConnectionHandler::DisconnectNetwork(
   CallShillDisconnect(service_path, success_callback, error_callback);
 }
 
-void NetworkConnectionHandler::CallShillConnect(
-    const std::string& service_path,
-    const base::Closure& success_callback,
-    const network_handler::ErrorCallback& error_callback) {
-  // TODO(stevenjb): Remove SetConnectingNetwork and use this class to maintain
-  // the connecting network(s) once NetworkLibrary path is eliminated.
-  network_state_handler_->SetConnectingNetwork(service_path);
-  NET_LOG_EVENT("Connect Request", service_path);
-  DBusThreadManager::Get()->GetShillServiceClient()->Connect(
-      dbus::ObjectPath(service_path),
-      base::Bind(&NetworkConnectionHandler::HandleShillSuccess,
-                 AsWeakPtr(), service_path, success_callback),
-      base::Bind(&NetworkConnectionHandler::HandleShillFailure,
-                 AsWeakPtr(), service_path, error_callback));
+bool NetworkConnectionHandler::HasConnectingNetwork(
+    const std::string& service_path) {
+  return pending_requests_.count(service_path) != 0;
 }
 
-void NetworkConnectionHandler::CallShillDisconnect(
-    const std::string& service_path,
-    const base::Closure& success_callback,
-    const network_handler::ErrorCallback& error_callback) {
-  NET_LOG_EVENT("Disconnect Request", service_path);
-  DBusThreadManager::Get()->GetShillServiceClient()->Disconnect(
-      dbus::ObjectPath(service_path),
-      base::Bind(&NetworkConnectionHandler::HandleShillSuccess,
-                 AsWeakPtr(), service_path, success_callback),
-      base::Bind(&NetworkConnectionHandler::HandleShillFailure,
-                 AsWeakPtr(), service_path, error_callback));
+void NetworkConnectionHandler::NetworkListChanged() {
+  CheckAllPendingRequests();
+}
+
+void NetworkConnectionHandler::NetworkPropertiesUpdated(
+    const NetworkState* network) {
+  if (HasConnectingNetwork(network->path()))
+    CheckPendingRequest(network->path());
+}
+
+NetworkConnectionHandler::ConnectRequest*
+NetworkConnectionHandler::pending_request(
+    const std::string& service_path) {
+  std::map<std::string, ConnectRequest>::iterator iter =
+      pending_requests_.find(service_path);
+  return iter != pending_requests_.end() ? &(iter->second) : NULL;
 }
 
+// ConnectToNetwork implementation
+
 void NetworkConnectionHandler::VerifyConfiguredAndConnect(
-    const base::Closure& success_callback,
-    const network_handler::ErrorCallback& error_callback,
     const std::string& service_path,
     const base::DictionaryValue& properties) {
+  NET_LOG_EVENT("VerifyConfiguredAndConnect", service_path);
+  ConnectRequest* request = pending_request(service_path);
+  DCHECK(request);
+  network_handler::ErrorCallback error_callback = request->error_callback;
+
   const NetworkState* network =
       network_state_handler_->GetNetworkState(service_path);
   if (!network) {
+    pending_requests_.erase(service_path);
     InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
     return;
   }
 
   // VPN requires a host and username to be set.
   if (network->type() == flimflam::kTypeVPN &&
-      !VPNIsConfigured(properties)) {
+      !VPNIsConfigured(service_path, properties)) {
+    pending_requests_.erase(service_path);
     InvokeErrorCallback(service_path, error_callback,
                         kErrorConfigurationRequired);
     return;
@@ -281,44 +345,152 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
   scoped_ptr<NetworkUIData> ui_data =
       ManagedNetworkConfigurationHandler::GetUIData(properties);
   if (ui_data && !CertificateIsConfigured(ui_data.get())) {
+    pending_requests_.erase(service_path);
     InvokeErrorCallback(service_path, error_callback,
                         kErrorCertificateRequired);
     return;
   }
 
-  CallShillConnect(service_path, success_callback, error_callback);
+  CallShillConnect(service_path);
+}
+
+void NetworkConnectionHandler::CallShillConnect(
+    const std::string& service_path) {
+  NET_LOG_EVENT("Connect Request", service_path);
+  DBusThreadManager::Get()->GetShillServiceClient()->Connect(
+      dbus::ObjectPath(service_path),
+      base::Bind(&NetworkConnectionHandler::HandleShillConnectSuccess,
+                 AsWeakPtr(), service_path),
+      base::Bind(&NetworkConnectionHandler::HandleShillConnectFailure,
+                 AsWeakPtr(), service_path));
 }
 
 void NetworkConnectionHandler::HandleConfigurationFailure(
     const std::string& service_path,
-    const network_handler::ErrorCallback& error_callback,
     const std::string& error_name,
     scoped_ptr<base::DictionaryValue> error_data) {
+  ConnectRequest* request = pending_request(service_path);
+  DCHECK(request);
+  network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
   if (!error_callback.is_null())
     error_callback.Run(error_name, error_data.Pass());
 }
 
-void NetworkConnectionHandler::HandleShillSuccess(
+void NetworkConnectionHandler::HandleShillConnectSuccess(
+    const std::string& service_path) {
+  ConnectRequest* request = pending_request(service_path);
+  DCHECK(request);
+  request->connect_state = ConnectRequest::CONNECT_STARTED;
+  NET_LOG_EVENT("Connect Request Sent", service_path);
+  // Do not call success_callback here, wait for one of the following
+  // conditions:
+  // * State transitions to a non connecting state indicating succes or failure
+  // * Network is no longer in the visible list, indicating failure
+  CheckPendingRequest(service_path);
+}
+
+void NetworkConnectionHandler::HandleShillConnectFailure(
     const std::string& service_path,
-    const base::Closure& success_callback) {
-  // TODO(stevenjb): Currently, this only indicates that the connect request
-  // succeeded. It might be preferable to wait for the actually connect
-  // attempt to succeed or fail here and only call |success_callback| at that
-  // point (or maybe call it twice, once indicating in-progress, then success
-  // or failure).
+    const std::string& error_name,
+    const std::string& error_message) {
+  ConnectRequest* request = pending_request(service_path);
+  DCHECK(request);
+  network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
-  NET_LOG_EVENT("Connect Request Sent", service_path);
+  std::string error = "Connect Failure: " + error_name + ": " + error_message;
+  network_handler::ShillErrorCallbackFunction(
+      service_path, error_callback, error_name, error_message);
+}
+
+void NetworkConnectionHandler::CheckPendingRequest(
+    const std::string service_path) {
+  ConnectRequest* request = pending_request(service_path);
+  DCHECK(request);
+  if (request->connect_state == ConnectRequest::CONNECT_REQUESTED)
+    return;  // Request has not started, ignore update
+  const NetworkState* network =
+      network_state_handler_->GetNetworkState(service_path);
+  if (!network) {
+    network_handler::ErrorCallback error_callback = request->error_callback;
+    pending_requests_.erase(service_path);
+    InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
+    return;
+  }
+  if (network->IsConnectingState()) {
+    request->connect_state = ConnectRequest::CONNECT_CONNECTING;
+    return;
+  }
+  if (network->IsConnectedState()) {
+    NET_LOG_EVENT("Connect Request Succeeded", service_path);
+    if (!request->success_callback.is_null())
+      request->success_callback.Run();
+    pending_requests_.erase(service_path);
+    return;
+  }
+  if (network->connection_state() == flimflam::kStateIdle &&
+      request->connect_state != ConnectRequest::CONNECT_CONNECTING) {
+    // Connection hasn't started yet, keep waiting.
+    return;
+  }
+
+  // Network is neither connecting or connected; an error occurred.
+  std::string error_name = kErrorConnectFailed;
+  std::string error_detail = network->error();
+  if (error_detail.empty()) {
+    if (network->connection_state() == flimflam::kStateFailure)
+      error_detail = flimflam::kUnknownString;
+    else
+      error_detail = "Unexpected State: " + network->connection_state();
+  }
+  std::string error_msg = error_name + ": " + error_detail;
+  NET_LOG_ERROR(error_msg, service_path);
+
+  network_handler::ErrorCallback error_callback = request->error_callback;
+  pending_requests_.erase(service_path);
+  if (error_callback.is_null())
+    return;
+  scoped_ptr<base::DictionaryValue> error_data(
+      network_handler::CreateErrorData(service_path, error_name, error_msg));
+  error_callback.Run(error_name, error_data.Pass());
+}
+
+void NetworkConnectionHandler::CheckAllPendingRequests() {
+  for (std::map<std::string, ConnectRequest>::iterator iter =
+           pending_requests_.begin(); iter != pending_requests_.end(); ++iter) {
+    CheckPendingRequest(iter->first);
+  }
+}
+
+// Disconnect
+
+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(&NetworkConnectionHandler::HandleShillDisconnectFailure,
+                 AsWeakPtr(), 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);
   success_callback.Run();
 }
 
-void NetworkConnectionHandler::HandleShillFailure(
+void NetworkConnectionHandler::HandleShillDisconnectFailure(
     const std::string& service_path,
     const network_handler::ErrorCallback& error_callback,
     const std::string& error_name,
     const std::string& error_message) {
-  pending_requests_.erase(service_path);
-  std::string error = "Connect Failure: " + error_name + ": " + error_message;
+  std::string error =
+      "Disconnect Failure: " + error_name + ": " + error_message;
   network_handler::ShillErrorCallbackFunction(
       service_path, error_callback, error_name, error_message);
 }
diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h
index a4d233de..42c2bdf 100644
--- a/chromeos/network/network_connection_handler.h
+++ b/chromeos/network/network_connection_handler.h
@@ -16,6 +16,7 @@
 #include "chromeos/dbus/dbus_method_call_status.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_handler_callbacks.h"
+#include "chromeos/network/network_state_handler_observer.h"
 
 namespace chromeos {
 
@@ -29,28 +30,33 @@ class NetworkState;
 // 2. Request additional information (e.g. user data which contains certificate
 //    information) and determine whether sufficient information is available.
 // 3. Send the connect request.
-// 4. Invoke the appropriate callback (always) on success or failure.
+// 4. Wait for the network state to change to a non connecting state.
+// 5. Invoke the appropriate callback (always) on success or failure.
 //
 // NetworkConnectionHandler depends on NetworkStateHandler for immediately
 // available State information, and NetworkConfigurationHandler for any
 // configuration calls.
 
 class CHROMEOS_EXPORT NetworkConnectionHandler
-    : public base::SupportsWeakPtr<NetworkConnectionHandler> {
+    : public NetworkStateHandlerObserver,
+      public base::SupportsWeakPtr<NetworkConnectionHandler> {
  public:
-  // Constants for |error_name| from |error_callback| for Connect/Disconnect.
+  // Constants for |error_name| from |error_callback| for Connect.
   static const char kErrorNotFound[];
   static const char kErrorConnected[];
   static const char kErrorConnecting[];
-  static const char kErrorNotConnected[];
   static const char kErrorPassphraseRequired[];
   static const char kErrorActivationRequired[];
   static const char kErrorCertificateRequired[];
   static const char kErrorConfigurationRequired[];
   static const char kErrorShillError[];
-  static const char kErrorPreviousConnectFailed[];
+  static const char kErrorConnectFailed[];
+  static const char kErrorUnknown[];
+
+  // Constants for |error_name| from |error_callback| for Disconnect.
+  static const char kErrorNotConnected[];
 
-  ~NetworkConnectionHandler();
+  virtual ~NetworkConnectionHandler();
 
   // ConnectToNetwork() will start an asynchronous connection attempt.
   // On success, |success_callback| will be called.
@@ -82,63 +88,75 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
                          const base::Closure& success_callback,
                          const network_handler::ErrorCallback& error_callback);
 
+  // Returns true if ConnectToNetwork has been called with |service_path| and
+  // has not completed (i.e. success or error callback has been called).
+  bool HasConnectingNetwork(const std::string& service_path);
+
+  // NetworkStateHandlerObserver
+  virtual void NetworkListChanged() OVERRIDE;
+  virtual void NetworkPropertiesUpdated(const NetworkState* network) OVERRIDE;
+
  private:
   friend class NetworkHandler;
   friend class NetworkConnectionHandlerTest;
+
+  struct ConnectRequest;
+
   NetworkConnectionHandler();
 
   void Init(NetworkStateHandler* network_state_handler,
             NetworkConfigurationHandler* network_configuration_handler);
 
-  // Calls Shill.Manager.Connect asynchronously.
-  void CallShillConnect(
-      const std::string& service_path,
-      const base::Closure& success_callback,
-      const network_handler::ErrorCallback& error_callback);
-
-  // Calls Shill.Manager.Disconnect asynchronously.
-  void CallShillDisconnect(
-      const std::string& service_path,
-      const base::Closure& success_callback,
-      const network_handler::ErrorCallback& error_callback);
+  ConnectRequest* pending_request(const std::string& service_path);
 
   // Callback from Shill.Service.GetProperties. Parses |properties| to verify
   // whether or not the network appears to be configured. If configured,
-  // attempts a connection, otherwise invokes |error_callback|.
-  void VerifyConfiguredAndConnect(
-      const base::Closure& success_callback,
-      const network_handler::ErrorCallback& error_callback,
-      const std::string& service_path,
-      const base::DictionaryValue& properties);
+  // attempts a connection, otherwise invokes error_callback from
+  // pending_requests_[service_path].
+  void VerifyConfiguredAndConnect(const std::string& service_path,
+                                  const base::DictionaryValue& properties);
 
-  // Sets the property for the service with an empty callback (logs errors).
-  void SetServiceProperty(const std::string& service_path,
-                          const std::string& property,
-                          const std::string& value) const;
+  // Calls Shill.Manager.Connect asynchronously.
+  void CallShillConnect(const std::string& service_path);
 
   // Handle failure from ConfigurationHandler calls.
   void HandleConfigurationFailure(
       const std::string& service_path,
-      const network_handler::ErrorCallback& error_callback,
       const std::string& error_name,
       scoped_ptr<base::DictionaryValue> error_data);
 
   // Handle success or failure from Shill.Service.Connect.
-  void HandleShillSuccess(const std::string& service_path,
-                          const base::Closure& success_callback);
-  void HandleShillFailure(const std::string& service_path,
-                          const network_handler::ErrorCallback& error_callback,
-                          const std::string& error_name,
-                          const std::string& error_message);
+  void HandleShillConnectSuccess(const std::string& service_path);
+  void HandleShillConnectFailure(const std::string& service_path,
+                                 const std::string& error_name,
+                                 const std::string& error_message);
+
+  void CheckPendingRequest(const std::string service_path);
+  void CheckAllPendingRequests();
+
+  // Calls Shill.Manager.Disconnect asynchronously.
+  void CallShillDisconnect(
+      const std::string& service_path,
+      const base::Closure& success_callback,
+      const network_handler::ErrorCallback& error_callback);
+
+  // Handle success or failure from Shill.Service.Disconnect.
+  void HandleShillDisconnectSuccess(const std::string& service_path,
+                                    const base::Closure& success_callback);
+  void HandleShillDisconnectFailure(
+      const std::string& service_path,
+      const network_handler::ErrorCallback& error_callback,
+      const std::string& error_name,
+      const std::string& error_message);
 
   // Local references to the associated handler instances.
   NetworkStateHandler* network_state_handler_;
   NetworkProfileHandler* network_profile_handler_;
   NetworkConfigurationHandler* network_configuration_handler_;
 
-  // Set of pending connect requests, used to prevent repeat attempts while
-  // waiting for Shill.
-  std::set<std::string> pending_requests_;
+  // Map of pending connect requests, used to prevent repeat attempts while
+  // waiting for Shill and to trigger callbacks on eventual success or failure.
+  std::map<std::string, ConnectRequest> pending_requests_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkConnectionHandler);
 };