[webkit-reviews] review granted: [Bug 80629] [chromium] Canvas acceleration is not restored after context loss : [Attachment 131350] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Stephen White
<senorblanco at chromium.org>'s request for review:
Bug 80629: [chromium] Canvas acceleration is not restored after context loss
https://bugs.webkit.org/show_bug.cgi?id=80629

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=131350&action=review


Looks good!

> Source/WebCore/platform/graphics/chromium/cc/CCRenderSurface.cpp:228
> +    return CCRenderSurfaceFilters::apply(m_filters,
m_contentsTexture->textureId(), m_contentRect.size(),
SharedGraphicsContext3D::get().get());

get().get() - nice. need some more data().data().data() up in here

> Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp:113
>      data->m_platformLayer = Canvas2DLayerChromium::create(context3D, size);

i think i'd find the ownership a bit easier to follow if this was a .release()
call, just to make it clear that ownership of the local is being transferred to
the Canvas2DLayerChromium and the layer was responsible for holding the context
alive. i couldn't tell when initially looking at this why this function was
taking a reference in a local and who was holding the ref after this function
returned.