Configure font font names in GFX unittests

Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.

In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
Committed: https://crrev.com/bdbc597424976a0b2f23b38b0c3630b636347ad2
Cr-Commit-Position: refs/heads/master@{#417244}

[drott, 2016-09-02 12:58:58]

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

[commit-bot: I haz the power, 2016-09-02 12:59:13]

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

[drott, 2016-09-02 13:01:15]

drott@chromium.org changed reviewers:
+ ananta@chromium.org, asvitkine@chromium.org, msw@chromium.org,
yukishiino@chromium.or

[commit-bot: I haz the power, 2016-09-02 13:02:22]

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

[commit-bot: I haz the power, 2016-09-02 13:02:23]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[drott, 2016-09-02 13:05:16]

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

[commit-bot: I haz the power, 2016-09-02 13:05:34]

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-09-02 13:52:26]

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

[commit-bot: I haz the power, 2016-09-02 13:52:27]

Dry run: 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_...)

[drott, 2016-09-02 15:08:08]

Description was changed from

==========
Make Symbol font name configurable in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
need to move to a different font on Linux for the unit tests that use it.

Make this font name configurable and replace it with "Symbola" on
Linux, which should be available through the ttf-ancient-scripts package
and has good coverage for a comparable set of codepoints.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

to

==========
Make Symbol font name configurable in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
need to move to a different font on Linux for the unit tests that use it.

Make this font name configurable and replace it with "OpenSymbol" on
Linux, which is default installed on Ubuntu 12.04 through
fonts-opensymbol.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

[drott, 2016-09-02 15:08:52]

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

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

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

[drott, 2016-09-02 15:11:59]

Description was changed from

==========
Make Symbol font name configurable in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
need to move to a different font on Linux for the unit tests that use it.

Make this font name configurable and replace it with "OpenSymbol" on
Linux, which is default installed on Ubuntu 12.04 through
fonts-opensymbol.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

to

==========
Make Symbol font name configurable in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
need to move to a different font on Linux for the unit tests that use it.

Make this font name configurable and replace it with "OpenSymbol" on
Linux, which is default installed on Ubuntu 12.04 through
fonts-opensymbol.

To make this work, due to the global metrics values of OpenSymbol,
StringSizeRespectsFontListMetrics needs to be updated. I believe I
captured the intention of the test correctly and hopefully made it more
robust on all platforms.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

[Alexei Svitkine (slow), 2016-09-02 15:19:58]

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
File ui/gfx/font_names_testing.h (right):

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:1: // Copyright (c) 2016 The Chromium Authors. All
rights reserved.
No (c) for new files.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:11: const char* const symbol_font_name =
"Opensymbol";
Naming conventions for constants is kSymbolFontName

Also, convention for defining character constants is const char[]
kSymbolFontName.

Finally, string constants should not be defined in .h files. Instead, make a
corresponding .cc file and have the declaration in the .h but the definition in
the .cc.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:16: };
No ;

Add a comment // namespace gfx

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_unittest.cc
File ui/gfx/font_unittest.cc (right):

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_unittest.cc...
ui/gfx/font_unittest.cc:132: 
Remove extra added lines.

[drott, 2016-09-02 15:34:21]

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

[commit-bot: I haz the power, 2016-09-02 15:34:32]

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

[drott, 2016-09-02 15:35:54]

Thanks for the quick review. CL updated.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
File ui/gfx/font_names_testing.h (right):

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:1: // Copyright (c) 2016 The Chromium Authors. All
rights reserved.
Done.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:11: const char* const symbol_font_name =
"Opensymbol";
Okay, split into .h and .cc.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_names_testi...
ui/gfx/font_names_testing.h:16: };
On 2016/09/02 at 15:19:58, Alexei Svitkine wrote:
> No ;
> 
> Add a comment // namespace gfx

Done in header and new .cc file.

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_unittest.cc
File ui/gfx/font_unittest.cc (right):

https://codereview.chromium.org/2302313002/diff/40001/ui/gfx/font_unittest.cc...
ui/gfx/font_unittest.cc:132: 
Done.

[commit-bot: I haz the power, 2016-09-02 15:37:37]

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

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

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Alexei Svitkine (slow), 2016-09-02 15:38:33]

https://codereview.chromium.org/2302313002/diff/60001/ui/gfx/BUILD.gn
File ui/gfx/BUILD.gn (right):

https://codereview.chromium.org/2302313002/diff/60001/ui/gfx/BUILD.gn#newcode553
ui/gfx/BUILD.gn:553: "font_names_testing.cc",
Looks like you forgot to add this file.

[drott, 2016-09-02 15:39:59]

Now with the new file

[drott, 2016-09-02 15:39:59]

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

[commit-bot: I haz the power, 2016-09-02 15:40:14]

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-09-02 16:23:48]

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

