[webkit-changes] [264665] trunk

Tue, 21 Jul 2020 11:29:15 -0700

Title: [264665] trunk
Revision
264665
Author
you...@apple.com
Date
2020-07-21 11:26:39 -0700 (Tue, 21 Jul 2020)

Log Message

NetworkConnectionToWebProcess should not handle NetworkRTCProvider messages
https://bugs.webkit.org/show_bug.cgi?id=214547

Reviewed by Alex Christensen.

Source/WebKit:

Normally NetworkRTCProvider is created when starting to monitor the network.
In case network process is crashing though, there might be requests to open RTC socket on the new network process which has not started to monitor the network.
In that case, we will go through NetworkConnectionToWebProcess as NetworkRTCProvider is not yet created.
To properly handle this case, on web process side, we send an IPC message to explictly create the NetworkRTCProvider.
Once done, we set the IPC connection to the socket factory which will start sending messages to open sockets if needed.
Until its connection is set, the socket factory will buffer messages to open sockets.
Covered by test no longer crashing in debug.

* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
(WebKit::NetworkConnectionToWebProcess::createRTCProvider):
* NetworkProcess/NetworkConnectionToWebProcess.h:
* NetworkProcess/NetworkConnectionToWebProcess.messages.in:
* WebProcess/Network/webrtc/LibWebRTCNetwork.cpp:
(WebKit::LibWebRTCNetwork::setAsActive):
(WebKit::LibWebRTCNetwork::setConnection):
(WebKit::LibWebRTCNetwork::setSocketFactoryConnection):
* WebProcess/Network/webrtc/LibWebRTCNetwork.h:
* WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:
(WebKit::LibWebRTCSocketFactory::setConnection):
(WebKit::LibWebRTCSocketFactory::createServerTcpSocket):
(WebKit::LibWebRTCSocketFactory::createUdpSocket):
(WebKit::LibWebRTCSocketFactory::createClientTcpSocket):
(WebKit::LibWebRTCSocketFactory::createNewConnectionSocket):
* WebProcess/Network/webrtc/LibWebRTCSocketFactory.h:

LayoutTests:

* platform/mac/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (264664 => 264665)


--- trunk/LayoutTests/ChangeLog	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/LayoutTests/ChangeLog	2020-07-21 18:26:39 UTC (rev 264665)
@@ -1,3 +1,12 @@
+2020-07-21  Youenn Fablet  <you...@apple.com>
+
+        NetworkConnectionToWebProcess should not handle NetworkRTCProvider messages
+        https://bugs.webkit.org/show_bug.cgi?id=214547
+
+        Reviewed by Alex Christensen.
+
+        * platform/mac/TestExpectations:
+
 2020-07-21  Sihui Liu  <sihui_...@appe.com>
 
         REGRESSION(r264486): ASSERTION FAILED: ASSERT_NOT_REACHED() in NetworkProcessPlatformStrategies::createBlobRegistry

Modified: trunk/LayoutTests/platform/mac/TestExpectations (264664 => 264665)


--- trunk/LayoutTests/platform/mac/TestExpectations	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2020-07-21 18:26:39 UTC (rev 264665)
@@ -1910,8 +1910,6 @@
 
 webkit.org/b/212219 media/track/track-cue-missing.html [ Pass Failure ]
 
-webkit.org/b/212218 webrtc/datachannel/gather-candidates-networkprocess-crash.html [ Pass Timeout ]
-
 webkit.org/b/212226 imported/w3c/web-platform-tests/css/css-grid/alignment/self-baseline/grid-self-baseline-changes-grid-area-size-008.html [ ImageOnlyFailure ]
 
 webkit.org/b/212488 [ Catalina ] media/video-play-audio-require-user-gesture.html [ Pass Timeout ]

Modified: trunk/Source/WebKit/ChangeLog (264664 => 264665)


--- trunk/Source/WebKit/ChangeLog	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/ChangeLog	2020-07-21 18:26:39 UTC (rev 264665)
@@ -1,5 +1,38 @@
 2020-07-21  Youenn Fablet  <you...@apple.com>
 
