Title: [259673] trunk/Source/WebKit
Revision
259673
Author
commit-qu...@webkit.org
Date
2020-04-07 15:07:54 -0700 (Tue, 07 Apr 2020)

Log Message

Simplify and fortify network getNetworkProcessConnection and getGPUProcessConnection
https://bugs.webkit.org/show_bug.cgi?id=210142
<rdar://problem/59488963>

Patch by Alex Christensen <achristen...@webkit.org> on 2020-04-07
Reviewed by Youenn Fablet.

We have reports of hangs inside WebKit::getNetworkProcessConnection that seem to last forever.
Some of the reports indicate the network process is being suspended while a connection is being established with it.

To fix this issue we do three things:
1. We take a foregroundActivity when sending an async message to establish a connection.
2. We use sendWithAsyncReply which already has logic to handle the case where we are currently launching the process.
   Instead of the complicated retry logic, we add a retry attempt in WebProcessPool if the connection identifier is invalid.
3. Add some release logging so we can better diagnose problems with this flow in the future.

The functional change is adding the foreground activity, which should prevent some hangs.
The rest is just to make this code more sane to understand and debug.
I do the same changes to NetworkProcess and GPUProcess because they are intended to be the same.  The latter is based on the former.

The API test WebKit.NetworkProcessCrashWithPendingConnection covers what happens when the network process crashes during connection establishment.
It fails if we don't retry somewhere, which I did in WebProcessPool.  We also need to try again in getNetworkProcessConnection and getGPUProcessConnection.
If it fails twice, there's nothing we can do, and we crash the web process to avoid a crash loop.

* UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::getGPUProcessConnection):
(WebKit::GPUProcessProxy::didFinishLaunching):
(WebKit::GPUProcessProxy::~GPUProcessProxy): Deleted.
(WebKit::GPUProcessProxy::openGPUProcessConnection): Deleted.
* UIProcess/GPU/GPUProcessProxy.h:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::~NetworkProcessProxy):
(WebKit::NetworkProcessProxy::getNetworkProcessConnection):
(WebKit::NetworkProcessProxy::networkProcessCrashed):
(WebKit::NetworkProcessProxy::didFinishLaunching):
(WebKit::NetworkProcessProxy::openNetworkProcessConnection): Deleted.
* UIProcess/Network/NetworkProcessProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::networkProcessCrashed):
(WebKit::WebProcessPool::getNetworkProcessConnection):
(WebKit::WebProcessPool::getGPUProcessConnection):
* UIProcess/WebProcessPool.h:
* WebProcess/GPU/GPUProcessConnectionInfo.h:
(WebKit::GPUProcessConnectionInfo::identifier const):
(WebKit::GPUProcessConnectionInfo::identifier): Deleted.
* WebProcess/Network/NetworkProcessConnectionInfo.h:
(WebKit::NetworkProcessConnectionInfo::identifier const):
(WebKit::NetworkProcessConnectionInfo::identifier): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (259672 => 259673)


