Changeset 202696 in webkit

Timestamp:

Jun 30, 2016 1:17:30 PM (8 years ago)

Author:

Chris Dumez

Message:

[iOS] Process suspension is prevented for 30 seconds after closing a tab
​https://bugs.webkit.org/show_bug.cgi?id=159280
<rdar://problem/27014867>

Reviewed by Gavin Barraclough.

When closing a tab, ConnectionTerminationWatchdog takes a background
assertion on behalf of the WebContent process and only releases it
after 30 seconds, allowing for the WebContent process to exit cleanly
without worrying about getting suspended too early.

However, the child process normally exits much sooner than this and
we end up holding a process assertion for no reason for a full 30
seconds anyway. This patch addresses the issue by registering an
invalidation handler with the BKSProcessAssertion and releasing our
our assertion in such case. The invalidation handler gets called
as soon as the child process exits.

• Platform/spi/ios/AssertionServicesSPI.h:
• UIProcess/ProcessAssertion.cpp:

(WebKit::ProcessAssertion::ProcessAssertion):

• UIProcess/ProcessAssertion.h:

(WebKit::ProcessAssertion::ProcessAssertion):
(WebKit::ProcessAssertion::validity):

• UIProcess/ios/ProcessAssertionIOS.mm:

(WebKit::ProcessAssertion::ProcessAssertion):
(WebKit::ProcessAssertion::~ProcessAssertion):
(WebKit::ProcessAndUIAssertion::updateRunInBackgroundCount):
(WebKit::ProcessAndUIAssertion::ProcessAndUIAssertion):
(WebKit::ProcessAssertion::setState): Deleted.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Platform/spi/ios/AssertionServicesSPI.h (modified) (2 diffs)
• UIProcess/ProcessAssertion.cpp (modified) (1 diff)
• UIProcess/ProcessAssertion.h (modified) (3 diffs)
• UIProcess/ios/ProcessAssertionIOS.mm (modified) (4 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-06-30  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] Process suspension is prevented for 30 seconds after closing a tab
+        https://bugs.webkit.org/show_bug.cgi?id=159280
+        <rdar://problem/27014867>
+
+        Reviewed by Gavin Barraclough.
+
+        When closing a tab, ConnectionTerminationWatchdog takes a background
+        assertion on behalf of the WebContent process and only releases it
+        after 30 seconds, allowing for the WebContent process to exit cleanly
+        without worrying about getting suspended too early.
+
+        However, the child process normally exits much sooner than this and
+        we end up holding a process assertion for no reason for a full 30
+        seconds anyway. This patch addresses the issue by registering an
+        invalidation handler with the BKSProcessAssertion and releasing our
+        our assertion in such case. The invalidation handler gets called
+        as soon as the child process exits.
+
+        * Platform/spi/ios/AssertionServicesSPI.h:
+        * UIProcess/ProcessAssertion.cpp:
+        (WebKit::ProcessAssertion::ProcessAssertion):
+        * UIProcess/ProcessAssertion.h:
+        (WebKit::ProcessAssertion::ProcessAssertion):
+        (WebKit::ProcessAssertion::validity):
+        * UIProcess/ios/ProcessAssertionIOS.mm:
+        (WebKit::ProcessAssertion::ProcessAssertion):
+        (WebKit::ProcessAssertion::~ProcessAssertion):
+        (WebKit::ProcessAndUIAssertion::updateRunInBackgroundCount):
+        (WebKit::ProcessAndUIAssertion::ProcessAndUIAssertion):
+        (WebKit::ProcessAssertion::setState): Deleted.
+
 2016-06-30  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebKit2/Platform/spi/ios/AssertionServicesSPI.h

@@ -73 +73 @@
 typedef uint32_t BKSProcessAssertionReason;
 
+typedef void (^BKSProcessAssertionInvalidationHandler)(void);
 typedef void (^BKSProcessAssertionAcquisitionHandler)(BOOL acquired);
 
@@ -81 +82 @@
 @property (nonatomic, assign) BKSProcessAssertionFlags flags;
 - (id)initWithPID:(pid_t)pid flags:(BKSProcessAssertionFlags)flags reason:(BKSProcessAssertionReason)reason name:(NSString *)name withHandler:(BKSProcessAssertionAcquisitionHandler)handler;
