Fixed some network menu logic bugs.

Also modified NetworkMenu::ActivatedAt to use ConnectToNetworkAt() to reduce duplicated logic.

BUG=chromium-os:7343
TEST=Test network menu

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

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

4 files changed, 128 insertions, 71 deletions

diff --git a/chrome/browser/chromeos/dom_ui/network_menu_ui.cc b/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
index 2658e9f..7000269 100644
--- a/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/network_menu_ui.cc
@@ -10,7 +10,9 @@
 #include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/chromeos/status/network_menu.h"
+#include "chrome/browser/chromeos/views/domui_menu_widget.h"
 #include "chrome/browser/chromeos/views/native_menu_domui.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/gurl.h"
 #include "grit/browser_resources.h"
@@ -50,8 +52,21 @@ void NetworkMenuHandler::RegisterMessages() {
 
 void NetworkMenuHandler::HandleAction(const ListValue* values) {
   menus::MenuModel* model = GetMenuModel();
-  if (model)
-    static_cast<chromeos::NetworkMenuUI*>(dom_ui_)->ModelAction(model, values);
+  if (model) {
+    chromeos::NetworkMenuUI* network_menu_ui =
+        static_cast<chromeos::NetworkMenuUI*>(dom_ui_);
+    bool close_menu = network_menu_ui->ModelAction(model, values);
+    if (close_menu) {
+      chromeos::DOMUIMenuWidget* widget
+          = chromeos::DOMUIMenuWidget::FindDOMUIMenuWidget(
+              dom_ui_->tab_contents()->GetNativeView());
+      if (widget) {
+        chromeos::NativeMenuDOMUI* domui_menu = widget->domui_menu();
+        if (domui_menu)
+          domui_menu->Hide();
+      }
+    }
+  }
 }
 
 NetworkMenuHandler::NetworkMenuHandler() {
@@ -100,15 +115,16 @@ void NetworkMenuUI::AddLocalizedStrings(
       l10n_util::GetStringUTF16(IDS_NETWORK_PASSWORD_HINT));
 }
 
-void NetworkMenuUI::ModelAction(const menus::MenuModel* model,
+bool NetworkMenuUI::ModelAction(const menus::MenuModel* model,
                                 const ListValue* values) {
   const NetworkMenu* network_menu = static_cast<const NetworkMenu*>(model);
   std::string action;
   bool success = values->GetString(0, &action);
+  bool close_menu = true;
   if (!success) {
     LOG(WARNING) << "ModelAction called with no arguments from: "
                  << chrome::kChromeUINetworkMenu;
-    return;
+    return close_menu;
   }
   int index;
   std::string index_str;
@@ -117,18 +133,25 @@ void NetworkMenuUI::ModelAction(const menus::MenuModel* model,
   if (!success) {
     LOG(WARNING) << "ModelAction called with no index from: "
                  << chrome::kChromeUINetworkMenu;
-    return;
+    return close_menu;
   }
   std::string passphrase;
   values->GetString(2, &passphrase);  // Optional
-  std::string identity;
-  values->GetString(3, &identity);  // Optional
+  std::string ssid;
+  values->GetString(3, &ssid);  // Optional
+  int remember = -1;  // -1 indicates not set
+  std::string remember_str;
+  if (values->GetString(4, &remember_str))  // Optional
+    base::StringToInt(remember_str, &remember);
+
   if (action == "connect" || action == "reconnect") {
-    network_menu->ConnectToNetworkAt(index, passphrase, identity);
+    close_menu = network_menu->ConnectToNetworkAt(index, passphrase, ssid,
+                                                  remember);
   } else {
     LOG(WARNING) << "Unrecognized action: " << action
                  << " from: " << chrome::kChromeUINetworkMenu;
   }
+  return close_menu;
 }
 
 DictionaryValue* NetworkMenuUI::CreateMenuItem(const menus::MenuModel* model,
diff --git a/chrome/browser/chromeos/dom_ui/network_menu_ui.h b/chrome/browser/chromeos/dom_ui/network_menu_ui.h
index 4560c9c..67027c8 100644
--- a/chrome/browser/chromeos/dom_ui/network_menu_ui.h
+++ b/chrome/browser/chromeos/dom_ui/network_menu_ui.h
@@ -25,7 +25,8 @@ class NetworkMenuUI : public MenuUI {
   // A callback method that is invoked when the JavaScript wants
   // to invoke an action in the model.
   // By convention the first member of 'values' describes the action.
-  void ModelAction(const menus::MenuModel* model,
+  // Returns true if the menu should be closed.
+  bool ModelAction(const menus::MenuModel* model,
                    const ListValue* values);
 
   // MenuUI overrides
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index e738909..149721c 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -96,8 +96,17 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
         info->message = l10n_util::GetStringUTF8(
             IDS_STATUSBAR_NETWORK_DEVICE_DISCONNECTED);
       }
+      if (wifi.encrypted()) {
+        if (wifi.IsCertificateLoaded() ||
+            wifi.encryption() == SECURITY_8021X) {
+          info->need_passphrase = false;
+        } else {
+          info->need_passphrase = true;
+        }
+      } else {
+        info->need_passphrase = false;
+      }
       info->ip_address = wifi.ip_address();
-      info->need_passphrase = wifi.encrypted();
       info->remembered = wifi.favorite();
     } else {
       res = false;  // Network not found, hide entry.
@@ -151,9 +160,10 @@ bool NetworkMenu::GetNetworkAt(int index, NetworkInfo* info) const {
   return res;
 }
 
-void NetworkMenu::ConnectToNetworkAt(int index,
+bool NetworkMenu::ConnectToNetworkAt(int index,
                                      const std::string& passphrase,
-                                     const std::string& identity) const {
+                                     const std::string& ssid,
+                                     int remember) const {
   int flags = menu_items_[index].flags;
   NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
   if (flags & FLAG_WIFI) {
@@ -162,20 +172,22 @@ void NetworkMenu::ConnectToNetworkAt(int index,
         menu_items_[index].wireless_path, &wifi);
     if (found) {
       // Connect or reconnect.
+      if (remember >= 0)
+        wifi.set_favorite(remember ? true : false);
       if (wifi.encrypted()) {
         if (wifi.IsCertificateLoaded()) {
-          cros->ConnectToWifiNetwork(wifi, std::string(),
-                                     std::string(), wifi.cert_path());
+          cros->ConnectToWifiNetwork(wifi, std::string(), std::string(),
+                                     wifi.cert_path());
+        } else if (wifi.encryption() == SECURITY_8021X) {
+          // Show the wifi settings/dialog to load/select a certificate.
+          ShowWifi(wifi, true);
         } else {
-          // TODO(stevenjb): use passphrase/identity once provided.
-          // cros->ConnectToWifiNetwork(wifi, passphrase,
-          //                            identity, menu_items[index].path);
-          const bool kFocusLogin = true;
-          ShowNetworkConfigView(new NetworkConfigView(wifi, true), kFocusLogin);
+          cros->ConnectToWifiNetwork(wifi, passphrase, std::string(),
+                                     std::string());
         }
       } else {
-        cros->ConnectToWifiNetwork(wifi, std::string(),
-                                   std::string(), std::string());
+        cros->ConnectToWifiNetwork(wifi, std::string(), std::string(),
+                                   std::string());
       }
     }
   } else if (flags & FLAG_CELLULAR) {
@@ -187,9 +199,11 @@ void NetworkMenu::ConnectToNetworkAt(int index,
       cros->ConnectToCellularNetwork(cellular);
     }
   } else if (flags & FLAG_OTHER_NETWORK) {
-    const bool kFocusLogin = true;
-    ShowNetworkConfigView(new NetworkConfigView(), kFocusLogin);
+    bool favorite = remember == 0 ? false : true;  // default is true
+    cros->ConnectToWifiNetwork(ssid, passphrase, std::string(), std::string(),
+                               favorite);
   }
+  return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -248,17 +262,10 @@ void NetworkMenu::ActivatedAt(int index) {
   } else if (flags & FLAG_TOGGLE_OFFLINE) {
     cros->EnableOfflineMode(!cros->offline_mode());
   } else if (flags & FLAG_OTHER_NETWORK) {
-    const bool kFocusLogin = true;
-    ShowNetworkConfigView(new NetworkConfigView(), kFocusLogin);
+    ShowOther();
   } else if (flags & FLAG_ETHERNET) {
     if (cros->ethernet_connected()) {
-      if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableTabbedOptions)) {
-        ShowTabbedNetworkSettings(cros->ethernet_network());
-      } else {
-        ShowNetworkConfigView(
-            new NetworkConfigView(cros->ethernet_network()), false);
-      }
+      ShowEthernet(cros->ethernet_network());
     }
   } else if (flags & FLAG_WIFI) {
     WifiNetwork wifi;
@@ -269,52 +276,24 @@ void NetworkMenu::ActivatedAt(int index) {
       // display a notification.
       // TODO(stevenjb): Show notification.
     } else if (wifi.name() == cros->wifi_name()) {
-      if (cros->wifi_connected()) {
-        // If we are already connected, show the config settings.
-        if (CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kEnableTabbedOptions)) {
-          ShowTabbedNetworkSettings(wifi);
-        } else {
-          ShowNetworkConfigView(new NetworkConfigView(wifi, false), false);
-        }
-      } else {
-        // TODO(stevenjb): Connection in progress. Show dialog?
-      }
+      // Show the config settings for the active network.
+      ShowWifi(wifi, false);
     } else {
-      // If wifi network is not encrypted, then directly connect.
-      // Otherwise, open the password dialog window.
-      if (!wifi.encrypted()) {
-        cros->ConnectToWifiNetwork(wifi, std::string(),
-                                   std::string(), std::string());
-      } else {
-        const bool kFocusLogin = true;
-        ShowNetworkConfigView(new NetworkConfigView(wifi, true), kFocusLogin);
-      }
+      ConnectToNetworkAt(index, std::string(), std::string(), -1);
     }
   } else if (flags & FLAG_CELLULAR) {
     CellularNetwork cellular;
     bool cellular_exists = cros->FindCellularNetworkByPath(
         menu_items_[index].wireless_path, &cellular);
-
     if (!cellular_exists) {
       // If we are attempting to connect to a network that no longer exists,
       // display a notification.
       // TODO(stevenjb): Show notification.
     } else if (cellular.name() == cros->cellular_name()) {
-      // If clicked on a network that we are already connected to or we are
-      // currently trying to connect to, then open config dialog.
-      if (cros->cellular_connected()) {
-        if (CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kEnableTabbedOptions)) {
-          ShowTabbedNetworkSettings(cellular);
-        } else {
-          ShowNetworkConfigView(new NetworkConfigView(cellular), false);
-        }
-      } else {
-        // TODO(stevenjb): Connection in progress. Show dialog?
-      }
+      // Show the config settings for the cellular network.
+      ShowCellular(cellular, false);
     } else {
-      cros->ConnectToCellularNetwork(cellular);
+      ConnectToNetworkAt(index, std::string(), std::string(), -1);
     }
   }
 }
@@ -525,7 +504,7 @@ void NetworkMenu::InitMenuItems() {
   }
 }
 
-void NetworkMenu::ShowTabbedNetworkSettings(const Network& network) {
+void NetworkMenu::ShowTabbedNetworkSettings(const Network& network) const {
   Browser* browser = BrowserList::GetLastActive();
   if (!browser)
     return;
@@ -536,7 +515,8 @@ void NetworkMenu::ShowTabbedNetworkSettings(const Network& network) {
   browser->ShowOptionsTab(page);
 }
 
-// TODO(stevenjb): deprecate this once all of the UI is embedded in the menu.
+// TODO(stevenjb): deprecate this once we've committed to the embedded
+// menu UI and fully deprecated NetworkConfigView.
 void NetworkMenu::ShowNetworkConfigView(NetworkConfigView* view,
                                         bool focus_login) const {
   view->set_browser_mode(IsBrowserMode());
@@ -548,4 +528,48 @@ void NetworkMenu::ShowNetworkConfigView(NetworkConfigView* view,
     view->SetLoginTextfieldFocus();
 }
 
+void NetworkMenu::ShowWifi(const WifiNetwork& wifi, bool focus_login) const{
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTabbedOptions)) {
+    ShowTabbedNetworkSettings(wifi);
+  } else {
+    ShowNetworkConfigView(new NetworkConfigView(wifi, true), focus_login);
+  }
+}
+
+void NetworkMenu::ShowCellular(const CellularNetwork& cellular,
+                               bool focus_login) const {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTabbedOptions)) {
+    ShowTabbedNetworkSettings(cellular);
+  } else {
+    ShowNetworkConfigView(new NetworkConfigView(cellular), focus_login);
+  }
+}
+
+void NetworkMenu::ShowEthernet(const EthernetNetwork& ethernet) const {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTabbedOptions)) {
+    ShowTabbedNetworkSettings(ethernet);
+  } else {
+    ShowNetworkConfigView(new NetworkConfigView(ethernet), false);
+  }
+}
+
+void NetworkMenu::ShowOther() const {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableTabbedOptions)) {
+    Browser* browser = BrowserList::GetLastActive();
+    if (browser) {
+      std::string page = StringPrintf("%s?networkType=%d",
+                                      chrome::kInternetOptionsSubPage,
+                                      chromeos::TYPE_WIFI);
+      browser->ShowOptionsTab(page);
+    }
+  } else {
+    const bool kFocusLogin = true;
+    ShowNetworkConfigView(new NetworkConfigView(), kFocusLogin);
+  }
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/status/network_menu.h b/chrome/browser/chromeos/status/network_menu.h
index 240a573..8419cfe 100644
--- a/chrome/browser/chromeos/status/network_menu.h
+++ b/chrome/browser/chromeos/status/network_menu.h
@@ -75,9 +75,12 @@ class NetworkMenu : public views::ViewMenuDelegate,
   bool GetNetworkAt(int index, NetworkInfo* info) const;
 
   // Connect or reconnect to the network at |index|.
