Changeset 252712 in webkit

Timestamp:

Nov 20, 2019, 3:01:40 PM (7 years ago)

Author:

Chris Dumez

Message:

[iOS] Make sure WebContent process does not get suspended while it is holding a process assertion for the UIProcess
​https://bugs.webkit.org/show_bug.cgi?id=204418

Reviewed by Jer Noble.

Make sure WebContent process does not get suspended while it is holding a process assertion for the UIProcess. We
see this happening in sysdiagnoses, and it means the system ends up killing the WebContent process because it leaked
a process assertion.

• WebProcess/WebProcess.h:
• WebProcess/cocoa/WebProcessCocoa.mm:

(WebKit::WebProcess::processTaskStateDidChange):
(WebKit::WebProcess::releaseProcessWasResumedAssertions):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/WebProcess.h (modified) (2 diffs)
• WebProcess/cocoa/WebProcessCocoa.mm (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-11-20  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] Make sure WebContent process does not get suspended while it is holding a process assertion for the UIProcess
+        https://bugs.webkit.org/show_bug.cgi?id=204418
+
+        Reviewed by Jer Noble.
+
+        Make sure WebContent process does not get suspended while it is holding a process assertion for the UIProcess. We
+        see this happening in sysdiagnoses, and it means the system ends up killing the WebContent process because it leaked
+        a process assertion.
+
+        * WebProcess/WebProcess.h:
+        * WebProcess/cocoa/WebProcessCocoa.mm:
+        (WebKit::WebProcess::processTaskStateDidChange):
+        (WebKit::WebProcess::releaseProcessWasResumedAssertions):
+
 2019-11-19  Brian Burg  <bburg@apple.com>
 

trunk/Source/WebKit/WebProcess/WebProcess.h

@@ -465 +465 @@
     bool shouldFreezeOnSuspension() const;
     void updateFreezerStatus();
+
+    void releaseProcessWasResumedAssertions();
 #endif
 
@@ -543 +545 @@
     WebSQLiteDatabaseTracker m_webSQLiteDatabaseTracker;
     RefPtr<ProcessTaskStateObserver> m_taskStateObserver;
-    Lock m_processWasResumedUIAssertionLock;
+    Lock m_processWasResumedAssertionsLock;
     RetainPtr<BKSProcessAssertion> m_processWasResumedUIAssertion;
+    RetainPtr<BKSProcessAssertion> m_processWasResumedOwnAssertion;
 #endif
 

trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

@@ -301 +301 @@
         return;
 
-    LockHolder holder(m_processWasResumedUIAssertionLock);
-    if (m_processWasResumedUIAssertion)
+    LockHolder holder(m_processWasResumedAssertionsLock);
+    if (m_processWasResumedUIAssertion && m_processWasResumedOwnAssertion)
         return;
 
@@ -308 +308 @@
     // to ensure that it too is awakened.
     RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processTaskStateChanged() Taking 'WebProcess was resumed' assertion on behalf on UIProcess", this);
-    m_processWasResumedUIAssertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:parentProcessConnection()->remoteProcessID() flags:BKSProcessAssertionPreventTaskSuspend reason:BKSProcessAssertionReasonFinishTask name:@"WebProcess was resumed" withHandler:nil]);
-
+    m_processWasResumedUIAssertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:parentProcessConnection()->remoteProcessID() flags:BKSProcessAssertionPreventTaskSuspend reason:BKSProcessAssertionReasonFinishTask name:@"WebProcess was resumed" withHandler:^(BOOL acquired) {
+        if (!acquired)
+            RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebProcess::processTaskStateDidChange() failed to take 'WebProcess was resumed' assertion for parent process", this);
+    }]);
     m_processWasResumedUIAssertion.get().invalidationHandler = [this] {
-        LockHolder holder(m_processWasResumedUIAssertionLock);
-        RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processTaskStateChanged() Releasing 'WebProcess was resumed' assertion on behalf on UIProcess due invalidation", this);
+        RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebProcess::processTaskStateChanged() Releasing 'WebProcess was resumed' assertion on behalf on UIProcess due to invalidation", this);
+        releaseProcessWasResumedAssertions();
+    };
+    m_processWasResumedOwnAssertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:getpid() flags:BKSProcessAssertionPreventTaskSuspend reason:BKSProcessAssertionReasonFinishTask name:@"WebProcess was resumed" withHandler:^(BOOL acquired) {
+        if (!acquired)
+            RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebProcess::processTaskStateDidChange() failed to take 'WebProcess was resumed' assertion for WebContent process", this);
+    }]);
+    m_processWasResumedOwnAssertion.get().invalidationHandler = [this] {
+        RELEASE_LOG_ERROR(ProcessSuspension, "%p - WebProcess::processTaskStateChanged() Releasing 'WebProcess was resumed' assertion on behalf on WebContent process due to invalidation", this);
+        releaseProcessWasResumedAssertions();
+    };
+
+    parentProcessConnection()->sendWithAsyncReply(Messages::WebProcessProxy::ProcessWasResumed(), [this] {
+        RELEASE_LOG(ProcessSuspension, "%p - WebProcess::processTaskStateDidChange() Parent process handled ProcessWasResumed IPC, releasing our assertions", this);
+        releaseProcessWasResumedAssertions();
+    });
+}
+
+void WebProcess::releaseProcessWasResumedAssertions()
+{
+    LockHolder holder(m_processWasResumedAssertionsLock);
+    if (m_processWasResumedUIAssertion) {
+        RELEASE_LOG(ProcessSuspension, "%p - WebProcess::releaseProcessWasResumedAssertions() Releasing parent process 'WebProcess was resumed' assertion", this);
         [m_processWasResumedUIAssertion invalidate];
         m_processWasResumedUIAssertion = nullptr;
-    };
-
-    // This will cause the parent process to send a ParentProcessDidHandleProcessWasResumed IPC back, so that we can release our assertion on its behalf.
-    parentProcessConnection()->sendWithAsyncReply(Messages::WebProcessProxy::ProcessWasResumed(), [this] {
-        LockHolder holder(m_processWasResumedUIAssertionLock);
-        ASSERT(m_processWasResumedUIAssertion);
-        RELEASE_LOG(ProcessSuspension, "%p - WebProcess::parentProcessDidHandleProcessWasResumed() Releasing 'WebProcess was resumed' assertion on behalf on UIProcess", this);
-        [m_processWasResumedUIAssertion invalidate];
-        m_processWasResumedUIAssertion = nullptr;
-    });
+    }
+    if (m_processWasResumedOwnAssertion) {
+        RELEASE_LOG(ProcessSuspension, "%p - WebProcess::releaseProcessWasResumedAssertions() Releasing WebContent process 'WebProcess was resumed' assertion", this);
+        [m_processWasResumedOwnAssertion invalidate];
+        m_processWasResumedOwnAssertion = nullptr;
+    }
 }