Bug 1818756 - Propagate stream error code from server to client, r=je…

…sup,necko-reviewers,saschanaz Differential Revision: https://phabricator.services.mozilla.com/D174237
iaxat · May 4, 2023 · e690ed2 · e690ed2
1 parent 2602613
commit e690ed2
Show file tree

Hide file tree

Showing 24 changed files with 435 additions and 151 deletions.
diff --git a/dom/webtransport/api/WebTransport.cpp b/dom/webtransport/api/WebTransport.cpp
@@ -41,23 +41,29 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(WebTransport)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIncomingBidirectionalStreams)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIncomingUnidirectionalAlgorithm)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIncomingBidirectionalAlgorithm)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSendStreams)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mReceiveStreams)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDatagrams)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mReady)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mClosed)
+  for (const auto& hashEntry : tmp->mSendStreams.Values()) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mSendStreams entry item");
+    cb.NoteXPCOMChild(hashEntry);
+  }
+  for (const auto& hashEntry : tmp->mReceiveStreams.Values()) {
+    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mReceiveStreams entry item");
+    cb.NoteXPCOMChild(hashEntry);
+  }
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(WebTransport)
+  tmp->mSendStreams.Clear();
+  tmp->mReceiveStreams.Clear();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mUnidirectionalStreams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mBidirectionalStreams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIncomingUnidirectionalStreams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIncomingBidirectionalStreams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIncomingUnidirectionalAlgorithm)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIncomingBidirectionalAlgorithm)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mSendStreams)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mReceiveStreams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mDatagrams)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mReady)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mClosed)
@@ -98,7 +104,7 @@ WebTransport::~WebTransport() {
 
 // From parent
 void WebTransport::NewBidirectionalStream(
-    const RefPtr<DataPipeReceiver>& aIncoming,
+    uint64_t aStreamId, const RefPtr<DataPipeReceiver>& aIncoming,
     const RefPtr<DataPipeSender>& aOutgoing) {
   LOG_VERBOSE(("NewBidirectionalStream()"));
   // Create a Bidirectional stream and push it into the
@@ -107,7 +113,9 @@ void WebTransport::NewBidirectionalStream(
 
   UniquePtr<BidirectionalPair> streams(
       new BidirectionalPair(aIncoming, aOutgoing));
-  mBidirectionalStreams.AppendElement(std::move(streams));
+  auto tuple = std::tuple<uint64_t, UniquePtr<BidirectionalPair>>(
+      aStreamId, std::move(streams));
+  mBidirectionalStreams.AppendElement(std::move(tuple));
   // We need to delete them all!
 
   // Notify something to wake up readers of IncomingReceiveStreams
@@ -122,13 +130,15 @@ void WebTransport::NewBidirectionalStream(
 }
 
 void WebTransport::NewUnidirectionalStream(
-    const RefPtr<mozilla::ipc::DataPipeReceiver>& aStream) {
+    uint64_t aStreamId, const RefPtr<mozilla::ipc::DataPipeReceiver>& aStream) {
   LOG_VERBOSE(("NewUnidirectionalStream()"));
   // Create a Unidirectional stream and push it into the
   // IncomingUnidirectionalStreams stream. Must be added to the ReceiveStreams
   // array
 
-  mUnidirectionalStreams.AppendElement(aStream);
+  mUnidirectionalStreams.AppendElement(
+      std::tuple<uint64_t, RefPtr<mozilla::ipc::DataPipeReceiver>>(aStreamId,
+                                                                   aStream));
   // Notify something to wake up readers of IncomingReceiveStreams
   // The callback is always set/used from the same thread (MainThread or a
   // Worker thread).
@@ -495,6 +505,56 @@ void WebTransport::RemoteClosed(bool aCleanly, const uint32_t& aCode,
   Cleanup(error, &closeinfo, errorresult);
 }
 
+template <typename Stream>
+void WebTransport::PropagateError(Stream* aStream, WebTransportError* aError) {
+  ErrorResult rv;
+  AutoJSAPI jsapi;
+  if (!jsapi.Init(mGlobal)) {
+    rv.ThrowUnknownError("Internal error");
+    return;
+  }
+  JSContext* cx = jsapi.cx();
+  JS::Rooted<JS::Value> errorValue(cx);
+  bool ok = ToJSValue(cx, aError, &errorValue);
+  if (!ok) {
+    rv.ThrowUnknownError("Internal error");
+    return;
+  }
+
+  aStream->ErrorNative(cx, errorValue, IgnoreErrors());
+}
+
+void WebTransport::OnStreamResetOrStopSending(
+    uint64_t aStreamId, const StreamResetOrStopSendingError& aError) {
+  LOG(("WebTransport::OnStreamResetOrStopSending %p id=%" PRIx64, this,
+       aStreamId));
+  if (aError.type() == StreamResetOrStopSendingError::TStopSendingError) {
+    RefPtr<WebTransportSendStream> stream = mSendStreams.Get(aStreamId);
+    if (!stream) {
+      return;
+    }
+    uint8_t errorCode = net::GetWebTransportErrorFromNSResult(
+        aError.get_StopSendingError().error());
+    RefPtr<WebTransportError> error = new WebTransportError(
+        "WebTransportStream StopSending"_ns, WebTransportErrorSource::Stream,
+        Nullable<uint8_t>(errorCode));
+    PropagateError(stream.get(), error);
+  } else if (aError.type() == StreamResetOrStopSendingError::TResetError) {
+    RefPtr<WebTransportReceiveStream> stream = mReceiveStreams.Get(aStreamId);
+    LOG(("WebTransport::OnStreamResetOrStopSending reset %p stream=%p", this,
+         stream.get()));
+    if (!stream) {
+      return;
+    }
+    uint8_t errorCode =
+        net::GetWebTransportErrorFromNSResult(aError.get_ResetError().error());
+    RefPtr<WebTransportError> error = new WebTransportError(
+        "WebTransportStream Reset"_ns, WebTransportErrorSource::Stream,
+        Nullable<uint8_t>(errorCode));
+    PropagateError(stream.get(), error);
+  }
+}
+
 void WebTransport::Close(const WebTransportCloseInfo& aOptions,
                          ErrorResult& aRv) {
   LOG(("Close() called"));
@@ -608,10 +668,12 @@ already_AddRefed<Promise> WebTransport::CreateBidirectionalStream(
               "Transport close/errored before CreateBidirectional finished");
           return;
         }
+        uint64_t id = aPipes.get_BidirectionalStream().streamId();
+        LOG(("Create WebTransportBidirectionalStream id=%" PRIx64, id));
         ErrorResult error;
         RefPtr<WebTransportBidirectionalStream> newStream =
             WebTransportBidirectionalStream::Create(
-                self, self->mGlobal,
+                self, self->mGlobal, id,
                 aPipes.get_BidirectionalStream().inStream(),
                 aPipes.get_BidirectionalStream().outStream(), error);
         LOG(("Returning a bidirectionalStream"));
@@ -658,8 +720,7 @@ already_AddRefed<Promise> WebTransport::CreateUnidirectionalStream(
   // Ask the parent to create the stream and send us the DataPipeSender
   mChild->SendCreateUnidirectionalStream(
       sendOrder,
-      [self = RefPtr{this},
-       promise](RefPtr<::mozilla::ipc::DataPipeSender>&& aPipe)
+      [self = RefPtr{this}, promise](UnidirectionalStreamResponse&& aResponse)
           MOZ_CAN_RUN_SCRIPT_BOUNDARY {
             LOG(("CreateUnidirectionalStream response"));
             // Step 5.1: Let internalStream be the result of creating an
@@ -669,7 +730,9 @@ already_AddRefed<Promise> WebTransport::CreateUnidirectionalStream(
             // Step 5.2.1 If transport.[[State]] is "closed" or "failed",
             // reject p with an InvalidStateError and abort these steps.
             if (self->mState == WebTransportState::CLOSED ||
-                self->mState == WebTransportState::FAILED) {
+                self->mState == WebTransportState::FAILED ||
+                aResponse.type() !=
+                    UnidirectionalStreamResponse::TUnidirectionalStream) {
               promise->MaybeRejectWithInvalidStateError(
                   "Transport close/errored during CreateUnidirectional");
               return;
@@ -679,16 +742,17 @@ already_AddRefed<Promise> WebTransport::CreateUnidirectionalStream(
             // WebTransportSendStream with internalStream, transport, and
             // sendOrder.
             ErrorResult error;
+            uint64_t id = aResponse.get_UnidirectionalStream().streamId();
+            LOG(("Create WebTransportSendStream id=%" PRIx64, id));
             RefPtr<WebTransportSendStream> writableStream =
-                WebTransportSendStream::Create(self, self->mGlobal, aPipe,
-                                               error);
+                WebTransportSendStream::Create(
+                    self, self->mGlobal, id,
+                    aResponse.get_UnidirectionalStream().outStream(), error);
             if (!writableStream) {
               promise->MaybeReject(std::move(error));
               return;
             }
             LOG(("Returning a writableStream"));
-            // https://w3c.github.io/webtransport/#send-stream-procedures step 7
-            self->mSendStreams.AppendElement(writableStream);
             // Step 5.2.3: Resolve p with stream.
             promise->MaybeResolve(writableStream);
           },
@@ -723,9 +787,9 @@ void WebTransport::Cleanup(WebTransportError* aError,
   // Step 7: Set transport.[[SendStreams]] to an empty set.
   // Step 8: Set transport.[[ReceiveStreams]] to an empty set.
   LOG(("Cleanup started"));
-  nsTArray<RefPtr<WebTransportSendStream>> sendStreams;
+  nsTHashMap<uint64_t, RefPtr<WebTransportSendStream>> sendStreams;
   sendStreams.SwapElements(mSendStreams);
-  nsTArray<RefPtr<WebTransportReceiveStream>> receiveStreams;
+  nsTHashMap<uint64_t, RefPtr<WebTransportReceiveStream>> receiveStreams;
   receiveStreams.SwapElements(mReceiveStreams);
 
   // Step 9: If closeInfo is given, then set transport.[[State]] to "closed".
@@ -746,13 +810,13 @@ void WebTransport::Cleanup(WebTransportError* aError,
     return;
   }
 
-  for (const auto& stream : sendStreams) {
+  for (const auto& stream : sendStreams.Values()) {
     // This MOZ_KnownLive is redundant, see bug 1620312
     MOZ_KnownLive(stream)->ErrorNative(cx, errorValue, IgnoreErrors());
   }
   // Step 11: For each receiveStream in receiveStreams, error receiveStream with
   // error.
-  for (const auto& stream : receiveStreams) {
+  for (const auto& stream : receiveStreams.Values()) {
     stream->ErrorNative(cx, errorValue, IgnoreErrors());
   }
   // Step 12:

diff --git a/dom/webtransport/api/WebTransport.h b/dom/webtransport/api/WebTransport.h
@@ -10,6 +10,7 @@
 #include "nsCOMPtr.h"
 #include "nsTArray.h"
 #include "nsISupports.h"
+#include "nsTHashMap.h"
 #include "nsWrapperCache.h"
 #include "nsPIDOMWindow.h"
 #include "mozilla/dom/Promise.h"
@@ -75,17 +76,22 @@ class WebTransport final : public nsISupports, public nsWrapperCache {
 
   // From Parent
   void NewBidirectionalStream(
+      uint64_t aStreamId,
       const RefPtr<mozilla::ipc::DataPipeReceiver>& aIncoming,
       const RefPtr<mozilla::ipc::DataPipeSender>& aOutgoing);
 
   void NewUnidirectionalStream(
+      uint64_t aStreamId,
       const RefPtr<mozilla::ipc::DataPipeReceiver>& aStream);
 
   void NewDatagramReceived(nsTArray<uint8_t>&& aData,
                            const mozilla::TimeStamp& aTimeStamp);
 
   void RemoteClosed(bool aCleanly, const uint32_t& aCode,
                     const nsACString& aReason);
+
+  void OnStreamResetOrStopSending(uint64_t aStreamId,
+                                  const StreamResetOrStopSendingError& aError);
   // WebIDL Boilerplate
   nsIGlobalObject* GetParentObject() const;
 
@@ -121,6 +127,10 @@ class WebTransport final : public nsISupports, public nsWrapperCache {
  private:
   ~WebTransport();
 
+  template <typename Stream>
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY void PropagateError(Stream* aStream,
+                                                  WebTransportError* aError);
+
   nsCOMPtr<nsIGlobalObject> mGlobal;
   // We are the owner of WebTransportChild.  We must call Shutdown() on it
   // before we're destroyed.
@@ -136,10 +146,8 @@ class WebTransport final : public nsISupports, public nsWrapperCache {
   // Order is visible due to
   // https://w3c.github.io/webtransport/#webtransport-procedures step 10: "For
   // each sendStream in sendStreams, error sendStream with error."
-  // XXX Use nsTArray.h for now, but switch to OrderHashSet/Table for release to
-  // improve remove performance (if needed)
-  nsTArray<RefPtr<WebTransportSendStream>> mSendStreams;
-  nsTArray<RefPtr<WebTransportReceiveStream>> mReceiveStreams;
+  nsTHashMap<uint64_t, RefPtr<WebTransportSendStream>> mSendStreams;
+  nsTHashMap<uint64_t, RefPtr<WebTransportReceiveStream>> mReceiveStreams;
 
   WebTransportState mState;
   RefPtr<Promise> mReady;
@@ -151,8 +159,10 @@ class WebTransport final : public nsISupports, public nsWrapperCache {
   // Incoming streams get queued here.  Use a TArray though it's working as
   // a FIFO - rarely will there be more than one entry in these arrays, so
   // the overhead of mozilla::Queue is unneeded
-  nsTArray<RefPtr<mozilla::ipc::DataPipeReceiver>> mUnidirectionalStreams;
-  nsTArray<UniquePtr<BidirectionalPair>> mBidirectionalStreams;
+  nsTArray<std::tuple<uint64_t, RefPtr<mozilla::ipc::DataPipeReceiver>>>
+      mUnidirectionalStreams;
+  nsTArray<std::tuple<uint64_t, UniquePtr<BidirectionalPair>>>
+      mBidirectionalStreams;
 
   // These are created in the constructor
   RefPtr<ReadableStream> mIncomingUnidirectionalStreams;

diff --git a/dom/webtransport/api/WebTransportBidirectionalStream.cpp b/dom/webtransport/api/WebTransportBidirectionalStream.cpp
@@ -34,23 +34,23 @@ JSObject* WebTransportBidirectionalStream::WrapObject(
 
 // static
 already_AddRefed<WebTransportBidirectionalStream>
-WebTransportBidirectionalStream::Create(WebTransport* aWebTransport,
-                                        nsIGlobalObject* aGlobal,
-                                        DataPipeReceiver* receiver,
-                                        DataPipeSender* sender,
-                                        ErrorResult& aRv) {
+WebTransportBidirectionalStream::Create(
+    WebTransport* aWebTransport, nsIGlobalObject* aGlobal, uint64_t aStreamId,
+    DataPipeReceiver* receiver, DataPipeSender* sender, ErrorResult& aRv) {
   // https://w3c.github.io/webtransport/#pullbidirectionalstream (and
   // createBidirectionalStream)
 
   // Step 7.1: Let stream be the result of creating a
   // WebTransportBidirectionalStream with internalStream and transport
   RefPtr<WebTransportReceiveStream> readableStream =
-      WebTransportReceiveStream::Create(aWebTransport, aGlobal, receiver, aRv);
+      WebTransportReceiveStream::Create(aWebTransport, aGlobal, aStreamId,
+                                        receiver, aRv);
   if (!readableStream) {
     return nullptr;
   }
   RefPtr<WebTransportSendStream> writableStream =
-      WebTransportSendStream::Create(aWebTransport, aGlobal, sender, aRv);
+      WebTransportSendStream::Create(aWebTransport, aGlobal, aStreamId, sender,
+                                     aRv);
   if (!writableStream) {
     return nullptr;
     ;

diff --git a/dom/webtransport/api/WebTransportBidirectionalStream.h b/dom/webtransport/api/WebTransportBidirectionalStream.h
@@ -33,7 +33,7 @@ class WebTransportBidirectionalStream final : public nsISupports,
   NS_DECL_CYCLE_COLLECTION_WRAPPERCACHE_CLASS(WebTransportBidirectionalStream)
 
   static already_AddRefed<WebTransportBidirectionalStream> Create(
-      WebTransport* aWebTransport, nsIGlobalObject* aGlobal,
+      WebTransport* aWebTransport, nsIGlobalObject* aGlobal, uint64_t aStreamId,
       ::mozilla::ipc::DataPipeReceiver* receiver,
       ::mozilla::ipc::DataPipeSender* sender, ErrorResult& aRv);
 

diff --git a/dom/webtransport/api/WebTransportReceiveStream.cpp b/dom/webtransport/api/WebTransportReceiveStream.cpp
@@ -40,7 +40,7 @@ JSObject* WebTransportReceiveStream::WrapObject(
 }
 
 already_AddRefed<WebTransportReceiveStream> WebTransportReceiveStream::Create(
-    WebTransport* aWebTransport, nsIGlobalObject* aGlobal,
+    WebTransport* aWebTransport, nsIGlobalObject* aGlobal, uint64_t aStreamId,
     DataPipeReceiver* receiver, ErrorResult& aRv) {
   // https://w3c.github.io/webtransport/#webtransportreceivestream-create
   AutoJSAPI jsapi;
@@ -60,7 +60,7 @@ already_AddRefed<WebTransportReceiveStream> WebTransportReceiveStream::Create(
     return nullptr;
   }
   // Add to ReceiveStreams
-  aWebTransport->mReceiveStreams.AppendElement(stream);
+  aWebTransport->mReceiveStreams.InsertOrUpdate(aStreamId, stream);
   return stream.forget();
 }
 

diff --git a/dom/webtransport/api/WebTransportReceiveStream.h b/dom/webtransport/api/WebTransportReceiveStream.h
@@ -27,7 +27,8 @@ class WebTransportReceiveStream final : public ReadableStream {
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY static already_AddRefed<WebTransportReceiveStream>
   Create(WebTransport* aWebTransport, nsIGlobalObject* aGlobal,
-         mozilla::ipc::DataPipeReceiver* receiver, ErrorResult& aRv);
+         uint64_t aStreamId, mozilla::ipc::DataPipeReceiver* receiver,
+         ErrorResult& aRv);
 
   // WebIDL Boilerplate
   JSObject* WrapObject(JSContext* aCx,

diff --git a/dom/webtransport/api/WebTransportSendStream.cpp b/dom/webtransport/api/WebTransportSendStream.cpp
@@ -39,7 +39,7 @@ JSObject* WebTransportSendStream::WrapObject(
 // NOTE: this does not yet implement SendOrder; see bug 1816925
 /* static */
 already_AddRefed<WebTransportSendStream> WebTransportSendStream::Create(
-    WebTransport* aWebTransport, nsIGlobalObject* aGlobal,
+    WebTransport* aWebTransport, nsIGlobalObject* aGlobal, uint64_t aStreamId,
     DataPipeSender* sender, ErrorResult& aRv) {
   // https://w3c.github.io/webtransport/#webtransportsendstream-create
   AutoJSAPI jsapi;
@@ -69,7 +69,7 @@ already_AddRefed<WebTransportSendStream> WebTransportSendStream::Create(
   // XXX TODO
 
   // Step 7: Append stream to SendStreams
-  aWebTransport->mSendStreams.AppendElement(stream);
+  aWebTransport->mSendStreams.InsertOrUpdate(aStreamId, stream);
   // Step 8: return stream
   return stream.forget();
 }

diff --git a/dom/webtransport/api/WebTransportSendStream.h b/dom/webtransport/api/WebTransportSendStream.h
@@ -27,7 +27,8 @@ class WebTransportSendStream final : public WritableStream {
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY static already_AddRefed<WebTransportSendStream>
   Create(WebTransport* aWebTransport, nsIGlobalObject* aGlobal,
-         mozilla::ipc::DataPipeSender* sender, ErrorResult& aRv);
+         uint64_t aStreamId, mozilla::ipc::DataPipeSender* sender,
+         ErrorResult& aRv);
 
   // WebIDL Boilerplate
   JSObject* WrapObject(JSContext* aCx,