+        NetworkConnectionToWebProcess should not handle NetworkRTCProvider messages
+        https://bugs.webkit.org/show_bug.cgi?id=214547
+
+        Reviewed by Alex Christensen.
+
+        Normally NetworkRTCProvider is created when starting to monitor the network.
+        In case network process is crashing though, there might be requests to open RTC socket on the new network process which has not started to monitor the network.
+        In that case, we will go through NetworkConnectionToWebProcess as NetworkRTCProvider is not yet created.
+        To properly handle this case, on web process side, we send an IPC message to explictly create the NetworkRTCProvider.
+        Once done, we set the IPC connection to the socket factory which will start sending messages to open sockets if needed.
+        Until its connection is set, the socket factory will buffer messages to open sockets.
+        Covered by test no longer crashing in debug.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
+        (WebKit::NetworkConnectionToWebProcess::createRTCProvider):
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+        * NetworkProcess/NetworkConnectionToWebProcess.messages.in:
+        * WebProcess/Network/webrtc/LibWebRTCNetwork.cpp:
+        (WebKit::LibWebRTCNetwork::setAsActive):
+        (WebKit::LibWebRTCNetwork::setConnection):
+        (WebKit::LibWebRTCNetwork::setSocketFactoryConnection):
+        * WebProcess/Network/webrtc/LibWebRTCNetwork.h:
+        * WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp:
+        (WebKit::LibWebRTCSocketFactory::setConnection):
+        (WebKit::LibWebRTCSocketFactory::createServerTcpSocket):
+        (WebKit::LibWebRTCSocketFactory::createUdpSocket):
+        (WebKit::LibWebRTCSocketFactory::createClientTcpSocket):
+        (WebKit::LibWebRTCSocketFactory::createNewConnectionSocket):
+        * WebProcess/Network/webrtc/LibWebRTCSocketFactory.h:
+
+2020-07-21  Youenn Fablet  <you...@apple.com>
+
         Fetch/XHR loads done by extensions should opt out of response sanitisation done in network process
         https://bugs.webkit.org/show_bug.cgi?id=214588
         <rdar://problem/65060560>

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (264664 => 264665)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2020-07-21 18:26:39 UTC (rev 264665)
@@ -215,10 +215,6 @@
     }
 
 #if USE(LIBWEBRTC)
-    if (decoder.messageReceiverName() == Messages::NetworkRTCProvider::messageReceiverName()) {
-        rtcProvider().didReceiveMessage(connection, decoder);
-        return;
-    }
     if (decoder.messageReceiverName() == Messages::NetworkRTCMonitor::messageReceiverName()) {
         rtcProvider().didReceiveNetworkRTCMonitorMessage(connection, decoder);
         return;
@@ -273,6 +269,14 @@
 }
 #endif
 
