Add settings to restrict media containers and codecs when in Captive Portal mode
Created attachment 451125 [details] Patch
Comment on attachment 451125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451125&action=review > Source/WTF/Scripts/Preferences/WebPreferences.yaml:1329 > +MediaAudioCodecIDsAllowedWhenCaptivePortalModeEnabled: The name looks a little strange. Is "MediaAudioCodecIDsAllowedInCaptivePortalMode" better? > Source/WebCore/page/SettingsBase.cpp:238 > + Vector<FourCC> newTypes; > + for (auto type : StringView(types).split(',')) { Nit. Maybe we can implement this in a function and reuse the function? > Source/WebKit/WebKit.xcodeproj/xcshareddata/xcschemes/WebKitSwift.xcscheme:1 > +<?xml version="1.0" encoding="UTF-8"?> Is this change relevant?
Comment on attachment 451125 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451125&action=review r=me once the bots are happy > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:362 > + auto& settings = document().settings(); > + if (!contentTypeMeetsContainerAndCodecTypeRequirements(contentType, settings.allowedMediaContainerTypes(), settings.allowedMediaCodecTypes())) > + return Exception { NotSupportedError }; You could add a public method to MediaSource instead of repeating the check here.
(In reply to Peng Liu from comment #2) > Comment on attachment 451125 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=451125&action=review > > > Source/WTF/Scripts/Preferences/WebPreferences.yaml:1329 > > +MediaAudioCodecIDsAllowedWhenCaptivePortalModeEnabled: > > The name looks a little strange. Is > "MediaAudioCodecIDsAllowedInCaptivePortalMode" better? Ok! > > Source/WebCore/page/SettingsBase.cpp:238 > > + Vector<FourCC> newTypes; > > + for (auto type : StringView(types).split(',')) { > > Nit. Maybe we can implement this in a function and reuse the function? I dunno, it's pretty concise as it is. > > Source/WebKit/WebKit.xcodeproj/xcshareddata/xcschemes/WebKitSwift.xcscheme:1 > > +<?xml version="1.0" encoding="UTF-8"?> > > Is this change relevant? Whoops! Nope. (In reply to Eric Carlson from comment #3) > Comment on attachment 451125 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=451125&action=review > > r=me once the bots are happy > > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:362 > > + auto& settings = document().settings(); > > + if (!contentTypeMeetsContainerAndCodecTypeRequirements(contentType, settings.allowedMediaContainerTypes(), settings.allowedMediaCodecTypes())) > > + return Exception { NotSupportedError }; > > You could add a public method to MediaSource instead of repeating the check > here. It's already been factored into a single utility method; I'm not sure it's worth further refactoring.
Created attachment 451255 [details] Patch
Created attachment 451264 [details] Patch
Created attachment 451267 [details] Patch
Comment on attachment 451267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451267&action=review > Source/WTF/ChangeLog:3 > + [Cocoa] Adopt -streamDataParser:didProvideContentKeySpecifier:forTrackID: delegate callback Is the right change for this bug? The title is quite different.
(In reply to Sam Weinig from comment #8) > Comment on attachment 451267 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=451267&action=review > > > Source/WTF/ChangeLog:3 > > + [Cocoa] Adopt -streamDataParser:didProvideContentKeySpecifier:forTrackID: delegate callback > > Is the right change for this bug? The title is quite different. Ugh. webkit-patch uploaded that patch to the wrong bug. Will fix.
Created attachment 451271 [details] Patch
Comment on attachment 451271 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451271&action=review > Source/WTF/ChangeLog:134 > +2022-02-07 Jer Noble <jer.noble@apple.com> > + > + Add settings to restrict media containers and codecs when in Captive Portal mode > + https://bugs.webkit.org/show_bug.cgi?id=236245 > + > + Reviewed by NOBODY (OOPS!). > + > + * Scripts/Preferences/WebPreferences.yaml: > + Nit: move this to the top of the ChangeLog > LayoutTests/media/video-test.js:243 > + return waitFor(element, type, false, true); s/false, true/false, false/
Comment on attachment 451271 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451271&action=review > LayoutTests/ChangeLog:373 > +2022-02-07 Jer Noble <jer.noble@apple.com> Nit: move this to the top of the ChangeLog. :-)
Created attachment 451713 [details] Patch for landing
Looks like settings are bleeding between runs. Will update with a patch that resets these new settings to consistent values between runs.
Created attachment 451764 [details] Patch for landing
Created attachment 451766 [details] Patch for landing
Created attachment 451785 [details] Patch for landing
Committed r289697 (247182@main): <https://commits.webkit.org/247182@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 451785 [details].
<rdar://problem/88857365>