[iOS] Take process assertion to prevent the service worker process from getting suspended
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 18:13:47 +0000 (18:13 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 18:13:47 +0000 (18:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180735

Reviewed by Brady Eidson.

Take process assertion to prevent the service worker process from getting suspended while
it is still needed. We use the same policy as for the network process, meaning that
unsuspended WebContent processes prevent the service worker process from getting suspended.

This patch still does not enable service workers on iOS. The demo at https://mdn.github.io/sw-test/
appears to work. However, things are not working as expected for mobile.twitter.com where I
see the fetches intercepted by the service worker fail when offline for some reason (unrelated
to process suspension).

* UIProcess/WebProcessPool.cpp:
(WebKit::m_foregroundWebProcessCounter):
(WebKit::m_backgroundWebProcessCounter):
(WebKit::WebProcessPool::ensureNetworkProcess):
(WebKit::WebProcessPool::establishWorkerContextConnectionToStorageProcess):
(WebKit::WebProcessPool::disconnectProcess):
(WebKit::WebProcessPool::updateProcessAssertions):
(WebKit::WebProcessPool::reinstateNetworkProcessAssertionState):
* UIProcess/WebProcessPool.h:
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didSetAssertionState):
* UIProcess/WebProcessProxy.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225850 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/UIProcess/WebProcessPool.h
Source/WebKit/UIProcess/WebProcessProxy.cpp
Source/WebKit/UIProcess/WebProcessProxy.h

index 0d7b72c..fbd6e1a 100644 (file)
@@ -1,3 +1,32 @@
+2017-12-13  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] Take process assertion to prevent the service worker process from getting suspended
+        https://bugs.webkit.org/show_bug.cgi?id=180735
+
+        Reviewed by Brady Eidson.
+
+        Take process assertion to prevent the service worker process from getting suspended while
+        it is still needed. We use the same policy as for the network process, meaning that
+        unsuspended WebContent processes prevent the service worker process from getting suspended.
+
+        This patch still does not enable service workers on iOS. The demo at https://mdn.github.io/sw-test/
+        appears to work. However, things are not working as expected for mobile.twitter.com where I
+        see the fetches intercepted by the service worker fail when offline for some reason (unrelated
+        to process suspension).
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::m_foregroundWebProcessCounter):
+        (WebKit::m_backgroundWebProcessCounter):
+        (WebKit::WebProcessPool::ensureNetworkProcess):
+        (WebKit::WebProcessPool::establishWorkerContextConnectionToStorageProcess):
+        (WebKit::WebProcessPool::disconnectProcess):
+        (WebKit::WebProcessPool::updateProcessAssertions):
+        (WebKit::WebProcessPool::reinstateNetworkProcessAssertionState):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::didSetAssertionState):
+        * UIProcess/WebProcessProxy.h:
+
 2017-12-13  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Update OptionsGTK.cmake and NEWS for 2.19.3 release.
index fc423b7..74f2553 100644 (file)
@@ -234,6 +234,10 @@ WebProcessPool::WebProcessPool(API::ProcessPoolConfiguration& configuration)
     , m_hiddenPageThrottlingAutoIncreasesCounter([this](RefCounterEvent) { m_hiddenPageThrottlingTimer.startOneShot(0_s); })
     , m_hiddenPageThrottlingTimer(RunLoop::main(), this, &WebProcessPool::updateHiddenPageThrottlingAutoIncreaseLimit)
     , m_serviceWorkerProcessTerminationTimer(RunLoop::main(), this, &WebProcessPool::terminateServiceWorkerProcess)
