[webkit-reviews] review granted: [Bug 36190] Lessen Include Path Build Fragility (FontPlatformData.h) : [Attachment 51045] Uploaded wrong version of diff. This is the correct file. (I.e., attachment 50975 with check-webkit-style fixes.)

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 36190: Lessen Include Path Build Fragility (FontPlatformData.h)
https://bugs.webkit.org/show_bug.cgi?id=36190

Attachment 51045: Uploaded wrong version of diff.  This is the correct file. 
(I.e., attachment 50975 with check-webkit-style fixes.)
https://bugs.webkit.org/attachment.cgi?id=51045&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> -    bool isHashTableDeletedValue() const {
> +    bool isHashTableDeletedValue() const
> +    {
>  #if defined(USE_FREETYPE)
>	   return m_pattern == hashTableDeletedFontValue();
>  #elif defined(USE_PANGO)
>	   return m_font == hashTableDeletedFontValue();
> +#elif PLATFORM(WIN)
> +	   return m_font.isHashTableDeletedValue();
>  #endif
>      };
>  
> +
>  #ifndef NDEBUG
>      String description() const;
>  #endif

Too much whitespace in there. (And the semicolon after the closing brace isn't
needed.)

> @@ -122,6 +161,13 @@ public:
>  
>      PangoContext* m_context;
>      PangoFont* m_font;
> +#elif PLATFORM(WIN)
> +private:
> +    void platformDataInit(HFONT font, float size, HDC hdc, WCHAR* faceName);


You could remove the "font" and "hdc" parameter names here.

> Index: WebCore/platform/graphics/cg/FontPlatformData.h

Having this Windows-specific file in the cg/ directory seems a little
confusing. Mac uses CG, too (and many files in the platform/graphics/cg
directory), for example. But I guess it's OK for now.

> +++ WebCore/platform/graphics/win/RefCountedHFONT.h	(revision 0)
> @@ -0,0 +1,59 @@
> +/*
> + * This file is part of the internal font implementation.  It should not be
included by anyone other than
> + * FontCairo.cpp, FontMac.cpp, FontWin.cpp and Font.cpp.

I don't think this comment is needed anymore.

r=me