Attachment 371230 Details for Bug 198488 – Patch

[patch] Patch

bug-198488-20190603164619.patch (text/plain), 11.43 KB, created by Chris Dumez on 2019-06-03 16:46:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-06-03 16:46:20 PDT

Size: 11.43 KB

patch

obsolete

>Subversion Revision: 246036
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 3638a0682ed018fabaabb5c75b40302d57e2d0e6..0a64e50ffeebec56ff24e5911067177a36e6d0b0 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-06-03  Chris Dumez  <cdumez@apple.com>
>+
>+        [iOS] Do not prevent app suspension for more than 20 seconds after getting backgrounded
>+        https://bugs.webkit.org/show_bug.cgi?id=198488
>+        <rdar://problem/50837208>
>+
>+        Reviewed by Geoff Garen.
>+
>+        Do not prevent app suspension for more than 20 seconds after getting backgrounded on iOS. We
>+        do this by implementing our own expiration handler which notifies our child processes of
>+        their imminent suspension before ending the background task that was preventing suspension.
>+
>+        * UIProcess/ios/ProcessAssertionIOS.mm:
>+        (isBackgroundState):
>+        (-[WKProcessAssertionBackgroundTaskManager init]):
>+        (-[WKProcessAssertionBackgroundTaskManager _scheduleTimeoutTask]):
>+        (-[WKProcessAssertionBackgroundTaskManager _cancelTimeoutTask]):
>+        (-[WKProcessAssertionBackgroundTaskManager _backgroundTaskExpired]):
>+        (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
>+        (-[WKProcessAssertionBackgroundTaskManager _releaseBackgroundTask]):
>+
> 2019-06-03  Darin Adler  <darin@apple.com>
> 
>         Finish cleanup of String::number for floating point
>diff --git a/Source/WebKit/UIProcess/ApplicationStateTracker.mm b/Source/WebKit/UIProcess/ApplicationStateTracker.mm
>index 507922faaf880b5ba2aecaad827fe911208d29f4..ba8a7a8452424679ef20fe4c14740d585d13b50d 100644
>--- a/Source/WebKit/UIProcess/ApplicationStateTracker.mm
>+++ b/Source/WebKit/UIProcess/ApplicationStateTracker.mm
>@@ -142,9 +142,11 @@ ApplicationStateTracker::ApplicationStateTracker(UIView *view, SEL didEnterBackg
>         pid_t applicationPID = serviceViewController._hostProcessIdentifier;
>         ASSERT(applicationPID);
> 
>-        auto applicationStateMonitor = adoptNS([[BKSApplicationStateMonitor alloc] init]);
>-        m_isInBackground = isBackgroundState([applicationStateMonitor mostElevatedApplicationStateForPID:applicationPID]);
>-        [applicationStateMonitor invalidate];
>+        {
>+            auto applicationStateMonitor = adoptNS([[BKSApplicationStateMonitor alloc] init]);
>+            m_isInBackground = isBackgroundState([applicationStateMonitor mostElevatedApplicationStateForPID:applicationPID]);
>+            [applicationStateMonitor invalidate];
>+        }
> 
>         // Workaround for <rdar://problem/34028921>. If the host application is StoreKitUIService then it is also a ViewService
>         // and is always in the background. We need to treat StoreKitUIService as foreground for the purpose of process suspension
>diff --git a/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm b/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>index 2aca868bb8b5713159c91d2abe5feca9461cc72f..f852b22ac878eb02f93a01895f1788efc2bf093f 100644
>--- a/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm
>@@ -41,6 +41,7 @@ using WebKit::ProcessAndUIAssertion;
> // the background task before the UIKit timeout (We get killed if we do not release the background task within 5 seconds
> // on the expiration handler getting called).
> static const Seconds releaseBackgroundTaskAfterExpirationDelay { 2_s };
>+static const Seconds maximumBackgroundTaskDuration { 20_s };
> 
> @interface WKProcessAssertionBackgroundTaskManager : NSObject
> 
>@@ -56,7 +57,8 @@ @implementation WKProcessAssertionBackgroundTaskManager
>     UIBackgroundTaskIdentifier _backgroundTask;
>     HashSet<ProcessAndUIAssertion*> _assertionsNeedingBackgroundTask;
>     BOOL _applicationIsBackgrounded;
>-    dispatch_block_t _pendingTaskReleaseTask;
>+    dispatch_block_t _releaseTask;
>+    dispatch_block_t _timeoutTask;
> }
> 
> + (WKProcessAssertionBackgroundTaskManager *)shared
>@@ -65,6 +67,11 @@ + (WKProcessAssertionBackgroundTaskManager *)shared
>     return shared;
> }
> 
>+static bool isBackgroundState(BKSApplicationState state)
>+{
>+    return state == BKSApplicationStateBackgroundRunning || state == BKSApplicationStateBackgroundTaskSuspended;
>+}
>+
> - (instancetype)init
> {
>     self = [super init];
>@@ -73,14 +80,30 @@ - (instancetype)init
> 
>     _backgroundTask = UIBackgroundTaskInvalid;
> 
>-    [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationWillEnterForegroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
>+    {
>+        auto applicationStateMonitor = adoptNS([[BKSApplicationStateMonitor alloc] init]);
>+        _applicationIsBackgrounded = isBackgroundState([applicationStateMonitor mostElevatedApplicationStateForPID:getpid()]);
>+        [applicationStateMonitor invalidate];
>+    }
>+
>+    NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
>+    UIApplication *application = [UIApplication sharedApplication];
>+
>+    [notificationCenter addObserverForName:UIApplicationWillEnterForegroundNotification object:application queue:nil usingBlock:^(NSNotification *) {
>+        RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Application will enter foreground", self);
>         _applicationIsBackgrounded = NO;
>-        [self _cancelPendingReleaseTask];
>+        [self _cancelReleaseTask];
>+        [self _cancelTimeoutTask];
>         [self _updateBackgroundTask];
>     }];
> 
>-    [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidEnterBackgroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
>+    [notificationCenter addObserverForName:UIApplicationDidEnterBackgroundNotification object:application queue:nil usingBlock:^(NSNotification *) {
>+        RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Application did enter background", self);
>         _applicationIsBackgrounded = YES;
>+
>+        // We do not want to keep the app awake for more than 20 seconds after it gets backgrounded, so start the timeout timer now.
>+        if (_backgroundTask != UIBackgroundTaskInvalid)
>+            [self _scheduleTimeoutTask];
>     }];
> 
>     return self;
>@@ -112,33 +135,81 @@ - (void)_notifyAssertionsOfImminentSuspension
>         assertion->uiAssertionWillExpireImminently();
> }
> 
>+- (void)_scheduleTimeoutTask
>+{
>+    ASSERT(_backgroundTask != UIBackgroundTaskInvalid);
>+
>+    if (_timeoutTask)
>+        return;
>+
>+    RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _scheduleTimeoutTask", self);
>+    _timeoutTask = dispatch_block_create((dispatch_block_flags_t)0, ^{
>+        _timeoutTask = nil;
>+        RELEASE_LOG_ERROR(ProcessSuspension, "Background task was running for too long so WebKit will end it shortly.");
>+        [self _backgroundTaskExpired];
>+    });
>+
>+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, maximumBackgroundTaskDuration.value() * NSEC_PER_SEC), dispatch_get_main_queue(), _timeoutTask);
>+#if !__has_feature(objc_arc)
>+    // dispatch_async() does a Block_copy() / Block_release() on behalf of the caller.
>+    Block_release(_timeoutTask);
>+#endif
>+}
>+
>+- (void)_cancelTimeoutTask
>+{
>+    if (!_timeoutTask)
>+        return;
>+
>+    RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _cancelTimeoutTask", self);
>+    dispatch_block_cancel(_timeoutTask);
>+    _timeoutTask = nil;
>+}
> 
> - (void)_scheduleReleaseTask
> {
>-    ASSERT(!_pendingTaskReleaseTask);
>-    if (_pendingTaskReleaseTask)
>+    ASSERT(!_releaseTask);
>+    if (_releaseTask)
>         return;
> 
>     RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _scheduleReleaseTask because the expiration handler has been called", self);
>-    _pendingTaskReleaseTask = dispatch_block_create((dispatch_block_flags_t)0, ^{
>-        _pendingTaskReleaseTask = nil;
>+    _releaseTask = dispatch_block_create((dispatch_block_flags_t)0, ^{
>+        _releaseTask = nil;
>         [self _releaseBackgroundTask];
>     });
>-    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, releaseBackgroundTaskAfterExpirationDelay.value() * NSEC_PER_SEC), dispatch_get_main_queue(), _pendingTaskReleaseTask);
>+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, releaseBackgroundTaskAfterExpirationDelay.value() * NSEC_PER_SEC), dispatch_get_main_queue(), _releaseTask);
> #if !__has_feature(objc_arc)
>     // dispatch_async() does a Block_copy() / Block_release() on behalf of the caller.
>-    Block_release(_pendingTaskReleaseTask);
>+    Block_release(_releaseTask);
> #endif
> }
> 
>-- (void)_cancelPendingReleaseTask
>+- (void)_cancelReleaseTask
> {
>-    if (!_pendingTaskReleaseTask)
>+    if (!_releaseTask)
>         return;
> 
>-    RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _cancelPendingReleaseTask because the application is foreground again", self);
>-    dispatch_block_cancel(_pendingTaskReleaseTask);
>-    _pendingTaskReleaseTask = nil;
>+    RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _cancelReleaseTask because the application is foreground again", self);
>+    dispatch_block_cancel(_releaseTask);
>+    _releaseTask = nil;
>+}
>+
>+- (void)_backgroundTaskExpired
>+{
>+    RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _backgroundTaskExpired", self);
>+    [self _cancelTimeoutTask];
>+
>+    // Tell our child processes they will suspend imminently.
>+    if (RunLoop::isMain())
>+        [self _notifyAssertionsOfImminentSuspension];
>+    else {
>+        // The expiration handler gets called on a non-main thread when the underlying assertion could not be taken (rdar://problem/27278419).
>+        dispatch_sync(dispatch_get_main_queue(), ^{
>+            [self _notifyAssertionsOfImminentSuspension];
>+        });
>+    }
>+
>+    [self _scheduleReleaseTask];
> }
> 
> - (void)_updateBackgroundTask
>@@ -150,17 +221,8 @@ - (void)_updateBackgroundTask
>         }
>         RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - beginBackgroundTaskWithName", self);
>         _backgroundTask = [[UIApplication sharedApplication] beginBackgroundTaskWithName:@"com.apple.WebKit.ProcessAssertion" expirationHandler:^{
>-            RELEASE_LOG_ERROR(ProcessSuspension, "Background task expired while holding WebKit ProcessAssertion (isMainThread? %d).", RunLoop::isMain());
>-            // The expiration handler gets called on a non-main thread when the underlying assertion could not be taken (rdar://problem/27278419).
>-            if (RunLoop::isMain())
>-                [self _notifyAssertionsOfImminentSuspension];
>-            else {
>-                dispatch_sync(dispatch_get_main_queue(), ^{
>-                    [self _notifyAssertionsOfImminentSuspension];
>-                });
>-            }
>-
>-            [self _scheduleReleaseTask];
>+            RELEASE_LOG_ERROR(ProcessSuspension, "Background task expired while holding WebKit ProcessAssertion (isMainThread? %d, _applicationIsBackgrounded? %d).", RunLoop::isMain(), _applicationIsBackgrounded);
>+            [self _backgroundTaskExpired];
>         }];
>     } else if (_assertionsNeedingBackgroundTask.isEmpty())
>         [self _releaseBackgroundTask];
>@@ -173,6 +235,8 @@ - (void)_releaseBackgroundTask
> 
>     RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - endBackgroundTask", self);
>     [[UIApplication sharedApplication] endBackgroundTask:_backgroundTask];
>+    [self _cancelReleaseTask];
>+    [self _cancelTimeoutTask];
>     _backgroundTask = UIBackgroundTaskInvalid;
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 198488: 371194 | 371201 | 371227 | 371230