CrOS Settings: Add skeleton page for multidevice section.

CrOS Settings: Add skeleton page for multidevice section.

This is gated on a Finch feature (enable with "--enable-features=Multidevice").

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2964293002
Cr-Commit-Position: refs/heads/master@{#486647}
Committed: https://chromium.googlesource.com/chromium/src/+/c23f8d441fb8c5924fbfa0fb32742a664d2c33cb

[Oren Blasberg, 2017-06-30 21:11:12]

Description was changed from

==========
CrOS Settings: Add skeleton page for multidevice section.

BUG=718617
==========

to

==========
CrOS Settings: Add skeleton page for multidevice section.

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Oren Blasberg, 2017-06-30 22:26:46]

Description was changed from

==========
CrOS Settings: Add skeleton page for multidevice section.

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
CrOS Settings: Add skeleton page for multidevice section.

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Oren Blasberg, 2017-06-30 22:26:47]

orenb@chromium.org changed reviewers:
+ dpapad@chromium.org

[Oren Blasberg, 2017-06-30 22:28:42]

Thanks for having a look!

Note 1: I noticed one issue perhaps you can help me with Demetrios. The routing
doesn't seem to work. I.e., clicking Multidevice in the settings drawer doesn't
work and the url doesn't update. What am I missing?

Note 2: The strings and exact layout of the setting within the page are subject
to change in a follow up cl once UX etc. decide on final strings.

[Oren Blasberg, 2017-06-30 22:29:30]

orenb@chromium.org changed reviewers:
- dpapad@chromium.org

[Oren Blasberg, 2017-06-30 22:29:30]

Err.. just realized I forgot to  gate this on a flag.

Will do that, then re-mail. Sorry about that :)

[Oren Blasberg, 2017-07-06 00:31:15]

Description was changed from

==========
CrOS Settings: Add skeleton page for multidevice section.

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
CrOS Settings: Add skeleton page for multidevice section.

This is gated on a Finch feature (enable with "--enable-features=Multidevice").

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Oren Blasberg, 2017-07-06 00:31:15]

orenb@chromium.org changed reviewers:
+ dpapad@chromium.org

[Oren Blasberg, 2017-07-06 00:32:09]

This is ready for review now! Thanks for your patience, General.   :)

Btw, the strings may change a bit, but unclear if that will happen before or
after landing this cl. May also need to update this UI slightly (i.e. with a
summary string beneath te "SMS connect" text) in a follow up CL.

