chromeos: Refactor some network related code in preparation for the uber tray.

chrome/browser/chromeos/status/network_menu.cc

Index: chrome/browser/chromeos/status/network_menu.cc
diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index 98de1d21373a1c676d591c48d25579ed1799125f..a0072117c676dcc4de0c0989a56f31930947572a 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -87,6 +87,15 @@ void SetMenuMargins(views::MenuItemView* menu_item_view, int top, int bottom) {
   }
 }
 
+// Activate a cellular network.
+void ActivateCellular(const chromeos::CellularNetwork* cellular) {
+  DCHECK(cellular);
+  Browser* browser = BrowserList::GetLastActive();
+  if (!browser)
+    return;
+  browser->OpenMobilePlanTabAndActivate();
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -202,15 +211,12 @@ class NetworkMenuModel : public ui::MenuModel {
  private:
   // Show a NetworkConfigView modal dialog instance.
   void ShowNetworkConfigView(NetworkConfigView* view) const;
-  // Activate a cellular network.
-  void ActivateCellular(const CellularNetwork* cellular) const;
   // Open a dialog to set up and connect to a network.
   void ShowOther(ConnectionType type) const;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkMenuModel);
 };
 
-
 class MoreMenuModel : public NetworkMenuModel {
  public:
   explicit MoreMenuModel(const base::WeakPtr<NetworkMenu> owner)
@@ -281,14 +287,7 @@ void NetworkMenuModel::ConnectToNetworkAt(int index,
       // Connect or reconnect.
       if (auto_connect >= 0)
         wifi->SetAutoConnect(auto_connect ? true : false);
-      if (wifi->connecting_or_connected()) {
-        // Show the config settings for the active network.
-        owner_->ShowTabbedNetworkSettings(wifi);
-      } else {
-        wifi->AttemptConnection(base::Bind(&NetworkMenu::DoConnect,
-                                           owner_,
-                                           wifi));
-      }
+      owner_->ConnectToNetwork(wifi);
     } else {
       // If we are attempting to connect to a network that no longer exists,
       // display a notification.
@@ -300,18 +299,7 @@ void NetworkMenuModel::ConnectToNetworkAt(int index,
     CellularNetwork* cellular = cros->FindCellularNetworkByPath(
         service_path);
     if (cellular) {
-      if ((cellular->activation_state() != ACTIVATION_STATE_ACTIVATED &&
-           cellular->activation_state() != ACTIVATION_STATE_UNKNOWN) ||
-          cellular->needs_new_plan()) {
-        ActivateCellular(cellular);
-      } else if (cellular->connecting_or_connected()) {
-        // Cellular network is connecting or connected,
-        // so we show the config settings for the cellular network.
-        owner_->ShowTabbedNetworkSettings(cellular);
-      } else {
-        // Clicked on a disconnected cellular network, so connect to it.
-        cros->ConnectToCellularNetwork(cellular);
-      }
+      owner_->ConnectToNetwork(cellular);
     } else {
       // If we are attempting to connect to a network that no longer exists,
       // display a notification.
@@ -328,22 +316,69 @@ void NetworkMenuModel::ConnectToNetworkAt(int index,
   } else if (flags & FLAG_VPN) {
     VirtualNetwork* vpn = cros->FindVirtualNetworkByPath(service_path);
     if (vpn) {
+      owner_->ConnectToNetwork(vpn);
+    } else {
+      // If we are attempting to connect to a network that no longer exists,
+      // display a notification.
+      LOG(WARNING) << "VPN does not exist to connect to: " << service_path;
+      // TODO(stevenjb): Show notification.
+    }
+  }
+}
+
+void NetworkMenu::ConnectToNetwork(Network* network) {
+  NetworkLibrary* cros = CrosLibrary::Get()->GetNetworkLibrary();
+  switch (network->type()) {
+    case TYPE_ETHERNET: {
+      ShowTabbedNetworkSettings(network);
+      break;
+    }
+    case TYPE_WIFI: {
+      WifiNetwork* wifi = static_cast<WifiNetwork*>(network);
+      if (wifi->connecting_or_connected()) {
+        ShowTabbedNetworkSettings(wifi);
+      } else {
+        wifi->AttemptConnection(base::Bind(&NetworkMenu::DoConnect,
+                                           weak_pointer_factory_.GetWeakPtr(),
+                                           wifi));
+      }
+      break;
+    }
+
+    case TYPE_CELLULAR: {
+      CellularNetwork* cell = static_cast<CellularNetwork*>(network);
+      if ((cell->activation_state() != ACTIVATION_STATE_ACTIVATED &&
+           cell->activation_state() != ACTIVATION_STATE_UNKNOWN) ||
+          cell->needs_new_plan()) {

[stevenjb, 2012/03/12 18:21:29]

As long as we're cleaning this up, can you wrap these checks into
CellularNetwork as NeedsActivation() ? This will help us with the refactoring
we're about to start. Thanks!

[sadrul, 2012/03/12 18:32:25]

Done.

+        ActivateCellular(cell);
+      } else if (cell->connecting_or_connected()) {
+        // Cellular network is connecting or connected,
+        // so we show the config settings for the cellular network.
+        ShowTabbedNetworkSettings(cell);
+      } else {
+        // Clicked on a disconnected cellular network, so connect to it.
+        cros->ConnectToCellularNetwork(cell);
+      }
+      break;
+    }
+
+    case TYPE_VPN: {
+      VirtualNetwork* vpn = static_cast<VirtualNetwork*>(network);
       // Connect or reconnect.
       if (vpn->connecting_or_connected()) {
         // Show the config settings for the connected network.
         if (cros->connected_network())
-          owner_->ShowTabbedNetworkSettings(cros->connected_network());
+          ShowTabbedNetworkSettings(cros->connected_network());

[stevenjb, 2012/03/12 18:21:29]

Can you put the cros->connected_network() bit into ShowTabbedNetworkSettings(),
i.e. have this just call ShowTabbedNetworkSettings(vpn), and have the logic to
show the conencted network in ShowTabbedNetworkSettings()? At some point we will
want to move that logic into the settings page since eventually we may want to
show properties for the VPN, not for the connected network.

[sadrul, 2012/03/12 18:32:25]

Done.

       } else {
         vpn->AttemptConnection(base::Bind(&NetworkMenu::DoConnect,
-                                          owner_,
+                                          weak_pointer_factory_.GetWeakPtr(),
                                           vpn));
       }
-    } else {
-      // If we are attempting to connect to a network that no longer exists,
-      // display a notification.
-      LOG(WARNING) << "VPN does not exist to connect to: " << service_path;
-      // TODO(stevenjb): Show notification.
+      break;
     }
+
+    default:
+      break;
   }
 }
 
@@ -451,9 +486,8 @@ void NetworkMenuModel::ActivatedAt(int index) {
   } else if (flags & FLAG_TOGGLE_OFFLINE) {
     cros->EnableOfflineMode(!cros->offline_mode());
   } else if (flags & FLAG_ETHERNET) {
-    if (cros->ethernet_connected()) {
+    if (cros->ethernet_connected())
       owner_->ShowTabbedNetworkSettings(cros->ethernet_network());
-    }
   } else if (flags & (FLAG_WIFI | FLAG_ADD_WIFI |
                       FLAG_CELLULAR | FLAG_ADD_CELLULAR |
                       FLAG_VPN | FLAG_ADD_VPN)) {
@@ -482,14 +516,6 @@ void NetworkMenuModel::ShowNetworkConfigView(NetworkConfigView* view) const {
   window->Show();
 }
 
-void NetworkMenuModel::ActivateCellular(const CellularNetwork* cellular) const {
-  DCHECK(cellular);
-  Browser* browser = BrowserList::GetLastActive();
-  if (!browser)
-    return;
-  browser->OpenMobilePlanTabAndActivate();
-}
-
 void NetworkMenuModel::ShowOther(ConnectionType type) const {
   if (type == TYPE_CELLULAR) {
     ChooseMobileNetworkDialog::ShowDialog(