Bug 1036500 - Fix crash caused by non-thread safe DOMFile. r=ehsan
authorYuan Xulei <xyuan@mozilla.com>
Thu, 24 Jul 2014 10:21:34 +0800
changeset 217457 d13295f54df11fbaf5883e6a41c0cad57843f508
parent 217456 7ec48291f26a726f0a585be9c13e4992ea68628c
child 217458 b97c05712090e560fc280fb3c0cd5a5b0e26c51e
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1036500
milestone34.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 1036500 - Fix crash caused by non-thread safe DOMFile. r=ehsan
dom/filesystem/CreateFileTask.h
dom/filesystem/DeviceStorageFileSystem.cpp
dom/filesystem/DeviceStorageFileSystem.h
dom/filesystem/Directory.cpp
dom/filesystem/FileSystemBase.h
dom/filesystem/RemoveTask.cpp
dom/filesystem/RemoveTask.h
--- a/dom/filesystem/CreateFileTask.h
+++ b/dom/filesystem/CreateFileTask.h
@@ -66,17 +66,17 @@ private:
   static uint32_t sOutputBufferSize;
   nsRefPtr<Promise> mPromise;
   nsString mTargetRealPath;
   nsCOMPtr<nsIDOMBlob> mBlobData;
   nsCOMPtr<nsIInputStream> mBlobStream;
   InfallibleTArray<uint8_t> mArrayData;
   bool mReplace;
 
-  // This cannot be a DOMFile bacause this object is created on a different
+  // This cannot be a DOMFile because this object is created on a different
   // thread and DOMFile is not thread-safe. Let's use the DOMFileImpl instead.
   nsRefPtr<DOMFileImpl> mTargetFileImpl;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_CreateFileTask_h
--- a/dom/filesystem/DeviceStorageFileSystem.cpp
+++ b/dom/filesystem/DeviceStorageFileSystem.cpp
@@ -8,17 +8,17 @@
 
 #include "DeviceStorage.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/Directory.h"
 #include "mozilla/dom/FileSystemUtils.h"
 #include "nsCOMPtr.h"
 #include "nsDebug.h"
 #include "nsDeviceStorage.h"
