[webkit-reviews] review denied: [Bug 62999] Use Skia if Skia on Mac Chrome is enabled : [Attachment 97970] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Cary Clark
<caryclark at google.com>'s request for review:
Bug 62999: Use Skia if Skia on Mac Chrome is enabled
https://bugs.webkit.org/show_bug.cgi?id=62999

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=97970&action=review

I'm having trouble figuring out what your design is here.  I also don't
understand
why we need another "use skia" macro when we already have WEBKIT_USING_SKIA. 
How
is that existing macro not sufficient??

> Source/JavaScriptCore/wtf/Platform.h:549
> +#if USE(SKIA_ON_MAC_CHROME)

nit: in WebKit, we don't say "Chrome", we say "Chromium"

> Source/WebKit/chromium/public/WebCommon.h:42
> +    #if !defined(__APPLE__) || defined(USE_SKIA)

it looks like you are trying to have the embedder define a global USE_SKIA
variable.
but in Platform.h, the code looks for a WTF_USE_SKIA_ON_MAC_CHROME define.  it
isn't
clear how that gets set, or why we need both USE_SKIA and
WTF_USE_SKIA_ON_MAC_CHROME.