+#if PLATFORM(IOS)
+    , m_foregroundWebProcessCounter([this](RefCounterEvent) { updateProcessAssertions(); })
+    , m_backgroundWebProcessCounter([this](RefCounterEvent) { updateProcessAssertions(); })
+#endif
 {
     if (m_configuration->shouldHaveLegacyDataStore())
         m_websiteDataStore = API::WebsiteDataStore::createLegacy(legacyWebsiteDataStoreConfiguration(m_configuration));
@@ -486,8 +490,7 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
 
     if (m_didNetworkProcessCrash) {
         m_didNetworkProcessCrash = false;
-        for (auto& process : m_processes)
-            process->reinstateNetworkProcessAssertionState(*m_networkProcess);
+        reinstateNetworkProcessAssertionState(*m_networkProcess);
         if (m_websiteDataStore)
             m_websiteDataStore->websiteDataStore().networkProcessDidCrash();
     }
@@ -598,6 +601,7 @@ void WebProcessPool::establishWorkerContextConnectionToStorageProcess(StoragePro
 
     auto serviceWorkerProcessProxy = ServiceWorkerProcessProxy::create(*this, m_websiteDataStore->websiteDataStore());
     m_serviceWorkerProcess = serviceWorkerProcessProxy.ptr();
+    updateProcessAssertions();
     initializeNewWebProcess(serviceWorkerProcessProxy.get(), m_websiteDataStore->websiteDataStore());
     m_processes.append(WTFMove(serviceWorkerProcessProxy));
 
@@ -923,8 +927,10 @@ void WebProcessPool::disconnectProcess(WebProcessProxy* process)
     if (m_processWithPageCache == process)
         m_processWithPageCache = nullptr;
 #if ENABLE(SERVICE_WORKER)
-    if (m_serviceWorkerProcess == process)
+    if (m_serviceWorkerProcess == process) {
         m_serviceWorkerProcess = nullptr;
+        updateProcessAssertions();
+    }
 #endif
 
     static_cast<WebContextSupplement*>(supplement<WebGeolocationManagerProxy>())->processDidClose(process);
@@ -1745,4 +1751,60 @@ void WebProcessPool::reportWebContentCPUTime(Seconds cpuTime, uint64_t activityS
 #endif
 }
 
+void WebProcessPool::updateProcessAssertions()
+{
+#if PLATFORM(IOS)
+#if ENABLE(SERVICE_WORKER)
+    auto updateServiceWorkerProcessAssertion = [&] {
+        if (m_serviceWorkerProcess && m_foregroundWebProcessCounter.value()) {
+            if (!m_foregroundTokenForServiceWorkerProcess)
+                m_foregroundTokenForServiceWorkerProcess = m_serviceWorkerProcess->throttler().foregroundActivityToken();
+            m_backgroundTokenForServiceWorkerProcess = nullptr;
+            return;
+        }
+        if (m_serviceWorkerProcess && m_backgroundWebProcessCounter.value()) {
+            if (!m_backgroundTokenForServiceWorkerProcess)
+                m_backgroundTokenForServiceWorkerProcess = m_serviceWorkerProcess->throttler().backgroundActivityToken();
+            m_foregroundTokenForServiceWorkerProcess = nullptr;
+            return;
+        }
+        m_foregroundTokenForServiceWorkerProcess = nullptr;
+        m_backgroundTokenForServiceWorkerProcess = nullptr;
+    };
+    updateServiceWorkerProcessAssertion();
+#endif
+
+    auto updateNetworkProcessAssertion = [&] {
+        if (m_foregroundWebProcessCounter.value()) {
+            if (!m_foregroundTokenForNetworkProcess)
+                m_foregroundTokenForNetworkProcess = ensureNetworkProcess().throttler().foregroundActivityToken();
+            m_backgroundTokenForNetworkProcess = nullptr;
+            return;
+        }
+        if (m_backgroundWebProcessCounter.value()) {
+            if (!m_backgroundTokenForNetworkProcess)
+                m_backgroundTokenForNetworkProcess = ensureNetworkProcess().throttler().backgroundActivityToken();
+            m_foregroundTokenForNetworkProcess = nullptr;
+            return;
+        }
+        m_foregroundTokenForNetworkProcess = nullptr;
+        m_backgroundTokenForNetworkProcess = nullptr;
+    };
+    updateNetworkProcessAssertion();
+#endif
+}
+
+void WebProcessPool::reinstateNetworkProcessAssertionState(NetworkProcessProxy& newNetworkProcessProxy)
+{
+#if PLATFORM(IOS)
+    // The network process crashed; take new tokens for the new network process.
+    if (m_backgroundTokenForNetworkProcess)
+        m_backgroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().backgroundActivityToken();
+    else if (m_foregroundTokenForNetworkProcess)
+        m_foregroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().foregroundActivityToken();
+#else
+    UNUSED_PARAM(newNetworkProcessProxy);
+#endif
+}
+
 } // namespace WebKit
index 1dd5351..988c33c 100644 (file)
@@ -425,6 +425,11 @@ public:
     static uint64_t registerProcessPoolCreationListener(Function<void(WebProcessPool&)>&&);
     static void unregisterProcessPoolCreationListener(uint64_t identifier);
 
+#if PLATFORM(IOS)
+    ForegroundWebProcessToken foregroundWebProcessToken() const { return ForegroundWebProcessToken(m_foregroundWebProcessCounter.count()); }
+    BackgroundWebProcessToken backgroundWebProcessToken() const { return BackgroundWebProcessToken(m_backgroundWebProcessCounter.count()); }
+#endif
+
 private:
     void platformInitialize();
 
@@ -451,6 +456,9 @@ private:
     void processStoppedUsingGamepads(WebProcessProxy&);
 #endif
 
+    void reinstateNetworkProcessAssertionState(NetworkProcessProxy&);
+    void updateProcessAssertions();
+
     // IPC::MessageReceiver.
     // Implemented in generated WebProcessPoolMessageReceiver.cpp
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
@@ -637,6 +645,17 @@ private:
 
     HashMap<PAL::SessionID, HashSet<WebPageProxy*>> m_sessionToPagesMap;
     RunLoop::Timer<WebProcessPool> m_serviceWorkerProcessTerminationTimer;
+
+#if PLATFORM(IOS)
+    ForegroundWebProcessCounter m_foregroundWebProcessCounter;
+    BackgroundWebProcessCounter m_backgroundWebProcessCounter;
+    ProcessThrottler::ForegroundActivityToken m_foregroundTokenForNetworkProcess;
+    ProcessThrottler::BackgroundActivityToken m_backgroundTokenForNetworkProcess;
+#if ENABLE(SERVICE_WORKER)
+    ProcessThrottler::ForegroundActivityToken m_foregroundTokenForServiceWorkerProcess;
+    ProcessThrottler::BackgroundActivityToken m_backgroundTokenForServiceWorkerProcess;
+#endif
+#endif
 };
 
 template<typename T>
