Attachment 339611 Details for Bug 185333 – Patch

[patch] Patch

bug-185333-20180504163549.patch (text/plain), 7.65 KB, created by Chris Dumez on 2018-05-04 16:35:50 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-04 16:35:50 PDT

Size: 7.65 KB

patch

obsolete

>Subversion Revision: 231383
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 32aceca8de47f91cefffa8f4431224e420e77245..b4c57282d8c70d2b6bf626257930169ef5b78dcc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-05-04  Chris Dumez  <cdumez@apple.com>
>+
>+        [iOS] Release page load process assertion if the screen is locked
>+        https://bugs.webkit.org/show_bug.cgi?id=185333
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We normally take a background process assertion during page loads to allow them to complete
>+        even if the tab / app is backgrounded. We should however avoid doing so when the backgrounding
>+        is caused by the screen locking. Keeping the process assertion in this case would prevent the
>+        whole device from sleeping longer than it should, thus negatively impacting power.
>+
>+        * UIProcess/Cocoa/NavigationState.h:
>+        * UIProcess/Cocoa/NavigationState.mm:
>+        (WebKit::NavigationState::NavigationState):
>+        (WebKit::NavigationState::releaseNetworkActivityToken):
>+        (WebKit::NavigationState::didChangeIsLoading):
>+        * UIProcess/ios/WebPageProxyIOS.mm:
>+        (WebKit::WebPageProxy::applicationDidEnterBackground):
>+
> 2018-05-04  Chris Dumez  <cdumez@apple.com>
> 
>         [iOS] Apps that are not visible may not get suspended if they trigger page loads while in the background
>diff --git a/Source/WebKit/UIProcess/Cocoa/NavigationState.h b/Source/WebKit/UIProcess/Cocoa/NavigationState.h
>index b273dbd81aea250086d9cc0e513dd4fb2ada336b..4a9c1cd8aa0bcf6353ba7e6615af2810f0fb2c37 100644
>--- a/Source/WebKit/UIProcess/Cocoa/NavigationState.h
>+++ b/Source/WebKit/UIProcess/Cocoa/NavigationState.h
>@@ -82,6 +82,11 @@ public:
> 
>     void didFirstPaint();
> 
>+#if PLATFORM(IOS)
>+    enum class NetworkActivityTokenReleaseReason { LoadCompleted, ScreenLocked };
>+    void releaseNetworkActivityToken(NetworkActivityTokenReleaseReason);
>+#endif
>+
> private:
>     class NavigationClient final : public API::NavigationClient {
>     public:
>@@ -172,7 +177,7 @@ private:
>     void didChangeWebProcessIsResponsive() override;
> 
> #if PLATFORM(IOS)
>-    void releaseNetworkActivityToken();
>+    void releaseNetworkActivityTokenAfterLoadCompletion() { releaseNetworkActivityToken(NetworkActivityTokenReleaseReason::LoadCompleted); }
> #endif
> 
>     WKWebView *m_webView;
>diff --git a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>index 203706afb66e833ca5967216bd2fb65ff13b2bad..8c3fa9d7496026b0b631aa7a11f9c2fa1287e0bb 100644
>--- a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm
>@@ -97,7 +97,7 @@ NavigationState::NavigationState(WKWebView *webView)
>     , m_navigationDelegateMethods()
>     , m_historyDelegateMethods()
> #if PLATFORM(IOS)
>-    , m_releaseActivityTimer(RunLoop::current(), this, &NavigationState::releaseNetworkActivityToken)
>+    , m_releaseActivityTimer(RunLoop::current(), this, &NavigationState::releaseNetworkActivityTokenAfterLoadCompletion)
> #endif
> {
>     ASSERT(m_webView->_page);
>@@ -1145,11 +1145,21 @@ void NavigationState::willChangeIsLoading()
> }
> 
> #if PLATFORM(IOS)
>-void NavigationState::releaseNetworkActivityToken()
>+void NavigationState::releaseNetworkActivityToken(NetworkActivityTokenReleaseReason reason)
> {
>-    RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p UIProcess is releasing a background assertion because a page load completed", this);
>-    ASSERT(m_activityToken);
>+    if (!m_activityToken)
>+        return;
>+
>+    switch (reason) {
>+    case NetworkActivityTokenReleaseReason::LoadCompleted:
>+        RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p NavigationState is releasing background process assertion because a page load completed", this);
>+        break;
>+    case NetworkActivityTokenReleaseReason::ScreenLocked:
>+        RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p NavigationState is releasing background process assertion because the screen was locked", this);
>+        break;
>+    }
>     m_activityToken = nullptr;
>+    m_releaseActivityTimer.stop();
> }
> #endif
> 
>@@ -1157,21 +1167,25 @@ void NavigationState::didChangeIsLoading()
> {
> #if PLATFORM(IOS)
>     if (m_webView->_page->pageLoadState().isLoading()) {
>+        // We do not take a network activity token if a load starts after the screen has been locked.
>+        if ([UIApp isSuspendedUnderLock])
>+            return;
>+
>         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);
>+            RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - NavigationState keeps its process network assertion because a new page load started", this);
>             m_releaseActivityTimer.stop();
>-        } 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);
>+        }
>+        if (!m_activityToken) {
>+            RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - NavigationState is taking a process network assertion because a page load started", this);
>             m_activityToken = m_webView->_page->process().throttler().backgroundActivityToken();
>         }
>     } else if (m_activityToken) {
>         if (m_webView._isBackground)
>-            releaseNetworkActivityToken();
>+            releaseNetworkActivityTokenAfterLoadCompletion();
>         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);
>+            RELEASE_LOG_IF(m_webView->_page->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - NavigationState will release its process network assertion soon because the page load completed", this);
>             m_releaseActivityTimer.startOneShot(3_s);
>         }
>     }
>diff --git a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>index 7098bb79462f3b808e6fc10f287fdc7766203869..93732367b15404b1aeb7a4c0f90ef77d15b898db 100644
>--- a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>@@ -34,6 +34,7 @@
> #import "InteractionInformationAtPosition.h"
> #import "Logging.h"
> #import "NativeWebKeyboardEvent.h"
>+#import "NavigationState.h"
> #import "PageClient.h"
> #import "PrintInfo.h"
> #import "RemoteLayerTreeDrawingAreaProxy.h"
>@@ -677,6 +678,11 @@ void WebPageProxy::saveImageToLibrary(const SharedMemory::Handle& imageHandle, u
> void WebPageProxy::applicationDidEnterBackground()
> {
>     bool isSuspendedUnderLock = [UIApp isSuspendedUnderLock];
>+
>+    // We normally delay process suspension when the app is backgrounded until the current page load completes. However,
>+    // we do not want to do so when the screen is locked for power reasons.
>+    if (isSuspendedUnderLock)
>+        NavigationState::fromWebPage(*this).releaseNetworkActivityToken(NavigationState::NetworkActivityTokenReleaseReason::ScreenLocked);
>     m_process->send(Messages::WebPage::ApplicationDidEnterBackground(isSuspendedUnderLock), m_pageID);
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185333: 339611 | 340136 | 340138