Bug 1507893 - Fetch() should consume DOM Files on the target thread only, r=twisniewski
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 20 Nov 2018 18:08:34 +0100
changeset 503764 d544aa6b069bd3d8bbf6549d43127d86b5afa693
parent 503763 da6b96f7f1210aa1e6d5aefe6a7b4db8fdd644fa
child 503765 637291128b2e0f395d52b5c7565681e00d115976
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstwisniewski
bugs1507893
milestone65.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 1507893 - Fetch() should consume DOM Files on the target thread only, r=twisniewski
dom/fetch/FetchConsumer.cpp
dom/fetch/FetchConsumer.h
--- a/dom/fetch/FetchConsumer.cpp
+++ b/dom/fetch/FetchConsumer.cpp
@@ -471,63 +471,68 @@ namespace {
 
 template <class Derived>
 class FileCreationHandler final : public PromiseNativeHandler
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
 
   static void
-  Create(Promise* aPromise, FetchBodyConsumer<Derived>* aConsumer)
+  Create(Promise* aPromise, FetchBodyConsumer<Derived>* aConsumer,
+         ThreadSafeWorkerRef* aWorkerRef)
   {
     AssertIsOnMainThread();
     MOZ_ASSERT(aPromise);
 
-    RefPtr<FileCreationHandler> handler = new FileCreationHandler<Derived>(aConsumer);
+    RefPtr<FileCreationHandler> handler =
+      new FileCreationHandler<Derived>(aConsumer, aWorkerRef);
     aPromise->AppendNativeHandler(handler);
   }
 
   void
   ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
   {
     AssertIsOnMainThread();
 
     if (NS_WARN_IF(!aValue.isObject())) {
-      mConsumer->OnBlobResult(nullptr);
+      mConsumer->OnBlobResult(nullptr, mWorkerRef);
       return;
     }
 
     RefPtr<Blob> blob;
     if (NS_WARN_IF(NS_FAILED(UNWRAP_OBJECT(Blob, &aValue.toObject(), blob)))) {
-      mConsumer->OnBlobResult(nullptr);
+      mConsumer->OnBlobResult(nullptr, mWorkerRef);
       return;
     }
 
-    mConsumer->OnBlobResult(blob);
+    mConsumer->OnBlobResult(blob, mWorkerRef);
   }
 
   void
   RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
   {
     AssertIsOnMainThread();
 
-    mConsumer->OnBlobResult(nullptr);
+    mConsumer->OnBlobResult(nullptr, mWorkerRef);
   }
 
 private:
-  explicit FileCreationHandler<Derived>(FetchBodyConsumer<Derived>* aConsumer)
+  FileCreationHandler<Derived>(FetchBodyConsumer<Derived>* aConsumer,
+                               ThreadSafeWorkerRef* aWorkerRef)
     : mConsumer(aConsumer)
+    , mWorkerRef(aWorkerRef)
   {
     AssertIsOnMainThread();
     MOZ_ASSERT(aConsumer);
   }
 
   ~FileCreationHandler() = default;
 
   RefPtr<FetchBodyConsumer<Derived>> mConsumer;
+  RefPtr<ThreadSafeWorkerRef> mWorkerRef;
 };
 
 template <class Derived>
 NS_IMPL_ADDREF(FileCreationHandler<Derived>)
 template <class Derived>
 NS_IMPL_RELEASE(FileCreationHandler<Derived>)
 template <class Derived>
 NS_INTERFACE_MAP_BEGIN(FileCreationHandler<Derived>)