+
+@property (nonatomic, copy) BKSProcessAssertionInvalidationHandler invalidationHandler;
 - (void)invalidate;
 @end

trunk/Source/WebKit2/UIProcess/ProcessAssertion.cpp

@@ -31 +31 @@
 namespace WebKit {
 
-ProcessAssertion::ProcessAssertion(pid_t, AssertionState assertionState)
+ProcessAssertion::ProcessAssertion(pid_t, AssertionState assertionState, std::function<void()>)
     : m_assertionState(assertionState)
 {

trunk/Source/WebKit2/UIProcess/ProcessAssertion.h

@@ -27 +27 @@
 #define ProcessAssertion_h
 
+#include <functional>
+
 #if PLATFORM(IOS) && !PLATFORM(IOS_SIMULATOR)
 #include <wtf/RetainPtr.h>
@@ -48 +50 @@
 class ProcessAssertion {
 public:
-    ProcessAssertion(pid_t, AssertionState);
+    ProcessAssertion(pid_t, AssertionState, std::function<void()> invalidationCallback = { });
     ~ProcessAssertion();
 
@@ -57 +59 @@
     void setState(AssertionState);
 
+#if PLATFORM(IOS) && !PLATFORM(IOS_SIMULATOR)
+protected:
+    enum class Validity { No, Yes, Unset };
+    Validity validity() const { return m_validity; }
+#endif
+
 private:
 #if PLATFORM(IOS) && !PLATFORM(IOS_SIMULATOR)
     RetainPtr<BKSProcessAssertion> m_assertion;
+    Validity m_validity { Validity::Unset };
 #endif
     AssertionState m_assertionState;

trunk/Source/WebKit2/UIProcess/ios/ProcessAssertionIOS.mm

@@ -144 +144 @@
 }
 
-ProcessAssertion::ProcessAssertion(pid_t pid, AssertionState assertionState)
-{
-    m_assertionState = assertionState;
-    
+ProcessAssertion::ProcessAssertion(pid_t pid, AssertionState assertionState, std::function<void()> invalidationCallback)
+    : m_assertionState(assertionState)
+{
     BKSProcessAssertionAcquisitionHandler handler = ^(BOOL acquired) {
         if (!acquired) {
-            LOG_ERROR("Unable to acquire assertion for process %d", pid);
+            LOG_ALWAYS_ERROR(true, "Unable to acquire assertion for process %d", pid);
             ASSERT_NOT_REACHED();
+            m_validity = Validity::No;
+            invalidationCallback();
         }
     };
     m_assertion = adoptNS([[BKSProcessAssertion alloc] initWithPID:pid flags:flagsForState(assertionState) reason:BKSProcessAssertionReasonExtension name:@"Web content visible" withHandler:handler]);
+    m_assertion.get().invalidationHandler = ^() {
+        m_validity = Validity::No;
+        invalidationCallback();
+    };
 }
 
 ProcessAssertion::~ProcessAssertion()
 {
+    m_assertion.get().invalidationHandler = nil;
+
     if (ProcessAssertionClient* client = this->client())
         [[WKProcessAssertionBackgroundTaskManager shared] removeClient:*client];
@@ -175 +182 @@
 void ProcessAndUIAssertion::updateRunInBackgroundCount()
 {
-    bool shouldHoldBackgroundAssertion = state() != AssertionState::Suspended;
+    bool shouldHoldBackgroundAssertion = validity() != Validity::No && state() != AssertionState::Suspended;
 
     if (shouldHoldBackgroundAssertion) {
@@ -189 +196 @@
 
 ProcessAndUIAssertion::ProcessAndUIAssertion(pid_t pid, AssertionState assertionState)
-    : ProcessAssertion(pid, assertionState)
+    : ProcessAssertion(pid, assertionState, [this] { updateRunInBackgroundCount(); })
 {
     updateRunInBackgroundCount();
@@ -220 +227 @@
 namespace WebKit {
 
-ProcessAssertion::ProcessAssertion(pid_t, AssertionState assertionState)
+ProcessAssertion::ProcessAssertion(pid_t, AssertionState assertionState, std::function<void()>)
     : m_assertionState(assertionState)
 {