Test CTFonts for equality, not just name and style.

Test CTFonts for equality, not just name and style.

The current code looks up CTFonts from descriptors by just name and style.
This is not sufficient, for example Avenir Book and Avenir Roman have the
same family name but the same basic style, but are obviously not the same.

BUG=skia:5162
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1860993004

Committed: https://skia.googlesource.com/skia/+/53d5c6e7448fb68766a05f7bee651aa25e5d7f2b

[bungeman-skia, 2016-04-05 20:24:43]

Description was changed from

==========
Test CTFonts for equality, not just name and style.

The current code looks up CTFonts from descriptors by just name and style.
This is not sufficient, for example Avenir Book and Avenir Roman have the
same family name but the same basic style, but are obviously not the same.

BUG=skia:5162
==========

to

==========
Test CTFonts for equality, not just name and style.

The current code looks up CTFonts from descriptors by just name and style.
This is not sufficient, for example Avenir Book and Avenir Roman have the
same family name but the same basic style, but are obviously not the same.

BUG=skia:5162
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[bungeman-skia, 2016-04-05 20:49:18]

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

[bungeman-skia, 2016-04-05 22:03:26]

https://codereview.chromium.org/1860993004/diff/20001/src/ports/SkFontHost_ma...
File src/ports/SkFontHost_mac.cpp (right):

https://codereview.chromium.org/1860993004/diff/20001/src/ports/SkFontHost_ma...
src/ports/SkFontHost_mac.cpp:2608: SkTypeface* face =
SkTypefaceCache::FindByProcAndRef(find_by_NameStyle, &cacheRequest);
Leaving this find_by_NameStyle here is trouble, which shows up in some gms. Some
gms like typefacestyles_kerningMac end up with some of the lines looking
different. Now that we're doing the other lookups correctly, there are a lot
more typefaces in the cache after running the font iteration gms and tests, so
it is more likely that the wrong one is going to be a cache hit. This is
normally observed as the fist line of typefacestyles_kerningMac being the
'wrong' font, it has the right name and style value, but it is not the one which
would have been selected had we done the lookup.

[bungeman-skia, 2016-04-06 20:22:57]

I ran this past blink layout tests with
https://codereview.chromium.org/1866773002/ and I don't think it changes
anything there. On the other hand, we aren't getting great coverage at the
moment on the trybots, filed a bug (linked to from the above issue) on that.

[bungeman-skia, 2016-04-07 22:52:11]

I have now run this on the Chromium trybots and this doesn't affect layout tests
at least. Please take a look, this may fix several issues currently being seen
in Chromium and CEF.

[mtklein, 2016-04-08 00:59:49]

lgtm, i think.

is there any hope of ever being able to write unit tests for any font code?

https://codereview.chromium.org/1860993004/diff/40001/src/ports/SkFontHost_ma...
File src/ports/SkFontHost_mac.cpp (left):

https://codereview.chromium.org/1860993004/diff/40001/src/ports/SkFontHost_ma...
src/ports/SkFontHost_mac.cpp:562: face = NewFromFontRef(ctFont.release(),
nullptr, nullptr, false);
this'd be easier to review if you keep the demorgans law refactoring bits
separate from the real logic changes...

not saying the refactor makes things worse, or better.  it'd just be nice as a
warmup or followup cl separate from the meat here.

[bungeman-skia, 2016-04-08 14:01:12]

The CQ bit was checked by bungeman@google.com

[commit-bot: I haz the power, 2016-04-08 14:01:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1860993004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1860993004/40001

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

Description was changed from

==========
Test CTFonts for equality, not just name and style.

The current code looks up CTFonts from descriptors by just name and style.
This is not sufficient, for example Avenir Book and Avenir Roman have the
same family name but the same basic style, but are obviously not the same.

BUG=skia:5162
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Test CTFonts for equality, not just name and style.

The current code looks up CTFonts from descriptors by just name and style.
This is not sufficient, for example Avenir Book and Avenir Roman have the
same family name but the same basic style, but are obviously not the same.

BUG=skia:5162
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/53d5c6e7448fb68766a05f7bee651aa25e5d7f2b
==========

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

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/53d5c6e7448fb68766a05f7bee651aa25e5d7f2b