ChromeOS: Implement Network Selection screen of material design OOBE.

ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/78696be690cf522455cf571c5c19848837c66791
Cr-Commit-Position: refs/heads/master@{#404955}

[Alexander Alekseev, 2016-06-15 04:29:56]

Description was changed from

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
==========

to

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Alexander Alekseev, 2016-06-15 04:30:52]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-15 04:31:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067153002/1

[Alexander Alekseev, 2016-06-15 04:33:07]

alemate@chromium.org changed reviewers:
+ asargent@chromium.org, stevenjb@chromium.org

[Alexander Alekseev, 2016-06-15 04:33:08]

Please review:

asargent@:
  extensions/common/api/_api_features.json

stevenjb@:
  all

[commit-bot: I haz the power, 2016-06-15 04:46:08]

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

[commit-bot: I haz the power, 2016-06-15 04:46:09]

Dry run: Try jobs failed on following builders:
  closure_compilation on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Alexander Alekseev, 2016-06-15 09:26:43]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-15 09:26:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067153002/20001

[commit-bot: I haz the power, 2016-06-15 09:38:07]

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

[commit-bot: I haz the power, 2016-06-15 09:38:09]

Dry run: Try jobs failed on following builders:
  closure_compilation on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[stevenjb, 2016-06-15 16:52:16]

On 2016/06/15 09:38:09, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   closure_compilation on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

Let's extract the ui/webui/resources/cr_elements/network changes into a separate
CL.

We are no longer including all of iron-icons in any web UI (it's very slow to
load). I'm not sure what the best way to do this will be. Can you add a link to
the mocks in the tracking issue?

[asargent_no_longer_on_chrome, 2016-06-15 20:46:42]

extensions/common/api/_api_features.json lgtm

[Alexander Alekseev, 2016-06-22 09:05:37]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-22 09:05:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067153002/60001

[commit-bot: I haz the power, 2016-06-22 09:14:55]

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

[commit-bot: I haz the power, 2016-06-22 09:14:56]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2016-06-30 03:44:31]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 03:44:44]

Dry run: 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-06-30 03:52:52]

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

[commit-bot: I haz the power, 2016-06-30 03:52:53]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Alexander Alekseev, 2016-06-30 04:38:41]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 04:38:53]

Dry run: 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-06-30 05:12:30]

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

[commit-bot: I haz the power, 2016-06-30 05:12:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Alexander Alekseev, 2016-07-07 04:54:23]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-07 04:54:38]

Dry run: 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-07-07 05:29:25]

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

[commit-bot: I haz the power, 2016-07-07 05:29:26]

Dry run: This issue passed the CQ dry run.

[Alexander Alekseev, 2016-07-07 05:35:01]

I modified polymer_config.js so that it won't complain if it was loaded before,
because we still need to load polymer from different files.

I also added additional error reporting to web_ui_browser_test.cc to make error
messages appear in test output just before "Encountered javascript console
error" message, because I spent quite a time to find out the source of test
failure.

Please review.

[stevenjb, 2016-07-07 16:58:41]

https://codereview.chromium.org/2067153002/diff/280001/ui/webui/resources/js/...
File ui/webui/resources/js/polymer_config.js (right):

https://codereview.chromium.org/2067153002/diff/280001/ui/webui/resources/js/...
ui/webui/resources/js/polymer_config.js:8: isChromePolymerInitialized = true;
Make this change in a separate CL. Add dbeam@ and michaelpg@ as reviewers.

[stevenjb, 2016-07-07 17:36:17]

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_dialog.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_dialog.html:14: <iron-icon ....
class="oobe-icon">
nit: 3 dots

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.html:44: <oobe-dialog
id="networkSection" class="fit" hidden>
Consider using a tamplate if here:

<template is="dom-if" if="[[showDialog_]]">

