Do not bundle Roboto on CrOS

Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

Make roboto.css empty on CrOS.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8e804d6180b3bee3e5eaf55b69ccddd9d075c5ce
Cr-Commit-Position: refs/heads/master@{#409920}

[jungshik at Google, 2016-07-19 20:53:51]

Description was changed from

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

BUG=
TEST=
==========

to

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

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

[jungshik at Google, 2016-07-19 21:47:07]

Description was changed from

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

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

to

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[jungshik at Google, 2016-07-19 21:51:53]

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

[commit-bot: I haz the power, 2016-07-19 21:52:32]

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

[jungshik at Google, 2016-07-19 22:01:35]

jshin@chromium.org changed reviewers:
+ dbeam@chromium.org, edwardjung@chromium.org

[jungshik at Google, 2016-07-19 22:01:36]

PTAL. Thanks

[commit-bot: I haz the power, 2016-07-19 22:33:55]

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

[commit-bot: I haz the power, 2016-07-19 22:33:56]

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

[jungshik at Google, 2016-07-20 22:48:45]

On 2016/07/19 22:01:36, jungshik at google wrote:
> PTAL. Thanks

roboto.css has a reference to chrome://roboto/roboto-regular.woff2 (even though
'local()' has a higher
priority) and the run-time check for its presence fails:

FATAL:shared_resources_data_source.cc(92)] Check failed: -1 != idr (-1 vs. -1)
path: roboto/roboto-regular.woff2

I need to come up with a way to include different versions of roboto.css
depending on whether OS==chrome or not.

[Dan Beam, 2016-07-20 23:51:17]

are you sure ChromeOS has all weights of Roboto?  is it in the format of

  font-family: Roboto;
  weight: <different weights>

or are different fonts used, i.e.

  font-family: Roboto{Light,Medium,Bold}

or something?

And regarding the CHECK_NE(-1, resource_id) you're hitting, doing something
like:

<if expr="not chromeos">
... contents of roboto.css ...
</if>

is probably OK (it means you just end up importing an empty file on ChromeOS). 
Conversely you can find all the places that load roboto.css and wrap them with
<if expr="not chromeos"> but that'll be more work (with mild benefit).

[jungshik at Google, 2016-07-21 21:21:24]

Description was changed from

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

Make roboto.css empty on CrOS.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[jungshik at Google, 2016-07-21 21:26:02]

On 2016/07/20 23:51:17, Dan Beam wrote:
> are you sure ChromeOS has all weights of Roboto?  is it in the format of
> 
>   font-family: Roboto;
>   weight: <different weights>
> 
> or are different fonts used, i.e.
> 
>   font-family: Roboto{Light,Medium,Bold}
> 
> or something?

In CSS, 'font-family: Roboto; font-weight: [134579]00;' can be used to specify 
Roboto {Thin,Light,Regular,Medium,Bold,Black}. 

See https://bugs.chromium.org/p/chromium/issues/detail?id=617281 (and a blocking
bug for that bug that was fixed recently) and CLs landed for the bug. All 6
weights of Robot were added. 

> 
> And regarding the CHECK_NE(-1, resource_id) you're hitting, doing something
> like:
> 
> <if expr="not chromeos">
> ... contents of roboto.css ...
> </if>
> 
> is probably OK (it means you just end up importing an empty file on ChromeOS).

> Conversely you can find all the places that load roboto.css and wrap them with
> <if expr="not chromeos"> but that'll be more work (with mild benefit).

Thanks a lot !
I didn't know that css is subject to that kind of pre-processing. Done in PS #2.

[Dan Beam, 2016-07-21 21:39:58]

lgtm

[jungshik at Google, 2016-07-22 22:16:39]

The CQ bit was checked by jshin@chromium.org

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

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-22 23:08:28]

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

[commit-bot: I haz the power, 2016-07-22 23:08:29]

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

[jungshik at Google, 2016-07-22 23:59:52]

On 2016/07/22 23:08:29, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
> (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Hmm... this check is still triggered even though references to roboto*woff2 is
guarded 
by <if expr="not chromeos"> in roboto.css 

FATAL:shared_resources_data_source.cc(92)] Check failed: -1 != idr (-1 vs. -1)
path: roboto/roboto-regular.woff2

[Dan Beam, 2016-07-23 00:05:45]

On 2016/07/22 23:59:52, jungshik at google wrote:
> On 2016/07/22 23:08:29, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
> > (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> Hmm... this check is still triggered even though references to roboto*woff2 is
> guarded 
> by <if expr="not chromeos"> in roboto.css 
> 
> FATAL:shared_resources_data_source.cc(92)] Check failed: -1 != idr (-1 vs. -1)
> path: roboto/roboto-regular.woff2

you need to add a preprocess="true" attribute to the .grd rule that builds it
into chrome:
https://cs.chromium.org/chromium/src/ui/webui/resources/webui_resources.grd?q...

