WifiConfigView - support networks with 802.1x authentication by...

WifiConfigView - support networks with 802.1x authentication by prompting the user to enter an ID, select a certificate/private key file, and enter a password for the key file, if any.
Pass these values down the stack into ConnectToWifiNetwork().

TEST=Manual - select certificate-protected network, enter ID, cert, and passphrase.
BUG=none

[Charlie Lee, 2010-04-20 16:23:01]

http://codereview.chromium.org/1676002/diff/10001/11005
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

http://codereview.chromium.org/1676002/diff/10001/11005#newcode85
chrome/browser/chromeos/options/wifi_config_view.cc:85: const std::wstring
dialog_title =
Where is this used?

http://codereview.chromium.org/1676002/diff/10001/11005#newcode91
chrome/browser/chromeos/options/wifi_config_view.cc:91: }
maybe put a NOTREACHED() in an else clause

http://codereview.chromium.org/1676002/diff/10001/11005#newcode98
chrome/browser/chromeos/options/wifi_config_view.cc:98: UpdateCanLogin();  //
Could we check if the passphrase decrypts the key?
Turn this into a TODO item if you plan to followup on it.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode105
chrome/browser/chromeos/options/wifi_config_view.cc:105: identity_string =
string16();
How about something like this so it's simple and clean?

string16 identity_string, certificate_path_string;
if (identity_textfield_ != NULL) {
  identity_string = identity_textfield_->text();
  certificate_path_string = ...
}

http://codereview.chromium.org/1676002/diff/10001/11005#newcode110
chrome/browser/chromeos/options/wifi_config_view.cc:110:
CrosLibrary::Get()->GetNetworkLibrary()->ConnectToWifiNetwork(
need 4 spaces here.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode214
chrome/browser/chromeos/options/wifi_config_view.cc:214: passphrase_textfield_ =
new views::Textfield(
This looks like the same code as above. Maybe pull them both out of the if/else
clause?

[Nathan Williams, 2010-04-20 20:46:20]

Patch updated.

http://codereview.chromium.org/1676002/diff/10001/11005
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

http://codereview.chromium.org/1676002/diff/10001/11005#newcode85
chrome/browser/chromeos/options/wifi_config_view.cc:85: const std::wstring
dialog_title =
On 2010/04/20 16:23:01, chocobo wrote:
> Where is this used?

Oops, it isn't - leftover from the code this was following. The "Open File"
dialog is OK as-is. Removed.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode91
chrome/browser/chromeos/options/wifi_config_view.cc:91: }
On 2010/04/20 16:23:01, chocobo wrote:
> maybe put a NOTREACHED() in an else clause

OK.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode98
chrome/browser/chromeos/options/wifi_config_view.cc:98: UpdateCanLogin();  //
Could we check if the passphrase decrypts the key?
On 2010/04/20 16:23:01, chocobo wrote:
> Turn this into a TODO item if you plan to followup on it.

OK.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode105
chrome/browser/chromeos/options/wifi_config_view.cc:105: identity_string =
string16();
On 2010/04/20 16:23:01, chocobo wrote:
> How about something like this so it's simple and clean?
> 
> string16 identity_string, certificate_path_string;
> if (identity_textfield_ != NULL) {
>   identity_string = identity_textfield_->text();
>   certificate_path_string = ...
> }

OK.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode110
chrome/browser/chromeos/options/wifi_config_view.cc:110:
CrosLibrary::Get()->GetNetworkLibrary()->ConnectToWifiNetwork(
On 2010/04/20 16:23:01, chocobo wrote:
> need 4 spaces here.

Fixed.

http://codereview.chromium.org/1676002/diff/10001/11005#newcode214
chrome/browser/chromeos/options/wifi_config_view.cc:214: passphrase_textfield_ =
new views::Textfield(
On 2010/04/20 16:23:01, chocobo wrote:
> This looks like the same code as above. Maybe pull them both out of the
if/else
> clause?

I've reworked this; the code can't be just pulled out because there's the
no-password case as well.

[Charlie Lee, 2010-04-20 21:00:48]

LGTM

I will ping you when cros_deps is updated.