--- trunk/Source/WebKit/ChangeLog	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/ChangeLog	2020-04-07 22:07:54 UTC (rev 259673)
@@ -1,3 +1,53 @@
+2020-04-07  Alex Christensen  <achristen...@webkit.org>
+
+        Simplify and fortify network getNetworkProcessConnection and getGPUProcessConnection
+        https://bugs.webkit.org/show_bug.cgi?id=210142
+        <rdar://problem/59488963>
+
+        Reviewed by Youenn Fablet.
+
+        We have reports of hangs inside WebKit::getNetworkProcessConnection that seem to last forever.
+        Some of the reports indicate the network process is being suspended while a connection is being established with it.
+
+        To fix this issue we do three things:
+        1. We take a foregroundActivity when sending an async message to establish a connection.
+        2. We use sendWithAsyncReply which already has logic to handle the case where we are currently launching the process.
+           Instead of the complicated retry logic, we add a retry attempt in WebProcessPool if the connection identifier is invalid.
+        3. Add some release logging so we can better diagnose problems with this flow in the future.
+
+        The functional change is adding the foreground activity, which should prevent some hangs.
+        The rest is just to make this code more sane to understand and debug.
+        I do the same changes to NetworkProcess and GPUProcess because they are intended to be the same.  The latter is based on the former.
+
+        The API test WebKit.NetworkProcessCrashWithPendingConnection covers what happens when the network process crashes during connection establishment.
+        It fails if we don't retry somewhere, which I did in WebProcessPool.  We also need to try again in getNetworkProcessConnection and getGPUProcessConnection.
+        If it fails twice, there's nothing we can do, and we crash the web process to avoid a crash loop.
+
+        * UIProcess/GPU/GPUProcessProxy.cpp:
+        (WebKit::GPUProcessProxy::getGPUProcessConnection):
+        (WebKit::GPUProcessProxy::didFinishLaunching):
+        (WebKit::GPUProcessProxy::~GPUProcessProxy): Deleted.
+        (WebKit::GPUProcessProxy::openGPUProcessConnection): Deleted.
+        * UIProcess/GPU/GPUProcessProxy.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::~NetworkProcessProxy):
+        (WebKit::NetworkProcessProxy::getNetworkProcessConnection):
+        (WebKit::NetworkProcessProxy::networkProcessCrashed):
+        (WebKit::NetworkProcessProxy::didFinishLaunching):
+        (WebKit::NetworkProcessProxy::openNetworkProcessConnection): Deleted.
+        * UIProcess/Network/NetworkProcessProxy.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::networkProcessCrashed):
+        (WebKit::WebProcessPool::getNetworkProcessConnection):
+        (WebKit::WebProcessPool::getGPUProcessConnection):
+        * UIProcess/WebProcessPool.h:
+        * WebProcess/GPU/GPUProcessConnectionInfo.h:
+        (WebKit::GPUProcessConnectionInfo::identifier const):
+        (WebKit::GPUProcessConnectionInfo::identifier): Deleted.
+        * WebProcess/Network/NetworkProcessConnectionInfo.h:
+        (WebKit::NetworkProcessConnectionInfo::identifier const):
+        (WebKit::NetworkProcessConnectionInfo::identifier): Deleted.
+
 2020-04-07  Simon Fraser  <simon.fra...@apple.com>
 
         Use RectEdges<> in some scrolling tree code

Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp	2020-04-07 22:07:54 UTC (rev 259673)
@@ -132,11 +132,7 @@
     connect();
 }
 
-GPUProcessProxy::~GPUProcessProxy()
-{
-    for (auto& request : m_connectionRequests.values())
-        request.reply({ });
-}
+GPUProcessProxy::~GPUProcessProxy() = default;
 
 #if ENABLE(MEDIA_STREAM)
 void GPUProcessProxy::setUseMockCaptureDevices(bool value)
@@ -178,16 +174,7 @@
 
 void GPUProcessProxy::getGPUProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetGPUProcessConnection::DelayedReply&& reply)
 {
-    m_connectionRequests.add(++m_connectionRequestIdentifier, ConnectionRequest { makeWeakPtr(webProcessProxy), WTFMove(reply) });
-    if (state() == State::Launching)
-        return;
-    openGPUProcessConnection(m_connectionRequestIdentifier, webProcessProxy);
-}
-
-void GPUProcessProxy::openGPUProcessConnection(ConnectionRequestIdentifier connectionRequestIdentifier, WebProcessProxy& webProcessProxy)
-{
     addSession(webProcessProxy.websiteDataStore());
-    
 #if HAVE(VISIBILITY_PROPAGATION_VIEW)
     if (m_contextIDForVisibilityPropagation)
         webProcessProxy.didCreateContextInGPUProcessForVisibilityPropagation(m_contextIDForVisibilityPropagation);
@@ -195,24 +182,17 @@
         m_processesPendingVisibilityPropagationNotification.append(makeWeakPtr(webProcessProxy));
 #endif
 
-    sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(this), webProcessProxy = makeWeakPtr(webProcessProxy), connectionRequestIdentifier](auto&& connectionIdentifier) mutable {
-        if (!weakThis)
-            return;
-
-        if (!connectionIdentifier) {
-            // GPU process probably crashed, the connection request should have been moved.
-            ASSERT(m_connectionRequests.isEmpty());
-            return;
+    RELEASE_LOG(ProcessSuspension, "%p - GPUProcessProxy is taking a background assertion because a web process is requesting a connection", this);
+    sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply), activity = throttler().backgroundActivity("GPUProcessProxy::getGPUProcessConnection"_s)](auto&& connectionIdentifier) mutable {
+        if (!weakThis) {
+            RELEASE_LOG_ERROR(Process, "GPUProcessProxy::getGPUProcessConnection: GPUProcessProxy deallocated during connection establishment");
+            return reply({ });
         }
-
-        ASSERT(m_connectionRequests.contains(connectionRequestIdentifier));
-        auto request = m_connectionRequests.take(connectionRequestIdentifier);
-
 #if USE(UNIX_DOMAIN_SOCKETS) || OS(WINDOWS)
-        request.reply(GPUProcessConnectionInfo { WTFMove(*connectionIdentifier) });
+        reply(GPUProcessConnectionInfo { WTFMove(*connectionIdentifier) });
 #elif OS(DARWIN)
         MESSAGE_CHECK(MACH_PORT_VALID(connectionIdentifier->port()));
-        request.reply(GPUProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, this->connection()->getAuditToken() });
+        reply(GPUProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, this->connection()->getAuditToken() });
 #else
         notImplemented();
 #endif
