Bug 105378 - Adopt new assertion SPI for process suppression on Mac
Summary: Adopt new assertion SPI for process suppression on Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac Unspecified
: P2 Normal
Assignee: Kiran Muppala
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-12-18 19:15 PST by Kiran Muppala
Modified: 2012-12-19 21:58 PST (History)
4 users (show)

See Also:


Attachments
WIP Patch (7.77 KB, patch)
2012-12-18 19:26 PST, Kiran Muppala
no flags Details | Formatted Diff | Diff
WIP Patch (7.94 KB, patch)
2012-12-18 19:51 PST, Kiran Muppala
no flags Details | Formatted Diff | Diff
Patch: not ready for commit (8.09 KB, patch)
2012-12-18 22:11 PST, Kiran Muppala
no flags Details | Formatted Diff | Diff
Patch (7.67 KB, patch)
2012-12-19 19:24 PST, Kiran Muppala
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kiran Muppala 2012-12-18 19:15:13 PST
Process suppression for WebKit2 child processes is currently enabled or disabled using AutomaticTermination.  This should be replaced with a new assertion SPI specific to process suppression.
Comment 1 Kiran Muppala 2012-12-18 19:15:29 PST
<rdar://problem/12893550>
Comment 2 Kiran Muppala 2012-12-18 19:26:49 PST
Created attachment 180076 [details]
WIP Patch
Comment 3 Kiran Muppala 2012-12-18 19:51:32 PST
Created attachment 180077 [details]
WIP Patch
Comment 4 Kiran Muppala 2012-12-18 19:53:50 PST
(In reply to comment #3)
> Created an attachment (id=180077) [details]
> Patch

In ChildProcess::setApplicationIsOccluded.

Simplified the check for occlusion change, by using ChildProcess::isApplicaitonOccluded().  Added a ASSERT.
Comment 5 Kiran Muppala 2012-12-18 22:11:37 PST
Created attachment 180092 [details]
Patch: not ready for commit
Comment 6 Kiran Muppala 2012-12-18 22:13:00 PST
(In reply to comment #5)
> Created an attachment (id=180092) [details]
> Patch

Renamed WKSI function to WKNSProcessInfoProcessAssertionWithTypes() to more closely reflect the underlying SPI method and renamed ChildProcess::m_processAssertionVisible to ChildProcess::m_processVisibleAssertion, since the former sounded like the assertion was visible rather than the process being visible.
Comment 7 Kiran Muppala 2012-12-19 19:24:01 PST
Created attachment 180261 [details]
Patch
Comment 8 Kiran Muppala 2012-12-19 19:29:45 PST
(In reply to comment #7)
> Created an attachment (id=180261) [details]
> Patch

Mark Rowe reviewed the patch and commented that ChildProcess::enableProcessSuppression and ChildProcess::disableProcessSuppression should be renamed since they are just taking a assertion for process visibility or alternatively he suggested that these methods can be removed entirely and the logic be moved to ChildProcess::setApplicaitonIsOccluded.  I preferred the latter idea and hence moved the logic to manage the process visible assertion into ChildProcess::setApplicaitonIsOccluded itself.
Comment 9 WebKit Review Bot 2012-12-19 21:58:54 PST
Comment on attachment 180261 [details]
Patch

Clearing flags on attachment: 180261

Committed r138226: <http://trac.webkit.org/changeset/138226>
Comment 10 WebKit Review Bot 2012-12-19 21:58:58 PST
All reviewed patches have been landed.  Closing bug.