Bug 44757 – [GStreamer] ImageGStreamer doesn't need to hold a Cairo surface

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Philippe Normand]

The BitmapImage created already holds it and destroy it when needed. So there could be cases where ImageGStreamerCairo tries to free the already freed surface in its destructor. It happened once on the 64-bits bot:

Thread 1 (Thread 1100):
#0  0x00007f476eb71f45 in raise () from /lib/libc.so.6
#1  0x00007f476eb74d80 in abort () from /lib/libc.so.6
#2  0x00007f476eb6b08a in __assert_fail () from /lib/libc.so.6
#3  0x00007f476fb2b046 in cairo_surface_destroy () from /usr/lib/libcairo.so.2
#4  0x00007f4774e9ed4d in ~ImageGStreamer (this=0xc64750, 
    __in_chrg=<value optimized out>)
    at ../../WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp:69
#5  0x00007f4774e9c197 in WTF::RefCounted<WebCore::ImageGStreamer>::deref (
    this=0xc64750) at ../../JavaScriptCore/wtf/RefCounted.h:139
#6  0x00007f4774e9c006 in WTF::derefIfNotNull<WebCore::ImageGStreamer> (
    ptr=0xc64750) at ../../JavaScriptCore/wtf/PassRefPtr.h:58
#7  0x00007f4774e9be85 in ~RefPtr (this=0x7fffffff9260, 
    __in_chrg=<value optimized out>) at ../../JavaScriptCore/wtf/RefPtr.h:58
#8  0x00007f4774e9a857 in WebCore::MediaPlayerPrivateGStreamer::paint (
    this=0xbf2c30, context=0x7fffffffb350, rect=...)
    at ../../WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1218
#9  0x00007f4774cabc02 in WebCore::MediaPlayer::paint (this=0xa2f0c0, 
    p=0x7fffffffb350, r=...)
    at ../../WebCore/platform/graphics/MediaPlayer.cpp:549
#10 0x00007f4774cb8e36 in WebCore::RenderVideo::paintReplaced (this=0xbab678, 
    paintInfo=..., tx=0, ty=-53)
    at ../../WebCore/rendering/RenderVideo.cpp:207
#11 0x00007f4774b9d33e in WebCore::RenderReplaced::paint (this=0xbab678, 
    paintInfo=..., tx=0, ty=-53)

[Philippe Normand]

Created attachment 65694 [details]
proposed patch

[Philippe Normand]

It happened again with media/video-seek-past-end-paused.html on the 30th of august:

http://webkit-bots.igalia.com/amd64/svn_66395.core-when_1283190163-_-who_DumpRenderTree-_-why_6.11551.trace.html

[Xan Lopez]

Comment on attachment 65694 [details]
proposed patch

I don't quite see in the code who will be handling the lifecycle of the surface. Can you point me to it?

[Xan Lopez]

Comment on attachment 65694 [details]
proposed patch

OK, I think it's in ImageCairo.cpp, the BitmapImage constructor steals the surface (I suppose this was desired, seems a bit obscure).

[Philippe Normand]

Thanks, see http://trac.webkit.org/changeset/66600 \m/ \m/