[webkit-reviews] review granted: [Bug 66935] [Chromium] Change OOP Font loading code to use CGFont*() APIs : [Attachment 105606] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Jeremy Moskovich
<playmobil at google.com>'s request for review:
Bug 66935: [Chromium] Change OOP Font loading code to use CGFont*() APIs
https://bugs.webkit.org/show_bug.cgi?id=66935

Attachment 105606: Patch
https://bugs.webkit.org/attachment.cgi?id=105606&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105606&action=review


I think this is fine.  It could be better with more RetainPtr usage.

> Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:135
> +    CGFontRef cgFont;

I think ideally we'd use a RetainPtr here.

> Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:150
> +	   CGFontRelease(cgFont);

Then this goes away.

> Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:162
> +  MemoryActivatedFont* font = new MemoryActivatedFont(fontID, nsFont,
cgFont);
>    return adoptRef(font);

You don't need the local here.