you might also... consider manually checking that the <if> works this time ;)

[jungshik at Google, 2016-08-01 16:44:56]

On 2016/07/23 00:05:45, Dan Beam wrote:
> On 2016/07/22 23:59:52, jungshik at google wrote:
> > On 2016/07/22 23:08:29, commit-bot: I haz the power wrote:
> > > Try jobs failed on following builders:
> > >   linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
> > > (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > 
> > Hmm... this check is still triggered even though references to roboto*woff2
is
> > guarded 
> > by <if expr="not chromeos"> in roboto.css 
> > 
> > FATAL:shared_resources_data_source.cc(92)] Check failed: -1 != idr (-1 vs.
-1)
> > path: roboto/roboto-regular.woff2
> 
> you need to add a preprocess="true" attribute to the .grd rule that builds it
> into chrome:
>
https://cs.chromium.org/chromium/src/ui/webui/resources/webui_resources.grd?q...
> 
> you might also... consider manually checking that the <if> works this time ;)

Thanks a lot. 
PS #3 does that, but browser_tests still fails at
shared_resources_data_source.cc(92).  

On Linux, I built chromeos Chrome and chrome://resources/roboto.css does not
have any reference to roboto*woff2 (it has only comment lines as it should). I'm
building browser_tests (for chromeos on Linux)  to see what's going on.

[jungshik at Google, 2016-08-04 17:20:42]

On 2016/08/01 16:44:56, jungshik at google wrote:
> On 2016/07/23 00:05:45, Dan Beam wrote:
> > On 2016/07/22 23:59:52, jungshik at google wrote:
> > > On 2016/07/22 23:08:29, commit-bot: I haz the power wrote:
> > > > Try jobs failed on following builders:
> > > >   linux_chromium_chromeos_ozone_rel_ng on
master.tryserver.chromium.linux
> > > > (JOB_FAILED,
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > > 
> > > Hmm... this check is still triggered even though references to
roboto*woff2
> is
> > > guarded 
> > > by <if expr="not chromeos"> in roboto.css 
> > > 
> > > FATAL:shared_resources_data_source.cc(92)] Check failed: -1 != idr (-1 vs.
> -1)
> > > path: roboto/roboto-regular.woff2
> > 
> > you need to add a preprocess="true" attribute to the .grd rule that builds
it
> > into chrome:
> >
>
https://cs.chromium.org/chromium/src/ui/webui/resources/webui_resources.grd?q...
> > 
> > you might also... consider manually checking that the <if> works this time
;)
> 
> Thanks a lot. 
> PS #3 does that, but browser_tests still fails at
> shared_resources_data_source.cc(92).  
> 
> On Linux, I built chromeos Chrome and chrome://resources/roboto.css does not
> have any reference to roboto*woff2 (it has only comment lines as it should).
I'm
> building browser_tests (for chromeos on Linux)  to see what's going on.

It turned out that the download ui uses a 'vulcanized' version that inlines all
the CSS files. I enclosed roboto font-face block in vulcanized.html with
<if></if>. (I didn't install node etc but made sure that what's in
vulcanized.html is identical to roboto.css). 

Although 'preprocess=true' is not necessary, I kept them in PS 4 because it
saves a bit of disk space/run-time cost on CrOS while increasing the build time.
(should we set it for other files where <if expr> is used? ). 

Dan, can you take another look? (just to make sure what I did in vulcanized.html
is ok). Thanks

[Dan Beam, 2016-08-04 18:08:15]

lgtm (I get the same results locally when using vulcanize.py)

[jungshik at Google, 2016-08-04 18:25:51]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2016-08-04 18:26:47]

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-08-04 18:49:46]

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

[commit-bot: I haz the power, 2016-08-04 18:49:46]

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

[jungshik at Google, 2016-08-04 22:17:54]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2016-08-04 22:18:18]

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

[jungshik at Google, 2016-08-04 22:18:40]

Thanks, Dan !
On 2016/08/04 18:49:46, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
> (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

This bot failed to compile even without a patch applied.

[commit-bot: I haz the power, 2016-08-04 22:38:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-04 22:42:37]

Description was changed from

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

Make roboto.css empty on CrOS.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Do not bundle Roboto on CrOS

Roboto (all weights) are included in CrOS. No need to bundle
them in resources.

Make roboto.css empty on CrOS.

BUG=617281
TEST=Roboto is still used in web UI on CrOS.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8e804d6180b3bee3e5eaf55b69ccddd9d075c5ce
Cr-Commit-Position: refs/heads/master@{#409920}
==========

[commit-bot: I haz the power, 2016-08-04 22:42:38]

Patchset 4 (id:??) landed as
https://crrev.com/8e804d6180b3bee3e5eaf55b69ccddd9d075c5ce
Cr-Commit-Position: refs/heads/master@{#409920}