+void NetworkConnectionToWebProcess::createRTCProvider(CompletionHandler<void()>&& callback)
+{
+#if USE(LIBWEBRTC)
+    rtcProvider();
+#endif
+    callback();
+}
+
 CacheStorageEngineConnection& NetworkConnectionToWebProcess::cacheStorageConnection()
 {
     if (!m_cacheStorageConnection)

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (264664 => 264665)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h	2020-07-21 18:26:39 UTC (rev 264665)
@@ -239,6 +239,8 @@
     void unregisterSWConnection();
 #endif
 
+    void createRTCProvider(CompletionHandler<void()>&&);
+
     void createNewMessagePortChannel(const WebCore::MessagePortIdentifier& port1, const WebCore::MessagePortIdentifier& port2);
     void entangleLocalPortInThisProcessToRemote(const WebCore::MessagePortIdentifier& local, const WebCore::MessagePortIdentifier& remote);
     void messagePortDisentangled(const WebCore::MessagePortIdentifier&);

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in (264664 => 264665)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.messages.in	2020-07-21 18:26:39 UTC (rev 264665)
@@ -84,6 +84,8 @@
     CloseSWContextConnection()
 #endif
 
+    CreateRTCProvider() -> () Async
+
     UpdateQuotaBasedOnSpaceUsageForTesting(struct WebCore::ClientOrigin origin)
     CreateNewMessagePortChannel(struct WebCore::MessagePortIdentifier port1, struct WebCore::MessagePortIdentifier port2)
     EntangleLocalPortInThisProcessToRemote(struct WebCore::MessagePortIdentifier local, struct WebCore::MessagePortIdentifier remote)

Modified: trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.cpp (264664 => 264665)


--- trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.cpp	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.cpp	2020-07-21 18:26:39 UTC (rev 264665)
@@ -29,6 +29,7 @@
 #include "DataReference.h"
 #include "LibWebRTCNetworkMessages.h"
 #include "Logging.h"
+#include "NetworkConnectionToWebProcessMessages.h"
 #include <WebCore/SharedBuffer.h>
 #include <wtf/MainThread.h>
 
@@ -44,11 +45,8 @@
     ASSERT(!m_isActive);
     m_isActive = true;
 #if USE(LIBWEBRTC)
-    if (m_connection) {
-        WebCore::LibWebRTCProvider::callOnWebRTCNetworkThread([this, connection = m_connection]() mutable {
-            m_socketFactory.setConnection(WTFMove(connection));
-        });
-    }
+    if (m_connection)
+        setSocketFactoryConnection();
 #endif
 }
 
@@ -66,19 +64,36 @@
 #if USE(LIBWEBRTC)
     if (m_connection)
         m_connection->removeThreadMessageReceiver(Messages::LibWebRTCNetwork::messageReceiverName());
-    if (m_isActive) {
-        WebCore::LibWebRTCProvider::callOnWebRTCNetworkThread([this, connection]() mutable {
-            m_socketFactory.setConnection(WTFMove(connection));
-        });
-    }
 #endif
     m_connection = WTFMove(connection);
 #if USE(LIBWEBRTC)
+    if (m_isActive)
+        setSocketFactoryConnection();
     if (m_connection)
         m_connection->addThreadMessageReceiver(Messages::LibWebRTCNetwork::messageReceiverName(), this);
 #endif
 }
 