@@ -253,14 +233,6 @@
         gpuProcessCrashed();
         return;
     }
-
-    auto connectionRequests = WTFMove(m_connectionRequests);
-    for (auto& connectionRequest : connectionRequests.values()) {
-        if (connectionRequest.webProcess)
-            getGPUProcessConnection(*connectionRequest.webProcess, WTFMove(connectionRequest.reply));
-        else
-            connectionRequest.reply({ });
-    }
     
 #if PLATFORM(IOS_FAMILY)
     if (xpc_connection_t connection = this->connection()->xpcConnection())

Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h	2020-04-07 22:07:54 UTC (rev 259673)
@@ -96,9 +96,6 @@
     // ProcessLauncher::Client
     void didFinishLaunching(ProcessLauncher*, IPC::Connection::Identifier) override;
 
-    typedef uint64_t ConnectionRequestIdentifier;
-    void openGPUProcessConnection(ConnectionRequestIdentifier, WebProcessProxy&);
-
     // IPC::Connection::Client
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
     void didClose(IPC::Connection&) override;
@@ -110,13 +107,6 @@
 
     static GPUProcessProxy* m_singleton;
 
-    struct ConnectionRequest {
-        WeakPtr<WebProcessProxy> webProcess;
-        Messages::WebProcessProxy::GetGPUProcessConnectionDelayedReply reply;
-    };
-    ConnectionRequestIdentifier m_connectionRequestIdentifier { 0 };
-    HashMap<ConnectionRequestIdentifier, ConnectionRequest> m_connectionRequests;
-
     ProcessThrottler m_throttler;
     ProcessThrottler::ActivityVariant m_activityFromWebProcesses;
 #if ENABLE(MEDIA_STREAM)

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2020-04-07 22:07:54 UTC (rev 259673)
@@ -108,9 +108,6 @@
 
     if (m_downloadProxyMap)
         m_downloadProxyMap->invalidate();
-
-    for (auto& request : m_connectionRequests.values())
-        request.reply({ });
 }
 
 void NetworkProcessProxy::getLaunchOptions(ProcessLauncher::LaunchOptions& launchOptions)
