Bug 99430 – hb_face_t instances should not depend on FontPlatformData

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Kenichi Ishibashi]

The lifetime of hb_face_t instances should correspond with the lifetime of underlying font data(e.g. SkTypeface and CTFont). HarfBuzzNGFace has its cache mechanism to allow hb_face_t instances live as long as underlying font data live. Since the lifetime of underlying font data and FontPlatformData are different, hb_face_t instances should not depend on FontPlatformData.

harfbuzzSkiaGetTable(), harfbuzzCoreTextGetTable() and harfbuzzCairoGetTable() violate this restriction. These functions uses FontPlatformData to get font tables. We should pass underlying font data (or handle of underlying font data) to these functions instead. Otherwise, these functions can access freed FontPlatformData objects (http://crbug.com/156015 is an instance).

Note: we can use FontPlatformData in HarfBuzzNGFace::createFont(). This should be safe.
Note: we need not add a reference to underlying font data because the cache mechanism takes care of it.

[Kenichi Ishibashi]

Created attachment 168886 [details]
Patch

[Kenichi Ishibashi]

Kent-san, could you take a look? The change itself is trivial. I'm ccing you on crbug.com.

I confirmed the fix on chromium linux, and compiled the patch for chromium mac.

I'll wait and see whether efl port can compile the patch. (for changes of harfbuzzCairoGetTable())

[Kent Tamura]

Comment on attachment 168886 [details]
Patch

rubber-stamped

[Kenichi Ishibashi]

Comment on attachment 168886 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 168886 [details]
Patch

Clearing flags on attachment: 168886

Committed r131432: <http://trac.webkit.org/changeset/131432>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.