Bug 1841702 - Add proper handling for failed worker refs; r=dom-storage-reviewers,jari

Differential Revision: https://phabricator.services.mozilla.com/D188431

dom/fs/api/FileSystemWritableFileStream.cpp

@@ -305,7 +305,8 @@ FileSystemWritableFileStream::Create(
     RefPtr<FileSystemManager>& aManager,
     RefPtr<FileSystemWritableFileStreamChild> aActor,
     mozilla::ipc::RandomAccessStreamParams&& aStreamParams,
-    fs::FileSystemEntryMetadata&& aMetadata) {
+    fs::FileSystemEntryMetadata&& aMetadata,
+    RefPtr<StrongWorkerRef> aBuildWorkerRef) {
   using StreamPromise = MozPromise<NotNull<nsCOMPtr<nsIRandomAccessStream>>,
                                    nsresult, /* IsExclusive */ true>;
 
@@ -323,68 +324,80 @@ FileSystemWritableFileStream::Create(
       TaskQueue::Create(streamTransportService.forget(), "WritableStreamQueue");
   MOZ_ASSERT(taskQueue);
 
-  auto streamSetup =
-      [aGlobal, aManager, actor = std::move(aActor), taskQueue,
-       metadata =
-           std::move(aMetadata)](StreamPromise::ResolveOrRejectValue&& aValue)
-          MOZ_CAN_RUN_SCRIPT mutable {
-            auto rejectAndReturn = [](const nsresult aRv) {
-              return CreatePromise::CreateAndReject(aRv, __func__);
-            };
+  auto streamSetup = [aGlobal, aManager, actor = std::move(aActor), taskQueue,
+                      metadata = std::move(aMetadata),
+                      buildWorkerRef = std::move(aBuildWorkerRef)](
+                         StreamPromise::ResolveOrRejectValue&&
+                             aValue) MOZ_CAN_RUN_SCRIPT mutable {
+    auto rejectAndReturn = [](const nsresult aRv) {
+      return CreatePromise::CreateAndReject(aRv, __func__);
+    };
 
-            if (!aValue.IsResolve()) {
-              MOZ_ASSERT(aValue.IsReject());
-              return rejectAndReturn(aValue.RejectValue());
-            }
+    if (!aValue.IsResolve()) {
+      MOZ_ASSERT(aValue.IsReject());
+      return rejectAndReturn(aValue.RejectValue());
+    }
 
-            AutoJSAPI jsapi;
-            if (!jsapi.Init(aGlobal)) {
-              return rejectAndReturn(NS_ERROR_FAILURE);
-            }
-            JSContext* cx = jsapi.cx();
+    AutoJSAPI jsapi;
+    if (!jsapi.Init(aGlobal)) {
+      return rejectAndReturn(NS_ERROR_FAILURE);
+    }
+    JSContext* cx = jsapi.cx();
 
-            // Step 5. Perform ! InitializeWritableStream(stream).
-            // (Done by the constructor)
-            nsCOMPtr<nsIRandomAccessStream> inputOutputStream =
-                aValue.ResolveValue();
-            RefPtr<FileSystemWritableFileStream> stream =
-                new FileSystemWritableFileStream(
-                    aGlobal, aManager, std::move(actor), taskQueue.forget(),
-                    /* random access stream */ inputOutputStream,
-                    std::move(metadata));
+    // Step 5. Perform ! InitializeWritableStream(stream).
+    // (Done by the constructor)
+    nsCOMPtr<nsIRandomAccessStream> inputOutputStream = aValue.ResolveValue();
+    RefPtr<FileSystemWritableFileStream> stream =
+        new FileSystemWritableFileStream(
+            aGlobal, aManager, std::move(actor), taskQueue.forget(),
+            /* random access stream */ inputOutputStream, std::move(metadata));
 
-            auto autoClose = MakeScopeExit([stream] {
-              stream->mCloseHandler->Close();
-              stream->mActor->SendClose(/* aAbort */ true);
-            });
+    auto autoClose = MakeScopeExit([stream] {
+      stream->mCloseHandler->Close();
+      stream->mActor->SendClose(/* aAbort */ true);
+    });
 
-            // Step 3 - 5
-            auto algorithms =
-                MakeRefPtr<WritableFileStreamUnderlyingSinkAlgorithms>(*stream);
+    RefPtr<StrongWorkerRef> workerRef;
+    if (buildWorkerRef) {
+      workerRef =
+          StrongWorkerRef::Create(buildWorkerRef->Private(),
+                                  "FileSystemWritableFileStream", [stream]() {
+                                    if (stream->IsOpen()) {
+                                      // We don't need the promise, we just
+                                      // begin the closing process.
+                                      Unused << stream->BeginAbort();
+                                    }
+                                  });
+      QM_TRY(MOZ_TO_RESULT(workerRef), rejectAndReturn);
+    }
 
-            // Step 8: Set up stream with writeAlgorithm set to writeAlgorithm,
-            // closeAlgorithm set to closeAlgorithm, abortAlgorithm set to
-            // abortAlgorithm, highWaterMark set to highWaterMark, and
-            // sizeAlgorithm set to sizeAlgorithm.
-            IgnoredErrorResult rv;
-            stream->SetUpNative(cx, *algorithms,
-                                // Step 6. Let highWaterMark be 1.
-                                Some(1),
-                                // Step 7. Let sizeAlgorithm be an algorithm
-                                // that returns 1. (nullptr returns 1, See
-                                // WritableStream::Constructor for details)
-                                nullptr, rv);
-            if (rv.Failed()) {
-              return CreatePromise::CreateAndReject(rv.StealNSResult(),
-                                                    __func__);
-            }
+    // Step 3 - 5
+    auto algorithms =
+        MakeRefPtr<WritableFileStreamUnderlyingSinkAlgorithms>(*stream);
 
-            autoClose.release();
-            stream->mCloseHandler->Open();
+    // Step 8: Set up stream with writeAlgorithm set to writeAlgorithm,
+    // closeAlgorithm set to closeAlgorithm, abortAlgorithm set to
+    // abortAlgorithm, highWaterMark set to highWaterMark, and
+    // sizeAlgorithm set to sizeAlgorithm.
+    IgnoredErrorResult rv;
+    stream->SetUpNative(cx, *algorithms,
+                        // Step 6. Let highWaterMark be 1.
+                        Some(1),
+                        // Step 7. Let sizeAlgorithm be an algorithm
+                        // that returns 1. (nullptr returns 1, See
+                        // WritableStream::Constructor for details)
+                        nullptr, rv);
+    if (rv.Failed()) {
+      return CreatePromise::CreateAndReject(rv.StealNSResult(), __func__);
+    }
 
-            // Step 9: Return stream.
-            return CreatePromise::CreateAndResolve(stream.forget(), __func__);
-          };
+    autoClose.release();
+    stream->mWorkerRef = std::move(workerRef);
+    stream->mCloseHandler->Open();
+
+    // Step 9: Return stream.
+    return CreatePromise::CreateAndResolve(stream.forget(), __func__);
+  };
 
   return InvokeAsync(taskQueue, __func__,
                      [streamParams = std::move(aStreamParams)]() mutable {
@@ -527,11 +540,6 @@ RefPtr<BoolPromise> FileSystemWritableFileStream::OnDone() {
   return mCloseHandler->GetClosePromise();
 }
 
-void FileSystemWritableFileStream::SetWorkerRef(
-    RefPtr<StrongWorkerRef>&& aWorkerRef) {
-  mWorkerRef = std::move(aWorkerRef);
-}
-
 already_AddRefed<Promise> FileSystemWritableFileStream::Write(
     JSContext* aCx, JS::Handle<JS::Value> aChunk, ErrorResult& aError) {
   // https://fs.spec.whatwg.org/#create-a-new-filesystemwritablefilestream

dom/fs/api/FileSystemWritableFileStream.h

@@ -57,7 +57,8 @@ class FileSystemWritableFileStream final : public WritableStream {
       RefPtr<FileSystemManager>& aManager,
       RefPtr<FileSystemWritableFileStreamChild> aActor,
       mozilla::ipc::RandomAccessStreamParams&& aStreamParams,
-      fs::FileSystemEntryMetadata&& aMetadata);
+      fs::FileSystemEntryMetadata&& aMetadata,
+      RefPtr<StrongWorkerRef> aBuildWorkerRef);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(FileSystemWritableFileStream,
@@ -84,8 +85,6 @@ class FileSystemWritableFileStream final : public WritableStream {
 
   [[nodiscard]] RefPtr<BoolPromise> OnDone();
 
-  void SetWorkerRef(RefPtr<StrongWorkerRef>&& aWorkerRef);
-
   already_AddRefed<Promise> Write(JSContext* aCx, JS::Handle<JS::Value> aChunk,
                                   ErrorResult& aError);
 

dom/fs/child/FileSystemRequestHandler.cpp

@@ -287,30 +287,15 @@ void ResolveCallback(FileSystemGetWritableFileStreamResponse&& aResponse,
 
   autoDelete.release();
 
-  FileSystemWritableFileStream::Create(aPromise->GetParentObject(), aManager,
-                                       actor, std::move(params),
-                                       std::move(metadata))
+  FileSystemWritableFileStream::Create(
+      aPromise->GetParentObject(), aManager, actor, std::move(params),
+      std::move(metadata), std::move(buildWorkerRef))
       ->Then(GetCurrentSerialEventTarget(), __func__,
-             [buildWorkerRef,
-              aPromise](CreatePromise::ResolveOrRejectValue&& aValue) {
+             [aPromise](CreatePromise::ResolveOrRejectValue&& aValue) {
                if (aValue.IsResolve()) {
                  RefPtr<FileSystemWritableFileStream> stream =
                      aValue.ResolveValue();
 
-                 if (buildWorkerRef) {
-                   RefPtr<StrongWorkerRef> workerRef = StrongWorkerRef::Create(
-                       buildWorkerRef->Private(),
-                       "FileSystemWritableFileStream", [stream]() {
-                         if (stream->IsOpen()) {
-                           // We don't need the promise, we just begin the
-                           // closing process.
-                           Unused << stream->BeginAbort();
-                         }
-                       });
-
-                   stream->SetWorkerRef(std::move(workerRef));
-                 }
-
                  aPromise->MaybeResolve(stream);
                  return;
                }

dom/fs/test/crashtests/1841702.html

@@ -5,6 +5,7 @@ self.onmessage = async function(e) {
     await a.createWritable({})
 }
 </script>
+
 <script>
 window.addEventListener("load", async () => {
     let a = await self.clientInformation.storage.getDirectory()