Bug 1258095 - patch 1/3 - OSFileSystem should have the root == the directory root, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 30 Mar 2016 07:17:15 +0100
changeset 291017 1910aa3c10fc6dd93c7d49a59d6c8290587a8936
parent 291016 907c2536632e727727eb107779d1fccfea93c295
child 291018 554e58d8b27df2d58241421f8de97b1502e60702
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1258095
milestone48.0a1
Bug 1258095 - patch 1/3 - OSFileSystem should have the root == the directory root, r=smaug
dom/filesystem/DeviceStorageFileSystem.cpp
dom/filesystem/Directory.cpp
dom/filesystem/FileSystemBase.h
dom/filesystem/GetDirectoryListingTask.cpp
dom/filesystem/OSFileSystem.cpp
--- a/dom/filesystem/DeviceStorageFileSystem.cpp
+++ b/dom/filesystem/DeviceStorageFileSystem.cpp
@@ -39,17 +39,17 @@ DeviceStorageFileSystem::DeviceStorageFi
   NS_WARN_IF(NS_FAILED(rv));
 
   // Get the local path of the file system root.
   nsCOMPtr<nsIFile> rootFile;
   DeviceStorageFile::GetRootDirectoryForType(aStorageType,
                                              aStorageName,
                                              getter_AddRefs(rootFile));
 
-  NS_WARN_IF(!rootFile || NS_FAILED(rootFile->GetPath(mLocalRootPath)));
+  NS_WARN_IF(!rootFile || NS_FAILED(rootFile->GetPath(mLocalOrDeviceStorageRootPath)));
 
   if (!XRE_IsParentProcess()) {
     return;
   }
 
   // DeviceStorageTypeChecker is a singleton object and must be initialized on
   // the main thread. We initialize it here so that we can use it on the worker
   // thread.
