Title: [278895] trunk/Source/WebKit
Revision
278895
Author
cdu...@apple.com
Date
2021-06-15 13:53:28 -0700 (Tue, 15 Jun 2021)

Log Message

Add basic detection of unresponsive Network / GPU Processes
https://bugs.webkit.org/show_bug.cgi?id=226994

Reviewed by Geoffrey Garen.

If a WebProcess A attempts to connect to the GPUProcess / NetworkProcess B and process B
fails to respond within 3 seconds, we consider process B as unresponsive and terminate
it. As a result, it will re-attempt to launch process B and connect to it again.

This helps in the following scenario:
1. User is in a tab and something looks broken due to the GPUProcess or NetworkProcess
   becoming unresponsive
2. The user tries to reload the page in a new tab

Before this patch, the tab would be similarly broken / hung because the GPUProcess or
NetworkProcess would be unresponsive. After this patch, we would detect the process is
hung and kill it. As a result, the page would load correctly in a new tab (with a delay).

In a follow-up, I think we should consider doing the same thing when the user requests
a reload, so that we don't require opening a new tab to recover. However, this keeps
the patch small and is a decent first step.

* Shared/ProcessTerminationReason.h:
* UIProcess/AuxiliaryProcessProxy.cpp:
(WebKit::AuxiliaryProcessProxy::AuxiliaryProcessProxy):
(WebKit::AuxiliaryProcessProxy::didFinishLaunching):
(WebKit::AuxiliaryProcessProxy::shutDownProcess):
(WebKit::AuxiliaryProcessProxy::platformIsBeingDebugged const):
(WebKit::AuxiliaryProcessProxy::stopResponsivenessTimer):
(WebKit::AuxiliaryProcessProxy::startResponsivenessTimer):
(WebKit::AuxiliaryProcessProxy::mayBecomeUnresponsive):
(WebKit::AuxiliaryProcessProxy::didBecomeUnresponsive):
* UIProcess/AuxiliaryProcessProxy.h:
(WebKit::AuxiliaryProcessProxy::responsivenessTimer):
(WebKit::AuxiliaryProcessProxy::responsivenessTimer const):
* UIProcess/Cocoa/WebProcessProxyCocoa.mm:
(WebKit::WebProcessProxy::platformIsBeingDebugged const): Deleted.
* UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::getGPUProcessConnection):
(WebKit::GPUProcessProxy::gpuProcessExited):
(WebKit::GPUProcessProxy::didBecomeUnresponsive):
* UIProcess/GPU/GPUProcessProxy.h:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::didBecomeUnresponsive):
(WebKit::NetworkProcessProxy::getNetworkProcessConnection):
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/ResponsivenessTimer.cpp:
(WebKit::ResponsivenessTimer::timerFired):
(WebKit::ResponsivenessTimer::mayBecomeUnresponsive const):
* UIProcess/ResponsivenessTimer.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::gpuProcessExited):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::WebProcessProxy):
(WebKit::WebProcessProxy::shutDown):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::isResponsive const):
(WebKit::WebProcessProxy::processTerminated):
(WebKit::WebProcessProxy::platformIsBeingDebugged const): Deleted.
(WebKit::WebProcessProxy::mayBecomeUnresponsive): Deleted.
(WebKit::WebProcessProxy::stopResponsivenessTimer): Deleted.
(WebKit::WebProcessProxy::startResponsivenessTimer): Deleted.
* UIProcess/WebProcessProxy.h:
(WebKit::WebProcessProxy::responsivenessTimer): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (278894 => 278895)


