Show a spinner UI for zero-touch enrollment

Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/dd7115ab179b8909ad4d41bfc53cfa6f16263052
Cr-Commit-Position: refs/heads/master@{#419291}

[kumarniranjan, 2016-09-16 18:09:05]

Description was changed from

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=manually inspect spinner
==========

to

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=manually inspect spinner
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[kumarniranjan, 2016-09-16 18:12:06]

Description was changed from

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=manually inspect spinner
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=manually inspect spinner
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[kumarniranjan, 2016-09-16 18:12:06]

kumarniranjan@google.com changed reviewers:
+ xiyuan@chromium.org

[kumarniranjan, 2016-09-16 18:12:48]

Description was changed from

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=manually inspect spinner
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[kumarniranjan, 2016-09-16 18:14:16]

kumarniranjan@google.com changed reviewers:
+ drustsmith@google.com, joth@chromium.org

[kumarniranjan, 2016-09-16 19:26:06]

kumarniranjan@google.com changed required reviewers:
+ xiyuan@chromium.org

[kumarniranjan, 2016-09-16 19:26:07]

I appreciate you taking the time to review this. Thank you!

[xiyuan, 2016-09-16 19:45:16]

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc:133:
IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, TestFlow) {
TestFlow -> EnrollmentSpinner

TestFlow is vague and not clear what the test case overs.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc:145:
LoginDisplayHost::default_host()->GetWebUILoginView()->GetWebContents());
Combine L143-145 to:

test::JSChecker checker(
    LoginDisplayHost::default_host()->GetWebUILoginView()->GetWebContents());

since JSChecker has a ctor that takes a WebContents*

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.css
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.css:26:
.oauth-enroll-state-working #oauth-enroll-step-working {
Can we add class="fit" to the throbber-notice and put this into L33-35 and use
the style there?

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html:12:
<throbber-notice id="oauth-enroll-working-card"
Get rid of attribute "id" since it is not use. If it is used, we need a better
name, e.g. oauth-enroll-working-spiner, since this is not a card.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.cc
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.cc:383:
builder->Add("oauthEnrollWorking", IDS_ENTERPRISE_ENROLLMENT_WORKING_MESSAGE);
nit: Move this before L 381.

[xiyuan, 2016-09-16 19:45:25]

xiyuan@chromium.org changed required reviewers:
- xiyuan@chromium.org

[kumarniranjan, 2016-09-16 20:53:40]

Hi Xiyuan,
I made changes according to your comments.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc:133:
IN_PROC_BROWSER_TEST_F(EnrollmentScreenTest, TestFlow) {
On 2016/09/16 19:45:15, xiyuan wrote:
> TestFlow -> EnrollmentSpinner
> 
> TestFlow is vague and not clear what the test case overs.

Done.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/enrollment/enrollment_screen_browsertest.cc:145:
LoginDisplayHost::default_host()->GetWebUILoginView()->GetWebContents());
On 2016/09/16 19:45:15, xiyuan wrote:
> Combine L143-145 to:
> 
> test::JSChecker checker(
>     LoginDisplayHost::default_host()->GetWebUILoginView()->GetWebContents());
> 
> since JSChecker has a ctor that takes a WebContents*

Done.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.css
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.css:26:
.oauth-enroll-state-working #oauth-enroll-step-working {
On 2016/09/16 19:45:15, xiyuan wrote:
> Can we add class="fit" to the throbber-notice and put this into L33-35 and use
> the style there?

Done.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html:12:
<throbber-notice id="oauth-enroll-working-card"
On 2016/09/16 19:45:15, xiyuan wrote:
> Get rid of attribute "id" since it is not use. If it is used, we need a better
> name, e.g. oauth-enroll-working-spiner, since this is not a card.

Done.

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.cc
(right):

https://codereview.chromium.org/2343183003/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.cc:383:
builder->Add("oauthEnrollWorking", IDS_ENTERPRISE_ENROLLMENT_WORKING_MESSAGE);
On 2016/09/16 19:45:15, xiyuan wrote:
> nit: Move this before L 381.

Done.

[xiyuan, 2016-09-16 20:54:03]

lgtm

[kumarniranjan, 2016-09-16 20:56:39]

The CQ bit was checked by kumarniranjan@google.com

[commit-bot: I haz the power, 2016-09-16 20:57:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 22:05:19]

Description was changed from

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-16 22:05:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-16 22:09:10]

Description was changed from

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Show a spinner UI for zero-touch enrollment

BUG=639044
TEST=browser test
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/dd7115ab179b8909ad4d41bfc53cfa6f16263052
Cr-Commit-Position: refs/heads/master@{#419291}
==========

[commit-bot: I haz the power, 2016-09-16 22:09:11]

Patchset 2 (id:??) landed as
https://crrev.com/dd7115ab179b8909ad4d41bfc53cfa6f16263052
Cr-Commit-Position: refs/heads/master@{#419291}