Bug 1211266 - Remote blobs coming from a different thread and a different manager must be kept alive until the creation of depending RemoteBlobs is not completed, r=bent, f=gerard-majax
☠☠ backed out by 71ba07014e12 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 24 Nov 2015 19:51:28 +0000
changeset 274084 cbc2c8a244380a83ba857d0c315462490b1652e8
parent 274083 72cf0adcfb2796c1e7b439e1851144fb818e1469
child 274085 21d5fede3c77e25a9e1ceb1d0e74db2859a31ccc
push idunknown
push userunknown
push dateunknown
reviewersbent
bugs1211266
milestone45.0a1
Bug 1211266 - Remote blobs coming from a different thread and a different manager must be kept alive until the creation of depending RemoteBlobs is not completed, r=bent, f=gerard-majax
dom/base/StructuredCloneHolder.cpp
dom/ipc/Blob.cpp
dom/ipc/BlobChild.h
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PBlob.ipdl
ipc/glue/BackgroundParentImpl.cpp
ipc/glue/BackgroundParentImpl.h
--- a/dom/base/StructuredCloneHolder.cpp
+++ b/dom/base/StructuredCloneHolder.cpp
@@ -553,24 +553,25 @@ StructuredCloneHolder::WriteFullySeriali
 namespace {
 
 // Recursive!
 already_AddRefed<BlobImpl>
 EnsureBlobForBackgroundManager(BlobImpl* aBlobImpl,
                                PBackgroundChild* aManager = nullptr)
 {
   MOZ_ASSERT(aBlobImpl);
+  RefPtr<BlobImpl> blobImpl = aBlobImpl;
 
   if (!aManager) {
     aManager = BackgroundChild::GetForCurrentThread();
-    MOZ_ASSERT(aManager);
+    if (!aManager) {
+      return blobImpl.forget();
+    }
   }
 
-  RefPtr<BlobImpl> blobImpl = aBlobImpl;
-
   const nsTArray<RefPtr<BlobImpl>>* subBlobImpls =
     aBlobImpl->GetSubBlobImpls();
 
   if (!subBlobImpls || !subBlobImpls->Length()) {
     if (nsCOMPtr<nsIRemoteBlob> remoteBlob = do_QueryObject(blobImpl)) {
       // Always make sure we have a blob from an actor we can use on this
       // thread.
       BlobChild* blobChild = BlobChild::GetOrCreate(aManager, blobImpl);
@@ -665,16 +666,18 @@ WriteBlob(JSStructuredCloneWriter* aWrit
 {
   MOZ_ASSERT(aWriter);
   MOZ_ASSERT(aBlob);
   MOZ_ASSERT(aHolder);
 
   RefPtr<BlobImpl> blobImpl = EnsureBlobForBackgroundManager(aBlob->Impl());
   MOZ_ASSERT(blobImpl);
 
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(blobImpl->SetMutable(false)));
+
   // We store the position of the blobImpl in the array as index.
   if (JS_WriteUint32Pair(aWriter, SCTAG_DOM_BLOB,
                          aHolder->BlobImpls().Length())) {
     aHolder->BlobImpls().AppendElement(blobImpl);
     return true;
   }
 
   return false;
--- a/dom/ipc/Blob.cpp
+++ b/dom/ipc/Blob.cpp
@@ -1748,48 +1748,43 @@ class BlobChild::RemoteBlobImpl
   , public nsIRemoteBlob
 {
 protected:
   class CreateStreamHelper;
 
   BlobChild* mActor;
   nsCOMPtr<nsIEventTarget> mActorTarget;
 
+  // We use this pointer to keep a live a blobImpl coming from a different
+  // process until this one is fully created. We set it to null when
+  // SendCreatedFromKnownBlob() is received. This is used only with KnownBlob
+  // params in the CTOR of a IPC BlobImpl.
+  RefPtr<BlobImpl> mDifferentProcessBlobImpl;
+
   RefPtr<BlobImpl> mSameProcessBlobImpl;
 
   const bool mIsSlice;
 
 public:
   // For File.
   RemoteBlobImpl(BlobChild* aActor,
+                 BlobImpl* aRemoteBlobImpl,
                  const nsAString& aName,
                  const nsAString& aContentType,
                  uint64_t aLength,
                  int64_t aModDate,
-                 BlobDirState aDirState);
+                 BlobDirState aDirState,
+                 bool aIsSameProcessBlob);
 
   // For Blob.
   RemoteBlobImpl(BlobChild* aActor,
-                 const nsAString& aContentType,
-                 uint64_t aLength);
-
-  // For same-process blobs.
-  RemoteBlobImpl(BlobChild* aActor,
-                 BlobImpl* aSameProcessBlobImpl,
-                 const nsAString& aName,
+                 BlobImpl* aRemoteBlobImpl,
                  const nsAString& aContentType,
                  uint64_t aLength,
-                 int64_t aModDate,
-                 BlobDirState aDirState);
-
-  // For same-process blobs.
-  RemoteBlobImpl(BlobChild* aActor,
-                 BlobImpl* aSameProcessBlobImpl,
-                 const nsAString& aContentType,
-                 uint64_t aLength);
+                 bool aIsSameProcessBlob);
 
   // For mystery blobs.
   explicit
   RemoteBlobImpl(BlobChild* aActor);
 
   void
   NoteDyingActor();
 
@@ -1852,16 +1847,23 @@ public:
   SetMutable(bool aMutable) override;
 
   virtual BlobChild*
   GetBlobChild() override;
 
   virtual BlobParent*
   GetBlobParent() override;
 
+  void
+  NullifyDifferentProcessBlobImpl()
+  {
+    MOZ_ASSERT(mDifferentProcessBlobImpl);
+    mDifferentProcessBlobImpl = nullptr;
+  }
+
 protected:
   // For SliceImpl.
   RemoteBlobImpl(const nsAString& aContentType, uint64_t aLength);
 
   ~RemoteBlobImpl()
   {
     MOZ_ASSERT_IF(mActorTarget,
                   EventTargetIsOnCurrentThread(mActorTarget));
@@ -2081,66 +2083,53 @@ private:
 };
 
 /*******************************************************************************
  * BlobChild::RemoteBlobImpl
  ******************************************************************************/
 
 BlobChild::
 RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor,
+                               BlobImpl* aRemoteBlobImpl,
                                const nsAString& aName,
                                const nsAString& aContentType,
                                uint64_t aLength,
                                int64_t aModDate,
-                               BlobDirState aDirState)
+                               BlobDirState aDirState,
+                               bool aIsSameProcessBlob)
   : BlobImplBase(aName, aContentType, aLength, aModDate, aDirState)
   , mIsSlice(false)
 {
-  CommonInit(aActor);
-}
-
-BlobChild::
-RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor,
-                               const nsAString& aContentType,
-                               uint64_t aLength)
-  : BlobImplBase(aContentType, aLength)
-  , mIsSlice(false)
-{
-  CommonInit(aActor);
-}
-
-BlobChild::
-RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor,
-                               BlobImpl* aSameProcessBlobImpl,
-                               const nsAString& aName,
-                               const nsAString& aContentType,
-                               uint64_t aLength,
-                               int64_t aModDate,
-                               BlobDirState aDirState)
-  : BlobImplBase(aName, aContentType, aLength, aModDate, aDirState)
-  , mSameProcessBlobImpl(aSameProcessBlobImpl)
-  , mIsSlice(false)
-{
-  MOZ_ASSERT(aSameProcessBlobImpl);
-  MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
+  if (aIsSameProcessBlob) {
+    MOZ_ASSERT(aRemoteBlobImpl);
+    mSameProcessBlobImpl = aRemoteBlobImpl;
+    MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
+  } else {
+    mDifferentProcessBlobImpl = aRemoteBlobImpl;
+  }
 
   CommonInit(aActor);
 }
 
 BlobChild::
 RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor,