--- trunk/Source/WebKit/ChangeLog	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/ChangeLog	2021-06-15 20:53:28 UTC (rev 278895)
@@ -1,3 +1,70 @@
+2021-06-15  Chris Dumez  <cdu...@apple.com>
+
+        Add basic detection of unresponsive Network / GPU Processes
+        https://bugs.webkit.org/show_bug.cgi?id=226994
+
+        Reviewed by Geoffrey Garen.
+
+        If a WebProcess A attempts to connect to the GPUProcess / NetworkProcess B and process B
+        fails to respond within 3 seconds, we consider process B as unresponsive and terminate
+        it. As a result, it will re-attempt to launch process B and connect to it again.
+
+        This helps in the following scenario:
+        1. User is in a tab and something looks broken due to the GPUProcess or NetworkProcess
+           becoming unresponsive
+        2. The user tries to reload the page in a new tab
+
+        Before this patch, the tab would be similarly broken / hung because the GPUProcess or
+        NetworkProcess would be unresponsive. After this patch, we would detect the process is
+        hung and kill it. As a result, the page would load correctly in a new tab (with a delay).
+
+        In a follow-up, I think we should consider doing the same thing when the user requests
+        a reload, so that we don't require opening a new tab to recover. However, this keeps
+        the patch small and is a decent first step.
+
+        * Shared/ProcessTerminationReason.h:
+        * UIProcess/AuxiliaryProcessProxy.cpp:
+        (WebKit::AuxiliaryProcessProxy::AuxiliaryProcessProxy):
+        (WebKit::AuxiliaryProcessProxy::didFinishLaunching):
+        (WebKit::AuxiliaryProcessProxy::shutDownProcess):
+        (WebKit::AuxiliaryProcessProxy::platformIsBeingDebugged const):
+        (WebKit::AuxiliaryProcessProxy::stopResponsivenessTimer):
+        (WebKit::AuxiliaryProcessProxy::startResponsivenessTimer):
+        (WebKit::AuxiliaryProcessProxy::mayBecomeUnresponsive):
+        (WebKit::AuxiliaryProcessProxy::didBecomeUnresponsive):
+        * UIProcess/AuxiliaryProcessProxy.h:
+        (WebKit::AuxiliaryProcessProxy::responsivenessTimer):
+        (WebKit::AuxiliaryProcessProxy::responsivenessTimer const):
+        * UIProcess/Cocoa/WebProcessProxyCocoa.mm:
+        (WebKit::WebProcessProxy::platformIsBeingDebugged const): Deleted.
+        * UIProcess/GPU/GPUProcessProxy.cpp:
+        (WebKit::GPUProcessProxy::getGPUProcessConnection):
+        (WebKit::GPUProcessProxy::gpuProcessExited):
+        (WebKit::GPUProcessProxy::didBecomeUnresponsive):
+        * UIProcess/GPU/GPUProcessProxy.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::didBecomeUnresponsive):
+        (WebKit::NetworkProcessProxy::getNetworkProcessConnection):
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/ResponsivenessTimer.cpp:
+        (WebKit::ResponsivenessTimer::timerFired):
+        (WebKit::ResponsivenessTimer::mayBecomeUnresponsive const):
+        * UIProcess/ResponsivenessTimer.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::gpuProcessExited):
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::WebProcessProxy):
+        (WebKit::WebProcessProxy::shutDown):
+        (WebKit::WebProcessProxy::didFinishLaunching):
+        (WebKit::WebProcessProxy::isResponsive const):
+        (WebKit::WebProcessProxy::processTerminated):
+        (WebKit::WebProcessProxy::platformIsBeingDebugged const): Deleted.
+        (WebKit::WebProcessProxy::mayBecomeUnresponsive): Deleted.
+        (WebKit::WebProcessProxy::stopResponsivenessTimer): Deleted.
+        (WebKit::WebProcessProxy::startResponsivenessTimer): Deleted.
+        * UIProcess/WebProcessProxy.h:
+        (WebKit::WebProcessProxy::responsivenessTimer): Deleted.
+
 2021-06-15  Jonathan Bedard  <jbed...@apple.com>
 
         [Monterey] Support building WebKit (Follow-up)

Modified: trunk/Source/WebKit/Shared/ProcessTerminationReason.h (278894 => 278895)


--- trunk/Source/WebKit/Shared/ProcessTerminationReason.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/Shared/ProcessTerminationReason.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -39,7 +39,8 @@
 
 enum class GPUProcessTerminationReason {
     Crash,
-    IdleExit
+    IdleExit,
+    Unresponsive
 };
 
 }

Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -32,10 +32,17 @@
 #include "WebProcessProxy.h"
 #include <wtf/RunLoop.h>
 
