Bug 171205

Summary: [GStreamer] WebKit improperly handles missing GStreamer elements
Product: WebKit Reporter: dirk.fizzlebeef
Component: MediaAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Major CC: bugs-noreply, calvaris, cgarcia, clopez, dirk.fizzlebeef, magomez, mcatanzaro, pnormand, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=171211
https://bugs.webkit.org/show_bug.cgi?id=171161
Attachments:
Description Flags
Backtrace
none
Patch mcatanzaro: review+

Description dirk.fizzlebeef 2017-04-23 15:15:49 PDT
When I go to midori-browser.org in Midori, I can see the page for an instant before it goes blank. The following messages are produced in the console: 

    (WebKitWebProcess:27301): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed 

    ** (WebKitWebProcess:27301): CRITICAL **: file /var/tmp/portage/media-libs/gst-plugins-base-1.10.3/work/gst-plugins-base-1.10.3/gst-libs/gst/audio/gstaudioringbuffer.c: line 1993 (gst_audio_ring_buffer_set_channel_positions): should not be reached 


Second, every time I attempt to open the web inspector on any page, it shows an error page that begins with "Web Inspector encountered an internal error" and this error is in the console: 

    (midori4:27162): Gtk-WARNING **: Allocating size to WebKitWebViewBase 0x10f8700 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate? 

I asked about these bugs in the #midori channel on Freenode and was told it's a regression in WebKit GTK, so I'm opening the bug here instead of there.
Comment 1 Michael Catanzaro 2017-04-23 16:29:17 PDT
What version of WebKitGTK+?