[commit-bot: I haz the power, 2016-09-02 16:23:50]

Dry run: 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_...)

[drott, 2016-09-05 13:04:52]

One more attempt, changing font size in tests that rely on height differences,
symbol font choice

[drott, 2016-09-05 13:04:53]

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

[commit-bot: I haz the power, 2016-09-05 13:05:07]

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-09-05 13:28:19]

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

[commit-bot: I haz the power, 2016-09-05 13:28:20]

Dry run: 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...)

[drott, 2016-09-05 14:17:56]

Another attempt, introducing a CJK font and mixed font list related tests

[drott, 2016-09-05 14:17:57]

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

[commit-bot: I haz the power, 2016-09-05 14:18:03]

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-09-05 14:26:37]

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

[commit-bot: I haz the power, 2016-09-05 14:26:38]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[drott, 2016-09-06 06:34:51]

Fixed parenthesis

[drott, 2016-09-06 06:34:52]

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

[commit-bot: I haz the power, 2016-09-06 06:35:00]

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-09-06 07:01:47]

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

[commit-bot: I haz the power, 2016-09-06 07:01:48]

Dry run: 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...)

[drott, 2016-09-06 15:16:35]

Fix Symbol font name on Win, change CJK font to PingFang on Mac

[drott, 2016-09-06 15:16:36]

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

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

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-09-06 15:41:21]

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

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

Dry run: 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...)

[drott, 2016-09-06 16:10:56]

Back to Symbol font for OS X and iOS

[drott, 2016-09-06 16:10:57]

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

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

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

[drott, 2016-09-06 17:00:58]

Reintroduce height based assertions now that CJK font is used in
StringSizeRespectsFontListMetrics

[drott, 2016-09-06 17:00:59]

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

[commit-bot: I haz the power, 2016-09-06 17:01:08]

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

[drott, 2016-09-06 17:01:19]

Description was changed from

==========
Make Symbol font name configurable in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
need to move to a different font on Linux for the unit tests that use it.

Make this font name configurable and replace it with "OpenSymbol" on
Linux, which is default installed on Ubuntu 12.04 through
fonts-opensymbol.

To make this work, due to the global metrics values of OpenSymbol,
StringSizeRespectsFontListMetrics needs to be updated. I believe I
captured the intention of the test correctly and hopefully made it more
robust on all platforms.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

to

==========
Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.
        
In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

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

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

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

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexei Svitkine (slow), 2016-09-06 19:48:34]

LGTM % comment

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
File ui/gfx/font_names_testing.cc (right):

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
ui/gfx/font_names_testing.cc:25: } // end namespace gfx
This comment should just be "// namespace". You also need two spaces before the
// per the style guide.

[msw, 2016-09-06 20:06:06]

lgtm with nits

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
File ui/gfx/font_names_testing.cc (right):

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
ui/gfx/font_names_testing.cc:25: } // end namespace gfx
On 2016/09/06 19:48:34, Alexei Svitkine (slow) wrote:
> This comment should just be "// namespace". You also need two spaces before
the
> // per the style guide.

Actually, shouldn't it still say gfx? Like: "}  // namespace gfx"

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/render_text_uni...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:1989: // Check that Arial and Symbol have
different font metrics.
nit: update comment.

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2002: const char* cjk_font_text = u8"円";
nit: use the escape codes instead?

[Alexei Svitkine (slow), 2016-09-06 21:06:31]

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
File ui/gfx/font_names_testing.cc (right):

https://codereview.chromium.org/2302313002/diff/200001/ui/gfx/font_names_test...
ui/gfx/font_names_testing.cc:25: } // end namespace gfx
On 2016/09/06 20:06:06, msw wrote:
> On 2016/09/06 19:48:34, Alexei Svitkine (slow) wrote:
> > This comment should just be "// namespace". You also need two spaces before
> the
> > // per the style guide.
> 
> Actually, shouldn't it still say gfx? Like: "}  // namespace gfx"

You are correct, indeed it should be "// namespace gfx". Sorry for the
confusion.

[drott, 2016-09-08 09:26:23]

Use Heiti SC font, namespace, comment, hex escapes

[drott, 2016-09-08 09:26:24]

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

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

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-09-08 10:49:46]

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

[commit-bot: I haz the power, 2016-09-08 10:49:47]

Dry run: This issue passed the CQ dry run.

[drott, 2016-09-08 10:50:17]

The CQ bit was checked by drott@chromium.org

[drott, 2016-09-08 10:50:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2302313002/#ps220001
(title: "Use Heiti SC font, namespace, comment, hex escapes")

[commit-bot: I haz the power, 2016-09-08 10:50:39]

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-08 10:55:34]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-09-08 10:59:31]

Description was changed from

==========
Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.
        
In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142
==========

to

==========
Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.

In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142

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

[commit-bot: I haz the power, 2016-09-08 10:59:32]

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

