[webkit-reviews] review granted: [Bug 130529] Add WebCore::IOSurface wrapper : [Attachment 227333] style

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 130529: Add WebCore::IOSurface wrapper
https://bugs.webkit.org/show_bug.cgi?id=130529

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=227333&action=review


> Source/WebCore/platform/graphics/cocoa/IOSurface.mm:95
> +    ASSERT(m_surface);

I don't think this is very useful; it will fire if height or width is zero
(maybe?), and/or on system that have run out of graphics memory.

> Source/WebCore/platform/graphics/cocoa/IOSurface.mm:150
> +    uint32_t previousState;

Maybe initailize this to 0 in case the IOSurfaceSetPurgeable fails and we use
uninitialized bits.

> Source/WebCore/platform/graphics/cocoa/IOSurface.mm:151
> +    IOReturn ret = IOSurfaceSetPurgeable(m_surface.get(),
kIOSurfacePurgeableKeepCurrent, &previousState);

Weird that a const function is calling IOSurfaceSetPurgeable(). Is that call a
no-op?

> Source/WebCore/platform/graphics/cocoa/IOSurface.mm:163
> +    IOReturn ret = IOSurfaceSetPurgeable(m_surface.get(),
kIOSurfacePurgeableKeepCurrent, &previousState);

Again.

> Source/WebCore/platform/graphics/cocoa/IOSurface.mm:171
> +    IOReturn ret = IOSurfaceSetPurgeable(m_surface.get(), isPurgeable ?
kIOSurfacePurgeableVolatile : kIOSurfacePurgeableNonVolatile, &previousState);

Bored now.

Oh wait, this is a real setter.