@@ -599,17 +604,17 @@ FetchBodyConsumer<Derived>::BeginConsume
     // then just consume that URI's blob instance.
     if (!mBodyBlobURISpec.IsEmpty()) {
       RefPtr<BlobImpl> blobImpl;
       rv = NS_GetBlobForBlobURISpec(mBodyBlobURISpec, getter_AddRefs(blobImpl));
       if (NS_WARN_IF(NS_FAILED(rv)) || !blobImpl) {
         return;
       }
       autoReject.DontFail();
-      ContinueConsumeBlobBody(blobImpl);
+      DispatchContinueConsumeBlobBody(blobImpl, aWorkerRef);
       return;
     }
 
     // If we're trying to consume a blob, and the request was for a local
     // file, then generate and return a File blob.
     nsCOMPtr<nsIFile> file;
     rv = GetBodyLocalFile(getter_AddRefs(file));
     if (!NS_WARN_IF(NS_FAILED(rv)) && file) {
@@ -618,18 +623,18 @@ FetchBodyConsumer<Derived>::BeginConsume
 
       ErrorResult error;
       RefPtr<Promise> promise =
         FileCreatorHelper::CreateFile(mGlobal, file, bag, true, error);
       if (NS_WARN_IF(error.Failed())) {
         return;
       }
 
-      FileCreationHandler<Derived>::Create(promise, this);
       autoReject.DontFail();
+      FileCreationHandler<Derived>::Create(promise, this, aWorkerRef);
       return;
     }
   }
 
   nsCOMPtr<nsIInputStreamPump> pump;
   nsresult rv = NS_NewInputStreamPump(getter_AddRefs(pump),
                                       mBodyStream.forget(), 0, 0, false,
                                       mMainThreadEventTarget);
@@ -683,29 +688,52 @@ FetchBodyConsumer<Derived>::BeginConsume
  * network transfer completes in BeginConsumeBodyRunnable or its local File has
  * been wrapped by FileCreationHandler). The blob is sent to the target thread
  * and ContinueConsumeBody is called.
  */
 template <class Derived>
 void
 FetchBodyConsumer<Derived>::OnBlobResult(Blob* aBlob, ThreadSafeWorkerRef* aWorkerRef)
 {
-  MOZ_ASSERT(aBlob);
+  AssertIsOnMainThread();
+
+  DispatchContinueConsumeBlobBody(aBlob ? aBlob->Impl() : nullptr, aWorkerRef);
+}
+
+template <class Derived>
+void
+FetchBodyConsumer<Derived>::DispatchContinueConsumeBlobBody(BlobImpl* aBlobImpl,
+                                                            ThreadSafeWorkerRef* aWorkerRef)
+{
+  AssertIsOnMainThread();
 
   // Main-thread.
   if (!aWorkerRef) {
-    ContinueConsumeBlobBody(aBlob->Impl());
+    if (aBlobImpl) {
+      ContinueConsumeBlobBody(aBlobImpl);
+    } else {
+      ContinueConsumeBody(NS_ERROR_DOM_ABORT_ERR, 0, nullptr);
+    }
     return;
   }
 
   // Web Worker.
-  {
+  if (aBlobImpl) {
     RefPtr<ContinueConsumeBlobBodyRunnable<Derived>> r =
       new ContinueConsumeBlobBodyRunnable<Derived>(this, aWorkerRef->Private(),
-                                                   aBlob->Impl());
+                                                   aBlobImpl);
+
+    if (r->Dispatch()) {
+      return;
+    }
+  } else {
+    RefPtr<ContinueConsumeBodyRunnable<Derived>> r =
+      new ContinueConsumeBodyRunnable<Derived>(this, aWorkerRef->Private(),
+                                               NS_ERROR_DOM_ABORT_ERR, 0,
+                                               nullptr);
 
     if (r->Dispatch()) {
       return;
     }
   }
 
   // The worker is shutting down. Let's use a control runnable to complete the
   // shutting down procedure.
--- a/dom/fetch/FetchConsumer.h
+++ b/dom/fetch/FetchConsumer.h
@@ -56,16 +56,20 @@ public:
   void
   ContinueConsumeBody(nsresult aStatus, uint32_t aLength, uint8_t* aResult,
                       bool aShuttingDown = false);
 
   void
   ContinueConsumeBlobBody(BlobImpl* aBlobImpl, bool aShuttingDown = false);
 
   void
+  DispatchContinueConsumeBlobBody(BlobImpl* aBlobImpl,
+                                  ThreadSafeWorkerRef* aWorkerRef);
+
+  void
   ShutDownMainThreadConsuming();
 
   void
   NullifyConsumeBodyPump()
   {
     mShuttingDown = true;
     mConsumeBodyPump = nullptr;
   }