@@ -140,32 +137,18 @@
 
 void NetworkProcessProxy::getNetworkProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply&& reply)
 {
-    m_connectionRequests.add(++m_connectionRequestIdentifier, ConnectionRequest { makeWeakPtr(webProcessProxy), WTFMove(reply) });
-    if (state() == State::Launching)
-        return;
-    openNetworkProcessConnection(m_connectionRequestIdentifier, webProcessProxy);
-}
-
-void NetworkProcessProxy::openNetworkProcessConnection(uint64_t connectionRequestIdentifier, WebProcessProxy& webProcessProxy)
-{
-    connection()->sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(this), webProcessProxy = makeWeakPtr(webProcessProxy), connectionRequestIdentifier](auto&& connectionIdentifier, HTTPCookieAcceptPolicy cookieAcceptPolicy) mutable {
-        if (!weakThis)
-            return;
-
-        if (!connectionIdentifier) {
-            // Network process probably crashed, the connection request should have been moved.
-            ASSERT(m_connectionRequests.isEmpty());
-            return;
+    RELEASE_LOG(ProcessSuspension, "%p - NetworkProcessProxy is taking a background assertion because a web process is requesting a connection", this);
+    sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply), activity = throttler().backgroundActivity("NetworkProcessProxy::getNetworkProcessConnection"_s)](auto&& connectionIdentifier, auto cookieAcceptPolicy) mutable {
+        if (!weakThis) {
+            RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::getNetworkProcessConnection: NetworkProcessProxy deallocated during connection establishment");
+            return reply({ });
         }
 
-        ASSERT(m_connectionRequests.contains(connectionRequestIdentifier));
-        auto request = m_connectionRequests.take(connectionRequestIdentifier);
-
 #if USE(UNIX_DOMAIN_SOCKETS) || OS(WINDOWS)
-        request.reply(NetworkProcessConnectionInfo { WTFMove(*connectionIdentifier), cookieAcceptPolicy });
+        reply(NetworkProcessConnectionInfo { WTFMove(*connectionIdentifier), cookieAcceptPolicy });
 #elif OS(DARWIN)
         MESSAGE_CHECK(MACH_PORT_VALID(connectionIdentifier->port()));
-        request.reply(NetworkProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, cookieAcceptPolicy, connection()->getAuditToken() });
+        reply(NetworkProcessConnectionInfo { IPC::Attachment { connectionIdentifier->port(), MACH_MSG_TYPE_MOVE_SEND }, cookieAcceptPolicy, connection()->getAuditToken() });
 #else
         notImplemented();
 #endif
@@ -248,18 +231,8 @@
 {
     clearCallbackStates();
 
-    Vector<std::pair<RefPtr<WebProcessProxy>, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply>> pendingRequests;
-    pendingRequests.reserveInitialCapacity(m_connectionRequests.size());
-    for (auto& request : m_connectionRequests.values()) {
-        if (request.webProcess)
-            pendingRequests.uncheckedAppend(std::make_pair(makeRefPtr(request.webProcess.get()), WTFMove(request.reply)));
-        else
-            request.reply({ });
-    }
-    m_connectionRequests.clear();
-
     // Tell the network process manager to forget about this network process proxy. This will cause us to be deleted.
-    m_processPool.networkProcessCrashed(*this, WTFMove(pendingRequests));
+    m_processPool.networkProcessCrashed(*this);
 }
 
 void NetworkProcessProxy::clearCallbackStates()
@@ -409,14 +382,6 @@
         return;
     }
 
-    auto connectionRequests = WTFMove(m_connectionRequests);
-    for (auto& connectionRequest : connectionRequests.values()) {
-        if (connectionRequest.webProcess)
-            getNetworkProcessConnection(*connectionRequest.webProcess, WTFMove(connectionRequest.reply));
-        else
-            connectionRequest.reply({ });
-    }
-
 #if PLATFORM(COCOA)
     if (m_processPool.processSuppressionEnabled())
         setProcessSuppressionEnabled(true);

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h	2020-04-07 22:07:54 UTC (rev 259673)
@@ -286,19 +286,10 @@
     // ProcessLauncher::Client
     void didFinishLaunching(ProcessLauncher*, IPC::Connection::Identifier) override;
 
-    void openNetworkProcessConnection(uint64_t connectionRequestIdentifier, WebProcessProxy&);
-
     void processAuthenticationChallenge(PAL::SessionID, Ref<AuthenticationChallengeProxy>&&);
 
     WebProcessPool& m_processPool;
 
-    struct ConnectionRequest {
-        WeakPtr<WebProcessProxy> webProcess;
-        Messages::WebProcessProxy::GetNetworkProcessConnectionDelayedReply reply;
-    };
-    uint64_t m_connectionRequestIdentifier { 0 };
-    HashMap<uint64_t, ConnectionRequest> m_connectionRequests;
-
     HashMap<uint64_t, CompletionHandler<void(WebsiteData)>> m_pendingFetchWebsiteDataCallbacks;
     HashMap<uint64_t, CompletionHandler<void()>> m_pendingDeleteWebsiteDataCallbacks;
     HashMap<uint64_t, CompletionHandler<void()>> m_pendingDeleteWebsiteDataForOriginsCallbacks;

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp	2020-04-07 22:07:54 UTC (rev 259673)
@@ -39,11 +39,13 @@
 #include "AuxiliaryProcessMessages.h"
 #include "DownloadProxy.h"
 #include "DownloadProxyMessages.h"
