Update locally managed user creation flow according to mocks

Update locally managed user creation flow according to mocks

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

[ygorshenin1, 2013-04-12 12:49:34]

lgtm

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/chromeos/lo...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc
(right):

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc:75:
if (on_image_screen_) {
nit: you can omit curly braces here

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/chromeos/lo...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h
(right):

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:44:
// Called when manager is successfully authenticated against cryptohome, but
nit: insert empty line before #44

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/resources/c...
File
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js
(right):

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:219:
context_: {},
Could you please add comment for context_?

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:576:
var buttonNames = ['start',
optional: create string constants for 'start', 'prev', 'next', ...

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/resources/c...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:767:
recoverable) {
nit: incorrect spacing

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc
(right):

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc:260:
HandleCreateManagedUser(const base::ListValue* args) {
It's more common to move argument list on the individual line. The same is for
HandleManagerSelected() and ShowManagerInconsistentStateErrorPage().

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc:318:

nit: delete this line

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h
(right):

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h:38:
// Starts managed user creation flow, with manager identified by
nit: insert an empty line before #38 and #44

https://codereview.chromium.org/14028005/diff/2001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h:69:
void ShowErrorPage(string16 message, bool recoverable);
Why not to pass |message| by const ref?

[Denis Kuznetsov (DE-MUC), 2013-04-12 13:00:17]

Nikita for owner LGTM

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/chro...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h
(right):

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/chro...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:44:
// Called when manager is successfully authenticated against cryptohome, but
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> nit: insert empty line before #44

Done.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/reso...
File
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js
(right):

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:576:
var buttonNames = ['start',
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> optional: create string constants for 'start', 'prev', 'next', ... 
As they are derived from html element IDs I see no reason for it.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/reso...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:767:
recoverable) {
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> nit: incorrect spacing

Done.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc
(right):

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc:260:
HandleCreateManagedUser(const base::ListValue* args) {
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> It's more common to move argument list on the individual line. The same is for
> HandleManagerSelected() and ShowManagerInconsistentStateErrorPage().

Done for two, last one does will exceed 80 chars limit.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.cc:318:

On 2013/04/12 12:49:35, ygorshenin1 wrote:
> nit: delete this line

Done.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
File
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h
(right):

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h:38:
// Starts managed user creation flow, with manager identified by
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> nit: insert an empty line before #38 and #44

Done.

https://chromiumcodereview.appspot.com/14028005/diff/2001/chrome/browser/ui/w...
chrome/browser/ui/webui/chromeos/login/locally_managed_user_creation_screen_handler.h:69:
void ShowErrorPage(string16 message, bool recoverable);
On 2013/04/12 12:49:35, ygorshenin1 wrote:
> Why not to pass |message| by const ref?

Done.

[Nikita (slow), 2013-04-12 13:12:15]

lgtm

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/chr...
File chrome/browser/chromeos/login/managed/locally_managed_user_creation_flow.cc
(right):

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/chr...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_flow.cc:27:
const std::string& manager_id) : ExtendedUserFlow(manager_id),
nit: Please move this to next line and change alignment i.e. move to the
beginning of the line.

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/chr...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h
(right):

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/chr...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:46:
// OAUTH token validation have not comleted yet.
nit: hasn't

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/res...
File
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.html
(right):

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/res...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.html:59:
<div class = "screen-horizontal-splitter"></div>
nit: Remove extra spaces.

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/res...
File
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js
(right):

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/res...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:343:

nit: Drop empty line.

https://chromiumcodereview.appspot.com/14028005/diff/10001/chrome/browser/res...
chrome/browser/resources/chromeos/login/screen_locally_managed_user_creation.js:554:
var screenNames = ['intro',
nit: These should be really renamed in the upcoming 2 weeks.
They are called here screens and pages.

[Denis Kuznetsov (DE-MUC), 2013-04-15 11:15:07]

Done

[Denis Kuznetsov (DE-MUC), 2013-04-16 16:24:56]

Committed patchset #5 manually as r194368 (presubmit successful).