Title: [262477] trunk
Revision
262477
Author
cdu...@apple.com
Date
2020-06-02 22:29:14 -0700 (Tue, 02 Jun 2020)

Log Message

[iOS] WKProcessAssertionBackgroundTaskManager incorrectly ignores expiration notifications for daemons
https://bugs.webkit.org/show_bug.cgi?id=212619

Reviewed by Alex Christensen.

WKProcessAssertionBackgroundTaskManager was incorrectly ignoring process assertion expiration notifications
for daemons, because it was relying on visibility to make decisions. For daemons (or ViewServices), we would
not get application visibility notifications and would therefore always assume the app is visible.
As a result, _handleBackgroundTaskExpiration would think it received an outdated expiration notification for
a visible app and would simply re-take the assertion right away.

To address the issue, we now rely on a RunningBoard API that lets us know if the suspension timer has been
started or not (and how much remains on the timer). If the suspension timer is not active when get receive
the expiration notification, we know it is an outdated notification and we ignore it (release and re-take
assertion right away). This can happen if we did not have time to process the expiration notification before
suspended and thus only get it upon resuming. It can also happen if the user re-activates the app right after
the expiration notice has been sent.

* Platform/spi/ios/RunningBoardServicesSPI.h:
* UIProcess/ios/ProcessAssertionIOS.mm:
(processHasActiveRunTimeLimitation):
(-[WKProcessAssertionBackgroundTaskManager init]):
(-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
(-[WKProcessAssertionBackgroundTaskManager _handleBackgroundTaskExpiration]):
(-[WKProcessAssertionBackgroundTaskManager _handleBackgroundTaskExpirationOnMainThread]):
(-[WKProcessAssertionBackgroundTaskManager _releaseBackgroundTask]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (262476 => 262477)


--- trunk/Source/WebKit/ChangeLog	2020-06-03 04:54:21 UTC (rev 262476)
+++ trunk/Source/WebKit/ChangeLog	2020-06-03 05:29:14 UTC (rev 262477)
@@ -1,3 +1,32 @@
+2020-06-02  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] WKProcessAssertionBackgroundTaskManager incorrectly ignores expiration notifications for daemons
+        https://bugs.webkit.org/show_bug.cgi?id=212619
+
+        Reviewed by Alex Christensen.
+
+        WKProcessAssertionBackgroundTaskManager was incorrectly ignoring process assertion expiration notifications
+        for daemons, because it was relying on visibility to make decisions. For daemons (or ViewServices), we would
+        not get application visibility notifications and would therefore always assume the app is visible.
+        As a result, _handleBackgroundTaskExpiration would think it received an outdated expiration notification for
+        a visible app and would simply re-take the assertion right away.
+
+        To address the issue, we now rely on a RunningBoard API that lets us know if the suspension timer has been
+        started or not (and how much remains on the timer). If the suspension timer is not active when get receive
+        the expiration notification, we know it is an outdated notification and we ignore it (release and re-take
+        assertion right away). This can happen if we did not have time to process the expiration notification before
+        suspended and thus only get it upon resuming. It can also happen if the user re-activates the app right after
+        the expiration notice has been sent.
+
+        * Platform/spi/ios/RunningBoardServicesSPI.h:
+        * UIProcess/ios/ProcessAssertionIOS.mm:
+        (processHasActiveRunTimeLimitation):
+        (-[WKProcessAssertionBackgroundTaskManager init]):
+        (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
+        (-[WKProcessAssertionBackgroundTaskManager _handleBackgroundTaskExpiration]):
+        (-[WKProcessAssertionBackgroundTaskManager _handleBackgroundTaskExpirationOnMainThread]):
+        (-[WKProcessAssertionBackgroundTaskManager _releaseBackgroundTask]):
+
 2020-06-02  Christopher Reid  <chris.r...@sony.com>
 
         REGRESSION[r260844]: [GTK][WPE] Inspector GResource no longer updated after WebInspectorUI file updates

Modified: trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h (262476 => 262477)


--- trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h	2020-06-03 04:54:21 UTC (rev 262476)
+++ trunk/Source/WebKit/Platform/spi/ios/RunningBoardServicesSPI.h	2020-06-03 05:29:14 UTC (rev 262477)
@@ -29,6 +29,12 @@
 
 #import <RunningBoardServices/RunningBoardServices.h>
 
+extern const NSTimeInterval RBSProcessTimeLimitationNone;
+
+@interface RBSProcessLimitations : NSObject
+@property (nonatomic, readwrite, assign) NSTimeInterval runTime;
+@end
+
 #else
 
 @interface RBSAttribute : NSObject
@@ -76,9 +82,17 @@
 @property (nonatomic, readonly, copy) NSSet<NSString *> *endowmentNamespaces;
 @end
 
+extern const NSTimeInterval RBSProcessTimeLimitationNone;
+
+@interface RBSProcessLimitations : NSObject
+@property (nonatomic, readwrite, assign) NSTimeInterval runTime;
+@end
+
 @interface RBSProcessHandle : NSObject
 + (RBSProcessHandle *)handleForIdentifier:(RBSProcessIdentifier *)identifier error:(NSError **)outError;
++ (RBSProcessHandle *)currentProcess;
 @property (nonatomic, readonly, strong) RBSProcessState *currentState;
+@property (nonatomic, readonly, strong) RBSProcessLimitations *activeLimitations;
 @end
 
 #endif

Modified: trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (262476 => 262477)


--- trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm	2020-06-03 04:54:21 UTC (rev 262476)
+++ trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm	2020-06-03 05:29:14 UTC (rev 262477)
@@ -48,6 +48,11 @@
 // on the expiration handler getting called).
 static const Seconds releaseBackgroundTaskAfterExpirationDelay { 2_s };
 
+static bool processHasActiveRunTimeLimitation()
+{
+    return [RBSProcessHandle currentProcess].activeLimitations.runTime != RBSProcessTimeLimitationNone;
+}
+
 @interface WKProcessAssertionBackgroundTaskManager
 #if HAVE(RUNNINGBOARD_VISIBILITY_ASSERTIONS)
     : NSObject <RBSAssertionObserving>
@@ -71,7 +76,6 @@
 #endif
 
     WeakHashSet<ProcessAndUIAssertion> _assertionsNeedingBackgroundTask;
-    BOOL _applicationIsBackgrounded;
     dispatch_block_t _pendingTaskReleaseTask;
 }
 
@@ -91,15 +95,14 @@
     _backgroundTask = UIBackgroundTaskInvalid;
 #endif
 
+    // FIXME: Stop relying on UIApplication notifications as this does not work as expected for daemons or ViewServices.
+    // We should likely use ProcessTaskStateObserver to monitor suspension state.
     [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationWillEnterForegroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
-        _applicationIsBackgrounded = NO;
         [self _cancelPendingReleaseTask];
         [self _updateBackgroundTask];
     }];
 
     [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidEnterBackgroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
-        _applicationIsBackgrounded = YES;
-        
         if (![self _hasBackgroundTask])
             WebKit::WebProcessPool::notifyProcessPoolsApplicationIsAboutToSuspend();
     }];
