Bug 1717185 - Fix mBufferedAmount and return value of send in TCPSocket. r=valentin
☠☠ backed out by a518036b2a45 ☠ ☠
authorPing Chen <remotenonsense@gmail.com>
Tue, 29 Jun 2021 23:48:44 +0000
changeset 584674 875cd4a49df6cb01ea0bf394b748e619a779470c
parent 584673 9d9554e0c2c7f8354b21412e7a99e97a20123a14
child 584675 a518036b2a45421b912ce00efe52b06bba66f330
push id145734
push userremotenonsense@gmail.com
push dateTue, 29 Jun 2021 23:51:08 +0000
treeherderautoland@875cd4a49df6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1717185
milestone91.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1717185 - Fix mBufferedAmount and return value of send in TCPSocket. r=valentin Previously, mBufferedAmount is only updated when copy complete. If I keep calling `send`, `NotifyCopyComplete` is only called after the last `send`. This patch changes to update mBufferedAmount in every `send` call, so that it returns `false` correctly if buffer is full. Differential Revision: https://phabricator.services.mozilla.com/D118258
dom/network/TCPSocket.cpp
dom/network/tests/test_tcpsocket_client_and_server_basics.js
--- a/dom/network/TCPSocket.cpp
+++ b/dom/network/TCPSocket.cpp
@@ -341,16 +341,27 @@ NS_IMETHODIMP
 CopierCallbacks::OnStopRequest(nsIRequest* aRequest, nsresult aStatus) {
   mOwner->NotifyCopyComplete(aStatus);
   mOwner = nullptr;
   return NS_OK;
 }
 }  // unnamed namespace
 
 nsresult TCPSocket::EnsureCopying() {
+  // Let's update the buffered amount of data.
+  uint64_t bufferedAmount = 0;
+  for (uint32_t i = 0, len = mPendingData.Length(); i < len; ++i) {
+    nsCOMPtr<nsIInputStream> stream = mPendingData[i];
+    uint64_t available = 0;
+    if (NS_SUCCEEDED(stream->Available(&available))) {
+      bufferedAmount += available;
+    }
+  }
+  mBufferedAmount = bufferedAmount;
+
   if (mAsyncCopierActive) {
     return NS_OK;
   }
 
   mAsyncCopierActive = true;
 
   nsresult rv;
 
@@ -386,38 +397,27 @@ nsresult TCPSocket::EnsureCopying() {
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 void TCPSocket::NotifyCopyComplete(nsresult aStatus) {
   mAsyncCopierActive = false;
 
-  // Let's update the buffered amount of data.
-  uint64_t bufferedAmount = 0;
-  for (uint32_t i = 0, len = mPendingData.Length(); i < len; ++i) {
-    nsCOMPtr<nsIInputStream> stream = mPendingData[i];
-    uint64_t available = 0;
-    if (NS_SUCCEEDED(stream->Available(&available))) {
-      bufferedAmount += available;
-    }
-  }
-  mBufferedAmount = bufferedAmount;
-
   if (mSocketBridgeParent && mSocketBridgeParent->IPCOpen()) {
     mozilla::Unused << mSocketBridgeParent->SendUpdateBufferedAmount(
         BufferedAmount(), mTrackingNumber);
   }
 
   if (NS_FAILED(aStatus)) {
     MaybeReportErrorAndCloseIfOpen(aStatus);
     return;
   }
 
-  if (bufferedAmount != 0) {
+  if (BufferedAmount() != 0) {
     EnsureCopying();
     return;
   }
 
   // Maybe we have some empty stream. We want to have an empty queue now.
   mPendingData.Clear();
 
   // If we are waiting for initiating starttls, we can begin to
--- a/dom/network/tests/test_tcpsocket_client_and_server_basics.js
+++ b/dom/network/tests/test_tcpsocket_client_and_server_basics.js
@@ -309,22 +309,33 @@ async function test_basics() {
   );
 
   // -- Send "big" data in both directions
   // (Enough to cross the buffering/drain threshold; 64KiB)
   let bigUint8Array = new Uint8Array(65536 + 3);
   for (let i = 0; i < bigUint8Array.length; i++) {
     bigUint8Array[i] = i % 256;
   }
+  // This can be anything from 1 to 65536. The idea is spliting and sending
+  // bigUint8Array in two chunks should trigger ondrain the same as sending
+  // bigUint8Array in one chunk.
+  let lengthOfChunk1 = 65536;
+  is(
+    clientSocket.send(bigUint8Array.buffer, 0, lengthOfChunk1),
+    true,
+    "Client sending chunk1 should not result in the buffer being full."
+  );
   // Do this twice so we have confidence that the 'drain' event machinery
-  // doesn't break after the first use.
+  // doesn't break after the first use. The first time we send bigUint8Array in
+  // two chunks, the second time we send bigUint8Array in one chunk.
   for (let iSend = 0; iSend < 2; iSend++) {
     // - Send "big" data from the client to the server
+    let offset = iSend == 0 ? lengthOfChunk1 : 0;
     is(
-      clientSocket.send(bigUint8Array.buffer, 0, bigUint8Array.length),
+      clientSocket.send(bigUint8Array.buffer, offset, bigUint8Array.length),
       false,
       "Client sending more than 64k should result in the buffer being full."
     );
     is(
       (await clientQueue.waitForEvent()).type,
       "drain",
       "The drain event should fire after a large send that indicated full."
     );
@@ -333,19 +344,26 @@ async function test_basics() {
       bigUint8Array.length
     );
     assertUint8ArraysEqual(
       serverReceived,
       bigUint8Array,
       "server received/client sent"
     );
 
+    if (iSend == 0) {
+      is(
+        serverSocket.send(bigUint8Array.buffer, 0, lengthOfChunk1),
+        true,
+        "Server sending chunk1 should not result in the buffer being full."
+      );
+    }
     // - Send "big" data from the server to the client
     is(
-      serverSocket.send(bigUint8Array.buffer, 0, bigUint8Array.length),
+      serverSocket.send(bigUint8Array.buffer, offset, bigUint8Array.length),
       false,
       "Server sending more than 64k should result in the buffer being full."
     );
     is(
       (await serverQueue.waitForEvent()).type,
       "drain",
       "The drain event should fire after a large send that indicated full."
     );