+#include "GPUProcessConnectionInfo.h"
 #include "GamepadData.h"
 #include "HighPerformanceGraphicsUsageSampler.h"
 #include "LegacyGlobalSettings.h"
 #include "LogInitialization.h"
 #include "Logging.h"
+#include "NetworkProcessConnectionInfo.h"
 #include "NetworkProcessCreationParameters.h"
 #include "NetworkProcessMessages.h"
 #include "NetworkProcessProxy.h"
@@ -711,7 +713,7 @@
     return *m_networkProcess;
 }
 
-void WebProcessPool::networkProcessCrashed(NetworkProcessProxy& networkProcessProxy, Vector<std::pair<RefPtr<WebProcessProxy>, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply>>&& pendingReplies)
+void WebProcessPool::networkProcessCrashed(NetworkProcessProxy& networkProcessProxy)
 {
     ASSERT(m_networkProcess);
     ASSERT(&networkProcessProxy == m_networkProcess.get());
@@ -727,13 +729,6 @@
     // Leave the process proxy around during client call, so that the client could query the process identifier.
     m_networkProcess = nullptr;
 
-    // Attempt to re-launch.
-    if (pendingReplies.isEmpty())
-        return;
-    auto& newNetworkProcess = ensureNetworkProcess();
-    for (auto& reply : pendingReplies)
-        newNetworkProcess.getNetworkProcessConnection(*reply.first, WTFMove(reply.second));
-
     terminateServiceWorkers();
 }
 
