Implement action interface in network_menu.js for 'connect'.

Implement action interface in network_menu.js for 'connect'.
* Added NetworkMenu::UpdateMenu()
* Added NetworkMenu::ConnectToNetworkAt()

BUG=http://code.google.com/p/chromium-os/issues/detail?id=7343 
TEST=Test the network menu (it still looks ugly). Specifically, selecting a non secure network, or one with a certificate installed should connect to it. Also, temporarily, using 'space' or 'return' to select a network should connect to it without closing the menu.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62598

[stevenjb, 2010-10-12 20:58:35]

Ready for review.

http://codereview.chromium.org/3678006/diff/1/2
File chrome/browser/chromeos/cros/network_library.cc (right):

http://codereview.chromium.org/3678006/diff/1/2#newcode397
chrome/browser/chromeos/cros/network_library.cc:397: // device. See
src/third_party/flimflam/files/doc/service-api.txt.
Note: This function was moved from wifi_config_view.cc (unchanged except for
using .cert_path_ instead of taking a path argument)

http://codereview.chromium.org/3678006/diff/1/5
File chrome/browser/chromeos/options/wifi_config_view.cc (left):

http://codereview.chromium.org/3678006/diff/1/5#oldcode201
chrome/browser/chromeos/options/wifi_config_view.cc:201: // device. See
src/third_party/flimflam/files/doc/service-api.txt.
Note: moved to network_library.cc

http://codereview.chromium.org/3678006/diff/1/6
File chrome/browser/chromeos/status/network_menu.cc (left):

http://codereview.chromium.org/3678006/diff/1/6#oldcode181
chrome/browser/chromeos/status/network_menu.cc:181: void
NetworkMenu::ShowTabbedNetworkSettings(const Network& network) {
NOTE: moved to bottom of file to match header.

http://codereview.chromium.org/3678006/diff/1/6#oldcode381
chrome/browser/chromeos/status/network_menu.cc:381:
network_menu_->UpdateStates();
The call to UpdateStates is unnecessary (Menu2 invokes UpdateStates before it
shows the menu)

http://codereview.chromium.org/3678006/diff/1/6
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3678006/diff/1/6#newcode504
chrome/browser/chromeos/status/network_menu.cc:504: void
NetworkMenu::ShowTabbedNetworkSettings(const Network& network) {
Moved from above to match header file.

http://codereview.chromium.org/3678006/diff/1/9
File chrome/browser/resources/network_menu.js (right):

http://codereview.chromium.org/3678006/diff/1/9#newcode82
chrome/browser/resources/network_menu.js:82: // Temporary, for testing
sendConnect()
Note: temporary hack for testing, although personally I think it's helpful to
have a way to see the status description while connecting to the network, so may
want to keep this, maybe assigned to a different key, like 'c'?

[xiyuan, 2010-10-12 21:06:04]

LGTM

http://codereview.chromium.org/3678006/diff/1/9
File chrome/browser/resources/network_menu.js (right):

http://codereview.chromium.org/3678006/diff/1/9#newcode9
chrome/browser/resources/network_menu.js:9: chrome.send('action', [ 'connect',
String(index), passphrase, identity ]);
nit: index + '' would convert it to a string.

[arv (Not doing code reviews), 2010-10-12 21:29:25]

FYI

http://codereview.chromium.org/3678006/diff/1/9
File chrome/browser/resources/network_menu.js (right):

http://codereview.chromium.org/3678006/diff/1/9#newcode9
chrome/browser/resources/network_menu.js:9: chrome.send('action', [ 'connect',
String(index), passphrase, identity ]);
On 2010/10/12 21:06:09, xiyuan wrote:
> nit: index + '' would convert it to a string.

String(expr) is the preferred way.

1. It is more readable
2. Less string allocations

[xiyuan, 2010-10-12 21:37:16]

http://codereview.chromium.org/3678006/diff/1/9
File chrome/browser/resources/network_menu.js (right):

http://codereview.chromium.org/3678006/diff/1/9#newcode9
chrome/browser/resources/network_menu.js:9: chrome.send('action', [ 'connect',
String(index), passphrase, identity ]);
On 2010/10/12 21:29:26, arv wrote:
> On 2010/10/12 21:06:09, xiyuan wrote:
> > nit: index + '' would convert it to a string.
> 
> String(expr) is the preferred way.
> 
> 1. It is more readable
> 2. Less string allocations

Good to know. Thanks.

[stevenjb, 2010-10-13 22:22:40]

I had to make some changes to fix browser_tests. Could someone please take a
look at the diffs from the first patch? Thanks.

[stevenjb, 2010-10-14 00:21:42]

Dave, can you review the changes I made to login_browsertest.cc? It wasn't using
CrosInProcessBrowserTest, which caused it to regress when the network library
expectations changed for the status area.

[Charlie Lee, 2010-10-14 04:22:48]

LGTM with a nit

http://codereview.chromium.org/3678006/diff/14001/15009
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3678006/diff/14001/15009#newcode241
chrome/browser/chromeos/status/network_menu.cc:241: ShowNetworkConfigView(new
NetworkConfigView(), focus_login);
any reason to still have focus_login here? should just pass in true.

http://codereview.chromium.org/3678006/diff/14001/15009#newcode280
chrome/browser/chromeos/status/network_menu.cc:280: ShowNetworkConfigView(new
NetworkConfigView(wifi, true), focus_login);
same comment here.

[stevenjb, 2010-10-14 17:43:56]

Addressed charlie's feedback, + one small change caught by xiyuan wrt the status
of non connected networks.

http://codereview.chromium.org/3678006/diff/14001/15009
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3678006/diff/14001/15009#newcode241
chrome/browser/chromeos/status/network_menu.cc:241: ShowNetworkConfigView(new
NetworkConfigView(), focus_login);
On 2010/10/14 04:22:48, Charlie Lee wrote:
> any reason to still have focus_login here? should just pass in true.
This is intentional to document the meaning of "true" in the second argument, as
described here:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Implementation...
I will change this to kFocusLogin = true; to better match the style guide.

[Charlie Lee, 2010-10-15 06:34:29]

LGTM

http://codereview.chromium.org/3678006/diff/20001/21009
File chrome/browser/chromeos/status/network_menu.cc (right):

http://codereview.chromium.org/3678006/diff/20001/21009#newcode259
chrome/browser/chromeos/status/network_menu.cc:259: ShowNetworkConfigView(
while you are at it, add the const here also

http://codereview.chromium.org/3678006/diff/20001/21009#newcode278
chrome/browser/chromeos/status/network_menu.cc:278: ShowNetworkConfigView(new
NetworkConfigView(wifi, false), false);
and here