Add test for typeface style round trip.

Add test for typeface style round trip.

This also fixes the CG and GDI ports so they pass the test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002

Committed: https://skia.googlesource.com/skia/+/6e45bda29edef867468cbdd7c062d0d99e884656

[bungeman-skia, 2016-07-22 03:18:56]

Description was changed from

==========
Add test to for typeface style round trip.
==========

to

==========
Add test to for typeface style round trip.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

[bungeman-skia, 2016-07-22 18:59:16]

Description was changed from

==========
Add test to for typeface style round trip.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

to

==========
Add test for typeface style round trip.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

[bungeman-skia, 2016-07-22 21:30:08]

Description was changed from

==========
Add test for typeface style round trip.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

to

==========
Add test for typeface style round trip.

This also fixes the CG and GDI ports so they pass the test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

[bungeman-skia, 2016-07-22 21:37:31]

bungeman@google.com changed reviewers:
+ mtklein@google.com, reed@google.com

[bungeman-skia, 2016-07-22 21:37:32]

The motivation for this was mostly to fix the Mac port, which has been giving
the wrong answer for a long time.

[bungeman-skia, 2016-07-25 13:23:08]

https://codereview.chromium.org/2171163002/diff/60001/tests/TypefaceTest.cpp
File tests/TypefaceTest.cpp (right):

https://codereview.chromium.org/2171163002/diff/60001/tests/TypefaceTest.cpp#...
tests/TypefaceTest.cpp:22: SkSFNTHeader* sfntHeader =
static_cast<SkSFNTHeader*>(data->writable_data());
This is what is actually failing on Mac (on the second and subsequent times
through). The previous typeface was destroyed but the data provider retain count
never went to 0, so that old data provider is still holding on to the data. It
appears that creating a CTFont from a CGFont from a CGDataProvider and actually
using it does all sorts of interesting things to the reference count, leading to
the data leaking. It appears that all fonts from streams are leaking all their
data on Mac. The weird thing is that this test passed on iOS.

https://codereview.chromium.org/2171163002/diff/80001/tests/TypefaceTest.cpp
File tests/TypefaceTest.cpp (right):

https://codereview.chromium.org/2171163002/diff/80001/tests/TypefaceTest.cpp#...
tests/TypefaceTest.cpp:25: if (!dataToUse->unique()) {
data should always be unique here, but it isn't on Mac at the moment. Once Mac
is fixed we can remove this block.

[reed1, 2016-07-25 16:48:51]

lgtm w/ nits/questions

https://codereview.chromium.org/2171163002/diff/80001/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

https://codereview.chromium.org/2171163002/diff/80001/include/core/SkTypeface...
include/core/SkTypeface.h:349: void setFontStyle(SkFontStyle style) { fStyle =
style; }
repeat comment/constraint about being only called from the constructor?

https://codereview.chromium.org/2171163002/diff/80001/src/core/SkFontStyle.cpp
File src/core/SkFontStyle.cpp (right):

https://codereview.chromium.org/2171163002/diff/80001/src/core/SkFontStyle.cp...
src/core/SkFontStyle.cpp:21: fUnion.fR.fWeight = SkTPin<int>(weight, 0, 1000);
expose as kMin_Weight and kMax_Weight ?

[bungeman-skia, 2016-07-25 21:03:58]

https://codereview.chromium.org/2171163002/diff/80001/include/core/SkTypeface.h
File include/core/SkTypeface.h (right):

https://codereview.chromium.org/2171163002/diff/80001/include/core/SkTypeface...
include/core/SkTypeface.h:349: void setFontStyle(SkFontStyle style) { fStyle =
style; }
On 2016/07/25 16:48:50, reed1 wrote:
> repeat comment/constraint about being only called from the constructor?

I made a mental note to do that and forgot.

Done.

https://codereview.chromium.org/2171163002/diff/80001/src/core/SkFontStyle.cpp
File src/core/SkFontStyle.cpp (right):

https://codereview.chromium.org/2171163002/diff/80001/src/core/SkFontStyle.cp...
src/core/SkFontStyle.cpp:21: fUnion.fR.fWeight = SkTPin<int>(weight, 0, 1000);
On 2016/07/25 16:48:50, reed1 wrote:
> expose as kMin_Weight and kMax_Weight ?

For 1000 it seems best to use kExtraBlack_Weight. Most systems seem to use the
terms 'extra black' or 'ultra black' for this (as synonyms), but I lean toward
'extra' as the current naming convention is that 'extra' is used in weight names
and 'ultra' in width names.

As for 0, existing naming seems some more confused. FontConfig actually doesn't
even have a setting for this it seems, it just bottoms out at 'Thin'. It seems
others just stick with 'Thin' being 100 and just describe it as the 'lightest'
weight. In some sense 0 implies that effectively nothing is drawn. Also,
usWeightClass is an unsigned short, so 0 really is as low as it goes (as opposed
to 1000 which is somewhat arbitrary). As a result kInvisible_Weight seems like a
good name.

[bungeman-skia, 2016-07-25 21:38:14]

The CQ bit was checked by bungeman@google.com

[bungeman-skia, 2016-07-25 21:38:16]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/2171163002/#ps100001
(title: "Name extremes, add comment.")

[commit-bot: I haz the power, 2016-07-25 21:38:21]

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-25 22:11:51]

Description was changed from

==========
Add test for typeface style round trip.

This also fixes the CG and GDI ports so they pass the test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002
==========

to

==========
Add test for typeface style round trip.

This also fixes the CG and GDI ports so they pass the test.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2171163002

Committed:
https://skia.googlesource.com/skia/+/6e45bda29edef867468cbdd7c062d0d99e884656
==========

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

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/6e45bda29edef867468cbdd7c062d0d99e884656