Comment 37 for bug 1203240

Revision history for this message
In , eocanha (eocanha) wrote :

Comment on attachment 300398
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:204
>> + g_signal_handlers_disconnect_by_func(GST_ELEMENT_PARENT(m_source.get()), reinterpret_cast<gpointer>(uriDecodeBinElementAddedCallback), this);
>
> What if it has been disconnected before? I think it would be better to save the handler id and use g_signal_handler_disconnect() and invalidate the id.

If the callback has been disconnected before, then g_signal_handlers_disconnect_matched() (the actual function called by the macro) just returns "0" matches and nothing happens.

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1350
>> + if (g_strcmp0(G_OBJECT_CLASS_NAME(G_OBJECT_GET_CLASS(G_OBJECT(element))), "GstDownloadBuffer"))
>
> GST_IS_DOWNLOAD_BUFFER?

Sorry, that's not possible. GstDownloadBuffer is an element, and as such, comes from a dynamically loaded plugin. It seems that the gstdownloadbuffer.h header isn't distributed for usage from third party applications. If I can't find it in WebKit/WebKitBuild/DependenciesGTK/Root/include/gstreamer-1.0/gst when using jhbuild, I doubt it's going to be available on regular distros.

>>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1395
>>> + *p = '?';
>>
>> I'm not sure I understand this, maybe you can simply do g_strdelimit (templateFile.get(), "X", '?');
>>
>> Can't you pass something like "*-XXXXXX*" as pattern to listDirectory?
>
> Maybe you can use String::replace instead of doing this (it can take position and length.

Carlos: It doesn't work like that. "X" isn't an actual letter X, but the character g_mkstemp()[1] uses as wildcard to construct actual random filenames. That character is equivalent to "?" in shell glob syntax.

Now that I think about it, if we're comfident about what the legacy base name was ("WebKitWebProcess-XXXXXX") and are sure that it doesn't have "X" in any other place than in the pattern, we can do a simple character replacement in the base name without fear to screw legitimate "X" letters up.

[1] https://developer.gnome.org/glib/stable/glib-File-Utilities.html#g-mkstemp