Bug 172244 – [WK2] Notify WebPageProxy client when an active process goes over the inactive memory limit

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Andreas Kling]

We should let the UI-side WK2 client know when the active process comes under memory pressure.

<rdar://problem/31800943>

[Andreas Kling]

Created attachment 310446 [details]
Proposed patch

Ok here's a cut, pls roast.

[Chris Dumez]

Comment on attachment 310446 [details]
Proposed patch

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

> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930
> +    WKPageDispatchMemoryPressureNotificationCallback                    dispatchMemoryPressureNotification;

When I discussed this with Geoff yesterday, my understanding was that the callback would be something like:
didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit);
enum class ResourceLimit { Memory, CPU };

Otherwise, we'll end up with 2 separate callback for memory and CPU.

[Andreas Kling]

(In reply to Chris Dumez from comment #2)
> Comment on attachment 310446 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310446&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930
> > +    WKPageDispatchMemoryPressureNotificationCallback                    dispatchMemoryPressureNotification;
> 
> When I discussed this with Geoff yesterday, my understanding was that the
> callback would be something like:
> didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit);
> enum class ResourceLimit { Memory, CPU };
> 
> Otherwise, we'll end up with 2 separate callback for memory and CPU.

Oh, okay, that sounds p good too. I can certainly make it look like that instead!

[Chris Dumez]

Comment on attachment 310446 [details]
Proposed patch

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

>> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930
>> +    WKPageDispatchMemoryPressureNotificationCallback                    dispatchMemoryPressureNotification;
> 
> When I discussed this with Geoff yesterday, my understanding was that the callback would be something like:
> didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit);
> enum class ResourceLimit { Memory, CPU };
> 
> Otherwise, we'll end up with 2 separate callback for memory and CPU.

Also, does this patch rely on memory pressure notifications from the system rather than using our inactive memory limit?

[Andreas Kling]

(In reply to Chris Dumez from comment #4)
> Comment on attachment 310446 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310446&action=review
> 
> >> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:930
> >> +    WKPageDispatchMemoryPressureNotificationCallback                    dispatchMemoryPressureNotification;
> > 
> > When I discussed this with Geoff yesterday, my understanding was that the callback would be something like:
> > didExceededBackgroundResourceLimitWhileInForeground(ResourceLimit);
> > enum class ResourceLimit { Memory, CPU };
> > 
> > Otherwise, we'll end up with 2 separate callback for memory and CPU.
> 
> Also, does this patch rely on memory pressure notifications from the system
> rather than using our inactive memory limit?

No, but this patch had some architectural mistakes so I'm redoing it to fit your suggested API name (which I love)

[Andreas Kling]

Created attachment 310527 [details]
Proposed patch

New version with Chris's suggested API using ResourceLimit { Memory, CPU } enum.

[Andreas Kling]

Created attachment 310556 [details]
Proposed patch

[Geoffrey Garen]

Comment on attachment 310556 [details]
Proposed patch

r=me

[Andreas Kling]

Created attachment 310559 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 310559 [details]
Patch for landing

Clearing flags on attachment: 310559

Committed r217101: <http://trac.webkit.org/changeset/217101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.