Bug 217532 – [GPU Process] Add additional support for painting video elements to 2D contexts

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

[Wenson Hsieh]

In particular:
- CanvasRenderingContext2D.createPattern(HTMLVideoElement, …);
- createImageBitmap(HTMLVideoElement, …);

[Wenson Hsieh]

Created attachment 410975 [details]
Patch

[Wenson Hsieh]

Created attachment 410976 [details]
Patch

[EWS]

Committed r268299: <https://trac.webkit.org/changeset/268299>

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

[Radar WebKit Bug Importer]

<rdar://problem/70158828>

[Sam Weinig]

Comment on attachment 410976 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:7208
> +    case RenderingPurpose::MediaPainting:
> +        return m_page->settings().useGPUProcessForMediaEnabled();

It's not ideal that WebCore::Settings is being used for this. WebCore really shouldn't know about the GPUProcess. Perhaps its ok, because Settings is just like "a bag of state", but we should try to avoid this. This could probably be avoided by just storing this state on the WebPage and/or keeping around the WebPreferencesStore that get's passed in preferencesDidChange.

[Wenson Hsieh]

Comment on attachment 410976 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:7208
>> +        return m_page->settings().useGPUProcessForMediaEnabled();
> 
> It's not ideal that WebCore::Settings is being used for this. WebCore really shouldn't know about the GPUProcess. Perhaps its ok, because Settings is just like "a bag of state", but we should try to avoid this. This could probably be avoided by just storing this state on the WebPage and/or keeping around the WebPreferencesStore that get's passed in preferencesDidChange.

Yeah — Said and I avoided adding a similar “render canvas in GPU process” flag in WebCore::Settings for this reason. I think ideally, the GPUP-for-media setting should be refactored to be more like the canvas one (but I didn’t try to fix that in this patch).