+#if USE(LIBWEBRTC)
+void LibWebRTCNetwork::setSocketFactoryConnection()
+{
+    if (!m_connection) {
+        WebCore::LibWebRTCProvider::callOnWebRTCNetworkThread([this]() mutable {
+            m_socketFactory.setConnection(nullptr);
+        });
+        return;
+    }
+    m_connection->sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::CreateRTCProvider(), [this, connection = m_connection]() mutable {
+        if (!connection->isValid())
+            return;
+
+        WebCore::LibWebRTCProvider::callOnWebRTCNetworkThread([this, connection = WTFMove(connection)]() mutable {
+            m_socketFactory.setConnection(WTFMove(connection));
+        });
+    }, 0);
+}
+#endif
+
 void LibWebRTCNetwork::dispatchToThread(Function<void()>&& callback)
 {
     if (!m_isActive) {

Modified: trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.h (264664 => 264665)


--- trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.h	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCNetwork.h	2020-07-21 18:26:39 UTC (rev 264665)
@@ -67,6 +67,8 @@
 
 private:
 #if USE(LIBWEBRTC)
+    void setSocketFactoryConnection();
+
     void signalReadPacket(WebCore::LibWebRTCSocketIdentifier, const IPC::DataReference&, const RTCNetwork::IPAddress&, uint16_t port, int64_t);
     void signalSentPacket(WebCore::LibWebRTCSocketIdentifier, int, int64_t);
     void signalAddressReady(WebCore::LibWebRTCSocketIdentifier, const RTCNetwork::SocketAddress&);

Modified: trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp (264664 => 264665)


--- trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.cpp	2020-07-21 18:26:39 UTC (rev 264665)
@@ -31,7 +31,6 @@
 #include "LibWebRTCNetwork.h"
 #include "Logging.h"
 #include "NetworkProcessConnection.h"
-#include "NetworkRTCMonitorMessages.h"
 #include "NetworkRTCProviderMessages.h"
 #include "WebProcess.h"
 #include <wtf/MainThread.h>
@@ -51,6 +50,11 @@
 {
     ASSERT(!WTF::isMainRunLoop());
     m_connection = WTFMove(connection);
+    if (!m_connection)
+        return;
+
+    while (!m_pendingMessageTasks.isEmpty())
+        m_pendingMessageTasks.takeFirst()(*m_connection);
 }
 
 IPC::Connection* LibWebRTCSocketFactory::connection()
@@ -62,18 +66,18 @@
 rtc::AsyncPacketSocket* LibWebRTCSocketFactory::createServerTcpSocket(const void* socketGroup, const rtc::SocketAddress& address, uint16_t minPort, uint16_t maxPort, int options)
 {
     ASSERT(!WTF::isMainRunLoop());
-    if (!m_connection) {
-        RELEASE_LOG(WebRTC, "No connection to create server TCP socket");
+    auto socket = makeUnique<LibWebRTCSocket>(*this, socketGroup, LibWebRTCSocket::Type::ServerTCP, address, rtc::SocketAddress());
+
+    if (m_connection)
+        m_connection->send(Messages::NetworkRTCProvider::CreateServerTCPSocket(socket->identifier(), RTCNetwork::SocketAddress(address), minPort, maxPort, options), 0);
+    else {
         callOnMainThread([] {
             WebProcess::singleton().ensureNetworkProcessConnection();
         });
-        return nullptr;
+        m_pendingMessageTasks.append([identifier = socket->identifier(), address = RTCNetwork::SocketAddress(address), minPort, maxPort, options](auto& connection) {
+            connection.send(Messages::NetworkRTCProvider::CreateServerTCPSocket(identifier, address, minPort, maxPort, options), 0);
+        });
     }
-
-    auto socket = makeUnique<LibWebRTCSocket>(*this, socketGroup, LibWebRTCSocket::Type::ServerTCP, address, rtc::SocketAddress());
-
-    m_connection->send(Messages::NetworkRTCProvider::CreateServerTCPSocket(socket->identifier(), RTCNetwork::SocketAddress(address), minPort, maxPort, options), 0);
-
     return socket.release();
 }
 
@@ -80,18 +84,19 @@
 rtc::AsyncPacketSocket* LibWebRTCSocketFactory::createUdpSocket(const void* socketGroup, const rtc::SocketAddress& address, uint16_t minPort, uint16_t maxPort)
 {
     ASSERT(!WTF::isMainRunLoop());
-    if (!m_connection) {
-        RELEASE_LOG(WebRTC, "No connection to create UDP socket");
+    auto socket = makeUnique<LibWebRTCSocket>(*this, socketGroup, LibWebRTCSocket::Type::UDP, address, rtc::SocketAddress());
+
+    if (m_connection)
+        m_connection->send(Messages::NetworkRTCProvider::CreateUDPSocket(socket->identifier(), RTCNetwork::SocketAddress(address), minPort, maxPort), 0);
+    else {
         callOnMainThread([] {
             WebProcess::singleton().ensureNetworkProcessConnection();
         });
-        return nullptr;
+        m_pendingMessageTasks.append([identifier = socket->identifier(), address = RTCNetwork::SocketAddress(address), minPort, maxPort](auto& connection) {
+            connection.send(Messages::NetworkRTCProvider::CreateUDPSocket(identifier, address, minPort, maxPort), 0);
+        });
     }
 
-    auto socket = makeUnique<LibWebRTCSocket>(*this, socketGroup, LibWebRTCSocket::Type::UDP, address, rtc::SocketAddress());
-
-    m_connection->send(Messages::NetworkRTCProvider::CreateUDPSocket(socket->identifier(), RTCNetwork::SocketAddress(address), minPort, maxPort), 0);
-
     return socket.release();
 }
 
@@ -98,19 +103,21 @@
 rtc::AsyncPacketSocket* LibWebRTCSocketFactory::createClientTcpSocket(const void* socketGroup, const rtc::SocketAddress& localAddress, const rtc::SocketAddress& remoteAddress, String&& userAgent, const rtc::PacketSocketTcpOptions& options)
 {
     ASSERT(!WTF::isMainRunLoop());
-    if (!m_connection) {
-        RELEASE_LOG(WebRTC, "No connection to create client TCP socket");
-        callOnMainThread([] {
-            WebProcess::singleton().ensureNetworkProcessConnection();
-        });
-        return nullptr;
-    }
 
     auto socket = makeUnique<LibWebRTCSocket>(*this, socketGroup, LibWebRTCSocket::Type::ClientTCP, localAddress, remoteAddress);
     socket->setState(LibWebRTCSocket::STATE_CONNECTING);
 
     // FIXME: We only transfer options.opts but should also handle other members.
-    m_connection->send(Messages::NetworkRTCProvider::CreateClientTCPSocket(socket->identifier(), RTCNetwork::SocketAddress(prepareSocketAddress(localAddress, m_disableNonLocalhostConnections)), RTCNetwork::SocketAddress(prepareSocketAddress(remoteAddress, m_disableNonLocalhostConnections)), userAgent, options.opts), 0);
+    if (m_connection)
+        m_connection->send(Messages::NetworkRTCProvider::CreateClientTCPSocket(socket->identifier(), RTCNetwork::SocketAddress(prepareSocketAddress(localAddress, m_disableNonLocalhostConnections)), RTCNetwork::SocketAddress(prepareSocketAddress(remoteAddress, m_disableNonLocalhostConnections)), userAgent, options.opts), 0);
+    else {
+        callOnMainThread([] {
+            WebProcess::singleton().ensureNetworkProcessConnection();
+        });
+        m_pendingMessageTasks.append([identifier = socket->identifier(), localAddress = RTCNetwork::SocketAddress(prepareSocketAddress(localAddress, m_disableNonLocalhostConnections)), remoteAddress = RTCNetwork::SocketAddress(prepareSocketAddress(remoteAddress, m_disableNonLocalhostConnections)), userAgent, opts = options.opts](auto& connection) {
+            connection.send(Messages::NetworkRTCProvider::CreateClientTCPSocket(identifier, localAddress, remoteAddress, userAgent, opts), 0);
+        });
+    }
 
     return socket.release();
 }
@@ -119,10 +126,8 @@
 {
     ASSERT(!WTF::isMainRunLoop());
     if (!m_connection) {
+        // No need to enqueue a message in this case since it means the network process handling the incoming socket is gone.
         RELEASE_LOG(WebRTC, "No connection to create incoming TCP socket");
-        callOnMainThread([] {
-            WebProcess::singleton().ensureNetworkProcessConnection();
-        });
         return nullptr;
     }

Modified: trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.h (264664 => 264665)


--- trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.h	2020-07-21 18:13:18 UTC (rev 264664)
+++ trunk/Source/WebKit/WebProcess/Network/webrtc/LibWebRTCSocketFactory.h	2020-07-21 18:26:39 UTC (rev 264665)
@@ -33,6 +33,8 @@
 #include <WebCore/LibWebRTCSocketIdentifier.h>
 #include <webrtc/rtc_base/net_helpers.h>
 #include <webrtc/api/packet_socket_factory.h>
+#include <wtf/Deque.h>
+#include <wtf/Function.h>
 #include <wtf/HashMap.h>
 
 namespace WebKit {
@@ -72,6 +74,7 @@
     bool m_disableNonLocalhostConnections { false };
 
     RefPtr<IPC::Connection> m_connection;
+    Deque<Function<void(IPC::Connection&)>> m_pendingMessageTasks;
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to