- Revision
- 284816
- Author
- [email protected]
- Date
- 2021-10-25 12:25:26 -0700 (Mon, 25 Oct 2021)
Log Message
WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
https://bugs.webkit.org/show_bug.cgi?id=232174
Patch by Alex Christensen <[email protected]> on 2021-10-25
Reviewed by Geoff Garen.
LayoutTests/imported/w3c:
* web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt:
Source/WebCore:
AVFoundation doesn't like it when you give it a range like 0-1/* because it doesn't know the content length.
To work around this, wait until the entire video is loaded then respond with a known length.
This isn't great, but it's better than not playing the video at all.
In order to fix this, I noticed that the setHTTPHeaderField and setHTTPStatusCode calls were not being reflected in the new NSURLResponse,
so I added a call to initNSURLResponse to update the NSURLResponse. I'm concerned about what other videos were not having the synthesized
response updated, and I'm surprised non-range-response-supporting videos played without this change.
This makes it so we can play videos like https://trac.webkit.org/export/284633/webkit/trunk/Tools/TestWebKitAPI/Tests/WebKit/test.mp4
which can play in Chrome and Firefox. Covered by an API test.
* platform/network/cf/ResourceResponse.h:
* platform/network/cocoa/RangeResponseGenerator.mm:
(WebCore::synthesizedResponseForRange):
(WebCore::RangeResponseGenerator::giveResponseToTaskIfBytesInRangeReceived):
* platform/network/cocoa/WebCoreNSURLSession.mm:
(-[WebCoreNSURLSessionDataTask resource:receivedResponse:completionHandler:]):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (284815 => 284816)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-10-25 19:25:26 UTC (rev 284816)
@@ -1,3 +1,12 @@
+2021-10-25 Alex Christensen <[email protected]>
+
+ WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
+ https://bugs.webkit.org/show_bug.cgi?id=232174
+
+ Reviewed by Geoff Garen.
+
+ * web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt:
+
2021-10-25 Ziran Sun <[email protected]>
[CSS-grid] Need to set prefer width dirty for the child that has constraints to the grid area
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt (284815 => 284816)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt 2021-10-25 19:25:26 UTC (rev 284816)
@@ -1,6 +1,4 @@
-Harness Error (TIMEOUT), message = null
+PASS Verify CORS XHR of fetch() in a Service Worker
-TIMEOUT Verify CORS XHR of fetch() in a Service Worker Test timed out
-
Modified: trunk/Source/WebCore/ChangeLog (284815 => 284816)
--- trunk/Source/WebCore/ChangeLog 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Source/WebCore/ChangeLog 2021-10-25 19:25:26 UTC (rev 284816)
@@ -1,3 +1,28 @@
+2021-10-25 Alex Christensen <[email protected]>
+
+ WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
+ https://bugs.webkit.org/show_bug.cgi?id=232174
+
+ Reviewed by Geoff Garen.
+
+ AVFoundation doesn't like it when you give it a range like 0-1/* because it doesn't know the content length.
+ To work around this, wait until the entire video is loaded then respond with a known length.
+ This isn't great, but it's better than not playing the video at all.
+
+ In order to fix this, I noticed that the setHTTPHeaderField and setHTTPStatusCode calls were not being reflected in the new NSURLResponse,
+ so I added a call to initNSURLResponse to update the NSURLResponse. I'm concerned about what other videos were not having the synthesized
+ response updated, and I'm surprised non-range-response-supporting videos played without this change.
+
+ This makes it so we can play videos like https://trac.webkit.org/export/284633/webkit/trunk/Tools/TestWebKitAPI/Tests/WebKit/test.mp4
+ which can play in Chrome and Firefox. Covered by an API test.
+
+ * platform/network/cf/ResourceResponse.h:
+ * platform/network/cocoa/RangeResponseGenerator.mm:
+ (WebCore::synthesizedResponseForRange):
+ (WebCore::RangeResponseGenerator::giveResponseToTaskIfBytesInRangeReceived):
+ * platform/network/cocoa/WebCoreNSURLSession.mm:
+ (-[WebCoreNSURLSessionDataTask resource:receivedResponse:completionHandler:]):
+
2021-10-25 Andres Gonzalez <[email protected]>
Remove childrenInitialized() from the AXCoreObject interface.
Modified: trunk/Source/WebCore/platform/network/cf/ResourceResponse.h (284815 => 284816)
--- trunk/Source/WebCore/platform/network/cf/ResourceResponse.h 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Source/WebCore/platform/network/cf/ResourceResponse.h 2021-10-25 19:25:26 UTC (rev 284816)
@@ -93,6 +93,10 @@
void setIsQuickLook(bool isQuickLook) { m_isQuickLook = isQuickLook; }
#endif
+#if PLATFORM(COCOA)
+ void initNSURLResponse() const;
+#endif
+
private:
friend class ResourceResponseBase;
@@ -100,10 +104,6 @@
String platformSuggestedFilename() const;
CertificateInfo platformCertificateInfo() const;
-#if PLATFORM(COCOA)
- void initNSURLResponse() const;
-#endif
-
static bool platformCompare(const ResourceResponse& a, const ResourceResponse& b);
#if USE(QUICK_LOOK)
Modified: trunk/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm (284815 => 284816)
--- trunk/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm 2021-10-25 19:25:26 UTC (rev 284816)
@@ -73,13 +73,13 @@
ASSERT(isMainThread());
}
-static ResourceResponse synthesizedResponseForRange(const ResourceResponse& originalResponse, const ParsedRequestRange& parsedRequestRange, std::optional<size_t> totalContentLength)
+static ResourceResponse synthesizedResponseForRange(const ResourceResponse& originalResponse, const ParsedRequestRange& parsedRequestRange, size_t totalContentLength)
{
ASSERT(isMainThread());
auto begin = parsedRequestRange.begin;
auto end = parsedRequestRange.end;
- auto newContentRange = makeString("bytes ", begin, "-", end, "/", (totalContentLength ? makeString(*totalContentLength) : "*"));
+ auto newContentRange = makeString("bytes ", begin, "-", end, "/", totalContentLength);
auto newContentLength = makeString(end - begin + 1);
ResourceResponse newResponse = originalResponse;
@@ -87,6 +87,9 @@
newResponse.setHTTPHeaderField(HTTPHeaderName::ContentLength, newContentLength);
constexpr auto partialContent = 206;
newResponse.setHTTPStatusCode(partialContent);
+
+ // Values from setHTTPStatusCode and setHTTPHeaderField are not reflected in the newly generated response without this.
+ newResponse.initNSURLResponse();
return newResponse;
}
@@ -106,6 +109,11 @@
auto buffer = data.buffer;
auto bufferSize = buffer->size();
+ // FIXME: We ought to be able to just make a range with a * after the / but AVFoundation doesn't accept such ranges.
+ // Instead, we just wait until the load has completed, at which time we will know the content length from the buffer length.
+ if (!expectedContentLength)
+ return;
+
if (bufferSize < range.begin)
return;
@@ -143,7 +151,7 @@
switch (taskData->responseState) {
case Data::TaskData::ResponseState::NotSynthesizedYet: {
- auto response = synthesizedResponseForRange(data.originalResponse, range, expectedContentLength);
+ auto response = synthesizedResponseForRange(data.originalResponse, range, *expectedContentLength);
[task resource:nullptr receivedResponse:response completionHandler:[giveBytesToTask = WTFMove(giveBytesToTask), taskData = WeakPtr { taskData }, task = retainPtr(task)] (WebCore::ShouldContinuePolicyCheck shouldContinue) {
if (taskData)
taskData->responseState = Data::TaskData::ResponseState::SessionCalledCompletionHandler;
Modified: trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm (284815 => 284816)
--- trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm 2021-10-25 19:25:26 UTC (rev 284816)
@@ -863,7 +863,6 @@
ASSERT_UNUSED(resource, !resource || resource == _resource);
ASSERT(isMainThread());
[self.session task:self didReceiveResponseFromOrigin:SecurityOrigin::create(response.url())];
- // FIXME: Think about this and make sure it's safe.
[self.session task:self didReceiveCORSAccessCheckResult:resource ? resource->didPassAccessControlCheck() : YES];
self.countOfBytesExpectedToReceive = response.expectedContentLength();
RetainPtr<NSURLResponse> strongResponse = response.nsURLResponse();
Modified: trunk/Tools/ChangeLog (284815 => 284816)
--- trunk/Tools/ChangeLog 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Tools/ChangeLog 2021-10-25 19:25:26 UTC (rev 284816)
@@ -1,3 +1,13 @@
+2021-10-25 Alex Christensen <[email protected]>
+
+ WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
+ https://bugs.webkit.org/show_bug.cgi?id=232174
+
+ Reviewed by Geoff Garen.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:
+ (TestWebKitAPI::TEST):
+
2021-10-25 Ryan Haddad <[email protected]>
Bring up queues for Monterey
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm (284815 => 284816)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm 2021-10-25 19:10:30 UTC (rev 284815)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm 2021-10-25 19:25:26 UTC (rev 284816)
@@ -163,8 +163,8 @@
respondToRequests = [&] (Connection connection) {
connection.receiveHTTPRequest([&, connection] (Vector<char>&& request) {
auto sendResponse = [&, connection] (HTTPResponse response, HTTPResponse::IncludeContentLength includeContentLength) {
- connection.send(response.serialize(includeContentLength), [&, connection] {
- respondToRequests(connection);
+ connection.send(response.serialize(includeContentLength), [connection] () mutable {
+ connection.terminate();
});
};
totalRequests++;
@@ -181,7 +181,7 @@
HTTPServer server([&](Connection connection) {
respondToRequests(connection);
});
- runVideoTest(server.request(), "error");
+ runVideoTest(server.request(), "playing");
EXPECT_EQ(totalRequests, 2u);
}
