Bug 218549

Summary: [GPU Process] Move the internal GPU rendering flags from WebPage to WebProcess
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, eric.carlson, ews-watchlist, glenn, jer.noble, peng.liu6, philipj, sam, sergio, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218600
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Said Abou-Hallawa 2020-11-03 18:29:45 PST
Because not all the clients have access to a WebPage such as a worker based ImageBitmap.
Comment 1 Said Abou-Hallawa 2020-11-03 18:52:01 PST
Created attachment 413133 [details]
Patch
Comment 2 Tim Horton 2020-11-03 21:11:29 PST
Comment on attachment 413133 [details]
Patch

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

> Source/WebKit/ChangeLog:8
> +        -- Setting UseGPUProcessForMedia is already in WebProcess.

This is wrong, not something to be emulated

> Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml:-697
> -  webcoreBinding: none

This is a layering violation.

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3266
> +- (BOOL)useGPUProcessForCanvasRenderingEnabled

Clearly not.
Comment 3 Said Abou-Hallawa 2020-11-03 22:49:54 PST
Created attachment 413141 [details]
Patch
Comment 4 Said Abou-Hallawa 2020-11-03 22:52:36 PST
Comment on attachment 413133 [details]
Patch

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

>> Source/WebKit/ChangeLog:8
>> +        -- Setting UseGPUProcessForMedia is already in WebProcess.
> 
> This is wrong, not something to be emulated

I tried to fix it but RemoteMediaPlayerManager::updatePreferences() uses settings.useGPUProcessForMediaEnabled(). So I kept it as is.

>> Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml:-697
>> -  webcoreBinding: none
> 
> This is a layering violation.

Change was reverted back.

>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3266
>> +- (BOOL)useGPUProcessForCanvasRenderingEnabled
> 
> Clearly not.

Deleted.
Comment 5 Simon Fraser (smfr) 2020-11-04 08:41:28 PST
So should we fix the media flag?
Comment 6 Said Abou-Hallawa 2020-11-04 09:40:26 PST
(In reply to Simon Fraser (smfr) from comment #5)
> So should we fix the media flag?

I'll try to fix it.
Comment 7 Said Abou-Hallawa 2020-11-04 10:45:44 PST
Created attachment 413178 [details]
Patch
Comment 8 Tim Horton 2020-11-04 11:34:28 PST
I wonder if we need to fail harder if something tries to make two WebPages with different GPUP settings coexist in the same WebProcess?
Comment 9 Tim Horton 2020-11-04 12:27:32 PST
Comment on attachment 413178 [details]
Patch

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

> Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml:719
> +  webcoreBinding: none

Nice!
Comment 10 Sam Weinig 2020-11-04 12:33:43 PST
If this is no longer per-page, WebPreferences is not the right API to use to surface this, as that is a per-page concept.
Comment 11 Said Abou-Hallawa 2020-11-04 21:39:09 PST
(In reply to Sam Weinig from comment #10)
> If this is no longer per-page, WebPreferences is not the right API to use to
> surface this, as that is a per-page concept.

I filed bug 218600 to track this issue.
Comment 12 EWS 2020-11-04 21:59:23 PST
Committed r269416: <https://trac.webkit.org/changeset/269416>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413178 [details].
Comment 13 Radar WebKit Bug Importer 2020-11-04 22:00:34 PST
<rdar://problem/71065314>