-  void ConnectToNetworkAt(int index,
+  // If remember >= 0, set the favorite state of the network.
+  // Returns true if a connect occurred (e.g. menu should be closed).
+  bool ConnectToNetworkAt(int index,
                           const std::string& passphrase,
-                          const std::string& identity) const;
+                          const std::string& ssid,
+                          int remember) const;
 
   // menus::MenuModel implementation.
   virtual bool HasIcons() const  { return true; }
@@ -169,12 +172,18 @@ class NetworkMenu : public views::ViewMenuDelegate,
   void InitMenuItems();
 
   // Shows network details in DOM UI options window.
-  void ShowTabbedNetworkSettings(const Network& network);
+  void ShowTabbedNetworkSettings(const Network& network) const;
 
   // Show a NetworkConfigView modal dialog instance.
   // TODO(stevenjb): deprecate this once all of the UI is embedded in the menu.
   void ShowNetworkConfigView(NetworkConfigView* view, bool focus_login) const;
 
+  // Wrappers for the ShowNetworkConfigView / ShowTabbedNetworkSettings.
+  void ShowWifi(const WifiNetwork& wifi, bool focus_login) const;
+  void ShowCellular(const CellularNetwork& cellular, bool focus_login) const;
+  void ShowEthernet(const EthernetNetwork& ethernet) const;
+  void ShowOther() const;
+
   // Set to true if we are currently refreshing the menu.
   bool refreshing_menu_;