That way the dialog won't be loaded initially, improving initial load time. This
is probably less of an issue for OOBE than for Settings, but it is still a good
practice.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:30: * Set proper focus.
Sets

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:37: * Handle "visible"
event.
Handles (here and below)

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:38: *
nit: Empty line before @private is not needed.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:51:
this.$.networkSelect.maxHeight = 280;
Instead of setting maxHeight to a specific value here, toggle a class and put
the value in CSS. (Or if you can just use #networkSelect[showActive=false] in
the CSS).

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:77: var self = this;
?

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:120: * This is called on
every network that has "Connected" state.
This gets called when a network enters the 'Connected' state.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:155: var
networkStateCopy = JSON.parse(JSON.stringify(state));
var networkStateCopy = Object.assign({}, state);

[Alexander Alekseev, 2016-07-07 22:45:06]

alemate@chromium.org changed reviewers:
+ dbeam@chromium.org

[Alexander Alekseev, 2016-07-07 22:45:07]

+dbeam

On 2016/07/07 22:39:08, Dan Beam wrote:
> not lgtm
> 
> you shouldn't need to do this and it circumvents the console.error() written
> basically to prevent this
> 
> how are you loading polymer_config.js?  from somewhere outside of
polymer.html?

After I added import line to oobe.html here:

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...

I was still getting console error message from polymer_config. I believe that my
change implies that this file is loaded first. So it looks like other imports of
this file conflict with each other.

Probably my assumptions are wrong somewhere, but where am I wrong?

[Dan Beam, 2016-07-07 23:09:32]

On 2016/07/07 22:45:07, Alexander Alekseev wrote:
> +dbeam
> 
> On 2016/07/07 22:39:08, Dan Beam wrote:
> > not lgtm
> > 
> > you shouldn't need to do this and it circumvents the console.error() written
> > basically to prevent this
> > 
> > how are you loading polymer_config.js?  from somewhere outside of
> polymer.html?
> 
> After I added import line to oobe.html here:
> 
>
https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
> 
> I was still getting console error message from polymer_config. I believe that
my
> change implies that this file is loaded first. So it looks like other imports
of
> this file conflict with each other.
> 
> Probably my assumptions are wrong somewhere, but where am I wrong?

https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/...

[Alexander Alekseev, 2016-07-08 01:12:15]

On 2016/07/07 23:09:32, Dan Beam wrote:
> On 2016/07/07 22:45:07, Alexander Alekseev wrote:
> > +dbeam
> > 
> > On 2016/07/07 22:39:08, Dan Beam wrote:
> > > not lgtm
> > > 
> > > you shouldn't need to do this and it circumvents the console.error()
written
> > > basically to prevent this
> > > 
> > > how are you loading polymer_config.js?  from somewhere outside of
> > polymer.html?
> > 
> > After I added import line to oobe.html here:
> > 
> >
>
https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
> > 
> > I was still getting console error message from polymer_config. I believe
that
> my
> > change implies that this file is loaded first. So it looks like other
imports
> of
> > this file conflict with each other.
> > 
> > Probably my assumptions are wrong somewhere, but where am I wrong?
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/...

Yes, removing this line fixed the console error. Thank you!

[Alexander Alekseev, 2016-07-08 01:12:29]

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_dialog.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_dialog.html:14: <iron-icon ....
class="oobe-icon">
On 2016/07/07 17:36:17, stevenjb wrote:
> nit: 3 dots

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.html:44: <oobe-dialog
id="networkSection" class="fit" hidden>
On 2016/07/07 17:36:17, stevenjb wrote:
> Consider using a tamplate if here:
> 
> <template is="dom-if" if="[[showDialog_]]">
> 
> That way the dialog won't be loaded initially, improving initial load time.
This
> is probably less of an issue for OOBE than for Settings, but it is still a
good
> practice.

