⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284816 in webkit


Ignore:
Timestamp:
Oct 25, 2021, 12:25:26 PM (5 years ago)
Author:
commit-queue@webkit.org
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 <​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):

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r284793 r284816  
     12021-10-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
     4        https://bugs.webkit.org/show_bug.cgi?id=232174
     5
     6        Reviewed by Geoff Garen.
     7
     8        * web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt:
     9
    1102021-10-25  Ziran Sun  <zsun@igalia.com>
    211

  • trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-audio-tainting.https-expected.txt

    r263198 r284816  
    11
    22
    3 Harness Error (TIMEOUT), message = null
     3PASS Verify CORS XHR of fetch() in a Service Worker
    44
    5 TIMEOUT Verify CORS XHR of fetch() in a Service Worker Test timed out
    6 

  • trunk/Source/WebCore/ChangeLog

    r284798 r284816  
     12021-10-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
     4        https://bugs.webkit.org/show_bug.cgi?id=232174
     5
     6        Reviewed by Geoff Garen.
     7
     8        AVFoundation doesn't like it when you give it a range like 0-1/* because it doesn't know the content length.
     9        To work around this, wait until the entire video is loaded then respond with a known length.
     10        This isn't great, but it's better than not playing the video at all.
     11
     12        In order to fix this, I noticed that the setHTTPHeaderField and setHTTPStatusCode calls were not being reflected in the new NSURLResponse,
     13        so I added a call to initNSURLResponse to update the NSURLResponse.  I'm concerned about what other videos were not having the synthesized
     14        response updated, and I'm surprised non-range-response-supporting videos played without this change.
     15
     16        This makes it so we can play videos like https://trac.webkit.org/export/284633/webkit/trunk/Tools/TestWebKitAPI/Tests/WebKit/test.mp4
     17        which can play in Chrome and Firefox.  Covered by an API test.
     18
     19        * platform/network/cf/ResourceResponse.h:
     20        * platform/network/cocoa/RangeResponseGenerator.mm:
     21        (WebCore::synthesizedResponseForRange):
     22        (WebCore::RangeResponseGenerator::giveResponseToTaskIfBytesInRangeReceived):
     23        * platform/network/cocoa/WebCoreNSURLSession.mm:
     24        (-[WebCoreNSURLSessionDataTask resource:receivedResponse:completionHandler:]):
     25
    1262021-10-25  Andres Gonzalez  <andresg_22@apple.com>
    227

  • trunk/Source/WebCore/platform/network/cf/ResourceResponse.h

    r264811 r284816  
    9494#endif
    9595
     96#if PLATFORM(COCOA)
     97    void initNSURLResponse() const;
     98#endif
     99
    96100private:
    97101    friend class ResourceResponseBase;
    … …  
    100104    String platformSuggestedFilename() const;
    101105    CertificateInfo platformCertificateInfo() const;
    102 
    103 #if PLATFORM(COCOA)
    104     void initNSURLResponse() const;
    105 #endif
    106106
    107107    static bool platformCompare(const ResourceResponse& a, const ResourceResponse& b);

  • trunk/Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm

    r284694 r284816  
    7474}
    7575
    76 static ResourceResponse synthesizedResponseForRange(const ResourceResponse& originalResponse, const ParsedRequestRange& parsedRequestRange, std::optional<size_t> totalContentLength)
     76static ResourceResponse synthesizedResponseForRange(const ResourceResponse& originalResponse, const ParsedRequestRange& parsedRequestRange, size_t totalContentLength)
    7777{
    7878    ASSERT(isMainThread());
    … …  
    8080    auto end = parsedRequestRange.end;
    8181
    82     auto newContentRange = makeString("bytes ", begin, "-", end, "/", (totalContentLength ? makeString(*totalContentLength) : "*"));
     82    auto newContentRange = makeString("bytes ", begin, "-", end, "/", totalContentLength);
    8383    auto newContentLength = makeString(end - begin + 1);
    8484
    … …  
    8888    constexpr auto partialContent = 206;
    8989    newResponse.setHTTPStatusCode(partialContent);
     90   
     91    // Values from setHTTPStatusCode and setHTTPHeaderField are not reflected in the newly generated response without this.
     92    newResponse.initNSURLResponse();
    9093
    9194    return newResponse;
    … …  
    106109    auto buffer = data.buffer;
    107110    auto bufferSize = buffer->size();
     111
     112    // FIXME: We ought to be able to just make a range with a * after the / but AVFoundation doesn't accept such ranges.
     113    // Instead, we just wait until the load has completed, at which time we will know the content length from the buffer length.
     114    if (!expectedContentLength)
     115        return;
    108116
    109117    if (bufferSize < range.begin)
    … …  
    144152    switch (taskData->responseState) {
    145153    case Data::TaskData::ResponseState::NotSynthesizedYet: {
    146         auto response = synthesizedResponseForRange(data.originalResponse, range, expectedContentLength);
     154        auto response = synthesizedResponseForRange(data.originalResponse, range, *expectedContentLength);
    147155        [task resource:nullptr receivedResponse:response completionHandler:[giveBytesToTask = WTFMove(giveBytesToTask), taskData = WeakPtr { taskData }, task = retainPtr(task)] (WebCore::ShouldContinuePolicyCheck shouldContinue) {
    148156            if (taskData)

  • trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm

    r284021 r284816  
    864864    ASSERT(isMainThread());
    865865    [self.session task:self didReceiveResponseFromOrigin:SecurityOrigin::create(response.url())];
    866     // FIXME: Think about this and make sure it's safe.
    867866    [self.session task:self didReceiveCORSAccessCheckResult:resource ? resource->didPassAccessControlCheck() : YES];
    868867    self.countOfBytesExpectedToReceive = response.expectedContentLength();

  • trunk/Tools/ChangeLog

    r284799 r284816  
     12021-10-25  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebKit ought to be able to play videos without Content-Length HTTP header fields and without range support
     4        https://bugs.webkit.org/show_bug.cgi?id=232174
     5
     6        Reviewed by Geoff Garen.
     7
     8        * TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:
     9        (TestWebKitAPI::TEST):
     10
    1112021-10-25  Ryan Haddad  <ryanhaddad@apple.com>
    212

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm

    r273287 r284816  
    164164        connection.receiveHTTPRequest([&, connection] (Vector<char>&& request) {
    165165            auto sendResponse = [&, connection] (HTTPResponse response, HTTPResponse::IncludeContentLength includeContentLength) {
    166                 connection.send(response.serialize(includeContentLength), [&, connection] {
    167                     respondToRequests(connection);
     166                connection.send(response.serialize(includeContentLength), [connection] () mutable {
     167                    connection.terminate();
    168168                });
    169169            };
    … …  
    182182        respondToRequests(connection);
    183183    });
    184     runVideoTest(server.request(), "error");
     184    runVideoTest(server.request(), "playing");
    185185    EXPECT_EQ(totalRequests, 2u);
    186186}
Note: See TracChangeset for help on using the changeset viewer.