The test is failing since it has been introduced in r134802. http://trac.webkit.org/changeset/134802 Here's the diff: --- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/video-src-blob-expected.txt +++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/media/video-src-blob-actual.txt @@ -1,5 +1,5 @@ This tests the ability of the <video> element to load blob URLs. In the browser, select a video file: EVENT(change) -EVENT(loadedmetadata) +EVENT(error) TEST(false) FAIL END OF TEST
Now that we use the SubresourceLoder to load videos this should be easy to fix.
Created attachment 206897 [details] Patch
Comment on attachment 206897 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206897&action=review Looks good to me. Great work! Just a couple questions... > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:465 > + DataBufferingPolicy bufferingPolicy = url.protocolIs("blob") ? BufferData : DoNotBufferData; > + CachedResourceRequest cacheRequest(request, ResourceLoaderOptions(SendCallbacks, DoNotSniffContent, bufferingPolicy, DoNotAllowStoredCredentials, DoNotAskClientForCrossOriginCredentials, DoSecurityCheck, UseDefaultOriginRestrictionsForType)); > priv->resource = loader->requestRawResource(cacheRequest); Why is it useful to buffer data that is already in memory on the client side? > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:605 > + if (!url.isValid() || (!url.protocolIsInHTTPFamily() && !url.protocolIs("blob"))) { I think this could be a helper. Something like urlIsValidAndHasSupportedProtocol or just urlHasSupportedProtocol.
(In reply to comment #3) > (From update of attachment 206897 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206897&action=review > > Looks good to me. Great work! Just a couple questions... Thanks! > > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:465 > > + DataBufferingPolicy bufferingPolicy = url.protocolIs("blob") ? BufferData : DoNotBufferData; > > + CachedResourceRequest cacheRequest(request, ResourceLoaderOptions(SendCallbacks, DoNotSniffContent, bufferingPolicy, DoNotAllowStoredCredentials, DoNotAskClientForCrossOriginCredentials, DoSecurityCheck, UseDefaultOriginRestrictionsForType)); > > priv->resource = loader->requestRawResource(cacheRequest); > > Why is it useful to buffer data that is already in memory on the client side? Because when buffered it can be cached, I think. I followed what apple does, TBH. > > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:605 > > + if (!url.isValid() || (!url.protocolIsInHTTPFamily() && !url.protocolIs("blob"))) { > > I think this could be a helper. Something like urlIsValidAndHasSupportedProtocol or just urlHasSupportedProtocol. I did it indeed :-) but in the end it looked too much code for a simple check and moved back to the current patch, I'll add the helper function since it makes the code easier to read.
LGTM but yeah that helper function would be good to have indeed.
Comment on attachment 206897 [details] Patch Looks good. Consider the helper when landing. :)
Committed r157091: <http://trac.webkit.org/changeset/157091>