[dpapad, 2017-07-06 01:26:50]

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/settings/basic_page/basic_page.js (right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:21: showAndroidApps:
Boolean,
Should lines 21-25 be wrapped in an <if expr="chromeos">...</if> ?

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:192: * @return
{boolean}
Nit: Move comment from line 190 to here, as
@return {boolean} Whether to show the multidevice settings page.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:195:
showMultidevicePage_: function(visibility) {
Where is this used?

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:3:
<link rel="import"
href="chrome://resources/cr_elements/policy/cr_policy_pref_indicator.html">
Probably copy pasted from elsewhere? Let's remove dependencies that are not
currently being used in this file.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:17:
</style>
Nit: Can fit in previous line.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:19:
<neon-animatable route-path="default">
I am a bit torn on how to review this file. A lot of the HTML markup is
currently unused. Can we make this as minimal as possible to serve its current
purpose (being a dummy placeholder)?

Specifically, how about removing all IDs and CSS classes that are not used. They
can be added when this UI component gets more fleshed out.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:26:
<paper-toggle-button id="enableSmsConnect"
Related to the comment above, this is unlikely to stay as is, as this file gets
more concrete.

For example, if you want a toggle button that automatically flips a pref, there
is already an element for that, called settings-toggle-button, see examples at
[1] below. Using that element instead, eliminates the need for the TODO you are
adding in multidevice_page.js, or to be more precise, the code that propagates
the pref change to the appropriate app would probably reside in C++ instead.

Not sure if that element throws errors if hooked up to a non-existing (for now)
pref though. If you want to keep this code as is, then I suggest to at least
update the TODO in the JS file to mention that a settings-toggle-button should
be used eventually. 

[1]
https://cs.chromium.org/search/?q=settings-toggle-button+file:%5Esrc/chrome/b...

https://codereview.chromium.org/2964293002/diff/90001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:1658: ROOT_PATH +
'ui/webui/resources/js/assert.js',
I don't think this is necessary. All assert methods used in tests are defined in
test_api.js which is already included (automatically).

[Oren Blasberg, 2017-07-06 18:17:59]

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/settings/basic_page/basic_page.js (right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:21: showAndroidApps:
Boolean,
On 2017/07/06 01:26:50, dpapad wrote:
> Should lines 21-25 be wrapped in an <if expr="chromeos">...</if> ?

Done. Btw, how does that work, if it's commented out and it's an "html" style
tag? I am guessing there is some nifty preprocessor which magically compiles
this out on non-CrOS platforms?

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:192: * @return
{boolean}
On 2017/07/06 01:26:50, dpapad wrote:
> Nit: Move comment from line 190 to here, as
> @return {boolean} Whether to show the multidevice settings page.

Done.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:195:
showMultidevicePage_: function(visibility) {
On 2017/07/06 01:26:50, dpapad wrote:
> Where is this used?

Oops. It was superseded by shouldShowMultidevice_ below. Removing this. Thanks!

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:3:
<link rel="import"
href="chrome://resources/cr_elements/policy/cr_policy_pref_indicator.html">
On 2017/07/06 01:26:50, dpapad wrote:
> Probably copy pasted from elsewhere? Let's remove dependencies that are not
> currently being used in this file.

Done.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:17:
</style>
On 2017/07/06 01:26:50, dpapad wrote:
> Nit: Can fit in previous line.

Done.

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:19:
<neon-animatable route-path="default">
On 2017/07/06 01:26:50, dpapad wrote:
> I am a bit torn on how to review this file. A lot of the HTML markup is
> currently unused. Can we make this as minimal as possible to serve its current
> purpose (being a dummy placeholder)?
> 
> Specifically, how about removing all IDs and CSS classes that are not used.
They
> can be added when this UI component gets more fleshed out.

Sure. Can you point out which IDs and classes are ok to keep and which to
remove?
I've removed  id="enableSmsConnect" on the paper-toggle-button since no one is
referencing that. 
The rest of the css classes seem to be just styling it so the setting looks
visually consistent with Material. Is it OK to leave those?

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:26:
<paper-toggle-button id="enableSmsConnect"
On 2017/07/06 01:26:50, dpapad wrote:
> Related to the comment above, this is unlikely to stay as is, as this file
gets
> more concrete.
> 
> For example, if you want a toggle button that automatically flips a pref,
there
> is already an element for that, called settings-toggle-button, see examples at
> [1] below. Using that element instead, eliminates the need for the TODO you
are
> adding in multidevice_page.js, or to be more precise, the code that propagates
> the pref change to the appropriate app would probably reside in C++ instead.
> 
> Not sure if that element throws errors if hooked up to a non-existing (for
now)
> pref though. If you want to keep this code as is, then I suggest to at least
> update the TODO in the JS file to mention that a settings-toggle-button should
> be used eventually. 
> 
> [1]
>
https://cs.chromium.org/search/?q=settings-toggle-button+file:%5Esrc/chrome/b...

Yes exactly, I can't use settings-toggle-button until there is an actual pref
there. It throws errors without a pref= attribute specified.

Updated the TODO in the js accordingly.
Thanks :)

https://codereview.chromium.org/2964293002/diff/90001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:1658: ROOT_PATH +
'ui/webui/resources/js/assert.js',
On 2017/07/06 01:26:50, dpapad wrote:
> I don't think this is necessary. All assert methods used in tests are defined
in
> test_api.js which is already included (automatically).

Done.

[dpapad, 2017-07-06 18:59:03]

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
File chrome/browser/resources/settings/basic_page/basic_page.js (right):

https://codereview.chromium.org/2964293002/diff/90001/chrome/browser/resource...
chrome/browser/resources/settings/basic_page/basic_page.js:21: showAndroidApps:
Boolean,
On 2017/07/06 at 18:17:58, Oren Blasberg wrote:
> On 2017/07/06 01:26:50, dpapad wrote:
> > Should lines 21-25 be wrapped in an <if expr="chromeos">...</if> ?
> 
> Done. Btw, how does that work, if it's commented out and it's an "html" style
tag? I am guessing there is some nifty preprocessor which magically compiles
this out on non-CrOS platforms?

Yes. GRIT is preprocessing those, see
https://cs.chromium.org/chromium/src/tools/grit/grit/format/html_inline_unitt....

The reason we put those "<if expr>" statements behind a comment is so that the
JS file still holds valid JS syntax, and therefore we can use still other tools
without them blowing up (like ESLint), and without having to preprocess the file
with GRIT first.

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/basic_page/basic_page.js (right):

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/basic_page/basic_page.js:196:
showMultidevicePage_: function(visibility) {
Still here.

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:5:
<link rel="import" href="../i18n_setup.html">
I don't think this is necessary. multidevice_page.js itself is not accessing
loadTimeData, so it does not directly depend on it.

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:15:
<settings-animated-pages id="pages" section="multidevice">
Currently this page does not have any subpages (settings-subpage). So
settings-animated-pages and neon-animatable usage is unnecessary for now. For a
minimal settings page that has no subpages you can see an example at
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/date_t....

[Oren Blasberg, 2017-07-06 19:20:30]

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/basic_page/basic_page.js (right):

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/basic_page/basic_page.js:196:
showMultidevicePage_: function(visibility) {
On 2017/07/06 18:59:02, dpapad wrote:
> Still here.

Done.

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:5:
<link rel="import" href="../i18n_setup.html">
On 2017/07/06 18:59:02, dpapad wrote:
> I don't think this is necessary. multidevice_page.js itself is not accessing
> loadTimeData, so it does not directly depend on it.

Done.

https://codereview.chromium.org/2964293002/diff/130001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:15:
<settings-animated-pages id="pages" section="multidevice">
On 2017/07/06 18:59:02, dpapad wrote:
> Currently this page does not have any subpages (settings-subpage). So
> settings-animated-pages and neon-animatable usage is unnecessary for now. For
a
> minimal settings page that has no subpages you can see an example at
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/date_t....

Done. I appreciate that example link.

Also got rid of the icon and separator to simplify further.

[dpapad, 2017-07-06 20:44:48]

LGTM with nits.

+scottchen as FYI, since some changes in this CL will conflict with the
uncommitted https://codereview.chromium.org/2957153003.

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/compiled_resources2.gyp
(right):

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/compiled_resources2.gyp:9:
'../compiled_resources2.gyp:route',
None of the dependencies in this list are actually needed to successfully
compile this file. Can you remove?

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:10:
<div id="multidevice" class="settings-box two-line">
All the IDs below this point are not used by any CSS rule, JS querySelector, or
any of the dummy tests. Let's remove them. The only thing that is needed is the
CSS classes for styling (setting-box, two-line, start).

[dpapad, 2017-07-06 20:45:09]

+scottchen, see previous comment (as FYI).

[Oren Blasberg, 2017-07-06 21:22:11]

Scott, no need to wait on this/me to land your cl.  :)

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/compiled_resources2.gyp
(right):

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/compiled_resources2.gyp:9:
'../compiled_resources2.gyp:route',
On 2017/07/06 20:44:47, dpapad wrote:
> None of the dependencies in this list are actually needed to successfully
> compile this file. Can you remove?

Done.

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/150001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:10:
<div id="multidevice" class="settings-box two-line">
On 2017/07/06 20:44:47, dpapad wrote:
> All the IDs below this point are not used by any CSS rule, JS querySelector,
or
> any of the dummy tests. Let's remove them. The only thing that is needed is
the
> CSS classes for styling (setting-box, two-line, start).


smsConnectToggleLabel is used by paper-toggle-button below, in the
aria-labelledby attribute
^ So I am keeping that one intact.

Removed the others.

[Oren Blasberg, 2017-07-13 18:37:52]

orenb@chromium.org changed reviewers:
+ thestig@chromium.org

[Oren Blasberg, 2017-07-13 18:37:54]

thestig@chromium.org: Please review changes in strings grdp and localized
strings provider.


dpapad@: The UX have finalized the layout and strings for the "SMS connect"
entry. I have updated very slightly the layout by adding a subtitle/summary text
and adding a new string for the summary text. I am emailing you an updated
screenshot.

Thanks both of you!

[Lei Zhang, 2017-07-13 20:56:59]

lgtm

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:2032:
LocalizedString localized_strings[] = {
Can this be const? (And all the others in this file)

[Oren Blasberg, 2017-07-13 21:16:13]

On 2017/07/13 20:56:59, Lei Zhang (OOO) wrote:
> lgtm
> 
>
https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
> File
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
> (right):
> 
>
https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
>
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:2032:
> LocalizedString localized_strings[] = {
> Can this be const? (And all the others in this file)


@dpapad, thoughts here? Shall I make this const and ignore the rest, or leave as
is? I don't think we ought to update the entire file in this cl but it could be
something worth doing separately.
Also @dpapad, can I still consider your lgtm valid or would you like to take a
look at the small tweaks I made to the settings page? Just diff against patch
10.
Thanks!

[dpapad, 2017-07-13 21:26:17]

Still LGTM.

Regarding making localized_strings const, if it can be const, my preference
would be to do it just for the one case you are adding in this CL, and convert
the remaining ones in a separate CL.

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:13:
<div class="secondary">
Nit: Can these three lines fit in one line?

[Oren Blasberg, 2017-07-13 23:07:06]

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/resourc...
File chrome/browser/resources/settings/multidevice_page/multidevice_page.html
(right):

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/resourc...
chrome/browser/resources/settings/multidevice_page/multidevice_page.html:13:
<div class="secondary">
On 2017/07/13 21:26:17, dpapad wrote:
> Nit: Can these three lines fit in one line?

Done.

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2964293002/diff/190001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:2032:
LocalizedString localized_strings[] = {
On 2017/07/13 20:56:58, Lei Zhang (OOO) wrote:
> Can this be const? (And all the others in this file)

Adding const does not work..

../../chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:2037:3:
error: no matching function for call to 'AddLocalizedStringsBulk'
  AddLocalizedStringsBulk(html_source, localized_strings,
  ^~~~~~~~~~~~~~~~~~~~~~~
../../chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:70:6:
note: candidate function not viable: no known conversion from 'const
settings::(anonymous namespace)::LocalizedString [3]' to 'settings::(anonymous
namespace)::LocalizedString *' for 2nd argument
void AddLocalizedStringsBulk(content::WebUIDataSource* html_source,
     ^



Deferring this -- as dpapad@ suggested, the change can be made orthogonally to
this CL.

[Oren Blasberg, 2017-07-14 00:40:35]

The CQ bit was checked by orenb@chromium.org

[Oren Blasberg, 2017-07-14 00:40:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2964293002/#ps230001
(title: "Sync and rebase")

[commit-bot: I haz the power, 2017-07-14 00:40:59]

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, 2017-07-14 01:36:11]

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

[commit-bot: I haz the power, 2017-07-14 01:36:12]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Oren Blasberg, 2017-07-14 02:21:52]

The CQ bit was checked by orenb@chromium.org

[commit-bot: I haz the power, 2017-07-14 02:22:10]

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, 2017-07-14 02:51:41]

CQ is committing da patch.

Bot data: {"patchset_id": 230001, "attempt_start_ts": 1499998912840820,
"parent_rev": "27a2627894bbcc779117d67b3529c35cfb229228", "commit_rev":
"c23f8d441fb8c5924fbfa0fb32742a664d2c33cb"}

[commit-bot: I haz the power, 2017-07-14 02:51:55]

Description was changed from

==========
CrOS Settings: Add skeleton page for multidevice section.

This is gated on a Finch feature (enable with "--enable-features=Multidevice").

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
CrOS Settings: Add skeleton page for multidevice section.

This is gated on a Finch feature (enable with "--enable-features=Multidevice").

BUG=718617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2964293002
Cr-Commit-Position: refs/heads/master@{#486647}
Committed:
https://chromium.googlesource.com/chromium/src/+/c23f8d441fb8c5924fbfa0fb3274...
==========

[commit-bot: I haz the power, 2017-07-14 02:51:57]

Committed patchset #13 (id:230001) as
https://chromium.googlesource.com/chromium/src/+/c23f8d441fb8c5924fbfa0fb3274...