Represent kiosk apps as user pods instead of menu items.

Represent kiosk apps as user pods instead of menu items.

Add a custom user pod icon for such apps.
Enable this UI by default.
Provide --disable-new-kiosk-ui flag which reverts back to the original UI (menu).

BUG=334304, 341884
R=xiyuan@chromium.org
TBR=oshima

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

[Nikita (slow), 2014-02-10 19:22:40]

I still need to reimplement AppsMenuButton.runAppForTesting(...) API so that
browser test works but I'll work on that tomorrow.
I think CL is somewhat ready for review as a concept.

[xiyuan, 2014-02-10 21:15:55]

Mostly good.

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
chrome/app/generated_resources.grd:14163: +          Launch App
nit: "Launch app" since it's in not title_case section.

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1208: *         found.
nit: should 4-space indent from @ at previous line

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1537: *         
podToFocus is already focused.
nit: 4-space indent from @

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1596: *         
podToFocus is already focused.
nit: 4-space indent from @

[Nikita (slow), 2014-02-12 17:19:24]

Ready for another round of review.

I'm still investigating why part of KioskTest.* is not working with new UI but I
expect diff will be quite small once I figure out the root cause.

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
chrome/app/generated_resources.grd:14163: +          Launch App
On 2014/02/10 21:15:56, xiyuan wrote:
> nit: "Launch app" since it's in not title_case section.

Done.
Do you know how this is used?

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1208: *         found.
On 2014/02/10 21:15:56, xiyuan wrote:
> nit: should 4-space indent from @ at previous line

Done.

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1537: *         
podToFocus is already focused.
On 2014/02/10 21:15:56, xiyuan wrote:
> nit: 4-space indent from @

Done.

https://codereview.chromium.org/158833003/diff/30001/chrome/browser/resources...
chrome/browser/resources/chromeos/login/user_pod_row.js:1596: *         
podToFocus is already focused.
On 2014/02/10 21:15:56, xiyuan wrote:
> nit: 4-space indent from @

Done.

[xiyuan, 2014-02-12 17:35:03]

LGTM

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/158833003/diff/30001/chrome/app/generated_res...
chrome/app/generated_resources.grd:14163: +          Launch App
On 2014/02/12 17:19:25, Nikita Kostylev wrote:
> On 2014/02/10 21:15:56, xiyuan wrote:
> > nit: "Launch app" since it's in not title_case section.
> 
> Done.
> Do you know how this is used?

It is used in common.gypi and true for gtk, mac and ios.
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...

Probably does not makes sense for chromeos but since we share code with desktop
user manager now, it is better to follow the convention.

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/user_pod_row.js:85: // main input field
gets focus and tab index 1;
nit: why remove the indent?

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/user_pod_row.js:1299:
activationEvent.originalEvent = fakeCtrlEnterEvent;
This is different from the expectation at line 1092 of KioskAppPod.activate,
where it expects the ctrlKey is on for the event itself. Might be the cause of
kiosk test failure.

[Nikita (slow), 2014-02-13 08:58:01]

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
File chrome/browser/resources/chromeos/login/user_pod_row.js (right):

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/user_pod_row.js:85: // main input field
gets focus and tab index 1;
On 2014/02/12 17:35:04, xiyuan wrote:
> nit: why remove the indent?

Done.

https://codereview.chromium.org/158833003/diff/210001/chrome/browser/resource...
chrome/browser/resources/chromeos/login/user_pod_row.js:1299:
activationEvent.originalEvent = fakeCtrlEnterEvent;
On 2014/02/12 17:35:04, xiyuan wrote:
> This is different from the expectation at line 1092 of KioskAppPod.activate,
> where it expects the ctrlKey is on for the event itself. Might be the cause of
> kiosk test failure.

Done. Yes, this one plus another JS error.

[xiyuan, 2014-02-13 15:34:18]

SLGTM

[Nikita (slow), 2014-02-14 10:37:44]

TBR'ing oshima for new image asset.

Xiyuan, I've had to change how we send list of apps.
What I've seen in the debugger on the startup KioskAppManager notifies
KioskAppMenuHandler that app state has changed (new app has been loaded) and JS
call sendApps is executed.

But at that time WebUI is not ready yet.

[Nikita (slow), 2014-02-14 11:14:16]

The CQ bit was checked by nkostylev@chromium.org

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nkostylev@chromium.org/158833003/840001

[commit-bot: I haz the power, 2014-02-14 14:21:21]

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

[commit-bot: I haz the power, 2014-02-14 14:21:22]

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

[Nikita (slow), 2014-02-14 14:57:37]

Committed patchset #8 manually as r251315 (presubmit successful).

[xiyuan, 2014-02-14 16:57:38]

On 2014/02/14 10:37:44, Nikita Kostylev wrote:
> TBR'ing oshima for new image asset.
> 
> Xiyuan, I've had to change how we send list of apps.
> What I've seen in the debugger on the startup KioskAppManager notifies
> KioskAppMenuHandler that app state has changed (new app has been loaded) and
JS
> call sendApps is executed.
> 
> But at that time WebUI is not ready yet.

We used to have that and gone in https://codereview.chromium.org/130613004/. I
forgot about this when I reviewed the CL. Thanks for fixing it.