This conflicts with onWelcomeNextButtonClicked_ method, because
this.$.networkSelect.customItems = ... will trigger "Uncaught TypeError: Cannot
set property 'customItems' of undefined" .

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:30: * Set proper focus.
On 2016/07/07 17:36:17, stevenjb wrote:
> Sets

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:37: * Handle "visible"
event.
On 2016/07/07 17:36:17, stevenjb wrote:
> Handles (here and below)

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:38: *
On 2016/07/07 17:36:17, stevenjb wrote:
> nit: Empty line before @private is not needed.

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:51:
this.$.networkSelect.maxHeight = 280;
On 2016/07/07 17:36:17, stevenjb wrote:
> Instead of setting maxHeight to a specific value here, toggle a class and put
> the value in CSS. (Or if you can just use #networkSelect[showActive=false] in
> the CSS).

maxHeight is not CSS attribute here, and setting it in HTML (or in CSS as
standard CSS property) doesn't work. (It probably gets overwritten by smth.)

So I don't know why I have to use this here.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:77: var self = this;
On 2016/07/07 17:36:17, stevenjb wrote:
> ?

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:120: * This is called on
every network that has "Connected" state.
On 2016/07/07 17:36:17, stevenjb wrote:
> This gets called when a network enters the 'Connected' state.

Done.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:155: var
networkStateCopy = JSON.parse(JSON.stringify(state));
On 2016/07/07 17:36:17, stevenjb wrote:
> var networkStateCopy = Object.assign({}, state);

Done.

https://codereview.chromium.org/2067153002/diff/280001/ui/webui/resources/js/...
File ui/webui/resources/js/polymer_config.js (right):

https://codereview.chromium.org/2067153002/diff/280001/ui/webui/resources/js/...
ui/webui/resources/js/polymer_config.js:8: isChromePolymerInitialized = true;
On 2016/07/07 16:58:41, stevenjb wrote:
> Make this change in a separate CL. Add dbeam@ and michaelpg@ as reviewers.

Done.

[stevenjb, 2016-07-08 17:43:35]

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.html:44: <oobe-dialog
id="networkSection" class="fit" hidden>
On 2016/07/08 01:12:28, Alexander Alekseev wrote:
> On 2016/07/07 17:36:17, stevenjb wrote:
> > Consider using a tamplate if here:
> > 
> > <template is="dom-if" if="[[showDialog_]]">
> > 
> > That way the dialog won't be loaded initially, improving initial load time.
> This
> > is probably less of an issue for OOBE than for Settings, but it is still a
> good
> > practice.
> 
> This conflicts with onWelcomeNextButtonClicked_ method, because
> this.$.networkSelect.customItems = ... will trigger "Uncaught TypeError:
Cannot
> set property 'customItems' of undefined" .

You would have to change that to a declarative approach, e.g.
custom-items="[[_getCustomItems()]]".

If that is problematic, you can disregard this sugegstion, but I do think that
removing the setting of custom-items from onWelcomeNextButtonClicked_ would be
cleaner.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:51:
this.$.networkSelect.maxHeight = 280;
On 2016/07/08 01:12:29, Alexander Alekseev wrote:
> On 2016/07/07 17:36:17, stevenjb wrote:
> > Instead of setting maxHeight to a specific value here, toggle a class and
put
> > the value in CSS. (Or if you can just use #networkSelect[showActive=false]
in
> > the CSS).
> 
> maxHeight is not CSS attribute here, and setting it in HTML (or in CSS as
> standard CSS property) doesn't work. (It probably gets overwritten by smth.)
> 
> So I don't know why I have to use this here.
> 

We should figure it out, because somebody later on is going to need to change
the height and may have a difficult time finding this.

Looking again at the HTML, do we ever need max-height to be anything other than
280? e.g. can't we just add 'max-height="280"' to the HTML? That seems to work
just fine in network_ui.html.

https://codereview.chromium.org/2067153002/diff/320001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/login_shared.html (right):

https://codereview.chromium.org/2067153002/diff/320001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/login_shared.html:9: <!-- This must be
first import in all login pages. -->
nit: 'the first import'

[Dan Beam, 2016-07-09 00:48:13]

Description was changed from

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-07-09 00:48:13]

