- Revision
- 184774
- Author
- barraclo...@apple.com
- Date
- 2015-05-22 10:19:39 -0700 (Fri, 22 May 2015)
Log Message
Networking process sometimes suspended while WebContent runnable
https://bugs.webkit.org/show_bug.cgi?id=145291
<rdar://problem/21033029>
Reviewed by Andreas Kling.
There is an asymmetry in how the Networking process suspension is managed.
We drop the assertion (& suspend the process) whenever the WebProcess says
it is ready to suspend, but we only take the assertion (& resume execution)
when the WebContent process is actually resumed. This leaves a race – where
the WebContent process was willing to be suspended, but the UI app changed
its mind suspending it. Since the WebContent process never actually suspended
(we only prepared it to), we never send a resume, and since we never resume,
we never wake the Networking process.
The fix is pretty clean. A web process should take a token on the networking
process precisely whenever it holds an assertion on the web content process.
This also enables a nice refinement, that when holding a BG assertion on the
WebContent process we only need to hold a BG assertion on the networking
process.
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didSetAssertionState):
- no-op
* UIProcess/Network/NetworkProcessProxy.h:
- added didSetAssertionState.
* UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::updateAssertionNow):
(WebKit::ProcessThrottler::updateAssertion):
(WebKit::ProcessThrottler::didConnectToProcess):
- whenever taking/updating the assertion, inform the client via didSetAssertionState.
* UIProcess/ProcessThrottlerClient.h:
- added didSetAssertionState.
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didFinishLaunching):
removed initializeNetworkProcessActivityToken - this is taken via ProcessThrottler::didConnectToProcess.
(WebKit::WebProcessProxy::sendProcessDidResume):
- removed call to initializeNetworkProcessActivityToken - updated via didSetAssertionState.
(WebKit::WebProcessProxy::processReadyToSuspend):
- removed update of m_tokenForNetworkProcess - updated via didSetAssertionState.
(WebKit::WebProcessProxy::didSetAssertionState):
- update m_backgroundTokenForNetworkProcess/m_foregroundTokenForNetworkProcess.
(WebKit::WebProcessProxy::initializeNetworkProcessActivityToken): Deleted.
- updated via didSetAssertionState.
* UIProcess/WebProcessProxy.h:
- added didSetAssertionState, added separate FG/BG activity tokens for Networking process.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (184773 => 184774)
--- trunk/Source/WebKit2/ChangeLog 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/ChangeLog 2015-05-22 17:19:39 UTC (rev 184774)
@@ -1,3 +1,52 @@
+2015-05-21 Gavin Barraclough <barraclo...@apple.com>
+
+ Networking process sometimes suspended while WebContent runnable
+ https://bugs.webkit.org/show_bug.cgi?id=145291
+ <rdar://problem/21033029>
+
+ Reviewed by Andreas Kling.
+
+ There is an asymmetry in how the Networking process suspension is managed.
+ We drop the assertion (& suspend the process) whenever the WebProcess says
+ it is ready to suspend, but we only take the assertion (& resume execution)
+ when the WebContent process is actually resumed. This leaves a race – where
+ the WebContent process was willing to be suspended, but the UI app changed
+ its mind suspending it. Since the WebContent process never actually suspended
+ (we only prepared it to), we never send a resume, and since we never resume,
+ we never wake the Networking process.
+
+ The fix is pretty clean. A web process should take a token on the networking
+ process precisely whenever it holds an assertion on the web content process.
+ This also enables a nice refinement, that when holding a BG assertion on the
+ WebContent process we only need to hold a BG assertion on the networking
+ process.
+
+ * UIProcess/Network/NetworkProcessProxy.cpp:
+ (WebKit::NetworkProcessProxy::didSetAssertionState):
+ - no-op
+ * UIProcess/Network/NetworkProcessProxy.h:
+ - added didSetAssertionState.
+ * UIProcess/ProcessThrottler.cpp:
+ (WebKit::ProcessThrottler::updateAssertionNow):
+ (WebKit::ProcessThrottler::updateAssertion):
+ (WebKit::ProcessThrottler::didConnectToProcess):
+ - whenever taking/updating the assertion, inform the client via didSetAssertionState.
+ * UIProcess/ProcessThrottlerClient.h:
+ - added didSetAssertionState.
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::didFinishLaunching):
+ removed initializeNetworkProcessActivityToken - this is taken via ProcessThrottler::didConnectToProcess.
+ (WebKit::WebProcessProxy::sendProcessDidResume):
+ - removed call to initializeNetworkProcessActivityToken - updated via didSetAssertionState.
+ (WebKit::WebProcessProxy::processReadyToSuspend):
+ - removed update of m_tokenForNetworkProcess - updated via didSetAssertionState.
+ (WebKit::WebProcessProxy::didSetAssertionState):
+ - update m_backgroundTokenForNetworkProcess/m_foregroundTokenForNetworkProcess.
+ (WebKit::WebProcessProxy::initializeNetworkProcessActivityToken): Deleted.
+ - updated via didSetAssertionState.
+ * UIProcess/WebProcessProxy.h:
+ - added didSetAssertionState, added separate FG/BG activity tokens for Networking process.
+
2015-05-22 Dan Bernstein <m...@apple.com>
Fixed the fix for <rdar://problem/20830673> Symlink the WebKit dylib instead of just the framework directory
Modified: trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp 2015-05-22 17:19:39 UTC (rev 184774)
@@ -356,6 +356,10 @@
m_throttler.processReadyToSuspend();
}
+void NetworkProcessProxy::didSetAssertionState(AssertionState)
+{
+}
+
void NetworkProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
{
if (!isHoldingLockedFiles) {
Modified: trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.h (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.h 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/Network/NetworkProcessProxy.h 2015-05-22 17:19:39 UTC (rev 184774)
@@ -92,6 +92,7 @@
void sendPrepareToSuspend() override;
void sendCancelPrepareToSuspend() override;
void sendProcessDidResume() override;
+ void didSetAssertionState(AssertionState) override;
// IPC::Connection::Client
virtual void didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) override;
Modified: trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/ProcessThrottler.cpp 2015-05-22 17:19:39 UTC (rev 184774)
@@ -55,8 +55,10 @@
void ProcessThrottler::updateAssertionNow()
{
m_suspendTimer.stop();
- if (m_assertion)
+ if (m_assertion) {
m_assertion->setState(assertionState());
+ m_process.didSetAssertionState(assertionState());
+ }
}
void ProcessThrottler::updateAssertion()
@@ -69,6 +71,7 @@
m_process.sendPrepareToSuspend();
m_suspendTimer.startOneShot(processSuspensionTimeout);
m_assertion->setState(AssertionState::Background);
+ m_process.didSetAssertionState(AssertionState::Background);
return;
}
@@ -88,6 +91,7 @@
{
m_suspendTimer.stop();
m_assertion = std::make_unique<ProcessAndUIAssertion>(pid, assertionState());
+ m_process.didSetAssertionState(assertionState());
m_assertion->setClient(*this);
}
Modified: trunk/Source/WebKit2/UIProcess/ProcessThrottlerClient.h (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/ProcessThrottlerClient.h 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/ProcessThrottlerClient.h 2015-05-22 17:19:39 UTC (rev 184774)
@@ -26,6 +26,8 @@
#ifndef ProcessThrottlerClient_h
#define ProcessThrottlerClient_h
+#include "ProcessAssertion.h"
+
namespace WebKit {
class ProcessThrottlerClient {
@@ -36,6 +38,8 @@
virtual void sendPrepareToSuspend() = 0;
virtual void sendCancelPrepareToSuspend() = 0;
virtual void sendProcessDidResume() = 0;
+
+ virtual void didSetAssertionState(AssertionState) = 0;
};
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp 2015-05-22 17:19:39 UTC (rev 184774)
@@ -580,8 +580,6 @@
ASSERT(xpcConnection);
m_throttler.didConnectToProcess(xpc_connection_get_pid(xpcConnection));
#endif
-
- initializeNetworkProcessActivityToken();
}
WebFrameProxy* WebProcessProxy::webFrame(uint64_t frameID) const
@@ -901,28 +899,15 @@
send(Messages::WebProcess::CancelPrepareToSuspend(), 0);
}
-void WebProcessProxy::initializeNetworkProcessActivityToken()
-{
-#if PLATFORM(IOS) && ENABLE(NETWORK_PROCESS)
- if (processPool().usesNetworkProcess())
- m_tokenForNetworkProcess = processPool().ensureNetworkProcess().throttler().foregroundActivityToken();
-#endif
-}
-
void WebProcessProxy::sendProcessDidResume()
{
- initializeNetworkProcessActivityToken();
-
if (canSendMessage())
send(Messages::WebProcess::ProcessDidResume(), 0);
}
-
+
void WebProcessProxy::processReadyToSuspend()
{
m_throttler.processReadyToSuspend();
-#if PLATFORM(IOS) && ENABLE(NETWORK_PROCESS)
- m_tokenForNetworkProcess = nullptr;
-#endif
}
void WebProcessProxy::didCancelProcessSuspension()
@@ -930,6 +915,32 @@
m_throttler.didCancelProcessSuspension();
}
+void WebProcessProxy::didSetAssertionState(AssertionState state)
+{
+#if PLATFORM(IOS) && ENABLE(NETWORK_PROCESS)
+ switch (state) {
+ case AssertionState::Suspended:
+ m_foregroundTokenForNetworkProcess = nullptr;
+ m_backgroundTokenForNetworkProcess = nullptr;
+ break;
+
+ case AssertionState::Background:
+ if (processPool().usesNetworkProcess())
+ m_backgroundTokenForNetworkProcess = processPool().ensureNetworkProcess().throttler().backgroundActivityToken();
+ m_foregroundTokenForNetworkProcess = nullptr;
+ break;
+
+ case AssertionState::Foreground:
+ if (processPool().usesNetworkProcess())
+ m_foregroundTokenForNetworkProcess = processPool().ensureNetworkProcess().throttler().foregroundActivityToken();
+ m_backgroundTokenForNetworkProcess = nullptr;
+ break;
+ }
+#else
+ UNUSED_PARAM(state);
+#endif
+}
+
void WebProcessProxy::setIsHoldingLockedFiles(bool isHoldingLockedFiles)
{
if (!isHoldingLockedFiles) {
Modified: trunk/Source/WebKit2/UIProcess/WebProcessProxy.h (184773 => 184774)
--- trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2015-05-22 17:09:12 UTC (rev 184773)
+++ trunk/Source/WebKit2/UIProcess/WebProcessProxy.h 2015-05-22 17:19:39 UTC (rev 184774)
@@ -210,6 +210,7 @@
void sendPrepareToSuspend() override;
void sendCancelPrepareToSuspend() override;
void sendProcessDidResume() override;
+ void didSetAssertionState(AssertionState) override;
// ProcessLauncher::Client
virtual void didFinishLaunching(ProcessLauncher*, IPC::Connection::Identifier) override;
@@ -220,8 +221,6 @@
bool canTerminateChildProcess();
- void initializeNetworkProcessActivityToken();
-
ResponsivenessTimer m_responsivenessTimer;
RefPtr<WebConnectionToWebProcess> m_webConnection;
@@ -248,7 +247,8 @@
ProcessThrottler m_throttler;
ProcessThrottler::BackgroundActivityToken m_tokenForHoldingLockedFiles;
#if PLATFORM(IOS) && ENABLE(NETWORK_PROCESS)
- ProcessThrottler::ForegroundActivityToken m_tokenForNetworkProcess;
+ ProcessThrottler::ForegroundActivityToken m_foregroundTokenForNetworkProcess;
+ ProcessThrottler::BackgroundActivityToken m_backgroundTokenForNetworkProcess;
#endif
HashMap<String, uint64_t> m_pageURLRetainCountMap;
