Bug 1426875 - Avoid adding/removing streams to nsMultiplexInputStream when reading in TCPSocket, r=jdm
authorAndrea Marchesini <amarchesini@mozilla.com>
Sat, 30 Dec 2017 12:47:12 +0100
changeset 449278 ad809e54748edeb50f0cc1d5a10e806304dc19f4
parent 449277 82cd16e43fed050ec7bc21fb3830cdf3b77212f7
child 449279 5a862b1b8997666093878eac4cb84dad3ab4d0d1
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1426875
milestone59.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 1426875 - Avoid adding/removing streams to nsMultiplexInputStream when reading in TCPSocket, r=jdm
dom/network/TCPSocket.cpp
dom/network/TCPSocket.h
--- a/dom/network/TCPSocket.cpp
+++ b/dom/network/TCPSocket.cpp
@@ -102,33 +102,29 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(TCPSocket,
                                                   DOMEventTargetHelper)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mTransport)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSocketInputStream)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSocketOutputStream)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mInputStreamPump)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mInputStreamScriptable)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mInputStreamBinary)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMultiplexStream)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMultiplexStreamCopier)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingDataAfterStartTLS)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSocketBridgeChild)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSocketBridgeParent)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(TCPSocket,
                                                 DOMEventTargetHelper)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTransport)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSocketInputStream)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSocketOutputStream)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mInputStreamPump)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mInputStreamScriptable)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mInputStreamBinary)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mMultiplexStream)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mMultiplexStreamCopier)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPendingDataAfterStartTLS)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSocketBridgeChild)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSocketBridgeParent)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_ADDREF_INHERITED(TCPSocket, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(TCPSocket, DOMEventTargetHelper)
 
@@ -203,36 +199,16 @@ TCPSocket::CreateStream()
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
     mInputStreamScriptable = do_CreateInstance("@mozilla.org/scriptableinputstream;1", &rv);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = mInputStreamScriptable->Init(mSocketInputStream);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  mMultiplexStream = do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1", &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-  nsCOMPtr<nsIInputStream> stream = do_QueryInterface(mMultiplexStream);
-
-  mMultiplexStreamCopier = do_CreateInstance("@mozilla.org/network/async-stream-copier;1", &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<nsISocketTransportService> sts =
-      do_GetService("@mozilla.org/network/socket-transport-service;1");
-
-  nsCOMPtr<nsIEventTarget> target = do_QueryInterface(sts);
-  rv = mMultiplexStreamCopier->Init(stream,
-                                    mSocketOutputStream,
-                                    target,
-                                    true, /* source buffered */
-                                    false, /* sink buffered */
-                                    BUFFER_SIZE,
-                                    false, /* close source */
-                                    false); /* close sink */
-  NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
 nsresult
 TCPSocket::InitWithUnconnectedTransport(nsISocketTransport* aTransport)
 {
   mReadyState = TCPReadyState::Connecting;
   mTransport = aTransport;
@@ -332,19 +308,17 @@ TCPSocket::UpgradeToSecure(mozilla::Erro
 
   mSsl = true;
 
   if (mSocketBridgeChild) {
     mSocketBridgeChild->SendStartTLS();
     return;
   }
 
-  uint32_t count = 0;
-  mMultiplexStream->GetCount(&count);
-  if (!count) {
+  if (!mAsyncCopierActive) {
     ActivateTLS();
   } else {
     mWaitingForStartTLS = true;
   }
 }
 
 namespace {
 class CopierCallbacks final : public nsIRequestObserver
@@ -379,71 +353,99 @@ CopierCallbacks::OnStopRequest(nsIReques
 nsresult
 TCPSocket::EnsureCopying()
 {
   if (mAsyncCopierActive) {
     return NS_OK;
   }
 
   mAsyncCopierActive = true;
+
+  nsresult rv;
+
+  nsCOMPtr<nsIMultiplexInputStream> multiplexStream =
+    do_CreateInstance("@mozilla.org/io/multiplex-input-stream;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIInputStream> stream = do_QueryInterface(multiplexStream);
+
+  while (!mPendingData.IsEmpty()) {
+    nsCOMPtr<nsIInputStream> stream = mPendingData[0];
+    multiplexStream->AppendStream(stream);
+    mPendingData.RemoveElementAt(0);
+  }
+
+  nsCOMPtr<nsIAsyncStreamCopier> copier =
+    do_CreateInstance("@mozilla.org/network/async-stream-copier;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsISocketTransportService> sts =
+      do_GetService("@mozilla.org/network/socket-transport-service;1");
+
+  nsCOMPtr<nsIEventTarget> target = do_QueryInterface(sts);
+  rv = copier->Init(stream,
+                    mSocketOutputStream,
+                    target,
+                    true, /* source buffered */
+                    false, /* sink buffered */
+                    BUFFER_SIZE,
+                    false, /* close source */
+                    false); /* close sink */
+  NS_ENSURE_SUCCESS(rv, rv);
+
   RefPtr<CopierCallbacks> callbacks = new CopierCallbacks(this);
-  return mMultiplexStreamCopier->AsyncCopy(callbacks, nullptr);
+  rv = copier->AsyncCopy(callbacks, nullptr);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
 }
 
 void
 TCPSocket::NotifyCopyComplete(nsresult aStatus)
 {
   mAsyncCopierActive = false;
 
-  uint32_t countRemaining;
-  nsresult rvRemaining = mMultiplexStream->GetCount(&countRemaining);
-  NS_ENSURE_SUCCESS_VOID(rvRemaining);
-
-  while (countRemaining--) {
-    mMultiplexStream->RemoveStream(0);
+  // 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;
+    }
   }
-
-  while (!mPendingDataWhileCopierActive.IsEmpty()) {
-      nsCOMPtr<nsIInputStream> stream = mPendingDataWhileCopierActive[0];
-      mMultiplexStream->AppendStream(stream);
-      mPendingDataWhileCopierActive.RemoveElementAt(0);
-  }
+  mBufferedAmount = bufferedAmount;
 
   if (mSocketBridgeParent) {
     mozilla::Unused << mSocketBridgeParent->SendUpdateBufferedAmount(BufferedAmount(),
                                                                      mTrackingNumber);
   }
 
   if (NS_FAILED(aStatus)) {
     MaybeReportErrorAndCloseIfOpen(aStatus);
     return;
   }
 
-  uint32_t count;
-  nsresult rv = mMultiplexStream->GetCount(&count);
-  NS_ENSURE_SUCCESS_VOID(rv);
-
-  if (count) {
+  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
   // activate tls now.
   if (mWaitingForStartTLS && mReadyState == TCPReadyState::Open) {
     ActivateTLS();
     mWaitingForStartTLS = false;
     // If we have pending data, we should send them, or fire
     // a drain event if we are waiting for it.
     if (!mPendingDataAfterStartTLS.IsEmpty()) {
-      while (!mPendingDataAfterStartTLS.IsEmpty()) {
-        nsCOMPtr<nsIInputStream> stream = mPendingDataAfterStartTLS[0];
-        mMultiplexStream->AppendStream(stream);
-        mPendingDataAfterStartTLS.RemoveElementAt(0);
-      }
+      mPendingData.SwapElements(mPendingDataAfterStartTLS);
       EnsureCopying();
       return;
     }
   }
 
   // If we have a connected child, we let the child decide whether
   // ondrain should be dispatched.
   if (mWaitingForDrain && !mSocketBridgeParent) {
@@ -584,32 +586,16 @@ TCPSocket::Port()
 }
 
 bool
 TCPSocket::Ssl()
 {
   return mSsl;
 }
 
-uint64_t
-TCPSocket::BufferedAmount()
-{
-  if (mSocketBridgeChild) {
-    return mBufferedAmount;
-  }
-  if (mMultiplexStream) {
-    uint64_t available = 0;
-    nsCOMPtr<nsIInputStream> stream(do_QueryInterface(mMultiplexStream));
-    nsresult rv = stream->Available(&available);
-    NS_ENSURE_SUCCESS(rv, 0);
-    return available;
-  }
-  return 0;
-}
-
 void
 TCPSocket::Suspend()
 {
   if (mSocketBridgeChild) {
     mSocketBridgeChild->SendSuspend();
     return;
   }
   if (mInputStreamPump) {
@@ -776,21 +762,21 @@ TCPSocket::CloseHelper(bool waitForUnsen
 
   mReadyState = TCPReadyState::Closing;
 
   if (mSocketBridgeChild) {
     mSocketBridgeChild->SendClose();
     return;
   }
 
-  uint32_t count = 0;
-  if (mMultiplexStream) {
-    mMultiplexStream->GetCount(&count);
-  }
-  if (!count || !waitForUnsentData) {
+  if (!mAsyncCopierActive || !waitForUnsentData) {
+
+    mPendingData.Clear();
+    mPendingDataAfterStartTLS.Clear();
+
     if (mSocketOutputStream) {
       mSocketOutputStream->Close();
       mSocketOutputStream = nullptr;
     }
   }
 
   if (mSocketInputStream) {
     mSocketInputStream->Close();
@@ -908,21 +894,18 @@ TCPSocket::Send(nsIInputStream* aStream,
     mBufferedAmount = newBufferedAmount;
     return !bufferFull;
   }
 
   if (mWaitingForStartTLS) {
     // When we are waiting for starttls, newStream is added to pendingData
     // and will be appended to multiplexStream after tls had been set up.
     mPendingDataAfterStartTLS.AppendElement(aStream);
-  } else if (mAsyncCopierActive) {
-    // While the AsyncCopier is still active..
-    mPendingDataWhileCopierActive.AppendElement(aStream);
   } else {
-    mMultiplexStream->AppendStream(aStream);
+    mPendingData.AppendElement(aStream);
   }
 
   EnsureCopying();
 
   return !bufferFull;
 }
 
 TCPReadyState
@@ -1094,24 +1077,19 @@ TCPSocket::OnDataAvailable(nsIRequest* a
   FireDataEvent(cx, NS_LITERAL_STRING("data"), value);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 TCPSocket::OnStopRequest(nsIRequest* aRequest, nsISupports* aContext, nsresult aStatus)
 {
-  uint32_t count;
-  nsresult rv = mMultiplexStream->GetCount(&count);
-  NS_ENSURE_SUCCESS(rv, rv);
-  bool bufferedOutput = count != 0;
-
   mInputStreamPump = nullptr;
 
-  if (bufferedOutput && NS_SUCCEEDED(aStatus)) {
+  if (mAsyncCopierActive && NS_SUCCEEDED(aStatus)) {
     // If we have some buffered output still, and status is not an
     // error, the other side has done a half-close, but we don't
     // want to be in the close state until we are done sending
     // everything that was buffered. We also don't want to call onclose
     // yet.
     return NS_OK;
   }
 
--- a/dom/network/TCPSocket.h
+++ b/dom/network/TCPSocket.h
@@ -90,17 +90,17 @@ public:
 
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   static bool ShouldTCPSocketExist(JSContext* aCx, JSObject* aGlobal);
 
   void GetHost(nsAString& aHost);
   uint32_t Port();
   bool Ssl();
-  uint64_t BufferedAmount();
+  uint64_t BufferedAmount() const { return mBufferedAmount; }
   void Suspend();
   void Resume(ErrorResult& aRv);
   void Close();
   void CloseImmediately();
   bool Send(JSContext* aCx, const nsACString& aData, ErrorResult& aRv);
   bool Send(JSContext* aCx,
             const ArrayBuffer& aData,
             uint32_t aByteOffset,
@@ -202,20 +202,16 @@ private:
   nsCOMPtr<nsIInputStream> mSocketInputStream;
   nsCOMPtr<nsIOutputStream> mSocketOutputStream;
 
   // Input stream machinery
   nsCOMPtr<nsIInputStreamPump> mInputStreamPump;
   nsCOMPtr<nsIScriptableInputStream> mInputStreamScriptable;
   nsCOMPtr<nsIBinaryInputStream> mInputStreamBinary;
 
-  // Output stream machinery
-  nsCOMPtr<nsIMultiplexInputStream> mMultiplexStream;
-  nsCOMPtr<nsIAsyncStreamCopier> mMultiplexStreamCopier;
-
   // Is there an async copy operation in progress?
   bool mAsyncCopierActive;
   // True if the buffer is full and a "drain" event is expected by the client.
   bool mWaitingForDrain;
 
   // The id of the window that created this socket.
   uint64_t mInnerWindowID;
 
@@ -231,18 +227,18 @@ private:
   uint32_t mTrackingNumber;
 
   // True if this socket has been upgraded to secure after the initial connection,
   // but the actual upgrade is waiting for an in-progress copy operation to complete.
   bool mWaitingForStartTLS;
   // The buffered data awaiting the TLS upgrade to finish.
   nsTArray<nsCOMPtr<nsIInputStream>> mPendingDataAfterStartTLS;
 
-  // The data to be sent while AsyncCopier is still active.
-  nsTArray<nsCOMPtr<nsIInputStream>> mPendingDataWhileCopierActive;
+  // The data to be sent.
+  nsTArray<nsCOMPtr<nsIInputStream>> mPendingData;
 
   bool mObserversActive;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_TCPSocket_h