Bug 219961 - [Cocoa] WebM format reader doesn't work with a url in a <source> element
Summary: [Cocoa] WebM format reader doesn't work with a url in a <source> element
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-16 14:07 PST by Eric Carlson
Modified: 2021-01-05 23:26 PST (History)
8 users (show)

See Also:

Attachments
Patch (21.10 KB, patch)
2020-12-16 17:21 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (20.77 KB, patch)
2020-12-16 17:27 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (21.20 KB, patch)
2020-12-16 17:35 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (21.08 KB, patch)
2020-12-17 06:31 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (21.25 KB, patch)
2020-12-17 08:04 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (21.45 KB, patch)
2020-12-17 08:41 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (21.97 KB, patch)
2020-12-17 09:03 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (22.01 KB, patch)
2020-12-17 09:28 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Final patch for landing (19.96 KB, patch)
2020-12-17 11:33 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Address post-review comments. (1.50 KB, patch)
2020-12-17 13:50 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Patch (37.17 KB, patch)
2021-01-05 16:23 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.27 KB, patch)
2021-01-05 17:05 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.26 KB, patch)
2021-01-05 17:13 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.38 KB, patch)
2021-01-05 17:23 PST, Eric Carlson 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (37.49 KB, patch)
2021-01-05 17:33 PST, Eric Carlson 		no flags Details | Formatted Diff | Diff
Show Obsolete (14) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2020-12-16 14:07:20 PST 
The WebM format reader doesn't work if the source url is in a <source> element
Comment 1 Radar WebKit Bug Importer 2020-12-16 14:07:35 PST 
<rdar://problem/72399014>
Comment 2 Eric Carlson 2020-12-16 17:21:45 PST 
Created attachment 416374 [details]
Patch
Comment 3 Eric Carlson 2020-12-16 17:27:34 PST 
Created attachment 416375 [details]
Patch
Comment 4 Eric Carlson 2020-12-16 17:35:00 PST 
Created attachment 416376 [details]
Patch
Comment 5 Eric Carlson 2020-12-17 06:31:34 PST 
Created attachment 416417 [details]
Patch
Comment 6 Andy Estes 2020-12-17 07:21:13 PST 
Comment on attachment 416417 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416417&action=review

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:484
> +    if (!os_variant_allows_internal_security_policies("com.apple.WebKit"))

Since the value won't change at runtime, can you break this out into a helper function that caches the result of os_variant_allows_internal_security_policies() in a static bool? This was my mistake from earlier, but maybe you can fix it while you're here :)
Comment 7 Andy Estes 2020-12-17 07:24:46 PST 
Comment on attachment 416417 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416417&action=review

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:447
> +bool SourceBufferParserWebM::m_formatReaderEnabled = false;

Since globals are initialized to 0 by default, no need for the ` = false` here.
Comment 8 Eric Carlson 2020-12-17 08:04:23 PST 
Created attachment 416419 [details]
Patch
Comment 9 Eric Carlson 2020-12-17 08:41:14 PST 
Created attachment 416420 [details]
Patch for landing
Comment 10 Eric Carlson 2020-12-17 09:03:31 PST 
Created attachment 416425 [details]
Patch for landing
Comment 11 Eric Carlson 2020-12-17 09:28:50 PST 
Created attachment 416428 [details]
Patch for landing
Comment 12 Eric Carlson 2020-12-17 11:33:48 PST 
Created attachment 416443 [details]
Final patch for landing
Comment 13 EWS 2020-12-17 12:14:50 PST 
Committed r270939: <https://trac.webkit.org/changeset/270939>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416443 [details].
Comment 14 Darin Adler 2020-12-17 12:32:22 PST 
Comment on attachment 416443 [details]
Final patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=416443&action=review

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:483
> +#endif
> +
> +#if USE(APPLE_INTERNAL_SDK)

This should be elif so we don’t compile multiple return statements.
Comment 15 Eric Carlson 2020-12-17 13:50:19 PST 
Reopening to attach new patch.
Comment 16 Eric Carlson 2020-12-17 13:50:19 PST 
Created attachment 416465 [details]
Address post-review comments.
Comment 17 EWS 2020-12-17 15:15:12 PST 
Committed r270952: <https://trac.webkit.org/changeset/270952>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416465 [details].
Comment 18 Darin Adler 2020-12-17 15:57:56 PST 
Comment on attachment 416419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416419&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:105
> +    static auto cache = makeNeverDestroyed([this] {
> +        UNUSED_PARAM(this);

Why do we capture "this"?
Comment 19 Eric Carlson 2020-12-17 16:17:03 PST 
Comment on attachment 416419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416419&action=review

>> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:105
>> +        UNUSED_PARAM(this);
> 
> Why do we capture "this"?

That was left over from a earlier version of the patch, but it was removed before landing.
Comment 20 Ryan Haddad 2020-12-21 12:49:26 PST 
Reverted r270939 and r270952 for reason:

Caused layout test timeouts on internal bots

Committed r271038: <https://trac.webkit.org/changeset/271038>
Comment 21 Ryan Haddad 2020-12-21 12:49:28 PST 
Reverted r270939 and r270952 for reason:

Caused layout test timeouts on internal bots

Committed r271038: <https://trac.webkit.org/changeset/271038>
Comment 22 Ryan Haddad 2020-12-21 13:03:46 PST 
Details in rdar://72484508
Comment 23 Eric Carlson 2021-01-05 16:23:31 PST 
Created attachment 417051 [details]
Patch
Comment 24 Eric Carlson 2021-01-05 17:05:02 PST 
Created attachment 417058 [details]
Patch
Comment 25 Eric Carlson 2021-01-05 17:13:42 PST 
Created attachment 417060 [details]
Patch
Comment 26 Eric Carlson 2021-01-05 17:23:53 PST 
Created attachment 417061 [details]
Patch
Comment 27 Eric Carlson 2021-01-05 17:33:16 PST 
Created attachment 417062 [details]
Patch
Comment 28 EWS 2021-01-05 23:26:41 PST 
Committed r271194: <https://trac.webkit.org/changeset/271194>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417062 [details].