Bug 1080867 - Tighten up threadsafety guarantees surrounding remote blobs. r=khuey, a=lsblakk
authorBen Turner <bent.mozilla@gmail.com>
Mon, 13 Oct 2014 09:55:52 -0700
changeset 223559 de32c9a4888da1b596d7b59d380d53294f2954d6
parent 223558 935f2c8109fb742ac30b9b1fe6ba109246e7c714
child 223560 9936bcca134f807e9295c6b1db0d0497e3525fec
push id5
push usergszorc@mozilla.com
push dateWed, 29 Oct 2014 02:51:31 +0000
reviewerskhuey, lsblakk
bugs1080867
milestone35.0a2
Bug 1080867 - Tighten up threadsafety guarantees surrounding remote blobs. r=khuey, a=lsblakk
dom/indexedDB/test/mochitest.ini
dom/ipc/Blob.cpp
--- a/dom/indexedDB/test/mochitest.ini
+++ b/dom/indexedDB/test/mochitest.ini
@@ -111,17 +111,17 @@ skip-if = (buildapp == 'b2g' && toolkit 
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_blob_archive.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_blob_simple.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_blob_worker_crash.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_blob_worker_xhr_post.html]
-skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
+skip-if = ((buildapp == 'b2g' && toolkit != 'gonk') || (e10s && toolkit == 'windows')) # Bug 931116
 [test_blocked_order.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_bug937006.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_clear.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_complex_keyPaths.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
--- a/dom/ipc/Blob.cpp
+++ b/dom/ipc/Blob.cpp
@@ -1092,16 +1092,36 @@ public:
   NoteDyingActor()
   {
     MOZ_ASSERT(mActor);
     mActor->AssertIsOnOwningThread();
 
     mActor = nullptr;
   }
 
+  BlobChild*
+  GetActor() const
+  {
+    AssertActorEventTargetIsOnCurrentThread();
+
+    return mActor;
+  }
+
+  nsIEventTarget*
+  GetActorEventTarget() const
+  {
+    return mActorTarget;
+  }
+
+  void
+  AssertActorEventTargetIsOnCurrentThread() const
+  {
+    MOZ_ASSERT(EventTargetIsOnCurrentThread(mActorTarget));
+  }
+
   NS_DECL_ISUPPORTS_INHERITED
 
   virtual void
   GetMozFullPathInternal(nsAString& aFileName, ErrorResult& aRv) MOZ_OVERRIDE;
 
   virtual already_AddRefed<FileImpl>
   CreateSlice(uint64_t aStart, uint64_t aLength,
               const nsAString& aContentType, ErrorResult& aRv) MOZ_OVERRIDE;
@@ -1163,46 +1183,43 @@ private:
     }
   }
 };
 
 class BlobChild::RemoteBlobImpl::StreamHelper MOZ_FINAL
   : public nsRunnable
 {
   Monitor mMonitor;
-  BlobChild* mActor;
-  nsRefPtr<FileImpl> mBlobImpl;
+  nsRefPtr<RemoteBlobImpl> mRemoteBlobImpl;
   nsRefPtr<RemoteInputStream> mInputStream;
   bool mDone;
 
 public:
-  StreamHelper(BlobChild* aActor, FileImpl* aBlobImpl)
+  StreamHelper(RemoteBlobImpl* aRemoteBlobImpl)
     : mMonitor("BlobChild::RemoteBlobImpl::StreamHelper::mMonitor")
-    , mActor(aActor)
-    , mBlobImpl(aBlobImpl)
+    , mRemoteBlobImpl(aRemoteBlobImpl)
     , mDone(false)
   {
     // This may be created on any thread.
-    MOZ_ASSERT(aActor);
-    MOZ_ASSERT(aBlobImpl);
+    MOZ_ASSERT(aRemoteBlobImpl);
   }
 
   nsresult
   GetStream(nsIInputStream** aInputStream)
   {
     // This may be called on any thread.
     MOZ_ASSERT(aInputStream);
-    MOZ_ASSERT(mActor);
+    MOZ_ASSERT(mRemoteBlobImpl);
     MOZ_ASSERT(!mInputStream);
     MOZ_ASSERT(!mDone);
 
-    if (mActor->IsOnOwningThread()) {
+    if (EventTargetIsOnCurrentThread(mRemoteBlobImpl->GetActorEventTarget())) {
       RunInternal(false);
     } else {
-      nsCOMPtr<nsIEventTarget> target = mActor->EventTarget();
+      nsCOMPtr<nsIEventTarget> target = mRemoteBlobImpl->GetActorEventTarget();
       if (!target) {
         target = do_GetMainThread();
       }
 
       MOZ_ASSERT(target);
 
       nsresult rv = target->Dispatch(this, NS_DISPATCH_NORMAL);
       NS_ENSURE_SUCCESS(rv, rv);
@@ -1210,108 +1227,109 @@ public:
       {
         MonitorAutoLock lock(mMonitor);
         while (!mDone) {
           lock.Wait();
         }
       }
     }
 
-    MOZ_ASSERT(!mActor);
+    MOZ_ASSERT(!mRemoteBlobImpl);
     MOZ_ASSERT(mDone);
 
     if (!mInputStream) {
       return NS_ERROR_UNEXPECTED;
     }
 
     mInputStream.forget(aInputStream);
     return NS_OK;
   }
 
   NS_IMETHOD
   Run() MOZ_OVERRIDE
   {
-    MOZ_ASSERT(mActor);
-    mActor->AssertIsOnOwningThread();
+    MOZ_ASSERT(mRemoteBlobImpl);
+    mRemoteBlobImpl->AssertActorEventTargetIsOnCurrentThread();
 
     RunInternal(true);
     return NS_OK;
   }
 
 private:
   void
   RunInternal(bool aNotify)
   {
-    MOZ_ASSERT(mActor);
-    mActor->AssertIsOnOwningThread();
+    MOZ_ASSERT(mRemoteBlobImpl);
+    mRemoteBlobImpl->AssertActorEventTargetIsOnCurrentThread();
     MOZ_ASSERT(!mInputStream);
     MOZ_ASSERT(!mDone);
 
-    nsRefPtr<RemoteInputStream> stream = new RemoteInputStream(mBlobImpl);
-
-    InputStreamChild* streamActor = new InputStreamChild(stream);
-    if (mActor->SendPBlobStreamConstructor(streamActor)) {
-      stream.swap(mInputStream);
+    if (BlobChild* actor = mRemoteBlobImpl->GetActor()) {
+      nsRefPtr<RemoteInputStream> stream =
+        new RemoteInputStream(mRemoteBlobImpl);
+
+      InputStreamChild* streamActor = new InputStreamChild(stream);
+      if (actor->SendPBlobStreamConstructor(streamActor)) {
+        stream.swap(mInputStream);
+      }
     }
 
-    mActor = nullptr;
+    mRemoteBlobImpl = nullptr;
 
     if (aNotify) {
       MonitorAutoLock lock(mMonitor);
       mDone = true;
       lock.Notify();
     }
     else {
       mDone = true;
     }
   }
 };
 
 class BlobChild::RemoteBlobImpl::SliceHelper MOZ_FINAL
   : public nsRunnable
 {
   Monitor mMonitor;
-  BlobChild* mActor;
+  nsRefPtr<RemoteBlobImpl> mRemoteBlobImpl;
   nsRefPtr<FileImpl> mSlice;
   uint64_t mStart;
   uint64_t mLength;
   nsString mContentType;
   bool mDone;
 
 public:
   explicit
-  SliceHelper(BlobChild* aActor)
+  SliceHelper(RemoteBlobImpl* aRemoteBlobImpl)
     : mMonitor("BlobChild::RemoteBlobImpl::SliceHelper::mMonitor")
-    , mActor(aActor)
+    , mRemoteBlobImpl(aRemoteBlobImpl)
     , mStart(0)
     , mLength(0)
     , mDone(false)
   {
     // This may be created on any thread.
-    MOZ_ASSERT(aActor);
+    MOZ_ASSERT(aRemoteBlobImpl);
   }
 
-  FileImpl*
-  GetSlice(uint64_t aStart,
-           uint64_t aLength,
-           const nsAString& aContentType)
+  already_AddRefed<FileImpl>
+  GetSlice(uint64_t aStart, uint64_t aLength, const nsAString& aContentType)
   {
     // This may be called on any thread.
-    MOZ_ASSERT(mActor);
+    MOZ_ASSERT(mRemoteBlobImpl);
     MOZ_ASSERT(!mSlice);
     MOZ_ASSERT(!mDone);
 
     mStart = aStart;
     mLength = aLength;
     mContentType = aContentType;
 
-    if (mActor->IsOnOwningThread()) {
+    if (EventTargetIsOnCurrentThread(mRemoteBlobImpl->GetActorEventTarget())) {
       RunInternal(false);
     } else {
-      nsCOMPtr<nsIEventTarget> target = mActor->EventTarget();
+      nsCOMPtr<nsIEventTarget> target = mRemoteBlobImpl->GetActorEventTarget();
       if (!target) {
         target = do_GetMainThread();
       }
 
       MOZ_ASSERT(target);
 
       nsresult rv = target->Dispatch(this, NS_DISPATCH_NORMAL);
       if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -1321,78 +1339,82 @@ public:
       {
         MonitorAutoLock lock(mMonitor);
         while (!mDone) {
           lock.Wait();
         }
       }
     }
 
-    MOZ_ASSERT(!mActor);
+    MOZ_ASSERT(!mRemoteBlobImpl);
     MOZ_ASSERT(mDone);
 
     if (NS_WARN_IF(!mSlice)) {
       return nullptr;
     }
 
-    return mSlice;
+    return mSlice.forget();
   }
 
   NS_IMETHOD
   Run() MOZ_OVERRIDE
   {
-    MOZ_ASSERT(mActor);
-    mActor->AssertIsOnOwningThread();
+    MOZ_ASSERT(mRemoteBlobImpl);
+    mRemoteBlobImpl->AssertActorEventTargetIsOnCurrentThread();
 
     RunInternal(true);
     return NS_OK;
   }
 
 private:
   void
   RunInternal(bool aNotify)
   {
-    MOZ_ASSERT(mActor);
-    mActor->AssertIsOnOwningThread();
+    MOZ_ASSERT(mRemoteBlobImpl);
+    mRemoteBlobImpl->AssertActorEventTargetIsOnCurrentThread();
     MOZ_ASSERT(!mSlice);
     MOZ_ASSERT(!mDone);
 
-    NS_ENSURE_TRUE_VOID(mActor->HasManager());
-
-    nsID id;
-    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(gUUIDGenerator->GenerateUUIDInPlace(&id)));
-
-    ChildBlobConstructorParams params(
-      id,
-      NormalBlobConstructorParams(mContentType /* contentType */,
-                                  mLength /* length */));
-
-    ParentBlobConstructorParams otherSideParams(
-      SlicedBlobConstructorParams(nullptr /* sourceParent */,
-                                  mActor /* sourceChild */,
-                                  id /* optionalID */,
-                                  mStart /* begin */,
-                                  mStart + mLength /* end */,
-                                  mContentType /* contentType */),
-      void_t() /* optionalInputStream */);
-
-    BlobChild* newActor;
-    if (nsIContentChild* contentManager = mActor->GetContentManager()) {
-      newActor = SendSliceConstructor(contentManager, params, otherSideParams);
-    } else {
-      newActor = SendSliceConstructor(mActor->GetBackgroundManager(),
-                                      params,
-                                      otherSideParams);
+    if (BlobChild* actor = mRemoteBlobImpl->GetActor()) {
+      MOZ_ASSERT(actor->HasManager());
+
+      nsID id;
+      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(gUUIDGenerator->GenerateUUIDInPlace(&id)));
+
+      ChildBlobConstructorParams params(
+        id,
+        NormalBlobConstructorParams(mContentType /* contentType */,
+                                    mLength /* length */));
+
+      ParentBlobConstructorParams otherSideParams(
+        SlicedBlobConstructorParams(nullptr /* sourceParent */,
+                                    actor /* sourceChild */,
+                                    id /* optionalID */,
+                                    mStart /* begin */,
+                                    mStart + mLength /* end */,
+                                    mContentType /* contentType */),
+        void_t() /* optionalInputStream */);
+
+      BlobChild* newActor;
+      if (nsIContentChild* contentManager = actor->GetContentManager()) {
+        newActor = SendSliceConstructor(contentManager,
+                                        params,
+                                        otherSideParams);
+      } else {
+        newActor = SendSliceConstructor(actor->GetBackgroundManager(),
+                                        params,
+                                        otherSideParams);
+      }
+
+      if (newActor) {
+        mSlice = newActor->GetBlobImpl();
+      }
     }
 
-    if (newActor) {
-      mSlice = newActor->GetBlobImpl();
-    }
-
-    mActor = nullptr;
+    mRemoteBlobImpl = nullptr;
 
     if (aNotify) {
       MonitorAutoLock lock(mMonitor);
       mDone = true;
       lock.Notify();
     }
     else {
       mDone = true;
@@ -1410,16 +1432,20 @@ NS_IMPL_QUERY_INTERFACE_INHERITED(BlobCh
                                   FileImpl,
                                   nsIRemoteBlob)
 
 void
 BlobChild::
 RemoteBlobImpl::GetMozFullPathInternal(nsAString& aFilePath,
                                        ErrorResult& aRv)
 {
+  if (!EventTargetIsOnCurrentThread(mActorTarget)) {
+    MOZ_CRASH("Not implemented!");
+  }
+
   if (!mActor) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
   nsString filePath;
   if (!mActor->SendGetFilePath(&filePath)) {
     aRv.Throw(NS_ERROR_FAILURE);
@@ -1429,48 +1455,43 @@ RemoteBlobImpl::GetMozFullPathInternal(n
   aFilePath = filePath;
 }
 
 already_AddRefed<FileImpl>
 BlobChild::
 RemoteBlobImpl::CreateSlice(uint64_t aStart, uint64_t aLength,
                             const nsAString& aContentType, ErrorResult& aRv)
 {
-  if (!mActor) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
-  nsRefPtr<SliceHelper> helper = new SliceHelper(mActor);
+  nsRefPtr<SliceHelper> helper = new SliceHelper(this);
 
   nsRefPtr<FileImpl> impl = helper->GetSlice(aStart, aLength, aContentType);
   if (NS_WARN_IF(!impl)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   return impl.forget();
 }
 
 nsresult
 BlobChild::
 RemoteBlobImpl::GetInternalStream(nsIInputStream** aStream)
 {
-  if (!mActor) {
-    return NS_ERROR_UNEXPECTED;
-  }
-
-  nsRefPtr<StreamHelper> helper = new StreamHelper(mActor, this);
+  nsRefPtr<StreamHelper> helper = new StreamHelper(this);
   return helper->GetStream(aStream);
 }
 
 int64_t
 BlobChild::
 RemoteBlobImpl::GetFileId()
 {
+  if (!EventTargetIsOnCurrentThread(mActorTarget)) {
+    MOZ_CRASH("Not implemented!");
+  }
+
   int64_t fileId;
   if (mActor && mActor->SendGetFileId(&fileId)) {
     return fileId;
   }
 
   return -1;
 }