Title: [231663] trunk
Revision
231663
Author
[email protected]
Date
2018-05-10 15:02:31 -0700 (Thu, 10 May 2018)

Log Message

[iOS] Apps that are not visible may not get suspended if they trigger page loads while in the background
https://bugs.webkit.org/show_bug.cgi?id=185318

Reviewed by Geoffrey Garen.

Whenever there is a page load going on, we take a background process assertion to delay process
suspension until this load completes. However, there is also a 3 seconds grace period after
a load is complete to allow the app to trigger a new load shortly after. This grace period was
introduced to support use cases where a visible app does loads in an offscreen view. However,
it can be abused by apps running in the background as they could trigger new page loads while
in the background to delay process suspension. This patch tightens the policy so that only
apps that are currently visible get to use this grace period. Apps that are in the background
get to finish their current load and will then get suspended.

* UIProcess/Cocoa/NavigationState.mm:
(WebKit::NavigationState::didChangeIsLoading):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (231662 => 231663)


--- trunk/Source/WebKit/ChangeLog	2018-05-10 21:56:58 UTC (rev 231662)
+++ trunk/Source/WebKit/ChangeLog	2018-05-10 22:02:31 UTC (rev 231663)
@@ -1,5 +1,24 @@
 2018-05-10  Chris Dumez  <[email protected]>
 
+        [iOS] Apps that are not visible may not get suspended if they trigger page loads while in the background
+        https://bugs.webkit.org/show_bug.cgi?id=185318
+
+        Reviewed by Geoffrey Garen.
+
+        Whenever there is a page load going on, we take a background process assertion to delay process
+        suspension until this load completes. However, there is also a 3 seconds grace period after
+        a load is complete to allow the app to trigger a new load shortly after. This grace period was
+        introduced to support use cases where a visible app does loads in an offscreen view. However,
+        it can be abused by apps running in the background as they could trigger new page loads while
+        in the background to delay process suspension. This patch tightens the policy so that only
+        apps that are currently visible get to use this grace period. Apps that are in the background
+        get to finish their current load and will then get suspended.
+
+        * UIProcess/Cocoa/NavigationState.mm:
+        (WebKit::NavigationState::didChangeIsLoading):
+
+2018-05-10  Chris Dumez  <[email protected]>
+
         'Cross-Origin-Options header implementation follow-up
         https://bugs.webkit.org/show_bug.cgi?id=185520
 

Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm (231662 => 231663)


--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm	2018-05-10 21:56:58 UTC (rev 231662)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm	2018-05-10 22:02:31 UTC (rev 231663)
@@ -1157,17 +1157,23 @@
 {
 #if PLATFORM(IOS)
     if (m_webView->_page->pageLoadState().isLoading()) {
-        if (m_releaseActivityTimer.isActive())
+        if (m_releaseActivityTimer.isActive()) {
+            RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - A new page load started while the UIProcess was still holding a page load background assertion", this);
             m_releaseActivityTimer.stop();
-        else {
+        } else {
             RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - UIProcess is taking a background assertion because a page load started", this);
             ASSERT(!m_activityToken);
             m_activityToken = m_webView->_page->process().throttler().backgroundActivityToken();
         }
-    } else {
-        // Delay releasing the background activity for 3 seconds to give the application a chance to start another navigation
-        // before suspending the WebContent process <rdar://problem/27910964>.
-        m_releaseActivityTimer.startOneShot(3_s);
+    } else if (m_activityToken) {
+        if (m_webView._isBackground)
+            releaseNetworkActivityToken();
+        else {
+            // The application is visible so we delay releasing the background activity for 3 seconds to give it a chance to start another navigation
+            // before suspending the WebContent process <rdar://problem/27910964>.
+            RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - Page load completed and UIProcess will be releasing background assertion soon unless a new load starts", this);
+            m_releaseActivityTimer.startOneShot(3_s);
+        }
     }
 #endif
 

Modified: trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm (231662 => 231663)


--- trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2018-05-10 21:56:58 UTC (rev 231662)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2018-05-10 22:02:31 UTC (rev 231663)
@@ -49,6 +49,18 @@
 #import <wtf/SoftLinking.h>
 SOFT_LINK_FRAMEWORK(UIKit)
 SOFT_LINK_CLASS(UIKit, UIWindow)
+
+@implementation WKWebView (WKWebViewTestingQuicks)
+
+// TestWebKitAPI is currently not a UIApplication so we are unable to track if it is in
+// the background or not (https://bugs.webkit.org/show_bug.cgi?id=175204). This can
+// cause our processes to get suspended on iOS. We work around this by having
+// WKWebView._isBackground always return NO in the context of API tests.
+- (BOOL)_isBackground
+{
+    return NO;
+}
+@end
 #endif
 
 @implementation TestMessageHandler {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to