Diff
Modified: branches/safari-608.1.24-branch/Source/WebKit/ChangeLog (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/ChangeLog 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/ChangeLog 2019-05-16 05:43:31 UTC (rev 245384)
@@ -1,5 +1,89 @@
2019-05-15 Kocsen Chung <[email protected]>
+ Cherry-pick r245334. rdar://problem/50234105
+
+ [WK2][iOS] UIProcess may get killed because it is taking too long to release its background task after expiration
+ https://bugs.webkit.org/show_bug.cgi?id=197893
+ <rdar://problem/50234105>
+
+ Reviewed by Alex Christensen.
+
+ The UIProcess may get killed because it is taking too long to release its background task after its expiration handler
+ was called. The reason is that the background task's expiration handler was sequentially sending a ProcessWillSuspendImminently
+ synchronous IPC to each of its child processes and only then ends the background task. By the time we receive the response from
+ all child processes, it may be too late and we get killed.
+
+ To address the issue, we now:
+ 1. Send the ProcessWillSuspendImminently asynchronously so that all processes can do their processing in parallel
+ 2. After 2 seconds, the UIProcess releases the background task (We get killed after ~5 seconds)
+
+ Also, to make sure that the UIProcess supends promptly, we now make sure we never start a new background task *after*
+ the app has been backgrounded. The intention of our background task is too finish critical work (like releasing locked
+ files) after the app gets backgrounded, not to start new work and delay process suspension.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::processWillSuspendImminently):
+ * NetworkProcess/NetworkProcess.h:
+ * NetworkProcess/NetworkProcess.messages.in:
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKProcessAssertionBackgroundTaskManager init]):
+ (-[WKProcessAssertionBackgroundTaskManager _scheduleReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _cancelPendingReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::didReceiveSyncMessage):
+ (WebKit::WebProcess::processWillSuspendImminently):
+ * WebProcess/WebProcess.h:
+ * WebProcess/WebProcess.messages.in:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245334 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-05-15 Chris Dumez <[email protected]>
+
+ [WK2][iOS] UIProcess may get killed because it is taking too long to release its background task after expiration
+ https://bugs.webkit.org/show_bug.cgi?id=197893
+ <rdar://problem/50234105>
+
+ Reviewed by Alex Christensen.
+
+ The UIProcess may get killed because it is taking too long to release its background task after its expiration handler
+ was called. The reason is that the background task's expiration handler was sequentially sending a ProcessWillSuspendImminently
+ synchronous IPC to each of its child processes and only then ends the background task. By the time we receive the response from
+ all child processes, it may be too late and we get killed.
+
+ To address the issue, we now:
+ 1. Send the ProcessWillSuspendImminently asynchronously so that all processes can do their processing in parallel
+ 2. After 2 seconds, the UIProcess releases the background task (We get killed after ~5 seconds)
+
+ Also, to make sure that the UIProcess supends promptly, we now make sure we never start a new background task *after*
+ the app has been backgrounded. The intention of our background task is too finish critical work (like releasing locked
+ files) after the app gets backgrounded, not to start new work and delay process suspension.
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::NetworkProcess::processWillSuspendImminently):
+ * NetworkProcess/NetworkProcess.h:
+ * NetworkProcess/NetworkProcess.messages.in:
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::sendProcessWillSuspendImminently):
+ * UIProcess/ios/ProcessAssertionIOS.mm:
+ (-[WKProcessAssertionBackgroundTaskManager init]):
+ (-[WKProcessAssertionBackgroundTaskManager _scheduleReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _cancelPendingReleaseTask]):
+ (-[WKProcessAssertionBackgroundTaskManager _updateBackgroundTask]):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::didReceiveSyncMessage):
+ (WebKit::WebProcess::processWillSuspendImminently):
+ * WebProcess/WebProcess.h:
+ * WebProcess/WebProcess.messages.in:
+
+2019-05-15 Kocsen Chung <[email protected]>
+
Cherry-pick r245322. rdar://problem/50107679
Allow NSFileCoordinator to be called from WebContent process
Modified: branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.cpp 2019-05-16 05:43:31 UTC (rev 245384)
@@ -2023,14 +2023,15 @@
#endif
}
-void NetworkProcess::processWillSuspendImminently(CompletionHandler<void(bool)>&& completionHandler)
+void NetworkProcess::processWillSuspendImminently()
{
+ RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently() BEGIN", this);
#if PLATFORM(IOS_FAMILY) && ENABLE(INDEXED_DATABASE)
for (auto& server : m_idbServers.values())
server->tryStop(IDBServer::ShouldForceStop::Yes);
#endif
actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
- completionHandler(true);
+ RELEASE_LOG(ProcessSuspension, "%p - NetworkProcess::processWillSuspendImminently() END", this);
}
void NetworkProcess::prepareToSuspend()
Modified: branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.h (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.h 2019-05-16 05:43:31 UTC (rev 245384)
@@ -179,7 +179,7 @@
bool canHandleHTTPSServerTrustEvaluation() const { return m_canHandleHTTPSServerTrustEvaluation; }
- void processWillSuspendImminently(CompletionHandler<void(bool)>&&);
+ void processWillSuspendImminently();
void prepareToSuspend();
void cancelPrepareToSuspend();
void processDidResume();
Modified: branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.messages.in (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.messages.in 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/NetworkProcess/NetworkProcess.messages.in 2019-05-16 05:43:31 UTC (rev 245384)
@@ -77,7 +77,7 @@
ProcessDidTransitionToBackground()
ProcessDidTransitionToForeground()
- ProcessWillSuspendImminently() -> (bool handled) Synchronous
+ ProcessWillSuspendImminently()
PrepareToSuspend()
CancelPrepareToSuspend()
ProcessDidResume()
Modified: branches/safari-608.1.24-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp 2019-05-16 05:43:31 UTC (rev 245384)
@@ -1013,11 +1013,8 @@
void NetworkProcessProxy::sendProcessWillSuspendImminently()
{
- if (!canSendMessage())
- return;
-
- bool handled = false;
- sendSync(Messages::NetworkProcess::ProcessWillSuspendImminently(), Messages::NetworkProcess::ProcessWillSuspendImminently::Reply(handled), 0, 1_s);
+ if (canSendMessage())
+ send(Messages::NetworkProcess::ProcessWillSuspendImminently(), 0);
}
void NetworkProcessProxy::sendPrepareToSuspend()
Modified: branches/safari-608.1.24-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-05-16 05:43:31 UTC (rev 245384)
@@ -1141,11 +1141,8 @@
void WebProcessProxy::sendProcessWillSuspendImminently()
{
- if (!canSendMessage())
- return;
-
- bool handled = false;
- sendSync(Messages::WebProcess::ProcessWillSuspendImminently(), Messages::WebProcess::ProcessWillSuspendImminently::Reply(handled), 0, 1_s);
+ if (canSendMessage())
+ send(Messages::WebProcess::ProcessWillSuspendImminently(), 0);
}
void WebProcessProxy::sendPrepareToSuspend()
Modified: branches/safari-608.1.24-branch/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm 2019-05-16 05:43:31 UTC (rev 245384)
@@ -37,6 +37,11 @@
using WebKit::ProcessAndUIAssertion;
+// This gives some time to our child processes to process the ProcessWillSuspendImminently IPC but makes sure we release
+// 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 };
+
@interface WKProcessAssertionBackgroundTaskManager : NSObject
+ (WKProcessAssertionBackgroundTaskManager *)shared;
@@ -50,7 +55,8 @@
{
UIBackgroundTaskIdentifier _backgroundTask;
HashSet<ProcessAndUIAssertion*> _assertionsNeedingBackgroundTask;
- BOOL _assertionHasExpiredInTheBackground;
+ BOOL _applicationIsBackgrounded;
+ dispatch_block_t _pendingTaskReleaseTask;
}
+ (WKProcessAssertionBackgroundTaskManager *)shared
@@ -68,10 +74,15 @@
_backgroundTask = UIBackgroundTaskInvalid;
[[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationWillEnterForegroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
- _assertionHasExpiredInTheBackground = NO;
+ _applicationIsBackgrounded = NO;
+ [self _cancelPendingReleaseTask];
[self _updateBackgroundTask];
}];
+ [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidEnterBackgroundNotification object:[UIApplication sharedApplication] queue:nil usingBlock:^(NSNotification *) {
+ _applicationIsBackgrounded = YES;
+ }];
+
return self;
}
@@ -101,12 +112,40 @@
assertion->uiAssertionWillExpireImminently();
}
+
+- (void)_scheduleReleaseTask
+{
+ ASSERT(!_pendingTaskReleaseTask);
+ if (_pendingTaskReleaseTask)
+ 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;
+ [self _releaseBackgroundTask];
+ });
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, releaseBackgroundTaskAfterExpirationDelay.value() * NSEC_PER_SEC), dispatch_get_main_queue(), _pendingTaskReleaseTask);
+#if !__has_feature(objc_arc)
+ // dispatch_async() does a Block_copy() / Block_release() on behalf of the caller.
+ Block_release(_pendingTaskReleaseTask);
+#endif
+}
+
+- (void)_cancelPendingReleaseTask
+{
+ if (!_pendingTaskReleaseTask)
+ return;
+
+ RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - _cancelPendingReleaseTask because the application is foreground again", self);
+ dispatch_block_cancel(_pendingTaskReleaseTask);
+ _pendingTaskReleaseTask = nil;
+}
+
- (void)_updateBackgroundTask
{
if (!_assertionsNeedingBackgroundTask.isEmpty() && _backgroundTask == UIBackgroundTaskInvalid) {
- if (_assertionHasExpiredInTheBackground) {
- RELEASE_LOG_ERROR(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager: Ignored request to start a background task because we're still in the background and the previous task expired", self);
- // Our invalidation handler would not get called if we tried to re-take a new background assertion at this point, and the UIProcess would get killed (rdar://problem/50001505).
+ 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);
return;
}
RELEASE_LOG(ProcessSuspension, "%p - WKProcessAssertionBackgroundTaskManager - beginBackgroundTaskWithName", self);
@@ -121,9 +160,7 @@
});
}
- // Remember that the assertion has expired in the background so we do not try to re-take it until the application becomes foreground again.
- _assertionHasExpiredInTheBackground = YES;
- [self _releaseBackgroundTask];
+ [self _scheduleReleaseTask];
}];
} else if (_assertionsNeedingBackgroundTask.isEmpty())
[self _releaseBackgroundTask];
Modified: branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.cpp (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.cpp 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.cpp 2019-05-16 05:43:31 UTC (rev 245384)
@@ -731,8 +731,6 @@
{
if (messageReceiverMap().dispatchSyncMessage(connection, decoder, replyEncoder))
return;
-
- didReceiveSyncWebProcessMessage(connection, decoder, replyEncoder);
}
void WebProcess::didReceiveMessage(IPC::Connection& connection, IPC::Decoder& decoder)
@@ -1489,19 +1487,11 @@
});
}
-void WebProcess::processWillSuspendImminently(CompletionHandler<void(bool)>&& completionHandler)
+void WebProcess::processWillSuspendImminently()
{
- if (parentProcessConnection()->inSendSync()) {
- // Avoid reentrency bugs such as rdar://problem/21605505 by just bailing
- // if we get an incoming ProcessWillSuspendImminently message when waiting for a
- // reply to a sync message.
- // FIXME: ProcessWillSuspendImminently should not be a sync message.
- return completionHandler(false);
- }
-
- RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently()", this);
+ RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently() BEGIN", this);
actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend::No);
- completionHandler(true);
+ RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processWillSuspendImminently() END", this);
}
void WebProcess::prepareToSuspend()
Modified: branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.h (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.h 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.h 2019-05-16 05:43:31 UTC (rev 245384)
@@ -216,7 +216,7 @@
void setHiddenPageDOMTimerThrottlingIncreaseLimit(int milliseconds);
- void processWillSuspendImminently(CompletionHandler<void(bool)>&&);
+ void processWillSuspendImminently();
void prepareToSuspend();
void cancelPrepareToSuspend();
void processDidResume();
@@ -418,7 +418,6 @@
// Implemented in generated WebProcessMessageReceiver.cpp
void didReceiveWebProcessMessage(IPC::Connection&, IPC::Decoder&);
- void didReceiveSyncWebProcessMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
#if PLATFORM(MAC)
void setScreenProperties(const WebCore::ScreenProperties&);
Modified: branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.messages.in (245383 => 245384)
--- branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.messages.in 2019-05-16 05:43:27 UTC (rev 245383)
+++ branches/safari-608.1.24-branch/Source/WebKit/WebProcess/WebProcess.messages.in 2019-05-16 05:43:31 UTC (rev 245384)
@@ -96,7 +96,7 @@
EnsureAutomationSessionProxy(String sessionIdentifier)
DestroyAutomationSessionProxy()
- ProcessWillSuspendImminently() -> (bool handled) Synchronous
+ ProcessWillSuspendImminently()
PrepareToSuspend()
CancelPrepareToSuspend()
ProcessDidResume()
