Bug 1113062 - FileImpls are kept alive when a blob URI is generated from them, r=smaug
☠☠ backed out by 034f69677784 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 05 Jan 2015 23:55:26 +0100
changeset 222146 e89d431543ae151c041870c9cf0a728fa68fb1f0
parent 222145 7070c9621457ef4383b674c8c8569b3c8a9c2ed7
child 222147 034f696777845b1cfb23464de7f469b110d0ec54
push id28059
push userryanvm@gmail.com
push dateTue, 06 Jan 2015 15:53:01 +0000
treeherdermozilla-central@4d91c33b351c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1113062
milestone37.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 1113062 - FileImpls are kept alive when a blob URI is generated from them, r=smaug
dom/base/File.cpp
dom/base/File.h
dom/base/nsHostObjectProtocolHandler.cpp
dom/workers/URL.cpp
--- a/dom/base/File.cpp
+++ b/dom/base/File.cpp
@@ -125,24 +125,31 @@ nsresult DataOwnerAdapter::Create(DataOw
 
 ////////////////////////////////////////////////////////////////////////////
 // mozilla::dom::File implementation
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(File)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(File)
   MOZ_ASSERT(tmp->mImpl);
-  tmp->mImpl->Unlink();
+  if (tmp->mImpl->IsCCed()) {
+    tmp->mImpl->ReleaseOwner();
+    if (!tmp->mImpl->HasOwners()) {
+      tmp->mImpl->Unlink();
+    }
+  }
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(File)
   MOZ_ASSERT(tmp->mImpl);
-  tmp->mImpl->Traverse(cb);
+  if (tmp->mImpl->IsCCed()) {
+    tmp->mImpl->Traverse(cb);
+  }
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(File)
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
@@ -285,16 +292,20 @@ File::File(nsISupports* aParent, FileImp
 #ifdef DEBUG
   {
     nsCOMPtr<nsPIDOMWindow> win = do_QueryInterface(aParent);
     if (win) {
       MOZ_ASSERT(win->IsInnerWindow());
     }
   }
 #endif
+
+  if (mImpl->IsCCed()) {
+    mImpl->AddOwner();
+  }
 }
 
 const nsTArray<nsRefPtr<FileImpl>>*
 File::GetSubBlobImpls() const
 {
   return mImpl->GetSubBlobImpls();
 }
 
@@ -710,16 +721,30 @@ FileImpl::Slice(const Optional<int64_t>&
   int64_t end = aEnd.WasPassed() ? aEnd.Value() : (int64_t)thisLength;
 
   ParseSize((int64_t)thisLength, start, end);
 
   return CreateSlice((uint64_t)start, (uint64_t)(end - start),
                      aContentType, aRv);
 }
 
+void
+FileImpl::AddOwner()
+{
+  ++mOwnersCount;
+}
+
+
+void
+FileImpl::ReleaseOwner()
+{
+  MOZ_ASSERT(mOwnersCount);
+  --mOwnersCount;
+}
+
 ////////////////////////////////////////////////////////////////////////////
 // FileImpl implementation
 
 NS_IMPL_ISUPPORTS(FileImpl, PIFileImpl)
 
 ////////////////////////////////////////////////////////////////////////////
 // FileImplFile implementation
 
--- a/dom/base/File.h
+++ b/dom/base/File.h
@@ -230,17 +230,17 @@ private:
 
 // This is the abstract class for any File backend. It must be nsISupports
 // because this class must be ref-counted and it has to work with IPC.
 class FileImpl : public PIFileImpl
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
 
-  FileImpl() {}
+  FileImpl() : mOwnersCount(0) {}
 
   virtual void GetName(nsAString& aName) = 0;
 
   virtual nsresult GetPath(nsAString& aName) = 0;
 
   virtual int64_t GetLastModified(ErrorResult& aRv) = 0;
 
   virtual void GetMozFullPath(nsAString& aName, ErrorResult& aRv) = 0;
@@ -297,18 +297,28 @@ public:
   virtual void Unlink() = 0;
   virtual void Traverse(nsCycleCollectionTraversalCallback &aCb) = 0;
 
   virtual bool IsCCed() const
   {
     return false;
   }
 
+  void AddOwner();
+  void ReleaseOwner();
+
+  bool HasOwners() const
+  {
+    return !!mOwnersCount;
+  }
+
 protected:
   virtual ~FileImpl() {}
+
+  uint32_t mOwnersCount;
 };
 
 class FileImplBase : public FileImpl
 {
 public:
   FileImplBase(const nsAString& aName, const nsAString& aContentType,
                uint64_t aLength, uint64_t aLastModifiedDate)
     : mIsFile(true)
--- a/dom/base/nsHostObjectProtocolHandler.cpp
+++ b/dom/base/nsHostObjectProtocolHandler.cpp
@@ -311,32 +311,49 @@ nsHostObjectProtocolHandler::AddDataEntr
 
   DataInfo* info = new DataInfo;
 
   info->mObject = aObject;
   info->mPrincipal = aPrincipal;
   mozilla::BlobURLsReporter::GetJSStackForBlob(info);
 
   gDataTable->Put(aUri, info);
+
+  nsCOMPtr<PIFileImpl> pFileImpl = do_QueryInterface(aObject);
+  if (pFileImpl) {
+    auto* fileImpl = static_cast<FileImpl*>(pFileImpl.get());
+    fileImpl->AddOwner();
+  }
+
   return NS_OK;
 }
 
 void
 nsHostObjectProtocolHandler::RemoveDataEntry(const nsACString& aUri)
 {
   if (gDataTable) {
     nsCString uriIgnoringRef;
     int32_t hashPos = aUri.FindChar('#');
     if (hashPos < 0) {
       uriIgnoringRef = aUri;
     }
     else {
       uriIgnoringRef = StringHead(aUri, hashPos);
     }
-    gDataTable->Remove(uriIgnoringRef);
+    nsAutoPtr<DataInfo> dataInfo;
+    gDataTable->RemoveAndForget(uriIgnoringRef, dataInfo);
+
+    if (dataInfo) {
+      nsCOMPtr<PIFileImpl> pFileImpl = do_QueryInterface(dataInfo->mObject);
+      if (pFileImpl) {
+        auto* fileImpl = static_cast<FileImpl*>(pFileImpl.get());
+        fileImpl->ReleaseOwner();
+      }
+    }
+
     if (gDataTable->Count() == 0) {
       delete gDataTable;
       gDataTable = nullptr;
     }
   }
 }
 
 nsresult