+#if PLATFORM(COCOA)
+#include "SandboxUtilities.h"
+#include <sys/sysctl.h>
+#include <wtf/spi/darwin/SandboxSPI.h>
+#endif
+
 namespace WebKit {
 
 AuxiliaryProcessProxy::AuxiliaryProcessProxy(bool alwaysRunsAtBackgroundPriority)
-    : m_alwaysRunsAtBackgroundPriority(alwaysRunsAtBackgroundPriority)
+    : m_responsivenessTimer(*this)
+    , m_alwaysRunsAtBackgroundPriority(alwaysRunsAtBackgroundPriority)
 {
 }
 
@@ -252,6 +259,11 @@
             IPC::addAsyncReplyHandler(*connection(), pendingMessage.asyncReplyInfo->second, WTFMove(pendingMessage.asyncReplyInfo->first));
         m_connection->sendMessage(WTFMove(pendingMessage.encoder), pendingMessage.sendOptions);
     }
+
+    if (m_shouldStartResponsivenessTimerWhenLaunched) {
+        auto useLazyStop = *std::exchange(m_shouldStartResponsivenessTimerWhenLaunched, std::nullopt);
+        startResponsivenessTimer(useLazyStop);
+    }
 }
 
 void AuxiliaryProcessProxy::replyToPendingMessages()
@@ -292,6 +304,7 @@
 
     m_connection->invalidate();
     m_connection = nullptr;
+    m_responsivenessTimer.invalidate();
 }
 
 void AuxiliaryProcessProxy::setProcessSuppressionEnabled(bool processSuppressionEnabled)
@@ -315,4 +328,51 @@
     RELEASE_LOG_FAULT(IPC, "Received an invalid message '%" PUBLIC_LOG_STRING "' from the %" PUBLIC_LOG_STRING " process.", description(messageName), processName().characters());
 }
 
+bool AuxiliaryProcessProxy::platformIsBeingDebugged() const
+{
+#if PLATFORM(COCOA)
+    // If the UI process is sandboxed and lacks 'process-info-pidinfo', it cannot find out whether other processes are being debugged.
+    if (currentProcessIsSandboxed() && !!sandbox_check(getpid(), "process-info-pidinfo", SANDBOX_CHECK_NO_REPORT))
+        return false;
+
+    struct kinfo_proc info;
+    int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, processIdentifier() };
+    size_t size = sizeof(info);
+    if (sysctl(mib, WTF_ARRAY_LENGTH(mib), &info, &size, nullptr, 0) == -1)
+        return false;
+
+    return info.kp_proc.p_flag & P_TRACED;
+#else
+    return false;
+#endif
+}
+
+void AuxiliaryProcessProxy::stopResponsivenessTimer()
+{
+    responsivenessTimer().stop();
+}
+
+void AuxiliaryProcessProxy::startResponsivenessTimer(UseLazyStop useLazyStop)
+{
+    if (isLaunching()) {
+        m_shouldStartResponsivenessTimerWhenLaunched = useLazyStop;
+        return;
+    }
+
+    if (useLazyStop == UseLazyStop::Yes)
+        responsivenessTimer().startWithLazyStop();
+    else
+        responsivenessTimer().start();
+}
+
+bool AuxiliaryProcessProxy::mayBecomeUnresponsive()
+{
+    return !platformIsBeingDebugged();
+}
+
+void AuxiliaryProcessProxy::didBecomeUnresponsive()
+{
+    RELEASE_LOG_ERROR(Process, "AuxiliaryProcessProxy::didBecomeUnresponsive: %" PUBLIC_LOG_STRING " process with PID %d became unresponsive", processName().characters(), processIdentifier());
+}
+
 } // namespace WebKit

Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -28,7 +28,7 @@
 #include "Connection.h"
 #include "MessageReceiverMap.h"
 #include "ProcessLauncher.h"
-
+#include "ResponsivenessTimer.h"
 #include <WebCore/ProcessIdentifier.h>
 #include <wtf/ProcessID.h>
 #include <wtf/SystemTracing.h>
@@ -39,7 +39,7 @@
 
 class ProcessThrottler;
 
