Bug 160975 – [iOS] Wait a few seconds before releasing network activity assertion after a load

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Some apps do several loads one after the other in a non-visible view. This causes us to release the background assertion every time a load completes and then take one again less than a second after. Every time we release the assertion, we send a PrepareToSuspend IPC to the WebContent process, which does all the clean up to get ready to suspend, only to get a CancelPrepareReadyToSuspend later on because the next load has started. To work around this problem, we now wait a few seconds before releasing the background activity after a load.

[Chris Dumez]

<rdar://problem/27910964>

[Chris Dumez]

Created attachment 286398 [details]
Patch

[Chris Dumez]

Created attachment 286399 [details]
Patch

[Darin Adler]

Comment on attachment 286399 [details]
Patch

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

> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:828
> +    RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), "%p UIProcess is releasing a background assertion because a page load completed", this);

Is there any concern about leaking object pointers into the log?

[Darin Adler]

Comment on attachment 286399 [details]
Patch

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

> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:846
> +        m_releaseActivityTimer.startOneShot(3s);

It’s best to document the reason we chose three seconds here.

[Keith Rollin]

(In reply to comment #4)
> Is there any concern about leaking object pointers into the log?

Not that we're aware of. We've submitted logging that contains object pointers for Data Collection Review and no one raised an issue.

[Chris Dumez]

Committed r204716: <http://trac.webkit.org/changeset/204716>

[Darin Adler]

Comment on attachment 286399 [details]
Patch

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

>> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:846
>> +        m_releaseActivityTimer.startOneShot(3s);
> 
> It’s best to document the reason we chose three seconds here.

Chris, the patch that you landed simply states the reason we are delaying, but sheds no light on why the delay is 3 seconds long as opposed to 1 second, 10 seconds, 100 seconds, or 100 ms.

[Chris Dumez]

(In reply to comment #8)
> Comment on attachment 286399 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=286399&action=review
> 
> >> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:846
> >> +        m_releaseActivityTimer.startOneShot(3s);
> > 
> > It’s best to document the reason we chose three seconds here.
> 
> Chris, the patch that you landed simply states the reason we are delaying,
> but sheds no light on why the delay is 3 seconds long as opposed to 1
> second, 10 seconds, 100 seconds, or 100 ms.

I do not have a great explanation for this value. I wanted a value short enough that delaying suspension for this period of time would be acceptable (3 seconds seemed acceptable for me). I also wanted a value large enough to give the application a chance to do something before trying the suspend the process too aggressively. Also, this value worked for the Facebook App.

[Darin Adler]

(In reply to comment #9)
> I wanted a value short
> enough that delaying suspension for this period of time would be acceptable
> (3 seconds seemed acceptable for me). I also wanted a value large enough to
> give the application a chance to do something before trying the suspend the
> process too aggressively. Also, this value worked for the Facebook App.

This is very close to the type of thing I wanted to see in the code. If someone wants to figure out later if this is the right value, this is the kind of information they need.

I would like to know more about why delaying suspension for three seconds is acceptable. What makes this acceptable? What happens during those three seconds?