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

@@ -558,14 +558,15 @@ 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();
 
@@ -670,6 +671,8 @@ WriteBlob(JSStructuredCloneWriter* aWriter,
   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())) {

dom/ipc/Blob.cpp

@@ -1753,6 +1753,12 @@ protected:
   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;
@@ -1760,31 +1766,20 @@ protected:
 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
@@ -1857,6 +1852,13 @@ public:
   virtual BlobParent*
   GetBlobParent() override;
 
+  void
+  NullifyDifferentProcessBlobImpl()
+  {
+    MOZ_ASSERT(mDifferentProcessBlobImpl);
+    mDifferentProcessBlobImpl = nullptr;
+  }
+
 protected:
   // For SliceImpl.
   RemoteBlobImpl(const nsAString& aContentType, uint64_t aLength);
@@ -2086,56 +2088,43 @@ private:
 
 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)
 {
+  if (aIsSameProcessBlob) {
+    MOZ_ASSERT(aRemoteBlobImpl);
+    mSameProcessBlobImpl = aRemoteBlobImpl;
+    MOZ_ASSERT(gProcessType == GeckoProcessType_Default);
+  } else {
+    mDifferentProcessBlobImpl = aRemoteBlobImpl;
+  }
+
   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,
+                               BlobImpl* aRemoteBlobImpl,
                                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);
-
-  CommonInit(aActor);
-}
-
-BlobChild::
-RemoteBlobImpl::RemoteBlobImpl(BlobChild* aActor,
-                               BlobImpl* aSameProcessBlobImpl,
-                               const nsAString& aContentType,
-                               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);
 }
@@ -3041,10 +3030,12 @@ BlobChild::CommonInit(BlobChild* aOther, BlobImpl* aBlobImpl)
     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);
@@ -3073,7 +3064,8 @@ BlobChild::CommonInit(const ChildBlobConstructorParams& aParams)
       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;
     }
 
@@ -3081,11 +3073,13 @@ BlobChild::CommonInit(const ChildBlobConstructorParams& aParams)
       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;
     }
 
@@ -3120,9 +3114,11 @@ BlobChild::CommonInit(const ChildBlobConstructorParams& aParams)
                              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;
@@ -3592,6 +3588,14 @@ BlobChild::DeallocPBlobStreamChild(PBlobStreamChild* aActor)
   return true;
 }
 
+bool
+BlobChild::RecvCreatedFromKnownBlob()
+{
+  // Releasing the other blob now that this blob is fully created.
+  mRemoteBlobImpl->NullifyDifferentProcessBlobImpl();
+  return true;
+}
+
 /*******************************************************************************
  * BlobParent
  ******************************************************************************/

dom/ipc/BlobChild.h

@@ -220,6 +220,9 @@ private:
 
   virtual bool
   DeallocPBlobStreamChild(PBlobStreamChild* aActor) override;
+
+  virtual bool
+  RecvCreatedFromKnownBlob() override;
 };
 
 // Only let ContentChild call BlobChild::Startup() and ensure that

dom/ipc/ContentParent.cpp

@@ -3515,6 +3515,18 @@ 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()
 {

dom/ipc/ContentParent.h

@@ -648,6 +648,9 @@ private:
                                           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,

dom/ipc/PBlob.ipdl

@@ -45,6 +45,11 @@ parent:
   // 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

ipc/glue/BackgroundParentImpl.cpp

@@ -13,6 +13,7 @@
 #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"
@@ -243,6 +244,18 @@ BackgroundParentImpl::DeallocPBlobParent(PBlobParent* 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)

ipc/glue/BackgroundParentImpl.h

@@ -64,6 +64,10 @@ protected:
   virtual bool
   DeallocPBlobParent(PBlobParent* aActor) override;
 
+  virtual bool
+  RecvPBlobConstructor(PBlobParent* aActor,
+                       const BlobConstructorParams& params) override;
+
   virtual PFileDescriptorSetParent*
   AllocPFileDescriptorSetParent(const FileDescriptor& aFileDescriptor)
                                 override;