@@ -182,8 +185,8 @@
 - (void)_updateBackgroundTask
 {
     if (!_assertionsNeedingBackgroundTask.computesEmpty() && ![self _hasBackgroundTask]) {
-        if (_applicationIsBackgrounded) {
-            RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Ignored request to start a new background task because the application is already in the background", self);
+        if (processHasActiveRunTimeLimitation()) {
+            RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Ignored request to start a new background task because RunningBoard has already started the expiration timer", self);
             return;
         }
         RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: beginBackgroundTaskWithName", self);
@@ -223,7 +226,9 @@
 
 - (void)_handleBackgroundTaskExpiration
 {
-    RELEASE_LOG(ProcessSuspension, "WKProcessAssertionBackgroundTaskManager: Background task expired while holding WebKit ProcessAssertion (isMainThread? %d).", RunLoop::isMain());
+    auto remainingTime = [RBSProcessHandle currentProcess].activeLimitations.runTime;
+    RELEASE_LOG(ProcessSuspension, "WKProcessAssertionBackgroundTaskManager: Background task expired while holding WebKit ProcessAssertion (isMainThread: %d, remainingTime: %g).", RunLoop::isMain(), remainingTime);
+
     callOnMainRunLoopAndWait([self] {
         [self _handleBackgroundTaskExpirationOnMainThread];
     });
@@ -233,12 +238,13 @@
 {
     ASSERT(RunLoop::isMain());
 
-    // FIXME: While relying on _applicationIsBackgrounded is acceptable for applications, this is inacurate in the case
-    // of daemons.
-    if (!_applicationIsBackgrounded) {
-        // We've received the invalidation warning after the app has become foreground again. In this case, we should not
-        // warn clients of imminent suspension. To be safe (avoid potential killing), we end the task right away and call
-        // _updateBackgroundTask asynchronously to start a new task if necessary.
+    auto remainingTime = [RBSProcessHandle currentProcess].activeLimitations.runTime;
+    RELEASE_LOG(ProcessSuspension, "WKProcessAssertionBackgroundTaskManager: _handleBackgroundTaskExpirationOnMainThread (remainingTime: %g).", remainingTime);
+
+    // If there is no time limitation, then it means that the process is now allowed to run again and the expiration notification
+    // is outdated (e.g. we did not have time to process the expiration notification before suspending and thus only process it
+    // upon resuming, or the user reactivated the app shortly after expiration).
+    if (remainingTime == RBSProcessTimeLimitationNone) {
         [self _releaseBackgroundTask];
         dispatch_async(dispatch_get_main_queue(), ^{
             [self _updateBackgroundTask];
@@ -256,7 +262,7 @@
         return;
 
     RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: endBackgroundTask", self);
-    if (_applicationIsBackgrounded)
+    if (processHasActiveRunTimeLimitation())
         WebKit::WebProcessPool::notifyProcessPoolsApplicationIsAboutToSuspend();
 
 #if HAVE(RUNNINGBOARD_VISIBILITY_ASSERTIONS)

Modified: trunk/WebKitLibraries/WebKitPrivateFrameworkStubs/iOS/13/RunningBoardServices.framework/RunningBoardServices.tbd (262476 => 262477)


--- trunk/WebKitLibraries/WebKitPrivateFrameworkStubs/iOS/13/RunningBoardServices.framework/RunningBoardServices.tbd	2020-06-03 04:54:21 UTC (rev 262476)
+++ trunk/WebKitLibraries/WebKitPrivateFrameworkStubs/iOS/13/RunningBoardServices.framework/RunningBoardServices.tbd	2020-06-03 05:29:14 UTC (rev 262477)
@@ -5,5 +5,6 @@
 platform: ios
 exports:
   - archs:           [ x86_64, arm64, arm64e ]
+    symbols:         [ _RBSProcessTimeLimitationNone ]
     objc-classes:    [ RBSAttribute, RBSDomainAttribute, RBSTarget, RBSAssertion, RBSProcessIdentifier, RBSProcessState, RBSProcessHandle ]
 ...
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to