-                               BlobImpl* aSameProcessBlobImpl,
+                               BlobImpl* aRemoteBlobImpl,
                                const nsAString& aContentType,
-                               uint64_t aLength)
+                               uint64_t aLength,
+                               bool aIsSameProcessBlob)
   : BlobImplBase(aContentType, aLength)
-  , mSameProcessBlobImpl(aSameProcessBlobImpl)
   , mIsSlice(false)
 {
-  MOZ_ASSERT(aSameProcessBlobImpl);
-  MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
+  if (aIsSameProcessBlob) {
+    MOZ_ASSERT(aRemoteBlobImpl);
+    mSameProcessBlobImpl = aRemoteBlobImpl;
+    MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
+  } else {
+    mDifferentProcessBlobImpl = aRemoteBlobImpl;
+  }
 
   CommonInit(aActor);
 }
 
 BlobChild::
 RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor)
   : BlobImplBase(EmptyString(), EmptyString(), UINT64_MAX, INT64_MAX)
   , mIsSlice(false)
@@ -3036,20 +3025,22 @@ BlobChild::CommonInit(BlobChild* aOther,
   RefPtr<RemoteBlobImpl> remoteBlob;
   if (otherImpl->IsFile()) {
     nsString name;
     otherImpl->GetName(name);
 
     int64_t modDate = otherImpl->GetLastModified(rv);
     MOZ_ASSERT(!rv.Failed());
 
-    remoteBlob = new RemoteBlobImpl(this, name, contentType, length, modDate,
-                                    otherImpl->GetDirState());
+    remoteBlob = new RemoteBlobImpl(this, otherImpl, name, contentType, length,
+                                    modDate, otherImpl->GetDirState(),
+                                    false /* SameProcessBlobImpl */);
   } else {
-    remoteBlob = new RemoteBlobImpl(this, contentType, length);
+    remoteBlob = new RemoteBlobImpl(this, otherImpl, contentType, length,
+                                    false /* SameProcessBlobImpl */);
   }
 
   CommonInit(aOther->ParentID(), remoteBlob);
 }
 
 void
 BlobChild::CommonInit(const ChildBlobConstructorParams& aParams)
 {
@@ -3068,29 +3059,32 @@ BlobChild::CommonInit(const ChildBlobCon
 
   RefPtr<RemoteBlobImpl> remoteBlob;
 
   switch (paramsType) {
     case AnyBlobConstructorParams::TNormalBlobConstructorParams: {
       const NormalBlobConstructorParams& params =
         blobParams.get_NormalBlobConstructorParams();
       remoteBlob =
-        new RemoteBlobImpl(this, params.contentType(), params.length());
+        new RemoteBlobImpl(this, nullptr, params.contentType(), params.length(),
+                           false /* SameProcessBlobImpl */);
       break;
     }
 
     case AnyBlobConstructorParams::TFileBlobConstructorParams: {
       const FileBlobConstructorParams& params =
         blobParams.get_FileBlobConstructorParams();
       remoteBlob = new RemoteBlobImpl(this,
+                                      nullptr,
                                       params.name(),
                                       params.contentType(),
                                       params.length(),
                                       params.modDate(),
-                                      BlobDirState(params.dirState()));
+                                      BlobDirState(params.dirState()),
+                                      false /* SameProcessBlobImpl */);
       break;
     }
 
     case AnyBlobConstructorParams::TSameProcessBlobConstructorParams: {
       MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
 
       const SameProcessBlobConstructorParams& params =
         blobParams.get_SameProcessBlobConstructorParams();
@@ -3115,19 +3109,21 @@ BlobChild::CommonInit(const ChildBlobCon
 
         remoteBlob =
           new RemoteBlobImpl(this,
                              blobImpl,
                              name,
                              contentType,
                              size,
                              lastModifiedDate,
-                             blobImpl->GetDirState());
+                             blobImpl->GetDirState(),
+                             true /* SameProcessBlobImpl */);
       } else {
-        remoteBlob = new RemoteBlobImpl(this, blobImpl, contentType, size);
+        remoteBlob = new RemoteBlobImpl(this, blobImpl, contentType, size,
+                                        true /* SameProcessBlobImpl */);
       }
 
       break;
     }
 
     case AnyBlobConstructorParams::TMysteryBlobConstructorParams: {
       remoteBlob = new RemoteBlobImpl(this);
       break;
@@ -3587,16 +3583,24 @@ bool
 BlobChild::DeallocPBlobStreamChild(PBlobStreamChild* aActor)
 {
   AssertIsOnOwningThread();
 
   delete static_cast<InputStreamChild*>(aActor);
   return true;
 }
 
+bool
+BlobChild::RecvCreatedFromKnownBlob()
+{
+  // Releasing the other blob now that this blob is fully created.
+  mRemoteBlobImpl->NullifyDifferentProcessBlobImpl();
+  return true;
+}
+
 /*******************************************************************************
  * BlobParent
  ******************************************************************************/
 
 BlobParent::BlobParent(nsIContentParent* aManager, IDTableEntry* aIDTableEntry)
   : mBackgroundManager(nullptr)
   , mContentManager(aManager)
 {
--- a/dom/ipc/BlobChild.h
+++ b/dom/ipc/BlobChild.h
@@ -215,16 +215,19 @@ private:
   ActorDestroy(ActorDestroyReason aWhy) override;
 
   virtual PBlobStreamChild*
   AllocPBlobStreamChild(const uint64_t& aStart,
                         const uint64_t& aLength) override;
 
   virtual bool
   DeallocPBlobStreamChild(PBlobStreamChild* aActor) override;
+
+  virtual bool
+  RecvCreatedFromKnownBlob() override;
 };
 
 // Only let ContentChild call BlobChild::Startup() and ensure that
 // ContentChild can't access any other BlobChild internals.
 class BlobChild::FriendKey final
 {
   friend class ContentChild;
 
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -3510,16 +3510,28 @@ ContentParent::AllocPBlobParent(const Bl
 }
 
 bool
 ContentParent::DeallocPBlobParent(PBlobParent* aActor)
 {
     return nsIContentParent::DeallocPBlobParent(aActor);
 }
 
+bool
+ContentParent::RecvPBlobConstructor(PBlobParent* aActor,
+                                    const BlobConstructorParams& aParams)
+{
+  const ParentBlobConstructorParams& params = aParams.get_ParentBlobConstructorParams();
+  if (params.blobParams().type() == AnyBlobConstructorParams::TKnownBlobConstructorParams) {
+    return aActor->SendCreatedFromKnownBlob();
+  }
+
+  return true;
+}
+
 mozilla::PRemoteSpellcheckEngineParent *
 ContentParent::AllocPRemoteSpellcheckEngineParent()
 {
     mozilla::RemoteSpellcheckEngineParent *parent = new mozilla::RemoteSpellcheckEngineParent();
     return parent;
 }
 
 bool
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -643,16 +643,19 @@ private:
 
     virtual bool
     DeallocPFileSystemRequestParent(PFileSystemRequestParent*) override;
 
     virtual PBlobParent* AllocPBlobParent(const BlobConstructorParams& aParams)
                                           override;
     virtual bool DeallocPBlobParent(PBlobParent* aActor) override;
 
+    virtual bool RecvPBlobConstructor(PBlobParent* aActor,
+                                      const BlobConstructorParams& params) override;
+
     virtual bool DeallocPCrashReporterParent(PCrashReporterParent* crashreporter) override;
 
     virtual bool RecvGetRandomValues(const uint32_t& length,
                                      InfallibleTArray<uint8_t>* randomValues) override;
 
     virtual bool RecvIsSecureURI(const uint32_t& aType, const URIParams& aURI,
                                  const uint32_t& aFlags, bool* aIsSecureURI) override;
 
--- a/dom/ipc/PBlob.ipdl
+++ b/dom/ipc/PBlob.ipdl
@@ -40,12 +40,17 @@ parent:
 
   // Use only for testing!
   sync GetFileId()
     returns (int64_t fileId);
 
   // Use only for testing!
   sync GetFilePath()
     returns (nsString filePath);
+
+child:
+  // This method must be called by the parent when the PBlobParent is fully
+  // created in order to release the known blob.
+  CreatedFromKnownBlob();
 };
 
 } // namespace dom
 } // namespace mozilla