-class AuxiliaryProcessProxy : public ThreadSafeRefCounted<AuxiliaryProcessProxy, WTF::DestructionThread::MainRunLoop>, private ProcessLauncher::Client, public IPC::Connection::Client {
+class AuxiliaryProcessProxy : public ThreadSafeRefCounted<AuxiliaryProcessProxy, WTF::DestructionThread::MainRunLoop>, public ResponsivenessTimer::Client, private ProcessLauncher::Client, public IPC::Connection::Client {
     WTF_MAKE_NONCOPYABLE(AuxiliaryProcessProxy);
 
 protected:
@@ -126,7 +126,18 @@
     WebCore::ProcessIdentifier coreProcessIdentifier() const { return m_processIdentifier; }
 
     void setProcessSuppressionEnabled(bool);
+    bool platformIsBeingDebugged() const;
 
+    enum class UseLazyStop : bool { No, Yes };
+    void startResponsivenessTimer(UseLazyStop = UseLazyStop::No);
+    void stopResponsivenessTimer();
+
+    ResponsivenessTimer& responsivenessTimer() { return m_responsivenessTimer; }
+    const ResponsivenessTimer& responsivenessTimer() const { return m_responsivenessTimer; }
+
+    void ref() final { ThreadSafeRefCounted::ref(); }
+    void deref() final { ThreadSafeRefCounted::deref(); }
+
 protected:
     // ProcessLauncher::Client
     void didFinishLaunching(ProcessLauncher*, IPC::Connection::Identifier) override;
@@ -148,10 +159,18 @@
 
     virtual bool shouldSendPendingMessage(const PendingMessage&) { return true; }
 
+    // ResponsivenessTimer::Client.
+    void didBecomeUnresponsive() override;
+    void didBecomeResponsive() override { }
+    void willChangeIsResponsive() override { }
+    void didChangeIsResponsive() override { }
+    bool mayBecomeUnresponsive() override;
+
 private:
     virtual void connectionWillOpen(IPC::Connection&);
     virtual void processWillShutDown(IPC::Connection&) = 0;
 
+    ResponsivenessTimer m_responsivenessTimer;
     Vector<PendingMessage> m_pendingMessages;
     RefPtr<ProcessLauncher> m_processLauncher;
     RefPtr<IPC::Connection> m_connection;
@@ -158,6 +177,7 @@
     IPC::MessageReceiverMap m_messageReceiverMap;
     bool m_alwaysRunsAtBackgroundPriority { false };
     WebCore::ProcessIdentifier m_processIdentifier { WebCore::ProcessIdentifier::generate() };
+    std::optional<UseLazyStop> m_shouldStartResponsivenessTimerWhenLaunched;
 };
 
 template<typename T>

Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebProcessProxyCocoa.mm	2021-06-15 20:53:28 UTC (rev 278895)
@@ -145,21 +145,6 @@
     return ObjCObjectGraph::create(ObjCObjectGraph::transform(objectGraph.rootObject(), Transformer()).get());
 }
 
-bool WebProcessProxy::platformIsBeingDebugged() const
-{
-    // If the UI process is sandboxed and lacks 'process-info-pidinfo', it cannot find out whether other processes are being debugged.
-    if (currentProcessIsSandboxed() && !!sandbox_check(getpid(), "process-info-pidinfo", SANDBOX_CHECK_NO_REPORT))
-        return false;
-
-    struct kinfo_proc info;
-    int mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, processIdentifier() };
-    size_t size = sizeof(info);
-    if (sysctl(mib, WTF_ARRAY_LENGTH(mib), &info, &size, nullptr, 0) == -1)
-        return false;
-
-    return info.kp_proc.p_flag & P_TRACED;
-}
-
 static Vector<String>& mediaTypeCache()
 {
     ASSERT(RunLoop::isMain());

Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -327,6 +327,7 @@
     addSession(webProcessProxy.websiteDataStore());
 
     RELEASE_LOG(ProcessSuspension, "%p - GPUProcessProxy is taking a background assertion because a web process is requesting a connection", this);
+    startResponsivenessTimer(UseLazyStop::No);
     sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID(), parameters }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& identifier) mutable {
         if (!weakThis) {
             RELEASE_LOG_ERROR(Process, "GPUProcessProxy::getGPUProcessConnection: GPUProcessProxy deallocated during connection establishment");
@@ -333,6 +334,7 @@
             return reply({ });
         }
 
+        stopResponsivenessTimer();
         if (!identifier) {
             RELEASE_LOG_ERROR(Process, "GPUProcessProxy::getGPUProcessConnection: connection identifier is empty");
             return reply({ });
@@ -361,6 +363,9 @@
     case GPUProcessTerminationReason::IdleExit:
         RELEASE_LOG(Process, "%p - GPUProcessProxy::gpuProcessExited: reason=idle-exit", this);
         break;
+    case GPUProcessTerminationReason::Unresponsive:
+        RELEASE_LOG(Process, "%p - GPUProcessProxy::gpuProcessExited: reason=unresponsive", this);
+        break;
     }
 
     if (singleton() == this)
@@ -592,6 +597,13 @@
 #endif
 }
 
