+++ This bug was initially created as a clone of Bug #761930 +++ MediaStorage needs to work from content processes. We will land support without cross-process Blobs. This bug is to track the changes required to support pblobs.

Comment on attachment 648454 [details] [diff] [review] patch v.1 Review of attachment 648454 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/devicestorage/DeviceStorageRequestParent.cpp @@ +156,5 @@ > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); > > + nsString mime; > + mime.AssignWithConversion(mMimeType); > + CopyASCIItoUTF16(mMimeType, mime); You want just this latter line I think. @@ +161,5 @@ > + > + nsCOMPtr<nsIDOMBlob> blob = new nsDOMFileFile(mFile->mPath, mime, mLength, mFile->mFile); > + > + ContentParent* cp = static_cast<ContentParent*>(mParent->Manager()); > + BlobParent* actor = cp->GetOrCreateActorForBlob(blob); This can fail if the child crashes. ::: dom/devicestorage/DeviceStorageRequestParent.h @@ +75,5 @@ > ~WriteFileEvent(); > NS_IMETHOD Run(); > private: > DeviceStorageRequestParent* mParent; > + nsRefPtr<nsIDOMBlob> mBlob; nsCOMPtr. Though, I think you have a patch waiting which makes this unnecessary now that you don't have to hold the blob alive while using the stream. ::: dom/devicestorage/nsDeviceStorage.cpp @@ +904,5 @@ > > NS_IMETHOD Run() > { > NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!"); > + Nit: You've got some extra whitespace here. @@ +906,5 @@ > { > NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!"); > + > + nsCOMPtr<nsIInputStream> stream; > + mBlob->GetInternalStream(getter_AddRefs(stream)); This can fail for the remote blobs if the child process crashes. ::: dom/devicestorage/nsDeviceStorage.h @@ +51,5 @@ > > // we want to make sure that the names of file can't reach > // outside of the type of storage the user asked for. > bool IsSafePath(); > Nit: You've got some extra whitespace here.

this changes the ownership of the parent to be refcounted. it also notifies any outstanding runnables if the parent is ipdl destroyed.

Comment on attachment 648791 [details] [diff] [review] patch v.2 Review of attachment 648791 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/devicestorage/DeviceStorageRequestParent.cpp @@ +33,5 @@ > + BlobParent* bp = static_cast<BlobParent*>(p.blobParent()); > + nsCOMPtr<nsIDOMBlob> blob = bp->GetBlob(); > + > + nsCOMPtr<nsIInputStream> stream; > + blob->GetInternalStream(getter_AddRefs(stream)); This will fail if the child crashes, need to handle it somehow. @@ +175,5 @@ > + > + nsCOMPtr<nsIDOMBlob> blob = new nsDOMFileFile(mFile->mPath, mime, mLength, mFile->mFile); > + > + ContentParent* cp = static_cast<ContentParent*>(mParent->Manager()); > + BlobParent* actor = cp->GetOrCreateActorForBlob(blob); This can also fail if the child crashes... Though I guess it doesn't really matter, the Send__delete__ call below will fail too and it seems like it doesn't matter here. ::: dom/devicestorage/DeviceStorageRequestParent.h @@ +20,5 @@ > class DeviceStorageRequestParent : public PDeviceStorageRequestParent > { > public: > DeviceStorageRequestParent(const DeviceStorageParams& aParams); > ~DeviceStorageRequestParent(); Nit: Now that you've made this refcounted please make this protected. @@ +30,4 @@ > private: > + nsAutoRefCnt mRefCnt; > + > + class nsCancelableRunnable : public nsRunnable Nit: The 'ns' prefix made me think this was living somewhere global, not an inner class. Not a big deal, but if you don't care then let's remove it. @@ +55,5 @@ > + void Cancel() { > + mCancelled = true; > + } > + > + bool IsCancelled() { This appears to be unused. @@ +64,5 @@ > + > + protected: > + nsRefPtr<DeviceStorageRequestParent> mParent; > + private: > + bool mCancelled; Nit: mCanceled. @@ +162,5 @@ > nsString mPath; > }; > > +protected: > + void AddRunnable(class nsCancelableRunnable* aRunnable) { Nit: is the 'class' needed here? (Or below?) ::: dom/devicestorage/nsDeviceStorage.cpp @@ +819,5 @@ > { > NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!"); > > + nsCOMPtr<nsIInputStream> stream; > + mBlob->GetInternalStream(getter_AddRefs(stream)); This can fail so you'd better check.