Bug 159001 - Add logging related to process assertions to help debug process suspension issues
Summary: Add logging related to process assertions to help debug process suspension is...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-21 15:01 PDT by Chris Dumez
Modified: 2016-06-22 10:00 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.69 KB, patch)
2016-06-21 15:03 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (9.67 KB, patch)
2016-06-21 16:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-06-21 15:01:54 PDT
Add logging related to process assertions to help debug process suspension issues.
Comment 1 Chris Dumez 2016-06-21 15:03:19 PDT
Created attachment 281781 [details]
Patch
Comment 2 Keith Rollin 2016-06-21 15:45:08 PDT
Some questions:

* How much logging does this emit? When opening a page? When closing a page? When putting a page in the background? When putting a page in the foreground? In short, will someone complain that there's too much logging?

* In the lambdas that call a completion handler, I'd put the logging before the call of the completion handler. If there's logging in the completion handlers, I think the result would read better.

* The common aspect of the added logging is that it involves calls to backgroundActivityToken and foregroundActivityToken. There are other places where these functions are invoked -- should they be flagged, too?

* Are any of these call sites session specific? I do see that WebPageProxy, for example, has a sessionID. If so, should the logging at those sites be suppressed for private sessions?
Comment 3 Chris Dumez 2016-06-21 15:49:37 PDT
(In reply to comment #2)
> Some questions:
> 
> * How much logging does this emit? When opening a page? When closing a page?
> When putting a page in the background? When putting a page in the
> foreground? In short, will someone complain that there's too much logging?

No, it logs on very specific operations (view visibility state change, load started / ended). It does very little logging.

> 
> * In the lambdas that call a completion handler, I'd put the logging before
> the call of the completion handler. If there's logging in the completion
> handlers, I think the result would read better.

The assertion really gets released then the lambda gets destroyed (because the token is captured in the lambda by value). Therefore, the *last* line in the lambda is really the best place to log that the assertion was released I believe.

> 
> * The common aspect of the added logging is that it involves calls to
> backgroundActivityToken and foregroundActivityToken. There are other places
> where these functions are invoked -- should they be flagged, too?

I focused on the most important ones for now. I will add more if this turns out to be insufficient.

> 
> * Are any of these call sites session specific? I do see that WebPageProxy,
> for example, has a sessionID. If so, should the logging at those sites be
> suppressed for private sessions?

I'll double check, I thought all the code paths I altered weren't specific to a particular session but I could be wrong.
Comment 4 Chris Dumez 2016-06-21 16:14:38 PDT
Created attachment 281792 [details]
Patch
Comment 5 Chris Dumez 2016-06-22 08:48:44 PDT
rdar://problem/17665473
Comment 6 Chris Dumez 2016-06-22 10:00:35 PDT
Comment on attachment 281792 [details]
Patch

Clearing flags on attachment: 281792

Committed r202331: <http://trac.webkit.org/changeset/202331>
Comment 7 Chris Dumez 2016-06-22 10:00:40 PDT
All reviewed patches have been landed.  Closing bug.