Bug 172428

Summary: [CoordinatedGraphics] BitmapTexturePool does not release textures properly
Product: WebKit Reporter: Gwang Yoon Hwang <yoon>
Component: New BugsAssignee: Gwang Yoon Hwang <yoon>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cgarcia, cmarcelo, commit-queue, kondapallykalyan, luiz, noam, zan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews112 for mac-elcapitan none

Description Gwang Yoon Hwang 2017-05-21 10:13:57 PDT
[CoordinatedGraphics] BitmapTexturePool does not release textures properly
Comment 1 Gwang Yoon Hwang 2017-05-21 10:14:41 PDT
Created attachment 310809 [details]
Patch
Comment 2 Build Bot 2017-05-21 11:33:39 PDT
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
Comment 3 Build Bot 2017-05-21 11:33:40 PDT
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
Comment 4 Zan Dobersek 2017-05-22 02:32:52 PDT
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?
Comment 5 Gwang Yoon Hwang 2017-05-22 02:50:26 PDT
(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.
Comment 6 WebKit Commit Bot 2017-05-22 03:19:09 PDT
Comment on attachment 310809 [details]
Patch

Clearing flags on attachment: 310809

Committed r217213: <http://trac.webkit.org/changeset/217213>
Comment 7 WebKit Commit Bot 2017-05-22 03:19:11 PDT
All reviewed patches have been landed.  Closing bug.