[webkit-reviews] review granted: [Bug 61800] Ready Chromium port for Skia on Mac : [Attachment 95466] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Cary Clark <caryclark at google.com>'s
request for review:
Bug 61800: Ready Chromium port for Skia on Mac
https://bugs.webkit.org/show_bug.cgi?id=61800

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

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

>>> Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438
>>> +	 gfx::SkiaBitLocker
bitLocker(drawingContext->platformContext()->canvas());
>> 
>> We might want a WebCore helper which knows how to bitlock directly from a
GraphicsContext instead of having to do this canvas lookup every time.
>> 
>> We could also use an abstraction around the drawingContext which would lock
in teh skia case, and otherwise just give you back the CGContext.  That would
remove a bunch of these #if USE(SKIA) branches.
> 
> The bitlock/canvas lookup combination only happens four times in the sources.
Also, the #if USE(SKIA) scaffolding is temporary; once the Chrome Mac port uses
Skia, it will be removed.

I don't think "temporary" is every a very good excuse for ugly code.  But I
think this patch is OK.  Best of luck. :)