[webkit-reviews] review granted: [Bug 65543] [Chromium] Fix OOP font loading to work on 10.6.6 and above. : [Attachment 102656] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Jeremy Moskovich
<playmobil at google.com>'s request for review:
Bug 65543: [Chromium] Fix OOP font loading to work on 10.6.6 and above.
https://bugs.webkit.org/show_bug.cgi?id=65543

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102656&action=review


The code looks good to me overall. (If this started failing in 10.6.6 why
wasn't it found until now?) Please fix the nits upon landing. r=me

> Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:132
> +    if (font.get())

You should be able to just write "if (font) ...".

> Source/WebCore/platform/graphics/chromium/CrossProcessFontLoading.mm:138
> +    if (!PlatformBridge::loadFont(nsFont, &container, &fontID)) {

No braces on single-line if statements.

> Source/WebKit/chromium/public/mac/WebSandboxSupport.h:59
> +    virtual bool loadFont(NSFont* srcFont, ATSFontContainerRef*, uint32_t*
fontID) = 0;

I see you've already landed the downstream override of the new signature --
great.