Introduce typeface cache in blink::FontCache

Introduce typeface cache in blink::FontCache

blink::FontCache stores fonts as FontPlatformData, that is associated
with the font size. However on other platform than Mac, the backing
SkTypeface are independent of the size and can be shared among font
queries that have different sizes.

This CL adds a cache to hold SkTypeface on non-Mac platform.

TODO: 
 * Measure the performance on Windows.
 * Set up finch experiment to measure it in wild.

For more context, see https://codereview.chromium.org/1919183002/

BUG=

[tzik, 2016-04-29 16:46:36]

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

[commit-bot: I haz the power, 2016-04-29 16:46:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1931393002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1931393002/1

[commit-bot: I haz the power, 2016-04-29 16:48:50]

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

[commit-bot: I haz the power, 2016-04-29 16:48:51]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[tzik, 2016-04-29 17:00:59]

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

[commit-bot: I haz the power, 2016-04-29 17:01:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1931393002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1931393002/20001

[tzik, 2016-04-29 17:59:02]

Description was changed from

==========
Introduce Typeface cache in blink::FontCache

BUG=
==========

to

==========
Introduce Typeface cache in blink::FontCache

TODO: 
 * Measure on Windows.
 * Set up finch experiment to measure in wild

BUG=
==========

[tzik, 2016-04-29 18:03:59]

Description was changed from

==========
Introduce Typeface cache in blink::FontCache

TODO: 
 * Measure on Windows.
 * Set up finch experiment to measure in wild

BUG=
==========

to

==========
Introduce Typeface cache in blink::FontCache

TODO: 
 * Measure on Windows.
 * Set up finch experiment to measure in wild

Will remove before landing:
Context: https://codereview.chromium.org/1919183002/

BUG=
==========

[commit-bot: I haz the power, 2016-04-29 18:24:45]

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

[commit-bot: I haz the power, 2016-04-29 18:24:46]

Dry run: This issue passed the CQ dry run.

[tzik, 2016-04-29 20:06:02]

Description was changed from

==========
Introduce Typeface cache in blink::FontCache

TODO: 
 * Measure on Windows.
 * Set up finch experiment to measure in wild

Will remove before landing:
Context: https://codereview.chromium.org/1919183002/

BUG=
==========

to

==========
blink::FontCache stores fonts as FontPlatformData, that is associated
with the font size. However on other platform than Mac, the backing
SkTypeface are independent of the size and can be shared among font
queries that have different sizes.

This CL adds a cache to hold SkTypeface on non-Mac platform.

TODO: 
 * Measure the performance on Windows.
 * Set up finch experiment to measure it in wild.

For more context, see https://codereview.chromium.org/1919183002/

BUG=
==========

[tzik, 2016-04-29 20:06:02]

tzik@chromium.org changed reviewers:
+ eae@chromium.org

[tzik, 2016-04-29 20:06:34]

Description was changed from

==========
blink::FontCache stores fonts as FontPlatformData, that is associated
with the font size. However on other platform than Mac, the backing
SkTypeface are independent of the size and can be shared among font
queries that have different sizes.

This CL adds a cache to hold SkTypeface on non-Mac platform.

TODO: 
 * Measure the performance on Windows.
 * Set up finch experiment to measure it in wild.

For more context, see https://codereview.chromium.org/1919183002/

BUG=
==========

to

==========
Introduce typeface cache in blink::FontCache

blink::FontCache stores fonts as FontPlatformData, that is associated
with the font size. However on other platform than Mac, the backing
SkTypeface are independent of the size and can be shared among font
queries that have different sizes.

This CL adds a cache to hold SkTypeface on non-Mac platform.

TODO: 
 * Measure the performance on Windows.
 * Set up finch experiment to measure it in wild.

For more context, see https://codereview.chromium.org/1919183002/

BUG=
==========

[eae, 2016-04-30 01:03:12]

This is great, thanks for working on this.
One thought though, what if instead we remove m_fontSize from FontCacheKey for
windows and linux?

[tzik, 2016-04-30 12:08:33]

On 2016/04/30 01:03:12, eae wrote:
> This is great, thanks for working on this.
> One thought though, what if instead we remove m_fontSize from FontCacheKey for
> windows and linux?

Hmm, that looks less transparent. If we remove m_fontSize from FontCacheKey, we
should also remove the font size from FontPlatformData and its users.

[eae, 2016-05-01 00:11:38]

On 2016/04/30 12:08:33, tzik wrote:
> On 2016/04/30 01:03:12, eae wrote:
> > This is great, thanks for working on this.
> > One thought though, what if instead we remove m_fontSize from FontCacheKey
for
> > windows and linux?
> 
> Hmm, that looks less transparent. If we remove m_fontSize from FontCacheKey,
we
> should also remove the font size from FontPlatformData and its users.

True but it would avoid adding yet another layer of caches.

[tzik, 2016-05-03 08:58:15]

On 2016/05/01 00:11:38, eae wrote:
> On 2016/04/30 12:08:33, tzik wrote:
> > On 2016/04/30 01:03:12, eae wrote:
> > > This is great, thanks for working on this.
> > > One thought though, what if instead we remove m_fontSize from FontCacheKey
> for
> > > windows and linux?
> > 
> > Hmm, that looks less transparent. If we remove m_fontSize from FontCacheKey,
> we
> > should also remove the font size from FontPlatformData and its users.
> 
> True but it would avoid adding yet another layer of caches.

Ok. Let me try it.
So we should probably do:
 * Move the font size and FontRenderStyle from FontPlatformData to
SimpleFontData.
 * Remove the font size from FontCacheKey and update FontDataCache.

[kojii, 2016-05-04 15:52:36]

It looks like WebKit is trying a similar attempt, though their intention isn't
clear from the bug.
https://bugs.webkit.org/show_bug.cgi?id=157061

[tzik, 2016-05-06 01:43:15]

On 2016/05/04 15:52:36, kojii wrote:
> It looks like WebKit is trying a similar attempt, though their intention isn't
> clear from the bug.
> https://bugs.webkit.org/show_bug.cgi?id=157061

Interesting. Yes, it's similar to mine.
I thought we can not move the font size out from FontPlatformData on Mac, but
they actually did it.

[tzik, 2016-05-10 03:56:45]

tzik@chromium.org changed reviewers:
+ kojii@chromium.org

[tzik, 2016-05-10 03:58:57]

On 2016/05/06 01:43:15, tzik wrote:
> On 2016/05/04 15:52:36, kojii wrote:
> > It looks like WebKit is trying a similar attempt, though their intention
isn't
> > clear from the bug.
> > https://bugs.webkit.org/show_bug.cgi?id=157061
> 
> Interesting. Yes, it's similar to mine.
> I thought we can not move the font size out from FontPlatformData on Mac, but
> they actually did it.

Hmm, it looks hard or even impossible to me to remove the size on Mac. Let me
leave the size as is on Mac.

[eae, 2016-05-10 18:37:35]

On 2016/05/10 03:58:57, tzik wrote:
> On 2016/05/06 01:43:15, tzik wrote:
> > On 2016/05/04 15:52:36, kojii wrote:
> > > It looks like WebKit is trying a similar attempt, though their intention
> isn't
> > > clear from the bug.
> > > https://bugs.webkit.org/show_bug.cgi?id=157061
> > 
> > Interesting. Yes, it's similar to mine.
> > I thought we can not move the font size out from FontPlatformData on Mac,
but
> > they actually did it.
> 
> Hmm, it looks hard or even impossible to me to remove the size on Mac. Let me
> leave the size as is on Mac.

Ok, we could do that as an interim step. Is Patch 6 ready for review or do you
have other changes you want to make first?

[tzik, 2016-05-17 07:59:09]

Patchset #7 (id:120001) has been deleted