Initial implementation of network screen WebUI handler.

Initial implementation of network screen WebUI handler.

R=altimofeev@chromium.org
BUG=chromium-os:15923
TEST=Manual

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

[altimofeev, 2011-06-07 17:02:22]

LGTM with nits

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/network_screen_browsertest.cc:119: actor_ =
static_cast<ViewsNetworkScreenActor*>(network_screen_->actor());
Comment on this please. Also consider adding TODO.

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.h (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.h:32: virtual void
ShowConnectingStatus(
nit: you can place it using only 2 lines

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:132:
l10n_util::GetStringUTF16(IDS_NETWORK_SELECTION_SELECT));
duplication (you have already set it in NetworkScreenActor)

[whywhat, 2011-06-07 17:24:51]

LGTM

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/network_screen_browsertest.cc (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/network_screen_browsertest.cc:119: actor_ =
static_cast<ViewsNetworkScreenActor*>(network_screen_->actor());
On 2011/06/07 17:02:22, altimofeev wrote:
> Comment on this please. Also consider adding TODO.

Done.

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.h (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.h:32: virtual void
ShowConnectingStatus(
On 2011/06/07 17:02:22, altimofeev wrote:
> nit: you can place it using only 2 lines

Done.

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right):

http://codereview.chromium.org/7121013/diff/1010/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:132:
l10n_util::GetStringUTF16(IDS_NETWORK_SELECTION_SELECT));
On 2011/06/07 17:02:22, altimofeev wrote:
> duplication (you have already set it in NetworkScreenActor)

Done.

[Nikita (slow), 2011-06-08 08:21:22]

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:31:
web_ui_->CallJavascriptFunction("toggleStep", *value);
cr.ui.Oobe.toggleStep

Same prefix for other functions.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:39:
web_ui_->CallJavascriptFunction("showError", *message_value);
I think it makes sense to comment these calls like
showError, clearErrors, showConnectingStatus till they're enabled in JS.

Some of them like showError/showConnectingStatus don't even have proper mocks.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:68:
localized_strings->SetString("networkScreenTitle",
You don't need this string.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:80: //
TODO(avayvod): Initialize languages, keyboards and networks lists.
I guess these lists will be initialized in Initialize instead.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right):

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:120:
localized_strings->SetString("welcomeScreenTitle",
Move this string to network screen handler.

[whywhat, 2011-06-08 08:29:56]

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc (right):

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:31:
web_ui_->CallJavascriptFunction("toggleStep", *value);
On 2011/06/08 08:21:23, Nikita Kostylev wrote:
> cr.ui.Oobe.toggleStep
> 
> Same prefix for other functions.

Done.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:39:
web_ui_->CallJavascriptFunction("showError", *message_value);
On 2011/06/08 08:21:23, Nikita Kostylev wrote:
> I think it makes sense to comment these calls like
> showError, clearErrors, showConnectingStatus till they're enabled in JS.
> 
> Some of them like showError/showConnectingStatus don't even have proper mocks.

Done.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:68:
localized_strings->SetString("networkScreenTitle",
On 2011/06/08 08:21:23, Nikita Kostylev wrote:
> You don't need this string.

Done.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/network_screen_handler.cc:80: //
TODO(avayvod): Initialize languages, keyboards and networks lists.
On 2011/06/08 08:21:23, Nikita Kostylev wrote:
> I guess these lists will be initialized in Initialize instead.

Done.

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/oobe_ui.cc (right):

http://codereview.chromium.org/7121013/diff/2003/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/oobe_ui.cc:120:
localized_strings->SetString("welcomeScreenTitle",
On 2011/06/08 08:21:23, Nikita Kostylev wrote:
> Move this string to network screen handler.

Done.

[Nikita (slow), 2011-06-08 08:39:22]

LGTM