@@ -387,17 +404,17 @@ GetDataInfo(const nsACString& aUri)
   int32_t hashPos = aUri.FindChar('#');
   if (hashPos < 0) {
     uriIgnoringRef = aUri;
   }
   else {
     uriIgnoringRef = StringHead(aUri, hashPos);
   }
   gDataTable->Get(uriIgnoringRef, &res);
-  
+
   return res;
 }
 
 nsIPrincipal*
 nsHostObjectProtocolHandler::GetDataEntryPrincipal(const nsACString& aUri)
 {
   if (!gDataTable) {
     return nullptr;
--- a/dom/workers/URL.cpp
+++ b/dom/workers/URL.cpp
@@ -884,16 +884,23 @@ URL::CreateObjectURL(const GlobalObject&
 }
 
 // static
 void
 URL::CreateObjectURL(const GlobalObject& aGlobal, File& aBlob,
                      const mozilla::dom::objectURLOptions& aOptions,
                      nsString& aResult, mozilla::ErrorResult& aRv)
 {
+  if (aBlob.Impl()->IsCCed()) {
+    // The creation of a blob URL happens on the main-thread and we cannot
+    // transfer a FileImpl if CCed.
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
   JSContext* cx = aGlobal.Context();
   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
 
   nsRefPtr<CreateURLRunnable> runnable =
     new CreateURLRunnable(workerPrivate, aBlob.Impl(), aOptions, aResult);
 
   if (!runnable->Dispatch(cx)) {
     JS_ReportPendingException(cx);