Bug 172428 – [CoordinatedGraphics] BitmapTexturePool does not release textures properly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gwang Yoon Hwang]

[CoordinatedGraphics] BitmapTexturePool does not release textures properly

[Gwang Yoon Hwang]

Created attachment 310809 [details]
Patch

[Build Bot]

Comment on attachment 310809 [details]
Patch

Attachment 310809 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3789763

New failing tests:
imported/w3c/web-platform-tests/media-source/mediasource-closed.html

[Build Bot]

Created attachment 310813 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Zan Dobersek]

Comment on attachment 310809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310809&action=review

> Source/WebCore/platform/graphics/texmap/BitmapTexturePool.h:60
> +        bool canBeReleased (double minUsedTime) const { return m_lastUsedTime < minUsedTime && m_texture->refCount() == 1; }

Not related to this patch, but do we properly manage BitmapTexture objects that are being constructed in different threads (GStreamer threads, main thread for WebGL/acc. canvas) and then used for rendering on the composition thread? Should BitmapTexture inherit from ThreadSafeRefCounted?

[Gwang Yoon Hwang]

(In reply to Zan Dobersek from comment #4)
> Comment on attachment 310809 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310809&action=review
> 
> > Source/WebCore/platform/graphics/texmap/BitmapTexturePool.h:60
> > +        bool canBeReleased (double minUsedTime) const { return m_lastUsedTime < minUsedTime && m_texture->refCount() == 1; }
> 
> Not related to this patch, but do we properly manage BitmapTexture objects
> that are being constructed in different threads (GStreamer threads, main
> thread for WebGL/acc. canvas) and then used for rendering on the composition
> thread? Should BitmapTexture inherit from ThreadSafeRefCounted?

At this point, bitmap textures for platform layers are not managed by bitmap texture pool in threaded compositing case. And those buffer's destruction working on should be done at the producer's thread.

I think BitmapTexture should be refcounted as a non-thread safe case. Since BitmapTexture uses GraphicsContext which is not thread-safe, and it is much clear creation and destruction should be done at the producer's thread.

What we are currently in platform layer's side, we are wrapping bitmap texture with a platform layer buffer, and passes ownership of buffer to thread to thread via unique_ptr.

[WebKit Commit Bot]

Comment on attachment 310809 [details]
Patch

Clearing flags on attachment: 310809

Committed r217213: <http://trac.webkit.org/changeset/217213>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.