dbeam@chromium.org changed reviewers:
- dbeam@chromium.org

[Alexander Alekseev, 2016-07-09 04:31:31]

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.html (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.html:44: <oobe-dialog
id="networkSection" class="fit" hidden>
On 2016/07/08 17:43:35, stevenjb wrote:
> On 2016/07/08 01:12:28, Alexander Alekseev wrote:
> > On 2016/07/07 17:36:17, stevenjb wrote:
> > > Consider using a tamplate if here:
> > > 
> > > <template is="dom-if" if="[[showDialog_]]">
> > > 
> > > That way the dialog won't be loaded initially, improving initial load
time.
> > This
> > > is probably less of an issue for OOBE than for Settings, but it is still a
> > good
> > > practice.
> > 
> > This conflicts with onWelcomeNextButtonClicked_ method, because
> > this.$.networkSelect.customItems = ... will trigger "Uncaught TypeError:
> Cannot
> > set property 'customItems' of undefined" .
> 
> You would have to change that to a declarative approach, e.g.
> custom-items="[[_getCustomItems()]]".
> 
> If that is problematic, you can disregard this sugegstion, but I do think that
> removing the setting of custom-items from onWelcomeNextButtonClicked_ would be
> cleaner.
> 

I changed most of code to declarative, but i18n-content is empty if this object
is initially uninitialized.

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/280001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:51:
this.$.networkSelect.maxHeight = 280;
On 2016/07/08 17:43:35, stevenjb wrote:
> On 2016/07/08 01:12:29, Alexander Alekseev wrote:
> > On 2016/07/07 17:36:17, stevenjb wrote:
> > > Instead of setting maxHeight to a specific value here, toggle a class and
> put
> > > the value in CSS. (Or if you can just use #networkSelect[showActive=false]
> in
> > > the CSS).
> > 
> > maxHeight is not CSS attribute here, and setting it in HTML (or in CSS as
> > standard CSS property) doesn't work. (It probably gets overwritten by smth.)
> > 
> > So I don't know why I have to use this here.
> > 
> 
> We should figure it out, because somebody later on is going to need to change
> the height and may have a difficult time finding this.
> 
> Looking again at the HTML, do we ever need max-height to be anything other
than
> 280? e.g. can't we just add 'max-height="280"' to the HTML? That seems to work
> just fine in network_ui.html.
> 

It seems that it works now, so this is done.

https://codereview.chromium.org/2067153002/diff/320001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/login_shared.html (right):

https://codereview.chromium.org/2067153002/diff/320001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/login_shared.html:9: <!-- This must be
first import in all login pages. -->
On 2016/07/08 17:43:35, stevenjb wrote:
> nit: 'the first import'

Done.

[Alexander Alekseev, 2016-07-09 04:33:11]

alemate@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Alexander Alekseev, 2016-07-09 04:33:12]

+phajdan.jr@ for web_ui_browser_test.cc

I added additional error reporting to web_ui_browser_test.cc to make error
messages appear in test output just before "Encountered javascript console
error" message, because I spent quite a time to find out the source of test
failure.

[Alexander Alekseev, 2016-07-09 04:33:35]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-09 04:33:48]

Dry run: 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-07-09 06:34:24]

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

[commit-bot: I haz the power, 2016-07-09 06:34:25]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  closure_compilation on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)

[Alexander Alekseev, 2016-07-09 06:40:46]

The CQ bit was checked by alemate@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-09 06:40:57]

Dry run: 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-07-09 08:41:35]

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

[commit-bot: I haz the power, 2016-07-09 08:41:36]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  closure_compilation on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)

