Bug 157172 - [OpenType] OpenTypeVerticalData object should not be created if the font is not OpenType
Summary: [OpenType] OpenTypeVerticalData object should not be created if the font is n...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-29 03:34 PDT by Carlos Garcia Campos
Modified: 2016-05-03 01:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.19 KB, patch)
2016-04-29 03:39 PDT, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2016-04-29 03:34:10 PDT
It's a bit weird that the object is always created and has an isOpenType() method to check whether it's an OpenType or not. The caller is always deleting the object when it's not an OpenType, so it would be better if the create method returned nullptr instead of creating the object when the font is not OpenType.
Comment 1 Carlos Garcia Campos 2016-04-29 03:39:49 PDT
Created attachment 277690 [details]
Patch
Comment 2 Darin Adler 2016-04-29 09:35:16 PDT
Comment on attachment 277690 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277690&action=review

> Source/WebCore/platform/graphics/FontCache.cpp:322
>      auto addResult = fontVerticalDataCache().add(platformData, nullptr);
> -    if (addResult.isNewEntry) {
> -        RefPtr<OpenTypeVerticalData> data = OpenTypeVerticalData::create(platformData);
> -        addResult.iterator->value = data->isOpenType() ? WTFMove(data) : nullptr;
> -    }
> +    if (addResult.isNewEntry)
> +        addResult.iterator->value = OpenTypeVerticalData::create(platformData);

This is now a perfect candidate for using the new "ensure" function instead of add.
Comment 3 Carlos Garcia Campos 2016-05-03 01:17:46 PDT
Committed r200364: <http://trac.webkit.org/changeset/200364>