Bug 219052

Summary: [macOS] Stop using RunLoopType=_WebKit starting in Big Sur
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, pvollan, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2020-11-17 13:56:33 PST 
Stop using RunLoopType=_WebKit starting in Big Sur. This was a temporary hack so that our WebProcesses would get the right scheduling priority. Starting with Big Sur, the right way to do this is to use _ProcessType=App and restore the RunLoopType to be NSRunLoop.
Comment 1 Chris Dumez 2020-11-17 13:56:44 PST 
<rdar://problem/61742969>
Comment 2 Chris Dumez 2020-11-17 13:58:30 PST 
Created attachment 414383 [details]
Patch
Comment 3 Geoffrey Garen 2020-11-17 14:10:21 PST 
It looks like r233932 was about app nap / tab nap. With this change, do tabs still nap / suspend properly?
Comment 4 Geoffrey Garen 2020-11-17 14:20:38 PST 
I have a vague memory that we made a sandboxing change that was incompatible with the App process type or the NSRunLoop runloop type. Maybe Per Arne remembers?
Comment 5 Chris Dumez 2020-11-17 14:22:02 PST 
(In reply to Geoffrey Garen from comment #4)
> I have a vague memory that we made a sandboxing change that was incompatible
> with the App process type or the NSRunLoop runloop type. Maybe Per Arne
> remembers?

I will verify AppNap shortly. I don't foresee any issue.
Comment 6 Chris Dumez 2020-11-17 14:24:10 PST 
(In reply to Geoffrey Garen from comment #4)
> I have a vague memory that we made a sandboxing change that was incompatible
> with the App process type or the NSRunLoop runloop type. Maybe Per Arne
> remembers?

The sandboxing change made it so that we could not use _NSApplicationMain RunLoopType anymore. We initially tried to use NSRunLoop RunLoopType instead but it broke AppNap. This is when the _WebKit RunLoopType was introduced to unbreak AppNap. I am being told that we can now revert to NSRunLoop RunLoopType as long as we use "App" as _ProcessType (See radar).
Comment 7 Chris Dumez 2020-11-17 14:30:22 PST 
Created attachment 414385 [details]
Patch
Comment 8 Chris Dumez 2020-11-17 14:30:46 PST 
Comment on attachment 414385 [details]
Patch

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

> Source/WebKit/ChangeLog:13
> +        After this change, I have verified that the WebContent's main thread still runs at UserInteractive QoS. App Nap is also

I have verified that AppNap is still working fine with this patch.
Comment 9 Geoffrey Garen 2020-11-17 14:31:32 PST 
Comment on attachment 414385 [details]
Patch

r=me

OK, I think that addresses all my vague memories :P
Comment 10 EWS 2020-11-17 15:30:31 PST 
Committed r269928: <https://trac.webkit.org/changeset/269928>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414385 [details].
Comment 11 Per Arne Vollan 2020-11-17 15:56:57 PST 
(In reply to Geoffrey Garen from comment #4)
> I have a vague memory that we made a sandboxing change that was incompatible
> with the App process type or the NSRunLoop runloop type. Maybe Per Arne
> remembers?

If I recall correctly, we stopped using the NSApplication runloop when blocking the WindowServer. As Chris noted, this introduced a process priority regression, which was fixed by using this custom runloop type. It was possibly also needed to fix the AppNap regression, but Chris seems to have that covered :)