--- a/ipc/glue/BackgroundParentImpl.cpp
+++ b/ipc/glue/BackgroundParentImpl.cpp
@@ -8,16 +8,17 @@
 #include "FileDescriptorSetParent.h"
 #ifdef MOZ_WEBRTC
 #include "CamerasParent.h"
 #endif
 #include "mozilla/media/MediaParent.h"
 #include "mozilla/AppProcessChecker.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/dom/ContentParent.h"
+#include "mozilla/dom/DOMTypes.h"
 #include "mozilla/dom/NuwaParent.h"
 #include "mozilla/dom/PBlobParent.h"
 #include "mozilla/dom/MessagePortParent.h"
 #include "mozilla/dom/ServiceWorkerRegistrar.h"
 #include "mozilla/dom/asmjscache/AsmJSCache.h"
 #include "mozilla/dom/cache/ActorUtils.h"
 #include "mozilla/dom/indexedDB/ActorsParent.h"
 #include "mozilla/dom/ipc/BlobParent.h"
@@ -238,16 +239,28 @@ BackgroundParentImpl::DeallocPBlobParent
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
   MOZ_ASSERT(aActor);
 
   mozilla::dom::BlobParent::Destroy(aActor);
   return true;
 }
 
+bool
+BackgroundParentImpl::RecvPBlobConstructor(PBlobParent* aActor,
+                                           const BlobConstructorParams& aParams)
+{
+  const ParentBlobConstructorParams& params = aParams;
+  if (params.blobParams().type() == AnyBlobConstructorParams::TKnownBlobConstructorParams) {
+    return aActor->SendCreatedFromKnownBlob();
+  }
+
+  return true;
+}
+
 PFileDescriptorSetParent*
 BackgroundParentImpl::AllocPFileDescriptorSetParent(
                                           const FileDescriptor& aFileDescriptor)
 {
   AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
 
   return new FileDescriptorSetParent(aFileDescriptor);
--- a/ipc/glue/BackgroundParentImpl.h
+++ b/ipc/glue/BackgroundParentImpl.h
@@ -59,16 +59,20 @@ protected:
                                         override;
 
   virtual PBlobParent*
   AllocPBlobParent(const BlobConstructorParams& aParams) override;
 
   virtual bool
   DeallocPBlobParent(PBlobParent* aActor) override;
 
+  virtual bool
+  RecvPBlobConstructor(PBlobParent* aActor,
+                       const BlobConstructorParams& params) override;
+
   virtual PFileDescriptorSetParent*
   AllocPFileDescriptorSetParent(const FileDescriptor& aFileDescriptor)
                                 override;
 
   virtual bool
   DeallocPFileDescriptorSetParent(PFileDescriptorSetParent* aActor)
                                   override;