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

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

The refactoring includes:
 * Move a function into anonymous namespace.
 * Add NetworkMenu::ConnectToNetwork, which Does The Right Thing when asked to
   connect to a specified network.

BUG=110130, 109480
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126185

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

Thanks for doing this, just a couple of additional things I'd like us to do here
if you don't mind.

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:352: cell->needs_new_plan()) {
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!

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:371:
ShowTabbedNetworkSettings(cros->connected_network());
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]

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:352: cell->needs_new_plan()) {
On 2012/03/12 18:21:29, stevenjb (chromium) wrote:
> 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!

Done.

http://codereview.chromium.org/9690021/diff/2001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:371:
ShowTabbedNetworkSettings(cros->connected_network());
On 2012/03/12 18:21:29, stevenjb (chromium) wrote:
> 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.

Done.

[stevenjb, 2012-03-12 18:44:55]

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:102: cell->needs_new_plan();
I was actually thinking this should a member of CellularNetwork in
network_library.h, but this is fine for now.

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:1074: network =
CrosLibrary::Get()->GetNetworkLibrary()->connected_network();
Need an early exit if |network| is NULL.

[sadrul, 2012-03-12 18:51:02]

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:102: cell->needs_new_plan();
On 2012/03/12 18:44:56, stevenjb (chromium) wrote:
> I was actually thinking this should a member of CellularNetwork in
> network_library.h, but this is fine for now.

That would make better sense. I have moved the code into network_library

http://codereview.chromium.org/9690021/diff/6001/chrome/browser/chromeos/stat...
chrome/browser/chromeos/status/network_menu.cc:1074: network =
CrosLibrary::Get()->GetNetworkLibrary()->connected_network();
On 2012/03/12 18:44:56, stevenjb (chromium) wrote:
> Need an early exit if |network| is NULL.

Done.

[stevenjb, 2012-03-12 18:55:52]

Thanks!
LGTM