Notify on network connect attempt when certificates not loaded

This also:
* Moves the check for certificates loaded to include any
connect attempt where a certificate is required, not just when the
certifciate properties are unconfigured (since the attempt will fail
unless we wait for certificates to load).
* Fixes a DCHECK in WifiConfigView when there are no user certificates 
installed (replaces an empty dropdown with 'None installed')

BUG=341518

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

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

2 files changed, 112 insertions, 39 deletions

diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index 2e3f1e7..42221bd 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/location.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/strings/string_number_conversions.h"
 #include "chromeos/cert_loader.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -118,6 +119,8 @@ const char NetworkConnectionHandler::kErrorConfigureFailed[] =
     "configure-failed";
 const char NetworkConnectionHandler::kErrorConnectCanceled[] =
     "connect-canceled";
+const char NetworkConnectionHandler::kErrorCertLoadTimeout[] =
+    "cert-load-timeout";
 
 struct NetworkConnectionHandler::ConnectRequest {
   ConnectRequest(const std::string& service_path,
@@ -165,14 +168,19 @@ void NetworkConnectionHandler::Init(
   if (LoginState::IsInitialized()) {
     LoginState::Get()->AddObserver(this);
     logged_in_ = LoginState::Get()->IsUserLoggedIn();
+    logged_in_time_ = base::TimeTicks::Now();
   }
 
   if (CertLoader::IsInitialized()) {
     cert_loader_ = CertLoader::Get();
     cert_loader_->AddObserver(this);
-    certificates_loaded_ = cert_loader_->certificates_loaded();
+    if (cert_loader_->certificates_loaded()) {
+      NET_LOG_EVENT("Certificates Loaded", "");
+      certificates_loaded_ = true;
+    }
   } else {
     // TODO(tbarzic): Require a mock or stub cert_loader in tests.
+    NET_LOG_EVENT("Certificate Loader not initialized", "");
     certificates_loaded_ = true;
   }
 
@@ -187,6 +195,7 @@ void NetworkConnectionHandler::LoggedInStateChanged() {
   if (LoginState::Get()->IsUserLoggedIn()) {
     logged_in_ = true;
     NET_LOG_EVENT("Logged In", "");
+    logged_in_time_ = base::TimeTicks::Now();
   }
 }
 
@@ -196,18 +205,7 @@ void NetworkConnectionHandler::OnCertificatesLoaded(
   certificates_loaded_ = true;
   NET_LOG_EVENT("Certificates Loaded", "");
   if (queued_connect_) {
-    NET_LOG_EVENT("Connecting to Queued Network",
-                  queued_connect_->service_path);
-
-    // Make a copy of |queued_connect_| parameters, because |queued_connect_|
-    // will get reset at the beginning of |ConnectToNetwork|.
-    std::string service_path = queued_connect_->service_path;
-    base::Closure success_callback = queued_connect_->success_callback;
-    network_handler::ErrorCallback error_callback =
-        queued_connect_->error_callback;
-
-    ConnectToNetwork(service_path, success_callback, error_callback,
-                     false /* check_error_state */);
+    ConnectToQueuedNetwork();
   } else if (initial_load) {
     // Once certificates have loaded, connect to the "best" available network.
     network_state_handler_->ConnectToBestWifiNetwork();
@@ -422,36 +420,31 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
 
   base::DictionaryValue config_properties;
   if (client_cert_type != client_cert::CONFIG_TYPE_NONE) {
+    // Note: if we get here then a certificate *may* be required, so we want
+    // to ensure that certificates have loaded successfully before attempting
+    // to connect.
+
+    // User must be logged in to connect to a network requiring a certificate.
+    if (!logged_in_ || !cert_loader_) {
+      NET_LOG_ERROR("User not logged in", "");
+      ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
+      return;
+    }
+    // If certificates have not been loaded yet, queue the connect request.
+    if (!certificates_loaded_) {
+      NET_LOG_EVENT("Certificates not loaded", "");
+      QueueConnectRequest(service_path);
+      return;
+    }
+
     // If the client certificate must be configured, this will be set to a
     // non-empty string.
     std::string pkcs11_id;
 
     // Check certificate properties in kUIDataProperty if configured.
     // Note: Wifi/VPNConfigView set these properties explicitly, in which case
-    //   only the TPM must be configured.
+    // only the TPM must be configured.
     if (ui_data && ui_data->certificate_type() == CLIENT_CERT_TYPE_PATTERN) {
-      // User must be logged in to connect to a network requiring a certificate.
-      if (!logged_in_ || !cert_loader_) {
-        ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
-        return;
-      }
-
-      // If certificates have not been loaded yet, queue the connect request.
-      if (!certificates_loaded_) {
-        NET_LOG_EVENT("Certificates not loaded", "");
-        ConnectRequest* request = GetPendingRequest(service_path);
-        if (!request) {
-          NET_LOG_ERROR("No pending request to queue", service_path);
-          return;
-        }
-        NET_LOG_EVENT("Connect Request Queued", service_path);
-        queued_connect_.reset(new ConnectRequest(
-            service_path, request->profile_path,
-            request->success_callback, request->error_callback));
-        pending_requests_.erase(service_path);
-        return;
-      }
-
       pkcs11_id = CertificateIsConfigured(ui_data.get());
       // Ensure the certificate is available and configured.
       if (!cert_loader_->IsHardwareBacked() || pkcs11_id.empty()) {
@@ -523,6 +516,70 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
     CallShillConnect(service_path);
 }
 
+void NetworkConnectionHandler::QueueConnectRequest(
+    const std::string& service_path) {
+  ConnectRequest* request = GetPendingRequest(service_path);
+  if (!request) {
+    NET_LOG_ERROR("No pending request to queue", service_path);
+    return;
+  }
+
+  const int kMaxCertLoadTimeSeconds = 15;
+  base::TimeDelta dtime = base::TimeTicks::Now() - logged_in_time_;
+  if (dtime > base::TimeDelta::FromSeconds(kMaxCertLoadTimeSeconds)) {
+    NET_LOG_ERROR("Certificate load timeout", service_path);
+    InvokeErrorCallback(service_path,
+                        request->error_callback,
+                        kErrorCertLoadTimeout);
+    return;
+  }
+
+  NET_LOG_EVENT("Connect Request Queued", service_path);
+  queued_connect_.reset(new ConnectRequest(
+      service_path, request->profile_path,
+      request->success_callback, request->error_callback));
+  pending_requests_.erase(service_path);
+
+  // Post a delayed task to check to see if certificates have loaded. If they
+  // haven't, and queued_connect_ has not been cleared (e.g. by a successful
+  // connect request), cancel the request and notify the user.
+  base::MessageLoopProxy::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&NetworkConnectionHandler::CheckCertificatesLoaded,
+                 AsWeakPtr()),
+      base::TimeDelta::FromSeconds(kMaxCertLoadTimeSeconds) - dtime);
+}
+
+void NetworkConnectionHandler::CheckCertificatesLoaded() {
+  if (certificates_loaded_)
+    return;
+  // If queued_connect_ has been cleared (e.g. another connect request occurred
+  // and wasn't queued), do nothing here.
+  if (!queued_connect_)
+    return;
+  // Otherwise, notify the user.
+  NET_LOG_ERROR("Certificate load timeout", queued_connect_->service_path);
+  InvokeErrorCallback(queued_connect_->service_path,
+                      queued_connect_->error_callback,
+                      kErrorCertLoadTimeout);
+  queued_connect_.reset();
+}
+
+void NetworkConnectionHandler::ConnectToQueuedNetwork() {
+  DCHECK(queued_connect_);
+
+  // Make a copy of |queued_connect_| parameters, because |queued_connect_|
+  // will get reset at the beginning of |ConnectToNetwork|.
+  std::string service_path = queued_connect_->service_path;
+  base::Closure success_callback = queued_connect_->success_callback;
+  network_handler::ErrorCallback error_callback =
+      queued_connect_->error_callback;
+
+  NET_LOG_EVENT("Connecting to Queued Network", service_path);
+  ConnectToNetwork(service_path, success_callback, error_callback,
+                   false /* check_error_state */);
+}
+
 void NetworkConnectionHandler::CallShillConnect(
     const std::string& service_path) {
   NET_LOG_EVENT("Sending Connect Request to Shill", service_path);
@@ -563,7 +620,7 @@ void NetworkConnectionHandler::HandleShillConnectSuccess(
   NET_LOG_EVENT("Connect Request Acknowledged", 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
+  // * State transitions to a non connecting state indicating success or failure
   // * Network is no longer in the visible list, indicating failure
   CheckPendingRequest(service_path);
 }
diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h
index 2bc40c4..b8e9a84 100644
--- a/chromeos/network/network_connection_handler.h
+++ b/chromeos/network/network_connection_handler.h
@@ -11,6 +11,7 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "base/values.h"
 #include "chromeos/cert_loader.h"
 #include "chromeos/chromeos_export.h"
@@ -86,6 +87,9 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
   // Constants for |error_name| from |error_callback| for Disconnect.
   static const char kErrorNotConnected[];
 
+  // Certificate load timed out.
+  static const char kErrorCertLoadTimeout[];
+
   virtual ~NetworkConnectionHandler();
 
   // ConnectToNetwork() will start an asynchronous connection attempt.
@@ -154,16 +158,27 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
                                   const std::string& service_path,
                                   const base::DictionaryValue& properties);
 
+  // Queues a connect request until certificates have loaded.
+  void QueueConnectRequest(const std::string& service_path);
+
+  // Checks to see if certificates have loaded and if not, cancels any queued
+  // connect request and notifies the user.
+  void CheckCertificatesLoaded();
+
+  // Handles connecting to a queued network after certificates are loaded or
+  // handle cert load timeout.
+  void ConnectToQueuedNetwork();
+
   // Calls Shill.Manager.Connect asynchronously.
   void CallShillConnect(const std::string& service_path);
 
-  // Handle failure from ConfigurationHandler calls.
+  // Handles failure from ConfigurationHandler calls.
   void HandleConfigurationFailure(
       const std::string& service_path,
       const std::string& error_name,
       scoped_ptr<base::DictionaryValue> error_data);
 
-  // Handle success or failure from Shill.Service.Connect.
+  // Handles success or failure from Shill.Service.Connect.
   void HandleShillConnectSuccess(const std::string& service_path);
   void HandleShillConnectFailure(const std::string& service_path,
                                  const std::string& error_name,
@@ -202,6 +217,7 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
   // Track certificate loading state.
   bool logged_in_;
   bool certificates_loaded_;
+  base::TimeTicks logged_in_time_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkConnectionHandler);
 };