Summary: | [WK2] Notify WebPageProxy client when an active process goes over the inactive memory limit | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||||||
Component: | WebKit2 | Assignee: | Andreas Kling <kling> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | benjamin, buildbot, cdumez, cmarcelo, commit-queue, dbates, ggaren, kling | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 172246 | ||||||||||||
Attachments: |
|
Description
Andreas Kling
2017-05-17 14:35:46 PDT
Created attachment 310446 [details]
Proposed patch
Ok here's a cut, pls roast.
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. (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! 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? (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) Created attachment 310527 [details]
Proposed patch
New version with Chris's suggested API using ResourceLimit { Memory, CPU } enum.
Created attachment 310556 [details]
Proposed patch
Comment on attachment 310556 [details]
Proposed patch
r=me
Created attachment 310559 [details]
Patch for landing
Comment on attachment 310559 [details] Patch for landing Clearing flags on attachment: 310559 Committed r217101: <http://trac.webkit.org/changeset/217101> All reviewed patches have been landed. Closing bug. |