@@ -108,17 +108,17 @@ DeviceStorageFileSystem::GetRootName(nsA
 bool
 DeviceStorageFileSystem::IsSafeFile(nsIFile* aFile) const
 {
   MOZ_ASSERT(XRE_IsParentProcess(),
              "Should be on parent process!");
   MOZ_ASSERT(aFile);
 
   nsCOMPtr<nsIFile> rootPath;
-  nsresult rv = NS_NewLocalFile(GetLocalRootPath(), false,
+  nsresult rv = NS_NewLocalFile(LocalOrDeviceStorageRootPath(), false,
                                 getter_AddRefs(rootPath));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return false;
   }
 
   // Check if this file belongs to this storage.
   if (NS_WARN_IF(!FileSystemUtils::IsDescendantPath(rootPath, aFile))) {
     return false;
--- a/dom/filesystem/Directory.cpp
+++ b/dom/filesystem/Directory.cpp
@@ -113,17 +113,17 @@ NS_INTERFACE_MAP_END
 
 // static
 already_AddRefed<Promise>
 Directory::GetRoot(FileSystemBase* aFileSystem, ErrorResult& aRv)
 {
   MOZ_ASSERT(aFileSystem);
 
   nsCOMPtr<nsIFile> path;
-  aRv = NS_NewNativeLocalFile(NS_ConvertUTF16toUTF8(aFileSystem->GetLocalRootPath()),
+  aRv = NS_NewNativeLocalFile(NS_ConvertUTF16toUTF8(aFileSystem->LocalOrDeviceStorageRootPath()),
                               true, getter_AddRefs(path));
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   RefPtr<GetFileOrDirectoryTask> task =
     GetFileOrDirectoryTask::Create(aFileSystem, path, eDOMRootDirectory, true, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
@@ -397,29 +397,22 @@ Directory::SetContentFilters(const nsASt
 {
   mFilters = aFilters;
 }
 
 FileSystemBase*
 Directory::GetFileSystem(ErrorResult& aRv)
 {
   if (!mFileSystem) {
-    nsCOMPtr<nsIFile> parent;
-    aRv = mFile->GetParent(getter_AddRefs(parent));
-    if (NS_WARN_IF(aRv.Failed())) {
-      return nullptr;
-    }
-
-    // Parent can be null if mFile is pointing to the top directory.
-    if (!parent) {
-      parent = mFile;
-    }
+    // Any subdir inherits the FileSystem of the parent Directory. If we are
+    // here it's because we are dealing with the DOM root.
+    MOZ_ASSERT(mType == eDOMRootDirectory);
 
     nsAutoString path;
-    aRv = parent->GetPath(path);
+    aRv = mFile->GetPath(path);
     if (NS_WARN_IF(aRv.Failed())) {
       return nullptr;
     }
 
     RefPtr<OSFileSystem> fs = new OSFileSystem(path);
     fs->Init(mParent);
 
     mFileSystem = fs;
--- a/dom/filesystem/FileSystemBase.h
+++ b/dom/filesystem/FileSystemBase.h
@@ -42,20 +42,26 @@ public:
 
   /*
    * Get the virtual name of the root directory. This name will be exposed to
    * the content page.
    */
   virtual void
   GetRootName(nsAString& aRetval) const = 0;
 
+  /*
+   * Return the local root path of the FileSystem implementation.
+   * For OSFileSystem, this is equal to the path of the root Directory;
+   * For DeviceStorageFileSystem, this is the path of the SDCard, parent
+   * directory of the exposed root Directory (per type).
+   */
   const nsAString&
-  GetLocalRootPath() const
+  LocalOrDeviceStorageRootPath() const
   {
-    return mLocalRootPath;
+    return mLocalOrDeviceStorageRootPath;
   }
 
   bool
   IsShutdown() const
   {
     return mShutdown;
   }
 
@@ -87,19 +93,27 @@ public:
   virtual void Unlink() {}
   virtual void Traverse(nsCycleCollectionTraversalCallback &cb) {}
 
 protected:
   virtual ~FileSystemBase();
 
   // The local path of the root (i.e. the OS path, with OS path separators, of
   // the OS directory that acts as the root of this OSFileSystem).
-  // Only available in the parent process.
-  // In the child process, we don't use it and its value should be empty.
-  nsString mLocalRootPath;
+  // This path must be set by the FileSystem implementation immediately
+  // because it will be used for the validation of any FileSystemTaskBase.
+  // The concept of this path is that, any task will never go out of it and this
+  // must be considered the OS 'root' of the current FileSystem. Different
+  // Directory object can have different OS 'root' path.
+  // To be more clear, any path managed by this FileSystem implementation must
+  // be discendant of this local root path.
+  // The reason why it's not just called 'localRootPath' is because for
+  // DeviceStorage this contains the path of the device storage SDCard, that is
+  // the parent directory of the exposed root path.
+  nsString mLocalOrDeviceStorageRootPath;
 
   bool mShutdown;
 
   // The permission name required to access the file system.
   nsCString mPermission;
 
   bool mRequiresPermissionChecks;
 };
--- a/dom/filesystem/GetDirectoryListingTask.cpp
+++ b/dom/filesystem/GetDirectoryListingTask.cpp
@@ -341,17 +341,17 @@ GetDirectoryListingTask::HandlerCallback
       if (NS_WARN_IF(NS_FAILED(rv))) {
         mPromise->MaybeReject(rv);
         mPromise = nullptr;
         return;
       }
 
 #ifdef DEBUG
       nsCOMPtr<nsIFile> rootPath;
-      rv = NS_NewLocalFile(mFileSystem->GetLocalRootPath(), false,
+      rv = NS_NewLocalFile(mFileSystem->LocalOrDeviceStorageRootPath(), false,
                            getter_AddRefs(rootPath));
       if (NS_WARN_IF(NS_FAILED(rv))) {
         mPromise->MaybeReject(rv);
         mPromise = nullptr;
         return;
       }
 
       MOZ_ASSERT(FileSystemUtils::IsDescendantPath(rootPath, directoryPath));
--- a/dom/filesystem/OSFileSystem.cpp
+++ b/dom/filesystem/OSFileSystem.cpp
@@ -14,31 +14,31 @@
 #include "nsDebug.h"
 #include "nsIFile.h"
 
 namespace mozilla {
 namespace dom {
 
 OSFileSystem::OSFileSystem(const nsAString& aRootDir)
 {
-  mLocalRootPath = aRootDir;
+  mLocalOrDeviceStorageRootPath = aRootDir;
 
   // Non-mobile devices don't have the concept of separate permissions to
   // access different parts of devices storage like Pictures, or Videos, etc.
   mRequiresPermissionChecks = false;
 
 #ifdef DEBUG
   mPermission.AssignLiteral("never-used");
 #endif
 }
 
 already_AddRefed<FileSystemBase>
 OSFileSystem::Clone()
 {
-  RefPtr<OSFileSystem> fs = new OSFileSystem(mLocalRootPath);
+  RefPtr<OSFileSystem> fs = new OSFileSystem(mLocalOrDeviceStorageRootPath);
   if (mParent) {
     fs->Init(mParent);
   }
 
   return fs.forget();
 }
 
 void
@@ -99,13 +99,13 @@ OSFileSystem::Traverse(nsCycleCollection
 {
   OSFileSystem* tmp = this;
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent);
 }
 
 void
 OSFileSystem::SerializeDOMPath(nsAString& aOutput) const
 {
-  aOutput = mLocalRootPath;
+  aOutput = mLocalOrDeviceStorageRootPath;
 }
 
 } // namespace dom
 } // namespace mozilla