[webkit-reviews] review denied: [Bug 22586] Remove FontDatabase from Cairo Build of WebKit (Windows) : [Attachment 25658] Remove FontDatabase from Windows Cairo build.

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Brent Fulgham
<bfulgham at gmail.com>'s request for review:
Bug 22586: Remove FontDatabase from Cairo Build of WebKit (Windows)
https://bugs.webkit.org/show_bug.cgi?id=22586

Attachment 25658: Remove FontDatabase from Windows Cairo build.
https://bugs.webkit.org/attachment.cgi?id=25658&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 38852)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,15 @@
> +2008-11-30  Brent Fulgham  <bfulgham at gmail.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=22586
> +	   Remove FontDatabase files from Cairo Windows build. 

Maybe you should make the name of the bug more general since you're making
other fixes in this change.

> +    static Color focusRingColor;
> +
> +    if (!focusRingColor.isValid())
> +	   focusRingColor = aquaFocusRingColor;

You can do this all in one statement:

static Color focusRingColor = aquaFocusRingColor;

> +    void populateFontDatabase() { /* Do nothing */ }

Why not just add this one stub to platform/win/TemporaryLinkStubs.cpp?

r- because I don't think it's good to add another link stub file with a
different name that's only for Cairo (at least not if the name doesn't indicate
it's only for Cairo).