WebKit ought to be able to play videos without Content-Length HTTP he…

…ader fields and without range support https://bugs.webkit.org/show_bug.cgi?id=232174 Patch by Alex Christensen <achristensen@webkit.org> 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): Canonical link: https://commits.webkit.org/243508@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284816 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Oct 25, 2021 · 934a244a26ac0fa8cf13a007742d4a51ea6e9186 · 934a244
1 parent 2ae6062
commit 934a244a26ac0fa8cf13a007742d4a51ea6e9186
Showing 8 changed files with 63 additions and 14 deletions.
diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,12 @@
+2021-10-25  Alex Christensen  <achristensen@webkit.org>
+
+        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  <zsun@igalia.com>
 
         [CSS-grid] Need to set prefer width dirty for the child that has constraints to the grid area

diff --git a/...web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt b/...web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt
@@ -1,6 +1,4 @@
 
 
-Harness Error (TIMEOUT), message = null
-
-TIMEOUT Verify CORS XHR of fetch() in a Service Worker Test timed out
+PASS Verify CORS XHR of fetch() in a Service Worker
 
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2021-10-25  Alex Christensen  <achristensen@webkit.org>
+
+        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  <andresg_22@apple.com>
 
         Remove childrenInitialized() from the AXCoreObject interface.

diff --git a/Source/WebCore/platform/network/cf/ResourceResponse.h b/Source/WebCore/platform/network/cf/ResourceResponse.h
@@ -93,17 +93,17 @@ class ResourceResponse : public ResourceResponseBase {
     void setIsQuickLook(bool isQuickLook) { m_isQuickLook = isQuickLook; }
 #endif
 
+#if PLATFORM(COCOA)
+    void initNSURLResponse() const;
+#endif
+
 private:
     friend class ResourceResponseBase;
 
     void platformLazyInit(InitLevel);
     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)

diff --git a/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm b/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm
@@ -73,20 +73,23 @@
     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;
     newResponse.setHTTPHeaderField(HTTPHeaderName::ContentRange, newContentRange);
     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 @@ static ResourceResponse synthesizedResponseForRange(const ResourceResponse& orig
     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 @@ static ResourceResponse synthesizedResponseForRange(const ResourceResponse& orig
 
     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;

diff --git a/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm b/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm
@@ -863,7 +863,6 @@ - (void)resource:(PlatformMediaResource*)resource receivedResponse:(const Resour
     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();

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,13 @@
+2021-10-25  Alex Christensen  <achristensen@webkit.org>
+
+        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  <ryanhaddad@apple.com>
 
         Bring up queues for Monterey

diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm
@@ -163,8 +163,8 @@ HTTPServer server({
     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({
     HTTPServer server([&](Connection connection) {
         respondToRequests(connection);
     });
-    runVideoTest(server.request(), "error");
+    runVideoTest(server.request(), "playing");
     EXPECT_EQ(totalRequests, 2u);
 }