[Paweł Hajdan Jr., 2016-07-11 15:37:38]

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
File chrome/test/base/web_ui_browser_test.cc (right):

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
chrome/test/base/web_ui_browser_test.cc:461: for (const auto& msg :
error_messages_.Get()) {
Consider extracting this change to a separate CL.

Why not change "Encountered javascript console error(s)" code below instead?
They operate on the same error_messages_ member variable.

By the way, more debugging info this way is a great idea. I like the change.

[stevenjb, 2016-07-11 16:37:23]

Thanks for making the declarative changes, LGTM!

https://codereview.chromium.org/2067153002/diff/340001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/340001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:19: },
nit: blank line

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
File chrome/test/base/web_ui_browser_test.cc (right):

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
chrome/test/base/web_ui_browser_test.cc:461: for (const auto& msg :
error_messages_.Get()) {
On 2016/07/11 15:37:38, Paweł Hajdan Jr. wrote:
> Consider extracting this change to a separate CL.
> 
> Why not change "Encountered javascript console error(s)" code below instead?
> They operate on the same error_messages_ member variable.
> 
> By the way, more debugging info this way is a great idea. I like the change.

+1, I like the idea behind the change, but agree that we should do this
separately and integrate it with the if clause below, e.g:

Encountered javascript console error(s):
JS ERROR: <first error>
etc

(or something similar)

[Alexander Alekseev, 2016-07-12 04:12:14]

https://codereview.chromium.org/2067153002/diff/340001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/oobe_welcome.js (right):

https://codereview.chromium.org/2067153002/diff/340001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/oobe_welcome.js:19: },
On 2016/07/11 16:37:23, stevenjb wrote:
> nit: blank line

Done.

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
File chrome/test/base/web_ui_browser_test.cc (right):

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
chrome/test/base/web_ui_browser_test.cc:461: for (const auto& msg :
error_messages_.Get()) {
On 2016/07/11 15:37:38, Paweł Hajdan Jr. wrote:
> Consider extracting this change to a separate CL.
> 
> Why not change "Encountered javascript console error(s)" code below instead?
> They operate on the same error_messages_ member variable.
> 
> By the way, more debugging info this way is a great idea. I like the change.

Done.

https://codereview.chromium.org/2067153002/diff/340001/chrome/test/base/web_u...
chrome/test/base/web_ui_browser_test.cc:461: for (const auto& msg :
error_messages_.Get()) {
On 2016/07/11 16:37:23, stevenjb wrote:
> On 2016/07/11 15:37:38, Paweł Hajdan Jr. wrote:
> > Consider extracting this change to a separate CL.
> > 
> > Why not change "Encountered javascript console error(s)" code below instead?
> > They operate on the same error_messages_ member variable.
> > 
> > By the way, more debugging info this way is a great idea. I like the change.
> 
> +1, I like the idea behind the change, but agree that we should do this
> separately and integrate it with the if clause below, e.g:
> 
> Encountered javascript console error(s):
> JS ERROR: <first error>
> etc
> 
> (or something similar)

I moved this change to amother CL: https://codereview.chromium.org/2145493002/ .

[Alexander Alekseev, 2016-07-12 04:21:22]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2016-07-12 04:21:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org, stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2067153002/#ps380001
(title: "Reverted change to web_ui_browser_test.cc that was moved to another
CL.")

[commit-bot: I haz the power, 2016-07-12 04:22:17]

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-07-12 05:54:13]

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

[commit-bot: I haz the power, 2016-07-12 05:54:15]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Alexander Alekseev, 2016-07-13 01:19:56]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2016-07-13 01:21:31]

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-07-13 03:24:05]

Description was changed from

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-13 03:24:07]

Committed patchset #20 (id:380001)

[commit-bot: I haz the power, 2016-07-13 03:25:01]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 03:26:06]

Description was changed from

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
ChromeOS: Implement Network Selection screen of material design OOBE.

This CL adds Network Selection screen to material design OOBE.

BUG=604119
TEST=none
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/78696be690cf522455cf571c5c19848837c66791
Cr-Commit-Position: refs/heads/master@{#404955}
==========

[commit-bot: I haz the power, 2016-07-13 03:26:08]

Patchset 20 (id:??) landed as
https://crrev.com/78696be690cf522455cf571c5c19848837c66791
Cr-Commit-Position: refs/heads/master@{#404955}