Bug 232818 – Make it possible to toggle several experimental media features when GPU Process is enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Wenson Hsieh]

In particular, these features:

• OpusDecoderEnabled (Opus audio decoder)
• VorbisDecoderEnabled (Vorbis audio decoder)
• WebMFormatReaderEnabled (WebM format reader)
• WebMParserEnabled (WebM MSE parser)
• MediaSourceInlinePaintingEnabled (Experimental MediaSource Inline Painting)

[Wenson Hsieh]

Created attachment 443572 [details]
For EWS

[Tim Horton]

Comment on attachment 443572 [details]
For EWS

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

> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:590
> +    for (auto page : webProcess.pages()) {

While this is still a wild layering problem, it's strictly better than using the WebPageGroup prefs.

I think you removed a bit too much of the "omg here be dragons" comment though; the caveats in your changelog should be mentioned here too.

> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:-670
> -    setScreenProperties(WebCore::collectScreenProperties());

as you pointed out yourself, you lost canSendMessage here.

[Wenson Hsieh]

Comment on attachment 443572 [details]
For EWS

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

Thanks for the review!

>> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:590
>> +    for (auto page : webProcess.pages()) {
> 
> While this is still a wild layering problem, it's strictly better than using the WebPageGroup prefs.
> 
> I think you removed a bit too much of the "omg here be dragons" comment though; the caveats in your changelog should be mentioned here too.

Yes — definitely agreed! I'll leave this comment behind:

// FIXME: It's not ideal that these features are controlled by preferences-level feature flags (i.e. per-web view), but there is only one GPU process and the runtime-enabled features backing these preferences are process-wide. We should refactor each of these features so that they aren't process-global, and then reimplement this feature flag propagation to the GPU Process in a way that respects the settings of the page that is hosting each media element.
// For the time being, each of the below features are enabled in the GPU Process if it is enabled by at least one web page's preferences. In practice, all web pages' preferences should agree on these feature flag values.

>> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:-670
>> -    setScreenProperties(WebCore::collectScreenProperties());
> 
> as you pointed out yourself, you lost canSendMessage here.

Fixed!

[Wenson Hsieh]

Created attachment 443619 [details]
Patch

[EWS]

Committed r285477 (244001@main): <https://commits.webkit.org/244001@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/85182754>