Bug 213245

Jer Noble would know better, but how is this a WebKit issue? This indicates that loading didn't go through WebKit, as it should have. Do you have steps to reproduce that would work for us?

<rdar://problem/64471582>

We pass a custom user-agent string to AVFoundation in a dictionary of header values, but that particular one appears to be ignored. We can work around this behavior by overriding the user-agent value at the WebCoreNSURLSession level.

And the issue is trivially reproducible. You can verify that we're sending the wrong UA string in WebInspector with any page with a <video> element.

I see, so this was a misunderstanding: "loading didn't go through WebKit".

Created attachment 402718 [details] Patch

Comment on attachment 402718 [details] Patch LGTM. In theory, CoreMedia should probably split their code so that they only set these headers (user-agent, accept...) in the code that actually sends the requests themselves. View in context: https://bugs.webkit.org/attachment.cgi?id=402718&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38 > +static String parseUserAgent(Vector<char>&& request) const Vector<char>& > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:40 > + Vector<String> headers = String::fromUTF8(request.data(), request.size()).split("\r\n"); auto > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:41 > + auto index = headers.findMatching([] (const String& header) { return header.startsWith("User-Agent:"); }); auto > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:57 > + connection.receiveHTTPRequest([&] (Vector<char>&& request) { auto&& > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:58 > + auto userAgent = parseUserAgent(WTFMove(request)); no need to move. Ditto below.

Created attachment 402744 [details] Patch for landing

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Comment on attachment 402744 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=402744&action=review source change looks fine > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38 > +static String parseUserAgent(const Vector<char>& request) Couldn't you just look for "User-Agent: TestWebKitAPI\r\n" in the request instead of this? > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:52 > + webView.get()._customUserAgent = @"TestWebKitAPI"; Could you use customUserAgent instead of _customUserAgent? > Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:105 > + connection.send(makeString("HTTP/1.1 200 OK\r\n", Does this test even need to send a reply?

Created attachment 402799 [details] Patch for landing

Comment on attachment 402744 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=402744&action=review >> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:38 >> +static String parseUserAgent(const Vector<char>& request) > > Couldn't you just look for "User-Agent: TestWebKitAPI\r\n" in the request instead of this? I could; but with this way, a failing test shows the incorrect User-Agent in the test output, rather than just a "strncmp expected TRUE, got FALSE". >> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:52 >> + webView.get()._customUserAgent = @"TestWebKitAPI"; > > Could you use customUserAgent instead of _customUserAgent? I didn't know that was a thing! Yes, it could. >> Tools/TestWebKitAPI/Tests/WebKitCocoa/MediaLoading.mm:105 >> + connection.send(makeString("HTTP/1.1 200 OK\r\n", > > Does this test even need to send a reply? Yes it does. I wanted to make sure both the initial request for the manifest contained the correct MIME type, and also follow-up requests for media segments also did. Without sending a correct reply, the follow up media request will never be sent.

Comment on attachment 402799 [details] Patch for landing Aha, you're right. I thought receivedManifestRequest and receivedMediaRequest were the same. It turns out, they're not the same.

Created attachment 402802 [details] Patch for landing

Committed r263537: <https://trac.webkit.org/changeset/263537> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402802 [details].

This broke tvOS and watchOS builds: https://build.webkit.org/builders/Apple-tvOS-13-Release-Build/builds/171 (we don't have EWS yet, operations has been consumed by Big Sur work, it's coming soon)

Re-opened since this is blocked by bug 213640

Okay, looks like tvOS and watchOS don't `HAVE(NETWORK_FRAMEWORK)`. Just needs a compile guard around the test case.

Created attachment 402926 [details] Patch for landing

Committed r263627: <https://trac.webkit.org/changeset/263627> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402926 [details].