[webkit-reviews] review granted: [Bug 83382] [Chromium] Refactor CCTextureUpdater to contain a raw pointer to a GraphicsContext3D. : [Attachment 136048] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted David Reveman
<reveman at chromium.org>'s request for review:
Bug 83382: [Chromium] Refactor CCTextureUpdater to contain a raw pointer to a
GraphicsContext3D.
https://bugs.webkit.org/show_bug.cgi?id=83382

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

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


Looks good. I'd like to hold a ref explicitly in the updater as well, just to
be safe (we currently manage the lifetime rather carefully in CCThreadProxy,
but somebody might not be so careful about lifetimes in a future refactor and I
don't want to leave the possibility of a dangling pointer).

> Source/WebCore/platform/graphics/chromium/cc/CCTextureUpdater.h:70
> +    GraphicsContext3D* m_context;

can this hold a RefPtr<> ? the lifetime should be maintained by the owner, but
it's safer to keep a ref here as well IMO

> Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.cpp:506
> +    m_currentTextureUpdaterOnImplThread = adoptPtr(new
CCTextureUpdater(m_layerTreeHostImpl->context(),
m_layerTreeHostImpl->contentsTextureAllocator(),
m_layerTreeHostImpl->layerRenderer()->textureCopier()));

Hm, we reset this on every frame?