(In reply to dirk.fizzlebeef from comment #0)
> When I go to midori-browser.org in Midori, I can see the page for an instant
> before it goes blank. The following messages are produced in the console: 
> 
>     (WebKitWebProcess:27301): GLib-GObject-CRITICAL **: g_object_set:
> assertion 'G_IS_OBJECT (object)' failed 

This is the first problem, so let's worry about this one first. Could you get a backtrace to it please, using G_DEBUG=fatal-criticals?

> Second, every time I attempt to open the web inspector on any page, it shows
> an error page that begins with "Web Inspector encountered an internal error"

Since this web inspector error is unrelated, please file a second bug for this.
Comment 2 dirk.fizzlebeef 2017-04-23 17:11:51 PDT
(In reply to Michael Catanzaro from comment #1)
> What version of WebKitGTK+?

Sorry about that. I'm using version 2.16.1 with the following features:

    [ Legend : U - final flag setting for installation]
    [        : I - package is installed with flag     ]
    [ Colors : set, unset                             ]
     * Found these USE flags for net-libs/webkit-gtk-2.16.1:
     U I
     - - coverage      : Enable code coverage support
     - - doc           : Add extra documentation (API, Javadoc, etc). It is recommended to enable per package instead of globally
     + + egl           : Enable EGL support
     - - geolocation   : Enable geolocation support through app-misc/geoclue
     - - gles2         : Enable GLESv2 support
     - - gnome-keyring : Enable support for storing passwords via gnome-keyring
     + + gstreamer     : Add support for media-libs/gstreamer (Streaming media)
     - - introspection : Add support for GObject based introspection
     + + jit           : Enable just-in-time compilation for improved performance. May prevent use of some PaX memory protection features in Gentoo Hardened.
     - - libnotify     : Enable desktop notification support
     - - nsplugin      : Build plugin for browsers supporting the Netscape plugin architecture (that is almost any modern browser)
     + + opengl        : Add support for OpenGL (3D graphics)
     - - spell         : Add dictionary support
     - - test          : Workaround to pull in packages needed to run with FEATURES=test. Portage-2.1.2 handles this internally, so don't set it in make.conf/package.use
                         anymore
     - - wayland       : Enable dev-libs/wayland backend
     + + webgl         : Build support for the WebGL HTML API using virtual/opengl

> > When I go to midori-browser.org in Midori, I can see the page for an instant
> > before it goes blank. The following messages are produced in the console: 
> > 
> >     (WebKitWebProcess:27301): GLib-GObject-CRITICAL **: g_object_set:
> > assertion 'G_IS_OBJECT (object)' failed 
> 
> This is the first problem, so let's worry about this one first. Could you
> get a backtrace to it please, using G_DEBUG=fatal-criticals?

I ran `G_DEBUG=fatal_criticals gdb midori` and hit that page, but I didn't see any additional information. I also tried setting `G_DEBUG=fatal-criticals` as well as setting it inside gdb using `set environment G_DEBUG=fatal_criticals` and `set environment G_DEBUG=fatal-criticals`. (I tried both because the glib docs say it is an underscore.) However, I only see a little additonal information:

    RenderNamedFlowThread 0x7fff90b7bc00 willBeDestroyed

    (WebKitWebProcess:18169): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed
    [New Thread 0x7fff7a12f700 (LWP 18334)]
    [Thread 0x7fff7892c700 (LWP 18174) exited]
    [Thread 0x7fff88be3700 (LWP 18043) exited]

> > Second, every time I attempt to open the web inspector on any page, it shows
> > an error page that begins with "Web Inspector encountered an internal error"
> 
> Since this web inspector error is unrelated, please file a second bug for
> this.

Done: https://bugs.webkit.org/show_bug.cgi?id=171207
Comment 3 Michael Catanzaro 2017-04-23 17:48:50 PDT
It's G_DEBUG=fatal-criticals with the hyphen, not the underscore. That will cause the web process to crash when it hits the critical, so you can get a backtrace out of coredumpctl. (If you don't have coredumpctl enabled, you'll have to attach to the web process with gdb before navigating to that page.)
Comment 4 dirk.fizzlebeef 2017-04-23 19:16:33 PDT
Sorry, I don't understand. What do I need to do?

By the way, I installed surf and ran into the same issue on the same website.
Comment 5 Carlos Alberto Lopez Perez 2017-04-23 19:44:37 PDT
Can you try to run a new instance of the browser after exporting this environment variable?

export WEBKIT_DISABLE_COMPOSITING_MODE=1


Does it make a difference?


Can you upload a txt file with the output of running the command glxinfo on your computer?
Comment 6 Michael Catanzaro 2017-04-23 20:18:44 PDT
(In reply to dirk.fizzlebeef from comment #4)
> Sorry, I don't understand. What do I need to do?
> 
> By the way, I installed surf and ran into the same issue on the same website.

You need to make the WebKitWebProcess crash on that critical, and then get a backtrace from the resulting core dump. That way we can see where the critical is occurring. Instructions for Gentoo:

https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Backtraces

Note: it's much easier to just let it crash and then open the backtrace in coredumpctl if you have it. 'coredumpctl gdb'.
Comment 7 Michael Catanzaro 2017-04-23 20:19:48 PDT
Hm, since you use Gentoo, please do make sure you have built all packages that appear in the backtrace with debuginfo. We don't want to see missing ????? lines.
Comment 8 Carlos Alberto Lopez Perez 2017-04-23 20:23:01 PDT
(In reply to dirk.fizzlebeef from comment #4)
> Sorry, I don't understand. What do I need to do?
> 
> By the way, I installed surf and ran into the same issue on the same website.

Here are some good instructions about how to generate a coredump and obtain a backtrace from it: https://trac.webkit.org/wiki/WebKitGTK/Debugging#Gettingabacktracefromacoredump
Comment 9 Michael Catanzaro 2017-04-23 20:27:43 PDT
Hmmm actually, I can reproduce this, except for the criticals you see, which must be a separate issue. Disabling compositing mode makes no difference: the Midori webpage disappears soon either way. (I doubt it's using compositing mode anyway.) I think this is probably bug #171161, but let's proceed separately for now. And please do still post a backtrace for that first critical, since I can't reproduce that.

Note: if I load the Midori page with trunk I hit this assertion in image decoders:

ASSERTION FAILED: m_deletionHasBegun
../../Source/WTF/wtf/RefCounted.h(84) : WTF::RefCountedBase::~RefCountedBase()
1   0x7fceae2bae31 /home/mcatanzaro/Projects/GNOME/install/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7fceae2bae31]
2   0x7fceb680990d /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF14RefCountedBaseD1Ev+0x3f) [0x7fceb680990d]
3   0x7fceb85ab3ec /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF10RefCountedIN7WebCore12ImageDecoderEED1Ev+0x18) [0x7fceb85ab3ec]
4   0x7fceb85aafc4 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore12ImageDecoderD1Ev+0x70) [0x7fceb85aafc4]
5   0x7fceb85bc448 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15PNGImageDecoderD1Ev+0x3c) [0x7fceb85bc448]
6   0x7fceb85bc464 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15PNGImageDecoderD0Ev+0x18) [0x7fceb85bc464]
7   0x7fceb85b80c7 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZNKSt14default_deleteIN7WebCore15PNGImageDecoderEEclEPS1_+0x23) [0x7fceb85b80c7]
8   0x7fceb85b7343 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZNSt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS1_EED1Ev+0x47) [0x7fceb85b7343]
9   0x7fceb85b8647 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF16VectorDestructorILb1ESt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS3_EEE8destructEPS6_S8_+0x2e) [0x7fceb85b8647]
10  0x7fceb85b7954 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF20VectorTypeOperationsISt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS3_EEE8destructEPS6_S8_+0x23) [0x7fceb85b7954]
11  0x7fceb85b8978 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF6VectorISt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS3_EELm0ENS_15CrashOnOverflowELm16EE6shrinkEm+0x78) [0x7fceb85b8978]
12  0x7fceb85b7be1 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF6VectorISt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS3_EELm0ENS_15CrashOnOverflowELm16EE14shrinkCapacityEm+0x55) [0x7fceb85b7be1]
13  0x7fceb85b7083 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN3WTF6VectorISt10unique_ptrIN7WebCore15PNGImageDecoderESt14default_deleteIS3_EELm0ENS_15CrashOnOverflowELm16EE5clearEv+0x1d) [0x7fceb85b7083]
14  0x7fceb85b5d36 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ICOImageDecoder9setFailedEv+0x30) [0x7fceb85b5d36]
15  0x7fceb85b65d1 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ICOImageDecoder13decodeAtIndexEm+0x3f1) [0x7fceb85b65d1]
16  0x7fceb85b603a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ICOImageDecoder6decodeEmb+0x5a) [0x7fceb85b603a]
17  0x7fceb85b5d00 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ICOImageDecoder18frameBufferAtIndexEm+0x74) [0x7fceb85b5d00]
18  0x7fceb85a9666 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore12ImageDecoder23createFrameImageAtIndexEmNS_16SubsamplingLevelERKNS_15DecodingOptionsE+0x7a) [0x7fceb85a9666]
19  0x7fceb7e75dab /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ImageFrameCache25frameAtIndexCacheIfNeededEmNS_10ImageFrame7CachingERKSt8optionalINS_16SubsamplingLevelEE+0x1a1) [0x7fceb7e75dab]
20  0x7fceb7e7a85b /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ImageFrameCache33frameMetadataAtIndexCacheIfNeededIN3WTF6RefPtrI14_cairo_surfaceEEJNS_10ImageFrame7CachingERNS_16SubsamplingLevelEEEET_mMS6_KFSA_vEPSt8optionalISA_EDpOT0_+0xd1) [0x7fceb7e7a85b]
21  0x7fceb7e76789 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore15ImageFrameCache30frameImageAtIndexCacheIfNeededEmNS_16SubsamplingLevelE+0x67) [0x7fceb7e76789]
22  0x7fceb7e7da6a /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11ImageSource30frameImageAtIndexCacheIfNeededEmNS_16SubsamplingLevelEPKNS_15GraphicsContextE+0x50) [0x7fceb7e7da6a]
23  0x7fceb7df42a8 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11BitmapImage30frameImageAtIndexCacheIfNeededEmNS_16SubsamplingLevelEPKNS_15GraphicsContextE+0xfc) [0x7fceb7df42a8]
24  0x7fceb7df4328 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN7WebCore11BitmapImage26nativeImageForCurrentFrameEPKNS_15GraphicsContextE+0x38) [0x7fceb7df4328]
25  0x7fceb6dac2ba /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x56fc2ba) [0x7fceb6dac2ba]
26  0x7fceb6dac449 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x56fc449) [0x7fceb6dac449]
27  0x7fceb6dac8a6 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x56fc8a6) [0x7fceb6dac8a6]
28  0x7fceb69fe3e2 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit21WebIconDatabaseClient23iconDataReadyForPageURLEPNS_15WebIconDatabaseEPN3API3URLE+0x5c) [0x7fceb69fe3e2]
29  0x7fceb69fb6dd /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit15WebIconDatabase29notifyIconDataReadyForPageURLERKN3WTF6StringE+0x49) [0x7fceb69fb6dd]
30  0x7fceb69fb321 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit15WebIconDatabase27didImportIconDataForPageURLERKN3WTF6StringE+0x23) [0x7fceb69fb321]
31  0x7fceb7bdb929 /home/mcatanzaro/Projects/GNOME/install/lib/libwebkit2gtk-4.0.so.37(+0x652b929) [0x7fceb7bdb929]
Comment 10 dirk.fizzlebeef 2017-04-23 20:33:42 PDT
(In reply to Carlos Alberto Lopez Perez from comment #5)
> Can you try to run a new instance of the browser after exporting this
> environment variable?
> 
> export WEBKIT_DISABLE_COMPOSITING_MODE=1
> 
> 
> Does it make a difference?

No difference.

> Can you upload a txt file with the output of running the command glxinfo on
> your computer?

http://ix.io/rDe

I'll work on getting that backtrace...
Comment 11 Michael Catanzaro 2017-04-23 20:34:23 PDT
I filed bug #171211 for the image decoder crash. Note: the attachment in that bug is an example of what your backtrace to the g_object_set() critical should look like.
Comment 12 dirk.fizzlebeef 2017-04-25 18:12:24 PDT
I'm still working on getting that core dump, but I'm having problems due to not really knowing what I'm doing. I'll link to the details in case anyone here can help troubleshoot on Gentoo: https://forums.gentoo.org/viewtopic-p-8060872.html#8060872
Comment 13 Michael Catanzaro 2017-04-25 18:44:43 PDT
Looks like you're on the right track... everything looks right except for the missing debuginfo. I don't know how to help with that on Gentoo. I would have expected adding -ggdb to CFLAGS would have done the trick. Maybe try removing splitdebug; that would make the build take longer (you might even think the linker has hung, even though it's still working) but that's my best guess as to the problem. Alternatively you could try changing the CMake build type using -DCMAKE_BUILD_TYPE=Debug, which should work just as well, though I'm not sure how to do that in make.conf.
Comment 14 dirk.fizzlebeef 2017-04-25 21:31:57 PDT
Created attachment 308210 [details]
Backtrace
Comment 15 Carlos Garcia Campos 2017-04-25 23:57:09 PDT
Could it be that gst_element_factory_make("videoflip", nullptr) is failing? Miguel?
Comment 16 Miguel Gomez 2017-04-26 00:31:11 PDT
(In reply to Carlos Garcia Campos from comment #15)
> Could it be that gst_element_factory_make("videoflip", nullptr) is failing?
> Miguel?

Checking the backtrace that seems to be the problem, yes. Seems that gst_element_factory_make is not able to create the videoflip element. Probably it was not enabled during the compilation of gst-plugins-good. You can check this by running:

gst-inspect-1.0 videoflip

That will show the plugin info or a message that it's not present.
Comment 17 Michael Catanzaro 2017-04-26 10:29:14 PDT
This is still a WebKit bug. There's no reason to expect video playback to work if GStreamer elements are missing, but WebKit should handle this gracefully without any critical warnings. (It's fine to print warning-level warnings about this misconfiguration, of course, but criticals are always application bugs.)
Comment 18 dirk.fizzlebeef 2017-04-26 18:37:49 PDT
(In reply to Miguel Gomez from comment #16)
> Checking the backtrace that seems to be the problem, yes. Seems that
> gst_element_factory_make is not able to create the videoflip element.
> Probably it was not enabled during the compilation of gst-plugins-good. You
> can check this by running:
> 
> gst-inspect-1.0 videoflip
> 
> That will show the plugin info or a message that it's not present.

When I run that command I get "No such element or plugin 'videoflip'".

Actually, I don't have gst-plugins-good at all. Is it a required dependency? If so then I can open a Gentoo bug against that package.
Comment 19 Michael Catanzaro 2017-04-26 20:16:42 PDT
It's required for multimedia playback, but WebKit should definitely not crash like this with it missing. It should just fail to play media.

Please file a separate bug for the web inspector crash; that's a different issue.
Comment 20 dirk.fizzlebeef 2017-04-27 08:08:19 PDT
(In reply to Michael Catanzaro from comment #19)
> Please file a separate bug for the web inspector crash; that's a different
> issue.

Already did: https://bugs.webkit.org/show_bug.cgi?id=171207
Comment 21 Philippe Normand 2018-02-08 04:12:53 PST
Created attachment 333368 [details]
Patch
Comment 22 Michael Catanzaro 2018-02-08 08:16:38 PST
Comment on attachment 333368 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2187
>          GstElement* videoFlip = gst_element_factory_make("videoflip", nullptr);

Do we need runtime checks after every call to gst_element_factory_make() in WebKit? Probably, right?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2190
> +            g_object_set(m_pipeline.get(), "video-filter", videoFlip, nullptr);

How does the memory management work here? Convention is for the property setter to take its own ref on the object, so now the pipeline should have its own ref of videoFlip. But you don't unref videoFlip yourself here. So it looks like it's leaking.

If GStreamer isn't taking its own ref on videoFlip, that's really unexpected and weird IMO.
Comment 23 Philippe Normand 2018-02-08 09:27:57 PST
(In reply to Michael Catanzaro from comment #22)
> Comment on attachment 333368 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=333368&action=review
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2187
> >          GstElement* videoFlip = gst_element_factory_make("videoflip", nullptr);
> 
> Do we need runtime checks after every call to gst_element_factory_make() in
> WebKit? Probably, right?
> 

It depends. I don't think it should be needed for elements shipped by GStreamer -core and -plugins-base. For the others, yes, runtime checks should be done.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2190
> > +            g_object_set(m_pipeline.get(), "video-filter", videoFlip, nullptr);
> 
> How does the memory management work here? Convention is for the property
> setter to take its own ref on the object, so now the pipeline should have
> its own ref of videoFlip. But you don't unref videoFlip yourself here. So it
> looks like it's leaking.
> 
> If GStreamer isn't taking its own ref on videoFlip, that's really unexpected
> and weird IMO.

I'll check this.
Comment 24 Philippe Normand 2018-02-08 09:37:11 PST
I don't think it leaks, the ownership is transferred to playsink with gst_object_ref_sink().
Comment 25 Michael Catanzaro 2018-02-08 09:53:13 PST
(In reply to Philippe Normand from comment #24)
> I don't think it leaks, the ownership is transferred to playsink with
> gst_object_ref_sink().

Inside the property setter?? OK then....
Comment 26 Philippe Normand 2018-02-08 10:39:01 PST
Committed r228281: <https://trac.webkit.org/changeset/228281>
Comment 27 Radar WebKit Bug Importer 2018-02-08 10:39:37 PST
<rdar://problem/37356862>