-#include "nsIDOMFile.h"
+#include "nsDOMFile.h"
 #include "nsIFile.h"
 #include "nsPIDOMWindow.h"
 
 namespace mozilla {
 namespace dom {
 
 DeviceStorageFileSystem::DeviceStorageFileSystem(
   const nsAString& aStorageType,
@@ -109,17 +109,17 @@ DeviceStorageFileSystem::GetLocalFile(co
   nsresult rv = NS_NewLocalFile(localPath, false, getter_AddRefs(file));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
   return file.forget();
 }
 
 bool
-DeviceStorageFileSystem::GetRealPath(nsIDOMFile* aFile, nsAString& aRealPath) const
+DeviceStorageFileSystem::GetRealPath(DOMFileImpl* aFile, nsAString& aRealPath) const
 {
   MOZ_ASSERT(FileSystemUtils::IsParentProcess(),
              "Should be on parent process!");
   MOZ_ASSERT(aFile, "aFile Should not be null.");
 
   aRealPath.Truncate();
 
   nsAutoString filePath;
--- a/dom/filesystem/DeviceStorageFileSystem.h
+++ b/dom/filesystem/DeviceStorageFileSystem.h
@@ -32,17 +32,17 @@ public:
 
   virtual nsPIDOMWindow*
   GetWindow() const MOZ_OVERRIDE;
 
   virtual already_AddRefed<nsIFile>
   GetLocalFile(const nsAString& aRealPath) const MOZ_OVERRIDE;
 
   virtual bool
-  GetRealPath(nsIDOMFile* aFile, nsAString& aRealPath) const MOZ_OVERRIDE;
+  GetRealPath(DOMFileImpl* aFile, nsAString& aRealPath) const MOZ_OVERRIDE;
 
   virtual const nsAString&
   GetRootName() const MOZ_OVERRIDE;
 
   virtual bool
   IsSafeFile(nsIFile* aFile) const MOZ_OVERRIDE;
 
   virtual bool
--- a/dom/filesystem/Directory.cpp
+++ b/dom/filesystem/Directory.cpp
@@ -190,22 +190,22 @@ Directory::RemoveDeep(const StringOrFile
 }
 
 already_AddRefed<Promise>
 Directory::RemoveInternal(const StringOrFileOrDirectory& aPath, bool aRecursive,
                           ErrorResult& aRv)
 {
   nsresult error = NS_OK;
   nsString realPath;
-  nsCOMPtr<nsIDOMFile> file;
+  nsRefPtr<DOMFileImpl> file;
 
   // Check and get the target path.
 
   if (aPath.IsFile()) {
-    file = aPath.GetAsFile();
+    file = static_cast<DOMFile*>(aPath.GetAsFile())->Impl();
     goto parameters_check_done;
   }
 
   if (aPath.IsString()) {
     if (!DOMPathToRealPath(aPath.GetAsString(), realPath)) {
       error = NS_ERROR_DOM_FILESYSTEM_INVALID_PATH_ERR;
     }
     goto parameters_check_done;
--- a/dom/filesystem/FileSystemBase.h
+++ b/dom/filesystem/FileSystemBase.h
@@ -5,23 +5,23 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_FileSystemBase_h
 #define mozilla_dom_FileSystemBase_h
 
 #include "nsAutoPtr.h"
 #include "nsString.h"
 
-class nsIDOMFile;
 class nsPIDOMWindow;
 
 namespace mozilla {
 namespace dom {
 
 class Directory;
+class DOMFileImpl;
 
 class FileSystemBase
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FileSystemBase)
 public:
 
   // Create file system object from its string representation.
   static already_AddRefed<FileSystemBase>
@@ -68,17 +68,17 @@ public:
   IsSafeDirectory(Directory* aDir) const;
 
   /*
    * Get the real path (absolute DOM path) of the DOM file in the file system.
    * If succeeded, returns true. Otherwise, returns false and set aRealPath to
    * empty string.
    */
   virtual bool
-  GetRealPath(nsIDOMFile* aFile, nsAString& aRealPath) const = 0;
+  GetRealPath(DOMFileImpl* aFile, nsAString& aRealPath) const = 0;
 
   /*
    * Get the permission name required to access this file system.
    */
   const nsCString&
   GetPermission() const
   {
     return mPermission;
--- a/dom/filesystem/RemoveTask.cpp
+++ b/dom/filesystem/RemoveTask.cpp
@@ -5,32 +5,32 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "RemoveTask.h"
 
 #include "DOMError.h"
 #include "mozilla/dom/FileSystemBase.h"
 #include "mozilla/dom/FileSystemUtils.h"
 #include "mozilla/dom/Promise.h"
-#include "nsIDOMFile.h"
+#include "nsDOMFile.h"
 #include "nsIFile.h"
 #include "nsStringGlue.h"
 
 namespace mozilla {
 namespace dom {
 
 RemoveTask::RemoveTask(FileSystemBase* aFileSystem,
                        const nsAString& aDirPath,
-                       nsIDOMFile* aTargetFile,
+                       DOMFileImpl* aTargetFile,
                        const nsAString& aTargetPath,
                        bool aRecursive,
                        ErrorResult& aRv)
   : FileSystemTaskBase(aFileSystem)
   , mDirRealPath(aDirPath)
-  , mTargetFile(aTargetFile)
+  , mTargetFileImpl(aTargetFile)
   , mTargetRealPath(aTargetPath)
   , mRecursive(aRecursive)
   , mReturnValue(false)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
   MOZ_ASSERT(aFileSystem);
   nsCOMPtr<nsIGlobalObject> globalObject =
     do_QueryInterface(aFileSystem->GetWindow());
@@ -60,18 +60,18 @@ RemoveTask::RemoveTask(FileSystemBase* a
 
   if (target.type() == FileSystemPathOrFileValue::TnsString) {
     mTargetRealPath = target;
     return;
   }
 
   BlobParent* bp = static_cast<BlobParent*>(static_cast<PBlobParent*>(target));
   nsCOMPtr<nsIDOMBlob> blob = bp->GetBlob();
-  mTargetFile = do_QueryInterface(blob);
-  MOZ_ASSERT(mTargetFile, "mTargetFile should not be null.");
+  MOZ_ASSERT(blob);
+  mTargetFileImpl = static_cast<DOMFile*>(blob.get())->Impl();
 }
 
 RemoveTask::~RemoveTask()
 {
   MOZ_ASSERT(!mPromise || NS_IsMainThread(),
              "mPromise should be released on main thread!");
 }
 
@@ -85,19 +85,20 @@ RemoveTask::GetPromise()
 FileSystemParams
 RemoveTask::GetRequestParams(const nsString& aFileSystem) const
 {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!");
   FileSystemRemoveParams param;
   param.filesystem() = aFileSystem;
   param.directory() = mDirRealPath;
   param.recursive() = mRecursive;
-  if (mTargetFile) {
+  if (mTargetFileImpl) {
+    nsRefPtr<DOMFile> file = new DOMFile(mTargetFileImpl);
     BlobChild* actor
-      = ContentChild::GetSingleton()->GetOrCreateActorForBlob(mTargetFile);
+      = ContentChild::GetSingleton()->GetOrCreateActorForBlob(file);
     if (actor) {
       param.target() = actor;
     }
   } else {
     param.target() = mTargetRealPath;
   }
   return param;
 }
@@ -124,18 +125,18 @@ RemoveTask::Work()
              "Only call from parent process!");
   MOZ_ASSERT(!NS_IsMainThread(), "Only call on worker thread!");
 
   if (mFileSystem->IsShutdown()) {
     return NS_ERROR_FAILURE;
   }
 
   // Get the DOM path if a DOMFile is passed as the target.
-  if (mTargetFile) {
-    if (!mFileSystem->GetRealPath(mTargetFile, mTargetRealPath)) {
+  if (mTargetFileImpl) {
+    if (!mFileSystem->GetRealPath(mTargetFileImpl, mTargetRealPath)) {
       return NS_ERROR_DOM_SECURITY_ERR;
     }
     if (!FileSystemUtils::IsDescendantPath(mDirRealPath, mTargetRealPath)) {
       return NS_ERROR_DOM_FILESYSTEM_NO_MODIFICATION_ALLOWED_ERR;
     }
   }
 
   nsCOMPtr<nsIFile> file = mFileSystem->GetLocalFile(mTargetRealPath);
--- a/dom/filesystem/RemoveTask.h
+++ b/dom/filesystem/RemoveTask.h
@@ -9,25 +9,26 @@
 
 #include "mozilla/dom/FileSystemTaskBase.h"
 #include "nsAutoPtr.h"
 #include "mozilla/ErrorResult.h"
 
 namespace mozilla {
 namespace dom {
 
+class DOMFileImpl;
 class Promise;
 
 class RemoveTask MOZ_FINAL
   : public FileSystemTaskBase
 {
 public:
   RemoveTask(FileSystemBase* aFileSystem,
              const nsAString& aDirPath,
-             nsIDOMFile* aTargetFile,
+             DOMFileImpl* aTargetFile,
              const nsAString& aTargetPath,
              bool aRecursive,
              ErrorResult& aRv);
   RemoveTask(FileSystemBase* aFileSystem,
              const FileSystemRemoveParams& aParam,
              FileSystemRequestParent* aParent);
 
   virtual
@@ -53,17 +54,19 @@ protected:
   Work() MOZ_OVERRIDE;
 
   virtual void
   HandlerCallback() MOZ_OVERRIDE;
 
 private:
   nsRefPtr<Promise> mPromise;
   nsString mDirRealPath;
-  nsCOMPtr<nsIDOMFile> mTargetFile;
+  // This cannot be a DOMFile because this object will be used on a different
+  // thread and DOMFile is not thread-safe. Let's use the DOMFileImpl instead.
+  nsRefPtr<DOMFileImpl> mTargetFileImpl;
   nsString mTargetRealPath;
   bool mRecursive;
   bool mReturnValue;
 };
 
 } // namespace dom
 } // namespace mozilla