@@ -747,9 +742,15 @@
 void WebProcessPool::getNetworkProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetNetworkProcessConnection::DelayedReply&& reply)
 {
     ensureNetworkProcess();
-    ASSERT(m_networkProcess);
-
-    m_networkProcess->getNetworkProcessConnection(webProcessProxy, WTFMove(reply));
+    m_networkProcess->getNetworkProcessConnection(webProcessProxy, [this, weakThis = makeWeakPtr(*this), webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {
+        if (UNLIKELY(!IPC::Connection::identifierIsValid(connectionInfo.identifier()) && webProcessProxy && weakThis)) {
+            WEBPROCESSPOOL_RELEASE_LOG_ERROR(Process, "getNetworkProcessConnection: Failed first attempt, retrying");
+            ensureNetworkProcess();
+            m_networkProcess->getNetworkProcessConnection(*webProcessProxy, WTFMove(reply));
+            return;
+        }
+        reply(connectionInfo);
+    });
 }
 
 #if ENABLE(GPU_PROCESS)
@@ -764,7 +765,14 @@
 
 void WebProcessPool::getGPUProcessConnection(WebProcessProxy& webProcessProxy, Messages::WebProcessProxy::GetGPUProcessConnection::DelayedReply&& reply)
 {
-    GPUProcessProxy::singleton().getGPUProcessConnection(webProcessProxy, WTFMove(reply));
+    GPUProcessProxy::singleton().getGPUProcessConnection(webProcessProxy, [this, weakThis = makeWeakPtr(*this), webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {
+        if (UNLIKELY(!IPC::Connection::identifierIsValid(connectionInfo.identifier()) && webProcessProxy && weakThis)) {
+            WEBPROCESSPOOL_RELEASE_LOG_ERROR(Process, "getGPUProcessConnection: Failed first attempt, retrying");
+            GPUProcessProxy::singleton().getGPUProcessConnection(*webProcessProxy, WTFMove(reply));
+            return;
+        }
+        reply(connectionInfo);
+    });
 }
 #endif
 

Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.h (259672 => 259673)


--- trunk/Source/WebKit/UIProcess/WebProcessPool.h	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.h	2020-04-07 22:07:54 UTC (rev 259673)
@@ -389,7 +389,7 @@
     // Network Process Management
     NetworkProcessProxy& ensureNetworkProcess(WebsiteDataStore* withWebsiteDataStore = nullptr);
     NetworkProcessProxy* networkProcess() { return m_networkProcess.get(); }
-    void networkProcessCrashed(NetworkProcessProxy&, Vector<std::pair<RefPtr<WebProcessProxy>, Messages::WebProcessProxy::GetNetworkProcessConnectionDelayedReply>>&&);
+    void networkProcessCrashed(NetworkProcessProxy&);
 
     void getNetworkProcessConnection(WebProcessProxy&, Messages::WebProcessProxy::GetNetworkProcessConnectionDelayedReply&&);
 

Modified: trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnectionInfo.h (259672 => 259673)


--- trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnectionInfo.h	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/WebProcess/GPU/GPUProcessConnectionInfo.h	2020-04-07 22:07:54 UTC (rev 259673)
@@ -35,7 +35,7 @@
     Optional<audit_token_t> auditToken;
 #endif
 
-    IPC::Connection::Identifier identifier()
+    IPC::Connection::Identifier identifier() const
     {
 #if USE(UNIX_DOMAIN_SOCKETS)
         return IPC::Connection::Identifier(connection.fileDescriptor());

Modified: trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnectionInfo.h (259672 => 259673)


--- trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnectionInfo.h	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnectionInfo.h	2020-04-07 22:07:54 UTC (rev 259673)
@@ -36,7 +36,7 @@
     Optional<audit_token_t> auditToken;
 #endif
 
-    IPC::Connection::Identifier identifier()
+    IPC::Connection::Identifier identifier() const
     {
 #if USE(UNIX_DOMAIN_SOCKETS)
         return IPC::Connection::Identifier(connection.fileDescriptor());

Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (259672 => 259673)


--- trunk/Source/WebKit/WebProcess/WebProcess.cpp	2020-04-07 21:33:04 UTC (rev 259672)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp	2020-04-07 22:07:54 UTC (rev 259673)
@@ -1094,16 +1094,20 @@
 {
     NetworkProcessConnectionInfo connectionInfo;
     if (!connection.sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(connectionInfo), 0)) {
+        // If we failed the first time, retry once. The attachment may have become invalid
+        // before it was received by the web process if the network process crashed.
+        if (!connection.sendSync(Messages::WebProcessProxy::GetNetworkProcessConnection(), Messages::WebProcessProxy::GetNetworkProcessConnection::Reply(connectionInfo), 0)) {
 #if PLATFORM(GTK) || PLATFORM(WPE)
-        // GTK+ and WPE ports don't exit on send sync message failure.
-        // In this particular case, the network process can be terminated by the UI process while the
-        // Web process is still initializing, so we always want to exit instead of crashing. This can
-        // happen when the WebView is created and then destroyed quickly.
-        // See https://bugs.webkit.org/show_bug.cgi?id=183348.
-        exit(0);
+            // GTK+ and WPE ports don't exit on send sync message failure.
+            // In this particular case, the network process can be terminated by the UI process while the
+            // Web process is still initializing, so we always want to exit instead of crashing. This can
+            // happen when the WebView is created and then destroyed quickly.
+            // See https://bugs.webkit.org/show_bug.cgi?id=183348.
+            exit(0);
 #else
-        CRASH();
+            CRASH();
 #endif
+        }
     }
 
     return connectionInfo;
@@ -1217,8 +1221,12 @@
 static GPUProcessConnectionInfo getGPUProcessConnection(IPC::Connection& connection)
 {
     GPUProcessConnectionInfo connectionInfo;
-    if (!connection.sendSync(Messages::WebProcessProxy::GetGPUProcessConnection(), Messages::WebProcessProxy::GetGPUProcessConnection::Reply(connectionInfo), 0))
-        CRASH();
+    if (!connection.sendSync(Messages::WebProcessProxy::GetGPUProcessConnection(), Messages::WebProcessProxy::GetGPUProcessConnection::Reply(connectionInfo), 0)) {
+        // If we failed the first time, retry once. The attachment may have become invalid
+        // before it was received by the web process if the network process crashed.
+        if (!connection.sendSync(Messages::WebProcessProxy::GetGPUProcessConnection(), Messages::WebProcessProxy::GetGPUProcessConnection::Reply(connectionInfo), 0))
+            CRASH();
+    }
 
     return connectionInfo;
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to