Comment 34 for bug 1203240

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:203
> + if (m_source && WEBKIT_IS_WEB_SRC(m_source.get()))

You don't need the m_source && as WEBKIT_IS_WEB_SRC checks for nulls as expected.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1362
> + GUniqueOutPtr<char> oldDownloadTemplate;
> + g_object_get(element, "temp-template", &oldDownloadTemplate.outPtr(), nullptr);
> +
> + GUniquePtr<char> newDownloadTemplate(g_build_filename(G_DIR_SEPARATOR_S, "var", "tmp", "WebKit-Media-XXXXXX", nullptr));
> + g_object_set(element, "temp-template", newDownloadTemplate.get(), nullptr);
> + GST_TRACE("Reconfiguring file download template from '%s' to '%s'", oldDownloadTemplate.get(), newDownloadTemplate.get());

Nit: you can move getting oldDownloadTemplate after setting newDownloadTemplate (it is going to be actually used before the GST_TRACE and specially relevant for the purge).

Another nit: "Reconfigured" is better because the relevant action already happened.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1395
> + // Transform "-XXXXXX" into "-??????" to get a file wildcard expression from the template.
> + for (char* p = &templateFile.get()[strlen(templateFile.get()) - 1]; p >= templateFile.get() && *p == 'X'; --p)
> + *p = '?';

Maybe you can use String::replace instead of doing this (it can take position and length.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1409
> + if (m_source && WEBKIT_IS_WEB_SRC(m_source.get()))

You don't need the m_source && as WEBKIT_IS_WEB_SRC checks for nulls as expected.