[jungshik at Google, 2016-09-16 19:34:54]

jshin@chromium.org changed reviewers:
+ jshin@chromium.org

[jungshik at Google, 2016-09-16 19:34:55]

Sorry for the post-check-in comment.

https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
File ui/gfx/font_names_testing.cc (right):

https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
ui/gfx/font_names_testing.cc:12: const char kSymbolFontName[] = "DejaVu Sans";
Will this test ever be run on an actual Chrome OS device (as opposed to on
Linux)? Perhaps, not. 

Just in case the answer is yes,  kSymbolsFontName had better be 'Noto Sans
Symbol" on CrOS. Similarly, kCJKFOntName had better be 'Noto Sans CJK JP' (or
KR, SC or TC). 

(I'm planning to remove DejaVu fonts from CrOS).

[drott, 2016-09-20 07:37:39]

https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
File ui/gfx/font_names_testing.cc (right):

https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
ui/gfx/font_names_testing.cc:12: const char kSymbolFontName[] = "DejaVu Sans";
On 2016/09/16 at 19:34:55, jungshik at google wrote:
> Will this test ever be run on an actual Chrome OS device (as opposed to on
Linux)? Perhaps, not. 
> 
> Just in case the answer is yes,  kSymbolsFontName had better be 'Noto Sans
Symbol" on CrOS. Similarly, kCJKFOntName had better be 'Noto Sans CJK JP' (or
KR, SC or TC). 
> (I'm planning to remove DejaVu fonts from CrOS).

It would be much better to have access to Noto indeed, even on Linux.
Unfortunately on Linux we still rely on the bot's/system's/Ubuntu's font
configuration instead of providing our own set at least for testing. I am not
sure whether this test runs on CrOS, probably the bots will tell you once you
remove DejaVu Sans - in that case you can extend the ifdef to put the Noto fonts
in there.

[jungshik at Google, 2016-09-20 18:10:02]

On 2016/09/20 07:37:39, drott wrote:
>
https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
> File ui/gfx/font_names_testing.cc (right):
> 
>
https://codereview.chromium.org/2302313002/diff/220001/ui/gfx/font_names_test...
> ui/gfx/font_names_testing.cc:12: const char kSymbolFontName[] = "DejaVu Sans";
> On 2016/09/16 at 19:34:55, jungshik at google wrote:
> > Will this test ever be run on an actual Chrome OS device (as opposed to on
> Linux)? Perhaps, not. 
> > 
> > Just in case the answer is yes,  kSymbolsFontName had better be 'Noto Sans
> Symbol" on CrOS. Similarly, kCJKFOntName had better be 'Noto Sans CJK JP' (or
> KR, SC or TC). 
> > (I'm planning to remove DejaVu fonts from CrOS).
> 
> It would be much better to have access to Noto indeed, even on Linux.

Hmm... there's a script to run for Chrome OS developers to install Noto and Noto
CJK locally on their Linux box (when they work on CrOS UI, they'd better have
the same set of fonts). The same can be run for trybots, I guess. 

> Unfortunately on Linux we still rely on the bot's/system's/Ubuntu's font
> configuration instead of providing our own set at least for testing. I am not
> sure whether this test runs on CrOS, probably the bots will tell you once you
> remove DejaVu Sans - in that case you can extend the ifdef to put the Noto
fonts
> in there.

Given that kCJKFontName is set to KochiMincho (NOT present on Chromebook) and
there's no test failure, I guess actual Chrome OS devices are not used for
running this test. So, no need to change for now.

[Yuki, 2016-11-01 13:07:26]

Description was changed from

==========
Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.

In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142

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

to

==========
Configure font font names in GFX unittests

Currently the GFX unittests assume a font called "Symbol" to be
available on all platforms. While the "Symbol" font may be available on
Ubuntu, it is a deprecated Type 1 package and stems from the
xfonts-mathml package on Debian, which is deprecated and no longer
maintained in favor of the newer STIX fonts [1].

In preparation for activating filtering for SNFT format font files, we
have to break with the assumption above and make at least some of the
used fonts in the gfx unittests configurable per OS.

This CL makes the symbol font name configurable and replace it with
"DejaVu Sans" on Linux.

In addition to reconfiguring the Symbol font this CL introduces a CJK
font as an example of a font that has a different descender/ascender
ratio to Arial as some unittests such as
StringSizeRespectsFontListMetrics require, since the newly chosen symbol
font on Linux does not that expose that property.

To make StringSizeRespectsFontListMetrics more robust, this CL adds
additional assertions based on GetFontSpansForTesting.

[1] https://tracker.debian.org/pkg/xfonts-mathml

BUG=632142

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

[Yuki, 2016-11-01 13:07:27]

yukishiino@chromium.org changed reviewers:
+ yukishiino@chromium.org
- yukishiino@chromium.or