+void GPUProcessProxy::didBecomeUnresponsive()
+{
+    RELEASE_LOG_ERROR(Process, "GPUProcessProxy::didBecomeUnresponsive: GPUProcess with PID %d became unresponsive, terminating it", processIdentifier());
+    terminate();
+    gpuProcessExited(GPUProcessTerminationReason::Unresponsive);
+}
+
 } // namespace WebKit
 
 #undef MESSAGE_CHECK

Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -118,6 +118,9 @@
     void didClose(IPC::Connection&) override;
     void didReceiveInvalidMessage(IPC::Connection&, IPC::MessageName) override;
 
+    // ResponsivenessTimer::Client
+    void didBecomeUnresponsive() final;
+
     void terminateWebProcess(WebCore::ProcessIdentifier);
     void processIsReadyToExit();
 

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -122,6 +122,12 @@
     networkProcessDidTerminate(TerminationReason::RequestedByClient);
 }
 
+void NetworkProcessProxy::didBecomeUnresponsive()
+{
+    RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::didBecomeUnresponsive: NetworkProcess with PID %d became unresponsive, terminating it", processIdentifier());
+    terminate();
+}
+
 void NetworkProcessProxy::sendCreationParametersToNewProcess()
 {
     ASSERT(RunLoop::isMain());
@@ -237,6 +243,7 @@
 void NetworkProcessProxy::getNetworkProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply&& reply)
 {
     RELEASE_LOG(ProcessSuspension, "%p - NetworkProcessProxy is taking a background assertion because a web process is requesting a connection", this);
+    startResponsivenessTimer(UseLazyStop::No);
     sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply)](auto&& identifier, auto cookieAcceptPolicy) mutable {
         if (!weakThis) {
             RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::getNetworkProcessConnection: NetworkProcessProxy deallocated during connection establishment");
@@ -243,6 +250,7 @@
             return reply({ });
         }
 
+        stopResponsivenessTimer();
         if (!identifier) {
             RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::getNetworkProcessConnection: connection identifier is empty");
             return reply({ });

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -286,6 +286,9 @@
     void didReceiveInvalidMessage(IPC::Connection&, IPC::MessageName) override;
     bool didReceiveSyncNetworkProcessProxyMessage(IPC::Connection&, IPC::Decoder&, UniqueRef<IPC::Encoder>&);
 
+    // ResponsivenessTimer::Client
+    void didBecomeUnresponsive() final;
+
     // Message handlers
     void didReceiveNetworkProcessProxyMessage(IPC::Connection&, IPC::Decoder&);
     void didReceiveAuthenticationChallenge(PAL::SessionID, WebPageProxyIdentifier, const std::optional<WebCore::SecurityOriginData>&, WebCore::AuthenticationChallenge&&, bool, uint64_t challengeID);

Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -70,7 +70,7 @@
 
     auto protectedClient = makeRef(m_client);
 
-    if (!m_client.mayBecomeUnresponsive()) {
+    if (!mayBecomeUnresponsive()) {
         m_waitingForTimer = true;
         m_timer.startOneShot(responsivenessTimeout);
         return;
@@ -104,6 +104,26 @@
     }
 }
 
+bool ResponsivenessTimer::mayBecomeUnresponsive() const
+{
+#if !defined(NDEBUG) || ASAN_ENABLED
+    return false;
+#else
+    static bool isLibgmallocEnabled = [] {
+        char* variable = getenv("DYLD_INSERT_LIBRARIES");
+        if (!variable)
+            return false;
+        if (!strstr(variable, "libgmalloc"))
+            return false;
+        return true;
+    }();
+    if (isLibgmallocEnabled)
+        return false;
+
+    return m_client.mayBecomeUnresponsive();
+#endif
+}
+
 void ResponsivenessTimer::startWithLazyStop()
 {
     if (!m_waitingForTimer) {

Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -77,6 +77,8 @@
 private:
     void timerFired();
 
+    bool mayBecomeUnresponsive() const;
+
     ResponsivenessTimer::Client& m_client;
 
     RunLoop::Timer<ResponsivenessTimer> m_timer;

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -502,7 +502,7 @@
     WEBPROCESSPOOL_RELEASE_LOG(Process, "gpuProcessDidExit: PID=%d, reason=%u", identifier, static_cast<unsigned>(reason));
     m_gpuProcess = nullptr;
 
-    if (reason == GPUProcessTerminationReason::Crash)
+    if (reason == GPUProcessTerminationReason::Crash || reason == GPUProcessTerminationReason::Unresponsive)
         m_client.gpuProcessDidCrash(this, identifier);
 
     Vector<Ref<WebProcessProxy>> processes = m_processes;
@@ -509,7 +509,7 @@
     for (auto& process : processes)
         process->gpuProcessExited(reason);
 
-    if (reason == GPUProcessTerminationReason::Crash) {
+    if (reason == GPUProcessTerminationReason::Crash || reason == GPUProcessTerminationReason::Unresponsive) {
         if (++m_recentGPUProcessCrashCount > maximumGPUProcessRelaunchAttemptsBeforeKillingWebProcesses) {
             WEBPROCESSPOOL_RELEASE_LOG_ERROR(Process, "gpuProcessDidExit: GPU Process has crashed more than %u times in the last %g seconds, terminating all WebProcesses", maximumGPUProcessRelaunchAttemptsBeforeKillingWebProcesses, resetGPUProcessCrashCountDelay.seconds());
             m_resetGPUProcessCrashCountTimer.stop();

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2021-06-15 20:53:28 UTC (rev 278895)
@@ -193,7 +193,6 @@
 
 WebProcessProxy::WebProcessProxy(WebProcessPool& processPool, WebsiteDataStore* websiteDataStore, IsPrewarmed isPrewarmed)
     : AuxiliaryProcessProxy(processPool.alwaysRunsAtBackgroundPriority())
-    , m_responsivenessTimer(*this)
     , m_backgroundResponsivenessTimer(*this)
     , m_processPool(processPool, isPrewarmed == IsPrewarmed::Yes ? IsWeak::Yes : IsWeak::No)
     , m_mayHaveUniversalFileReadSandboxExtension(false)
@@ -461,7 +460,6 @@
         m_webConnection = nullptr;
     }
 
-    m_responsivenessTimer.invalidate();
     m_backgroundResponsivenessTimer.invalidate();
     m_activityForHoldingLockedFiles = nullptr;
     m_audibleMediaActivity = std::nullopt;
@@ -806,13 +804,6 @@
 }
 #endif
 
-#if !PLATFORM(COCOA)
-bool WebProcessProxy::platformIsBeingDebugged() const
-{
-    return false;
-}
-#endif
-
 #if !PLATFORM(MAC)
 bool WebProcessProxy::shouldAllowNonValidInjectedCode() const
 {
@@ -980,30 +971,6 @@
         page->didChangeProcessIsResponsive();
 }
 
-bool WebProcessProxy::mayBecomeUnresponsive()
-{
-#if !defined(NDEBUG) || ASAN_ENABLED
-    // Disable responsiveness checks in slow builds to avoid false positives.
-    return false;
-#else
-    if (platformIsBeingDebugged())
-        return false;
-
-    static bool isLibgmallocEnabled = [] {
-        char* variable = getenv("DYLD_INSERT_LIBRARIES");
-        if (!variable)
-            return false;
-        if (!strstr(variable, "libgmalloc"))
-            return false;
-        return true;
-    }();
-    if (isLibgmallocEnabled)
-        return false;
-
-    return true;
-#endif
-}
-
 #if ENABLE(IPC_TESTING_API)
 void WebProcessProxy::setIgnoreInvalidMessageForTesting()
 {
@@ -1055,11 +1022,6 @@
     enableRemoteInspectorIfNeeded();
 #endif
 #endif
-
-    if (m_shouldStartResponsivenessTimerWhenLaunched) {
-        auto useLazyStop = *std::exchange(m_shouldStartResponsivenessTimerWhenLaunched, std::nullopt);
-        startResponsivenessTimer(useLazyStop);
-    }
 }
 
 WebFrameProxy* WebProcessProxy::webFrame(FrameIdentifier frameID) const
@@ -1131,7 +1093,7 @@
 
 bool WebProcessProxy::isResponsive() const
 {
-    return m_responsivenessTimer.isResponsive() && m_backgroundResponsivenessTimer.isResponsive();
+    return responsivenessTimer().isResponsive() && m_backgroundResponsivenessTimer.isResponsive();
 }
 
 void WebProcessProxy::didDestroyUserGestureToken(uint64_t identifier)
@@ -1287,24 +1249,6 @@
     processDidTerminateOrFailedToLaunch(reason);
 }
 
-void WebProcessProxy::stopResponsivenessTimer()
-{
-    responsivenessTimer().stop();
-}
-
-void WebProcessProxy::startResponsivenessTimer(UseLazyStop useLazyStop)
-{
-    if (isLaunching()) {
-        m_shouldStartResponsivenessTimerWhenLaunched = useLazyStop;
-        return;
-    }
-
-    if (useLazyStop == UseLazyStop::Yes)
-        responsivenessTimer().startWithLazyStop();
-    else
-        responsivenessTimer().start();
-}
-
 void WebProcessProxy::enableSuddenTermination()
 {
     if (state() != State::Running)
@@ -1577,7 +1521,6 @@
 
 void WebProcessProxy::processTerminated()
 {
-    m_responsivenessTimer.processTerminated();
     m_backgroundResponsivenessTimer.processTerminated();
 }
 

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (278894 => 278895)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2021-06-15 20:19:05 UTC (rev 278894)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h	2021-06-15 20:53:28 UTC (rev 278895)
@@ -118,7 +118,7 @@
 using WebProcessWithAudibleMediaToken = WebProcessWithAudibleMediaCounter::Token;
 enum class CheckBackForwardList : bool { No, Yes };
 
-class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer::Client, private ProcessThrottlerClient {
+class WebProcessProxy : public AuxiliaryProcessProxy, private ProcessThrottlerClient {
 public:
     typedef HashMap<WebCore::FrameIdentifier, RefPtr<WebFrameProxy>> WebFrameProxyMap;
     typedef HashMap<WebPageProxyIdentifier, WebPageProxy*> WebPageProxyMap;
@@ -243,10 +243,6 @@
 
     void requestTermination(ProcessTerminationReason);
 
-    enum class UseLazyStop : bool { No, Yes };
-    void startResponsivenessTimer(UseLazyStop = UseLazyStop::No);
-    void stopResponsivenessTimer();
-
     RefPtr<API::Object> transformHandlesToObjects(API::Object*);
     static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
 
@@ -352,9 +348,6 @@
 
     void updateAudibleMediaAssertions();
 
-    void ref() final { ThreadSafeRefCounted::ref(); }
-    void deref() final { ThreadSafeRefCounted::deref(); }
-
 #if ENABLE(SERVICE_WORKER)
     void establishServiceWorkerContext(const WebPreferencesStore&, CompletionHandler<void()>&&);
     void setServiceWorkerUserAgent(const String&);
@@ -471,12 +464,10 @@
     void getWebAuthnProcessConnection(Messages::WebProcessProxy::GetWebAuthnProcessConnectionDelayedReply&&);
 #endif
 
-    bool platformIsBeingDebugged() const;
     bool shouldAllowNonValidInjectedCode() const;
 
     static const MemoryCompactLookupOnlyRobinHoodHashSet<String>& platformPathsWithAssumedReadAccess();
 
-    ResponsivenessTimer& responsivenessTimer() { return m_responsivenessTimer; }
     void updateBackgroundResponsivenessTimer();
 
     void processDidTerminateOrFailedToLaunch(ProcessTerminationReason);
@@ -493,7 +484,6 @@
     void didBecomeResponsive() override;
     void willChangeIsResponsive() override;
     void didChangeIsResponsive() override;
-    bool mayBecomeUnresponsive() override;
 
     // Implemented in generated WebProcessProxyMessageReceiver.cpp
     void didReceiveWebProcessProxyMessage(IPC::Connection&, IPC::Decoder&);
@@ -563,7 +553,6 @@
         RefPtr<T> m_strongObject;
     };
 
-    ResponsivenessTimer m_responsivenessTimer;
     BackgroundProcessResponsivenessTimer m_backgroundResponsivenessTimer;
     
     RefPtr<WebConnectionToWebProcess> m_webConnection;
@@ -626,7 +615,6 @@
 #if PLATFORM(IOS)
     bool m_hasManagedSessionSandboxAccess { false };
 #endif
-    std::optional<UseLazyStop> m_shouldStartResponsivenessTimerWhenLaunched;
 
 #if PLATFORM(WATCHOS)
     std::unique_ptr<ProcessThrottler::BackgroundActivity> m_backgroundActivityForFullscreenFormControls;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to