From 1bc13dd88dc6b18876abf66fdd09faebc9f914a8 Mon Sep 17 00:00:00 2001 From: Kershaw Chang Date: Fri, 28 Aug 2020 08:04:15 +0000 Subject: [PATCH] Bug 1660361 - Send data in chunks from WebSocketChannelParent to WebSocketChannelChild r=valentin Differential Revision: https://phabricator.services.mozilla.com/D88155 --- dom/websocket/tests/mochitest.ini | 1 + .../tests/test_webSocket_longString.html | 48 ++++++++++++++++ netwerk/protocol/websocket/PWebSocket.ipdl | 4 +- .../websocket/WebSocketChannelChild.cpp | 25 ++++++--- .../websocket/WebSocketChannelChild.h | 10 +++- .../websocket/WebSocketChannelParent.cpp | 55 ++++++++++++++++++- 6 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 dom/websocket/tests/test_webSocket_longString.html diff --git a/dom/websocket/tests/mochitest.ini b/dom/websocket/tests/mochitest.ini index 5e8799572890..355d861fbb20 100644 --- a/dom/websocket/tests/mochitest.ini +++ b/dom/websocket/tests/mochitest.ini @@ -50,3 +50,4 @@ support-files = websocket_loadgroup_worker.js support-files = webSocket_sharedWorker.js [test_websocket_bigBlob.html] support-files = file_websocket_bigBlob_wsh.py +[test_webSocket_longString.html] diff --git a/dom/websocket/tests/test_webSocket_longString.html b/dom/websocket/tests/test_webSocket_longString.html new file mode 100644 index 000000000000..78c8e471c791 --- /dev/null +++ b/dom/websocket/tests/test_webSocket_longString.html @@ -0,0 +1,48 @@ + + + + + WebSocket test - big blob on content side + + + + + + + + diff --git a/netwerk/protocol/websocket/PWebSocket.ipdl b/netwerk/protocol/websocket/PWebSocket.ipdl index 55cd57ae9c06..f3d0eebe8243 100644 --- a/netwerk/protocol/websocket/PWebSocket.ipdl +++ b/netwerk/protocol/websocket/PWebSocket.ipdl @@ -53,8 +53,8 @@ child: nsString aEffectiveURL, bool aEncrypted, uint64_t aHttpChannelId); async OnStop(nsresult aStatusCode); - async OnMessageAvailable(nsCString aMsg); - async OnBinaryMessageAvailable(nsCString aMsg); + async OnMessageAvailable(nsDependentCSubstring aMsg, bool aMoreData); + async OnBinaryMessageAvailable(nsDependentCSubstring aMsg, bool aMoreData); async OnAcknowledge(uint32_t aSize); async OnServerClose(uint16_t code, nsCString aReason); diff --git a/netwerk/protocol/websocket/WebSocketChannelChild.cpp b/netwerk/protocol/websocket/WebSocketChannelChild.cpp index a9b7a52bb7a9..13c137f0f219 100644 --- a/netwerk/protocol/websocket/WebSocketChannelChild.cpp +++ b/netwerk/protocol/websocket/WebSocketChannelChild.cpp @@ -294,11 +294,22 @@ class MessageEvent : public WebSocketEvent { bool mBinary; }; -mozilla::ipc::IPCResult WebSocketChannelChild::RecvOnMessageAvailable( - const nsCString& aMsg) { - mEventQ->RunOrEnqueue(new EventTargetDispatcher( - this, new MessageEvent(aMsg, false), mTargetThread)); +void WebSocketChannelChild::RecvOnMessageAvailableInternal( + const nsDependentCSubstring& aMsg, bool aMoreData, bool aBinary) { + if (aMoreData) { + mReceivedMsgBuffer.Append(aMsg); + return; + } + mReceivedMsgBuffer.Append(aMsg); + mEventQ->RunOrEnqueue(new EventTargetDispatcher( + this, new MessageEvent(mReceivedMsgBuffer, aBinary), mTargetThread)); + mReceivedMsgBuffer.Truncate(); +} + +mozilla::ipc::IPCResult WebSocketChannelChild::RecvOnMessageAvailable( + const nsDependentCSubstring& aMsg, const bool& aMoreData) { + RecvOnMessageAvailableInternal(aMsg, aMoreData, false); return IPC_OK(); } @@ -319,10 +330,8 @@ void WebSocketChannelChild::OnMessageAvailable(const nsCString& aMsg) { } mozilla::ipc::IPCResult WebSocketChannelChild::RecvOnBinaryMessageAvailable( - const nsCString& aMsg) { - mEventQ->RunOrEnqueue(new EventTargetDispatcher( - this, new MessageEvent(aMsg, true), mTargetThread)); - + const nsDependentCSubstring& aMsg, const bool& aMoreData) { + RecvOnMessageAvailableInternal(aMsg, aMoreData, true); return IPC_OK(); } diff --git a/netwerk/protocol/websocket/WebSocketChannelChild.h b/netwerk/protocol/websocket/WebSocketChannelChild.h index 0598d2d943b1..57fa335bf8df 100644 --- a/netwerk/protocol/websocket/WebSocketChannelChild.h +++ b/netwerk/protocol/websocket/WebSocketChannelChild.h @@ -57,8 +57,10 @@ class WebSocketChannelChild final : public BaseWebSocketChannel, const bool& aSecure, const uint64_t& aHttpChannelId); mozilla::ipc::IPCResult RecvOnStop(const nsresult& aStatusCode); - mozilla::ipc::IPCResult RecvOnMessageAvailable(const nsCString& aMsg); - mozilla::ipc::IPCResult RecvOnBinaryMessageAvailable(const nsCString& aMsg); + mozilla::ipc::IPCResult RecvOnMessageAvailable( + const nsDependentCSubstring& aMsg, const bool& aMoreData); + mozilla::ipc::IPCResult RecvOnBinaryMessageAvailable( + const nsDependentCSubstring& aMsg, const bool& aMoreData); mozilla::ipc::IPCResult RecvOnAcknowledge(const uint32_t& aSize); mozilla::ipc::IPCResult RecvOnServerClose(const uint16_t& aCode, const nsCString& aReason); @@ -80,8 +82,12 @@ class WebSocketChannelChild final : public BaseWebSocketChannel, // This function tries to get a labeled event target for |mNeckoTarget|. void SetupNeckoTarget(); + void RecvOnMessageAvailableInternal(const nsDependentCSubstring& aMsg, + bool aMoreData, bool aBinary); + RefPtr mEventQ; nsString mEffectiveURL; + nsCString mReceivedMsgBuffer; // This variable is protected by mutex. enum { Opened, Closing, Closed } mIPCState; diff --git a/netwerk/protocol/websocket/WebSocketChannelParent.cpp b/netwerk/protocol/websocket/WebSocketChannelParent.cpp index f95c37905768..c6fcd23e4cb5 100644 --- a/netwerk/protocol/websocket/WebSocketChannelParent.cpp +++ b/netwerk/protocol/websocket/WebSocketChannelParent.cpp @@ -217,13 +217,53 @@ WebSocketChannelParent::OnStop(nsISupports* aContext, nsresult aStatusCode) { return NS_OK; } +static bool SendOnMessageAvailableHelper( + const nsACString& aMsg, + const std::function& aSendFunc) { + // To avoid the crash caused by too large IPC message, we have to split the + // data in small chunks and send them to child process. Note that the chunk + // size used here is the same as what we used for PHttpChannel. + static uint32_t const kCopyChunkSize = 128 * 1024; + uint32_t count = aMsg.Length(); + if (count <= kCopyChunkSize) { + return aSendFunc(nsDependentCSubstring(aMsg), false); + } + + uint32_t start = 0; + uint32_t toRead = std::min(count, kCopyChunkSize); + while (count) { + nsDependentCSubstring data(Substring(aMsg, start, toRead)); + + if (!aSendFunc(data, count > kCopyChunkSize)) { + return false; + } + + start += toRead; + count -= toRead; + toRead = std::min(count, kCopyChunkSize); + } + + return true; +} + NS_IMETHODIMP WebSocketChannelParent::OnMessageAvailable(nsISupports* aContext, const nsACString& aMsg) { LOG(("WebSocketChannelParent::OnMessageAvailable() %p\n", this)); - if (!CanRecv() || !SendOnMessageAvailable(nsCString(aMsg))) { + + if (!CanRecv()) { return NS_ERROR_FAILURE; } + + auto sendFunc = [self = UnsafePtr(this)]( + const nsDependentCSubstring& aMsg, bool aMoreData) { + return self->SendOnMessageAvailable(aMsg, aMoreData); + }; + + if (!SendOnMessageAvailableHelper(aMsg, sendFunc)) { + return NS_ERROR_FAILURE; + } + return NS_OK; } @@ -231,9 +271,20 @@ NS_IMETHODIMP WebSocketChannelParent::OnBinaryMessageAvailable(nsISupports* aContext, const nsACString& aMsg) { LOG(("WebSocketChannelParent::OnBinaryMessageAvailable() %p\n", this)); - if (!CanRecv() || !SendOnBinaryMessageAvailable(nsCString(aMsg))) { + + if (!CanRecv()) { return NS_ERROR_FAILURE; } + + auto sendFunc = [self = UnsafePtr(this)]( + const nsDependentCSubstring& aMsg, bool aMoreData) { + return self->SendOnBinaryMessageAvailable(aMsg, aMoreData); + }; + + if (!SendOnMessageAvailableHelper(aMsg, sendFunc)) { + return NS_ERROR_FAILURE; + } + return NS_OK; }