Added shortcut for NW config screen in kiosk.

Added a shortcut (Ctrl+Alt+N) for showing network config screen in during kiosk launch even if network connectivity is present. 

BUG=350065
TEST=added KioskTest.LaunchAppWithNetworkConfigAccelerator

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

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

[xiyuan, 2014-03-25 05:21:19]

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/kiosk_browsertest.cc (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/kiosk_browsertest.cc:592: // Mock network could be
configured with owner's password.
nit: The comments need to be updated. The 2nd arg of ScopedCanConfigureNetwork
ctor below is false, which means no owner password is needed.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/screen_error_message.html (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/screen_error_message.html:23: <span
id="show-with-error-state-kiosk-online"
"id" not used and its value looks more like a class name.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/screen_error_message.js (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/screen_error_message.js:201:
rebootButton.classList.add('show-with-error-state-kiosk-online');
Do we need this line? When error screen shows up during kiosk launch, it should
have "ui-state-kiosk-mode" class and 'show-with-ui-state-kiosk-mode" above
should make the button visible.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc
(right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc:215:
ShowNetworkConfigureUI();
This would by-pass the owner auth check or policy check. Should we only do this
after ShowNetworkConfigureUI() has been called from AppLaunchController, i.e.
when current screen is the error screen already?

And we probably still need to call delegate_->OnNetworkStateChanged when network
config is NOT requested.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc:279:
LOG(WARNING) << "No delegate set to handle cancel app launch";
The message needs to be updated.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/error_screen_handler.cc:240:
builder->Add("continueButton", IDS_NETWORK_SELECTION_CONTINUE_BUTTON);
nit: Suggest to use a different key so that if we later want to change the
string, we won't be surprised that "continueButton" in NetworkScreenHandler gets
changed too.

[zel, 2014-03-25 22:26:32]

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/kiosk_browsertest.cc (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/kiosk_browsertest.cc:592: // Mock network could be
configured with owner's password.
On 2014/03/25 05:21:19, xiyuan wrote:
> nit: The comments need to be updated. The 2nd arg of ScopedCanConfigureNetwork
> ctor below is false, which means no owner password is needed.

Done.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/screen_error_message.html (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/screen_error_message.html:23: <span
id="show-with-error-state-kiosk-online"
On 2014/03/25 05:21:19, xiyuan wrote:
> "id" not used and its value looks more like a class name. 

Done.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/screen_error_message.js (right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/screen_error_message.js:201:
rebootButton.classList.add('show-with-error-state-kiosk-online');
On 2014/03/25 05:21:19, xiyuan wrote:
> Do we need this line? When error screen shows up during kiosk launch, it
should
> have "ui-state-kiosk-mode" class and 'show-with-ui-state-kiosk-mode" above
> should make the button visible.

Done.

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc
(right):

https://codereview.chromium.org/209143002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc:215:
ShowNetworkConfigureUI();
On 2014/03/25 05:21:19, xiyuan wrote:
> This would by-pass the owner auth check or policy check. Should we only do
this
> after ShowNetworkConfigureUI() has been called from AppLaunchController, i.e.
> when current screen is the error screen already?
> 
> And we probably still need to call delegate_->OnNetworkStateChanged when
network
> config is NOT requested.

Done.

[xiyuan, 2014-03-26 16:24:16]

LGTM

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:54: const int
kRerunDelay = 2;
no longer needed.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:153: // If installer is
still running in the background, delay the restart.
nit: Update the comment to clarify that start up continues after the current
installer is finished.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:338: if
(!delegate_->IsShowingNetworkConfigScreen())
This "if" seem unnecessary.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/login/app_launch_controller.cc:221: LOG(ERROR) <<
"OnNetworkConfigRequested: " << requested;
nit: Do we still need the LOG here and around line 230? If we do, maybe change
it to LOG(WARNING) or VLOG?

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/login/app_launch_controller.cc:431: 
nit: nuke one empty line

[Nikita (slow), 2014-03-26 16:35:52]

+Yuri FYI

[zel, 2014-03-26 17:22:38]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-26 17:25:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/170001

[zel, 2014-03-26 17:30:57]

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/app_mode/startup_app_launcher.cc (right):

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:54: const int
kRerunDelay = 2;
On 2014/03/26 16:24:16, xiyuan wrote:
> no longer needed.

Done.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:153: // If installer is
still running in the background, delay the restart.
On 2014/03/26 16:24:16, xiyuan wrote:
> nit: Update the comment to clarify that start up continues after the current
> installer is finished.

Done.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/app_mode/startup_app_launcher.cc:338: if
(!delegate_->IsShowingNetworkConfigScreen())
On 2014/03/26 16:24:16, xiyuan wrote:
> This "if" seem unnecessary.

Done.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/app_launch_controller.cc (right):

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/login/app_launch_controller.cc:221: LOG(ERROR) <<
"OnNetworkConfigRequested: " << requested;
On 2014/03/26 16:24:16, xiyuan wrote:
> nit: Do we still need the LOG here and around line 230? If we do, maybe change
> it to LOG(WARNING) or VLOG?

Done.

https://codereview.chromium.org/209143002/diff/150001/chrome/browser/chromeos...
chrome/browser/chromeos/login/app_launch_controller.cc:431: 
On 2014/03/26 16:24:16, xiyuan wrote:
> nit: nuke one empty line

Done.

[commit-bot: I haz the power, 2014-03-26 20:51:25]

Change committed as 259679

[erikchen, 2014-03-26 22:17:07]

A revert of this CL has been created in
https://codereview.chromium.org/213253007/ by erikchen@chromium.org.

The reason for reverting is: The newly added test
LaunchAppWithNetworkConfigAccelerator fails.

http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2....

[zel, 2014-03-27 22:51:39]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:52:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/190001

[commit-bot: I haz the power, 2014-03-28 01:01:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 01:01:44]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[zel, 2014-03-28 01:13:32]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-28 01:13:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/190001

[commit-bot: I haz the power, 2014-03-28 02:51:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:51:47]

Retried try job too often on ios_dbg_simulator for step(s) components_unittests,
crypto_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[zel, 2014-03-28 02:52:27]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:54:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/190001

[commit-bot: I haz the power, 2014-03-28 03:16:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 03:16:19]

Retried try job too often on ios_dbg_simulator for step(s) crypto_unittests,
url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[zel, 2014-03-28 03:19:02]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-28 03:19:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/190001

[commit-bot: I haz the power, 2014-03-28 04:11:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 04:11:14]

Retried try job too often on ios_dbg_simulator for step(s) components_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[zel, 2014-03-28 14:17:08]

The CQ bit was checked by zelidrag@chromium.org

[commit-bot: I haz the power, 2014-03-28 14:18:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zelidrag@chromium.org/209143002/190001

[commit-bot: I haz the power, 2014-03-28 16:31:33]

Change committed as 260158