index 32f9464..9a12bc2 100644 (file)
@@ -1057,51 +1057,39 @@ void WebProcessProxy::didCancelProcessSuspension()
     m_throttler.didCancelProcessSuspension();
 }
 
-void WebProcessProxy::reinstateNetworkProcessAssertionState(NetworkProcessProxy& newNetworkProcessProxy)
-{
-#if PLATFORM(IOS)
-    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
-
-    // The network process crashed; take new tokens for the new network process.
-    if (m_backgroundTokenForNetworkProcess)
-        m_backgroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().backgroundActivityToken();
-    else if (m_foregroundTokenForNetworkProcess)
-        m_foregroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().foregroundActivityToken();
-#else
-    UNUSED_PARAM(newNetworkProcessProxy);
-#endif
-}
-
 void WebProcessProxy::didSetAssertionState(AssertionState state)
 {
 #if PLATFORM(IOS)
-    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
+    if (isServiceWorkerProcess())
+        return;
+
+    ASSERT(!m_backgroundToken || !m_foregroundToken);
 
     switch (state) {
     case AssertionState::Suspended:
         RELEASE_LOG(ProcessSuspension, "%p - WebProcessProxy::didSetAssertionState(Suspended) release all assertions for network process", this);
-        m_foregroundTokenForNetworkProcess = nullptr;
-        m_backgroundTokenForNetworkProcess = nullptr;
+        m_foregroundToken = nullptr;
+        m_backgroundToken = nullptr;
         for (auto& page : m_pageMap.values())
             page->processWillBecomeSuspended();
         break;
 
     case AssertionState::Background:
         RELEASE_LOG(ProcessSuspension, "%p - WebProcessProxy::didSetAssertionState(Background) taking background assertion for network process", this);
-        m_backgroundTokenForNetworkProcess = processPool().ensureNetworkProcess().throttler().backgroundActivityToken();
-        m_foregroundTokenForNetworkProcess = nullptr;
+        m_backgroundToken = processPool().backgroundWebProcessToken();
+        m_foregroundToken = nullptr;
         break;
     
     case AssertionState::Foreground:
         RELEASE_LOG(ProcessSuspension, "%p - WebProcessProxy::didSetAssertionState(Foreground) taking foreground assertion for network process", this);
-        m_foregroundTokenForNetworkProcess = processPool().ensureNetworkProcess().throttler().foregroundActivityToken();
-        m_backgroundTokenForNetworkProcess = nullptr;
+        m_foregroundToken = processPool().foregroundWebProcessToken();
+        m_backgroundToken = nullptr;
         for (auto& page : m_pageMap.values())
             page->processWillBecomeForeground();
         break;
     }
 
-    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
+    ASSERT(!m_backgroundToken || !m_foregroundToken);
 #else
     UNUSED_PARAM(state);
 #endif
index 36972a5..39b1d25 100644 (file)
@@ -77,6 +77,15 @@ struct WebNavigationDataStore;
 struct WebPageCreationParameters;
 struct WebsiteData;
 
+#if PLATFORM(IOS)
+enum ForegroundWebProcessCounterType { };
+typedef RefCounter<ForegroundWebProcessCounterType> ForegroundWebProcessCounter;
+typedef ForegroundWebProcessCounter::Token ForegroundWebProcessToken;
+enum BackgroundWebProcessCounterType { };
+typedef RefCounter<BackgroundWebProcessCounterType> BackgroundWebProcessCounter;
+typedef BackgroundWebProcessCounter::Token BackgroundWebProcessToken;
+#endif
+
 class WebProcessProxy : public ChildProcessProxy, public ResponsivenessTimer::Client, private ProcessThrottlerClient {
 public:
     typedef HashMap<uint64_t, RefPtr<WebBackForwardListItem>> WebBackForwardListItemMap;
@@ -174,8 +183,6 @@ public:
 
     ProcessThrottler& throttler() { return m_throttler; }
 
-    void reinstateNetworkProcessAssertionState(NetworkProcessProxy&);
-
     void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
     void didReceiveMainThreadPing();
     void didReceiveBackgroundResponsivenessPing();
@@ -280,8 +287,8 @@ private:
     ProcessThrottler m_throttler;
     ProcessThrottler::BackgroundActivityToken m_tokenForHoldingLockedFiles;
 #if PLATFORM(IOS)
-    ProcessThrottler::ForegroundActivityToken m_foregroundTokenForNetworkProcess;
-    ProcessThrottler::BackgroundActivityToken m_backgroundTokenForNetworkProcess;
+    ForegroundWebProcessToken m_foregroundToken;
+    BackgroundWebProcessToken m_backgroundToken;
 #endif
 
     HashMap<String, uint64_t> m_pageURLRetainCountMap;