Bug 209825 - [iOS] Replace UIKit background task with a RunningBoard FinishTaskInterruptable assertion
Summary: [iOS] Replace UIKit background task with a RunningBoard FinishTaskInterruptab...
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: InRadar
Depends on:
Blocks: 209990
  Show dependency treegraph
 
Reported: 2020-03-31 11:59 PDT by Chris Dumez
Modified: 2020-04-03 15:34 PDT (History)
9 users (show)

See Also:


Attachments
Patch (12.17 KB, patch)
2020-03-31 12:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.75 KB, patch)
2020-03-31 14:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.83 KB, patch)
2020-03-31 15:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (12.98 KB, patch)
2020-03-31 17:15 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 2020-03-31 11:59:25 PDT
Replace UIKit background task with a RunningBoard FinishTask assertion on iOS. Our UIProcess gets terminated too frequently when the UIKit background task expires when the UIProcess holds it for longer than 30 seconds in the background. The RunningBoard FinishTask assertion is supposed to be equivalent but would cause suspension of our UIProcess on expiration, instead of termination.
Comment 1 Radar WebKit Bug Importer 2020-03-31 11:59:39 PDT
<rdar://problem/61118503>
Comment 2 Chris Dumez 2020-03-31 12:21:47 PDT
Created attachment 395077 [details]
Patch
Comment 3 Chris Dumez 2020-03-31 14:07:20 PDT
Looks like I need to fix up the open source build. Will look into it shortly.
Comment 4 Chris Dumez 2020-03-31 14:57:24 PDT
Created attachment 395095 [details]
Patch
Comment 5 Chris Dumez 2020-03-31 15:25:22 PDT
Created attachment 395098 [details]
Patch
Comment 6 Darin Adler 2020-03-31 16:30:20 PDT
Comment on attachment 395098 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:461
> +#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000
> +#define HAVE_RUNNINGBOARD_WEBKIT_ASSERTIONS 1
> +#endif

This doesn’t work as you’d expect. It‘s the thing Tim has been reminding us about. IOS_FAMILY plus __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000 gives iOS *without* watchOS and tvOS.
Comment 7 Chris Dumez 2020-03-31 17:15:06 PDT
(In reply to Darin Adler from comment #6)
> Comment on attachment 395098 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=395098&action=review
> 
> > Source/WTF/wtf/PlatformHave.h:461
> > +#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000
> > +#define HAVE_RUNNINGBOARD_WEBKIT_ASSERTIONS 1
> > +#endif
> 
> This doesn’t work as you’d expect. It‘s the thing Tim has been reminding us
> about. IOS_FAMILY plus __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000 gives iOS
> *without* watchOS and tvOS.

I had copied an existing example in our code hoping bad things in the code base had been fixed.
Comment 8 Chris Dumez 2020-03-31 17:15:24 PDT
Created attachment 395109 [details]
Patch
Comment 9 Chris Dumez 2020-04-02 09:10:28 PDT
Ping review?
Comment 10 Geoffrey Garen 2020-04-02 13:22:45 PDT
Comment on attachment 395109 [details]
Patch

r=me
Comment 11 EWS 2020-04-02 13:49:09 PDT
Committed r259414: <https://trac.webkit.org/changeset/259414>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395109 [details].