Clear passphrase when canceling a failed connection attempt.

Also, keep the connect dialog up after an immediate conenction failure (e.g. due to an invalid password format)

BUG=chromium-os:8566
TEST=See issue. Also, test conencting to network with a combination of good / bad passwords in general. Note: the only way currently to re-enter the passphrase for a successfully remembered network is to forget it, log out, and log back in.

Review URL: http://codereview.chromium.org/5160004

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

5 files changed, 68 insertions, 15 deletions

diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index 33aa955..db3396a 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -1019,7 +1019,7 @@ class NetworkLibraryImpl : public NetworkLibrary  {
       WifiNetwork* wifi = GetWirelessNetworkByPath(
           wifi_networks_, network->service_path());
       if (wifi) {
-        wifi->set_passphrase(password);
+        // Note: don't save the passphrase here, it might be incorrect.
         wifi->set_identity(identity);
         wifi->set_cert_path(certpath);
         wifi->set_connecting(true);
@@ -1121,7 +1121,17 @@ class NetworkLibraryImpl : public NetworkLibrary  {
     // Update the cellular network with libcros.
     if (!EnsureCrosLoaded() || !network)
       return;
+    CellularNetwork* cellular =
+        GetWirelessNetworkByPath(cellular_networks_,
+                                 network->service_path());
+    if (!cellular) {
+      LOG(WARNING) << "Save to unknown network: " << cellular->service_path();
+      return;
+    }
 
+    // Immediately update properties in the cached structure.
+    cellular->set_auto_connect(network->auto_connect());
+    // Update libcros.
     SetAutoConnect(network->service_path().c_str(), network->auto_connect());
   }
 
@@ -1130,18 +1140,34 @@ class NetworkLibraryImpl : public NetworkLibrary  {
     // Update the wifi network with libcros.
     if (!EnsureCrosLoaded() || !network)
       return;
-    SetPassphrase(
-        network->service_path().c_str(), network->passphrase().c_str());
-    SetIdentity(network->service_path().c_str(),
-        network->identity().c_str());
-    SetCertPath(network->service_path().c_str(),
-        network->cert_path().c_str());
-    SetAutoConnect(network->service_path().c_str(), network->auto_connect());
+    WifiNetwork* wifi = GetWirelessNetworkByPath(wifi_networks_,
+                                                 network->service_path());
+    if (!wifi) {
+      LOG(WARNING) << "Save to unknown network: " << wifi->service_path();
+      return;
+    }
+    // Immediately update properties in the cached structure.
+    wifi->set_passphrase(network->passphrase());
+    wifi->set_identity(network->identity());
+    wifi->set_cert_path(network->cert_path());
+    wifi->set_auto_connect(network->auto_connect());
+    // Update libcros.
+    const char* service_path = network->service_path().c_str();
+    SetPassphrase(service_path, network->passphrase().c_str());
+    SetIdentity(service_path, network->identity().c_str());
+    SetCertPath(service_path, network->cert_path().c_str());
+    SetAutoConnect(service_path, network->auto_connect());
   }
 
   virtual void ForgetWifiNetwork(const std::string& service_path) {
     if (!EnsureCrosLoaded())
       return;
+    // NOTE: service paths for remembered wifi networks do not match the
+    // service paths in wifi_networks_; calling a libcros funtion that
+    // operates on the wifi_networks_ list with this service_path will
+    // trigger a crash because the DBUS path does not exist.
+    // TODO(stevenjb): modify libcros to warn and fail instead of crash.
+    // https://crosbug.com/9295
     if (DeleteRememberedService(service_path.c_str())) {
       // Update local cache and notify listeners.
       for (WifiNetworkVector::iterator iter =
diff --git a/chrome/browser/chromeos/options/network_config_view.cc b/chrome/browser/chromeos/options/network_config_view.cc
index 0c65eb6..48070e2 100644
--- a/chrome/browser/chromeos/options/network_config_view.cc
+++ b/chrome/browser/chromeos/options/network_config_view.cc
@@ -94,6 +94,8 @@ bool NetworkConfigView::IsDialogButtonEnabled(
 bool NetworkConfigView::Cancel() {
   if (delegate_)
     delegate_->OnDialogCancelled();
+  if (flags_ & FLAG_WIFI)
+    wificonfig_view_->Cancel();
   return true;
 }
 
diff --git a/chrome/browser/chromeos/options/wifi_config_view.cc b/chrome/browser/chromeos/options/wifi_config_view.cc
index 7cdad5b..f6c29dd 100644
--- a/chrome/browser/chromeos/options/wifi_config_view.cc
+++ b/chrome/browser/chromeos/options/wifi_config_view.cc
@@ -191,6 +191,7 @@ bool WifiConfigView::Login() {
   if (identity_textfield_ != NULL) {
     identity_string = UTF16ToUTF8(identity_textfield_->text());
   }
+  NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   bool connected = false;
   if (other_network_) {
     ConnectionSecurity sec = SECURITY_UNKNOWN;
@@ -203,15 +204,24 @@ bool WifiConfigView::Login() {
       sec = SECURITY_WPA;
     else if (index == INDEX_RSN)
       sec = SECURITY_RSN;
-    connected =  CrosLibrary::Get()->GetNetworkLibrary()->ConnectToWifiNetwork(
+    connected =  cros->ConnectToWifiNetwork(
         sec, GetSSID(), GetPassphrase(),
         identity_string, certificate_path_,
         autoconnect_checkbox_ ? autoconnect_checkbox_->checked() : true);
+    // TODO(stevenjb): Modify libcros to set an error code and return 'false'
+    // only on an invalid password or other UI failure.
   } else {
     Save();
-    connected = CrosLibrary::Get()->GetNetworkLibrary()->ConnectToWifiNetwork(
+    connected = cros->ConnectToWifiNetwork(
         wifi_.get(), GetPassphrase(),
         identity_string, certificate_path_);
+    if (!connected) {
+      // Assume this failed due to an invalid password.
+      // TODO(stevenjb): Modify libcros to set an error code and return 'false'
+      // only on an invalid password or other recoverable failure.
+      wifi_->set_passphrase(std::string());
+      cros->SaveWifiNetwork(wifi_.get());
+    }
   }
   return connected;
 }
@@ -230,8 +240,7 @@ bool WifiConfigView::Save() {
     }
 
     if (passphrase_textfield_) {
-      const std::string& passphrase =
-          UTF16ToUTF8(passphrase_textfield_->text());
+      std::string passphrase = UTF16ToUTF8(passphrase_textfield_->text());
       if (passphrase != wifi_->passphrase()) {
         wifi_->set_passphrase(passphrase);
         changed = true;
@@ -244,6 +253,15 @@ bool WifiConfigView::Save() {
   return true;
 }
 
+void WifiConfigView::Cancel() {
+  // If we have a bad passphrase error, clear the passphrase.
+  if (wifi_->error() == ERROR_BAD_PASSPHRASE ||
+      wifi_->error() == ERROR_BAD_WEPKEY) {
+    wifi_->set_passphrase(std::string());
+    CrosLibrary::Get()->GetNetworkLibrary()->SaveWifiNetwork(wifi_.get());
+  }
+}
+
 const std::string WifiConfigView::GetSSID() const {
   std::string result;
   if (ssid_textfield_ != NULL)
diff --git a/chrome/browser/chromeos/options/wifi_config_view.h b/chrome/browser/chromeos/options/wifi_config_view.h
index 9a72a25..c8beb38 100644
--- a/chrome/browser/chromeos/options/wifi_config_view.h
+++ b/chrome/browser/chromeos/options/wifi_config_view.h
@@ -54,12 +54,15 @@ class WifiConfigView : public views::View,
   // SelectFileDialog::Listener implementation.
   virtual void FileSelected(const FilePath& path, int index, void* params);
 
-  // Login to network.
+  // Login to network. Returns false if the dialog should remain open.
   virtual bool Login();
 
   // Save network information.
   virtual bool Save();
 
+  // Cancel the dialog.
+  virtual void Cancel();
+
   // Get the typed in ssid.
   const std::string GetSSID() const;
   // Get the typed in passphrase.
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index 90eaefb..080c13f 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -227,9 +227,13 @@ bool NetworkMenu::ConnectToNetworkAt(int index,
           ShowWifi(wifi, true);
           return true;
         } else {
-          if (MenuUI::IsEnabled() || !wifi->passphrase_required()) {
+          // If a passphrase is provided use it, otherwise use the saved one.
+          std::string pass =
+              !passphrase.empty() ? passphrase : wifi->passphrase();
+          if (MenuUI::IsEnabled() ||
+              (!pass.empty() && !wifi->passphrase_required())) {
             connected = cros->ConnectToWifiNetwork(
-                wifi, passphrase, std::string(), std::string());
+                wifi, pass, std::string(), std::string());
           }
         }
       } else {