Bug 111387

Summary: Use new assertion API for process suppression on Mac
Product: WebKit Reporter: Kiran Muppala <cmuppala>
Component: WebKit2Assignee: Kiran Muppala <cmuppala>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch: not ready for commit
none
Patch none

Description Kiran Muppala 2013-03-04 17:42:51 PST
Use new [NSProcessInfo beginSuspensionOfSystemBehaviors:] API to control process suppression on Mac in place of the current WKNSProcessInfoProcessAssertionWithTypes() SPI.
Comment 1 Kiran Muppala 2013-03-04 17:43:02 PST
<rdar://problem/13145081>
Comment 2 Kiran Muppala 2013-03-04 18:07:19 PST
Created attachment 191369 [details]
Patch: not ready for commit
Comment 3 Kiran Muppala 2013-03-04 18:47:35 PST
Created attachment 191374 [details]
Patch
Comment 4 Kiran Muppala 2013-03-04 18:48:25 PST
(In reply to comment #3)
> Created an attachment (id=191374) [details]
> Patch

Made a slight correction to the ChangeLog.  No source changes.
Comment 5 Alexey Proskuryakov 2013-03-04 23:03:54 PST
Comment on attachment 191374 [details]
Patch

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

> WebKitLibraries/WebKitSystemInterface.h:501
> +extern const NSSystemBehaviors WKProcessSuppressionSystemBehaviors;

I don't think that this should go to WebKitSystemInterface.h. We can declare the symbol anywhere, and ChildProcessMac.mm seems like the right place for that.
Comment 6 Alexey Proskuryakov 2013-03-04 23:05:02 PST
Comment on attachment 191374 [details]
Patch

The patch name says "not ready for commit", but presumably it's ready for review.
Comment 7 Kiran Muppala 2013-03-04 23:23:41 PST
(In reply to comment #5)
> (From update of attachment 191374 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191374&action=review
> 
> > WebKitLibraries/WebKitSystemInterface.h:501
> > +extern const NSSystemBehaviors WKProcessSuppressionSystemBehaviors;
> 
> I don't think that this should go to WebKitSystemInterface.h. We can declare the symbol anywhere, and ChildProcessMac.mm seems like the right place for that.

Ah, the catch is that the symbol is actually computed in WebKitSystemInterface.m, not a simple wrapper for a underlying SPI symbol.  So, I thought it made sense to put it there.  But, as you said, the symbol does belong in ChildProcessMac.mm and can be moved there when the API becomes is public.
Comment 8 Kiran Muppala 2013-03-04 23:25:10 PST
(In reply to comment #6)
> (From update of attachment 191374 [details])
> The patch name says "not ready for commit", but presumably it's ready for review.

Yes, I wanted it to be reviewed but not cq+'d because it needs a WebKitSystemInterface part to be committed before that.  Thanks for the review.
Comment 9 WebKit Review Bot 2013-03-05 15:48:15 PST
Comment on attachment 191374 [details]
Patch

Clearing flags on attachment: 191374

Committed r144832: <http://trac.webkit.org/changeset/144832>
Comment 10 WebKit Review Bot 2013-03-05 15:48:19 PST
All reviewed patches have been landed.  Closing bug.