Bug 1258095 - patch 2/3 - Implement Directory::GetPath() correctly, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 30 Mar 2016 07:17:56 +0100
changeset 291018 554e58d8b27df2d58241421f8de97b1502e60702
parent 291017 1910aa3c10fc6dd93c7d49a59d6c8290587a8936
child 291019 7f1ca2749ce7060fef60202c3125cad9e448436b
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 2/3 - Implement Directory::GetPath() correctly, r=smaug
dom/filesystem/Directory.cpp
dom/filesystem/Directory.h
dom/filesystem/FileSystemBase.cpp
dom/filesystem/FileSystemBase.h
dom/filesystem/tests/script_fileList.js
dom/filesystem/tests/test_basic.html
--- a/dom/filesystem/Directory.cpp
+++ b/dom/filesystem/Directory.cpp
@@ -350,22 +350,30 @@ Directory::RemoveInternal(const StringOr
   task->SetError(error);
   FileSystemPermissionRequest::RequestForTask(task);
   return task->GetPromise();
 }
 
 void
 Directory::GetPath(nsAString& aRetval, ErrorResult& aRv)
 {
-  if (mType == eDOMRootDirectory) {
-    aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL);
-  } else {
-    // TODO: this should be a bit different...
-    GetName(aRetval, aRv);
+  // This operation is expensive. Better to cache the result.
+  if (mPath.IsEmpty()) {
+    RefPtr<FileSystemBase> fs = GetFileSystem(aRv);
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
+
+    fs->GetDOMPath(mFile, mType, mPath, aRv);
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
   }
+
+  aRetval = mPath;
 }
 
 nsresult
 Directory::GetFullRealPath(nsAString& aPath)
 {
   nsresult rv = mFile->GetPath(aPath);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
--- a/dom/filesystem/Directory.h
+++ b/dom/filesystem/Directory.h
@@ -162,14 +162,15 @@ private:
                  ErrorResult& aRv);
 
   nsCOMPtr<nsISupports> mParent;
   RefPtr<FileSystemBase> mFileSystem;
   nsCOMPtr<nsIFile> mFile;
   DirectoryType mType;
 
   nsString mFilters;
+  nsString mPath;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_Directory_h
--- a/dom/filesystem/FileSystemBase.cpp
+++ b/dom/filesystem/FileSystemBase.cpp
@@ -94,10 +94,83 @@ FileSystemBase::IsSafeFile(nsIFile* aFil
 }
 
 bool
 FileSystemBase::IsSafeDirectory(Directory* aDir) const
 {
   return false;
 }
 
+void
+FileSystemBase::GetDOMPath(nsIFile* aFile,
+                           Directory::DirectoryType aType,
+                           nsAString& aRetval,
+                           ErrorResult& aRv) const
+{
+  MOZ_ASSERT(aFile);
+
+  if (aType == Directory::eDOMRootDirectory) {
+    aRetval.AssignLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL);
+    return;
+  }
+
+  nsCOMPtr<nsIFile> fileSystemPath;
+  aRv = NS_NewNativeLocalFile(NS_ConvertUTF16toUTF8(LocalOrDeviceStorageRootPath()),
+                              true, getter_AddRefs(fileSystemPath));
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+
+  MOZ_ASSERT(FileSystemUtils::IsDescendantPath(fileSystemPath, aFile));
+
+  nsCOMPtr<nsIFile> path;
+  aRv = aFile->Clone(getter_AddRefs(path));
+  if (NS_WARN_IF(aRv.Failed())) {
+    return;
+  }
+
+  nsTArray<nsString> parts;
+
+  while (true) {
+    bool equal = false;
+    aRv = fileSystemPath->Equals(path, &equal);
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
+
+    if (equal) {
+      break;
+    }
+
+    nsAutoString leafName;
+    aRv = path->GetLeafName(leafName);
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
+
+    parts.AppendElement(leafName);
+
+    nsCOMPtr<nsIFile> parentPath;
+    aRv = path->GetParent(getter_AddRefs(parentPath));
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
+
+    MOZ_ASSERT(parentPath);
+
+    aRv = parentPath->Clone(getter_AddRefs(path));
+    if (NS_WARN_IF(aRv.Failed())) {
+      return;
+    }
+  }
+
+  MOZ_ASSERT(!parts.IsEmpty());
+
+  aRetval.Truncate();
+
+  for (int32_t i = parts.Length() - 1; i >= 0; --i) {
+    aRetval.AppendLiteral(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL);
+    aRetval.Append(parts[i]);
+  }
+}
+
 } // namespace dom
 } // namespace mozilla
--- a/dom/filesystem/FileSystemBase.h
+++ b/dom/filesystem/FileSystemBase.h
@@ -4,22 +4,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * 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"
+#include "Directory.h"
 
 namespace mozilla {
 namespace dom {
 
 class BlobImpl;
-class Directory;
 
 class FileSystemBase
 {
   NS_INLINE_DECL_REFCOUNTING(FileSystemBase)
 public:
 
   // Create file system object from its string representation.
   static already_AddRefed<FileSystemBase>
@@ -42,16 +42,20 @@ 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;
 
+  void
+  GetDOMPath(nsIFile* aFile, Directory::DirectoryType aType,
+             nsAString& aRetval, ErrorResult& aRv) const;
+
   /*
    * 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&
   LocalOrDeviceStorageRootPath() const
--- a/dom/filesystem/tests/script_fileList.js
+++ b/dom/filesystem/tests/script_fileList.js
@@ -1,13 +1,25 @@
 var { classes: Cc, interfaces: Ci, utils: Cu } = Components;
 Cu.importGlobalProperties(["File"]);
 
-addMessageListener("dir.open", function () {
+addMessageListener("dir.open", function (e) {
   var testFile = Cc["@mozilla.org/file/directory_service;1"]
                    .getService(Ci.nsIDirectoryService)
                    .QueryInterface(Ci.nsIProperties)
-                   .get("ProfD", Ci.nsIFile);
+                   .get(e.path == 'root' ? 'ProfD' : e.path, Ci.nsIFile);
+
+  // Let's go back to the root of the FileSystem
+  if (e.path == 'root') {
+    while (true) {
+      var parent = testFile.parent;
+      if (!parent) {
+        break;
+      }
+
+      testFile = parent;
+    }
+  }
 
   sendAsyncMessage("dir.opened", {
     dir: testFile.path
   });
 });
--- a/dom/filesystem/tests/test_basic.html
+++ b/dom/filesystem/tests/test_basic.html
@@ -7,17 +7,17 @@
 </head>
 
 <body>
 <input id="fileList" type="file"></input>
 <script type="application/javascript;version=1.7">
 
 var directory;
 
-function create_fileList() {
+function create_fileList(aPath) {
   var url = SimpleTest.getTestFileURL("script_fileList.js");
   var script = SpecialPowers.loadChromeScript(url);
 
   function onOpened(message) {
     var fileList = document.getElementById('fileList');
     SpecialPowers.wrap(fileList).mozSetDirectory(message.dir);
 
     // Just a simple test
@@ -26,69 +26,76 @@ function create_fileList() {
 
     directory = fileList.files[0];
 
     script.destroy();
     next();
   }
 
   script.addMessageListener("dir.opened", onOpened);
-  script.sendAsyncMessage("dir.open");
+  script.sendAsyncMessage("dir.open", { path: aPath });
 }
 
 function test_basic() {
   ok(directory, "Directory exists.");
   ok(directory instanceof Directory, "We have a directory.");
   is(directory.name, '/', "directory.name must be '/'");
   is(directory.path, '/', "directory.path must be '/'");
   next();
 }
 
 function checkSubDir(dir) {
   return dir.getFilesAndDirectories().then(
     function(data) {
       for (var i = 0; i < data.length; ++i) {
         ok (data[i] instanceof File || data[i] instanceof Directory, "Just Files or Directories");
         if (data[i] instanceof Directory) {
-          isnot(data[i].name, '/', "Subdirectory should be called with the leafname");
-          isnot(data[i].path, '/', "Subdirectory path should be called with the leafname");
+          isnot(data[i].name, '/', "Subdirectory should be called with the leafname: " + data[i].name);
+          isnot(data[i].path, '/', "Subdirectory path should be called with the leafname:" + data[i].path);
+          isnot(data[i].path, dir.path, "Subdirectory path should contain the parent path.");
+          is(data[i].path,dir.path + '/' + data[i].name, "Subdirectory path should be called parentdir.path + '/' + leafname");
         }
       }
     }
   );
 }
 
-function getFilesAndDirectories() {
+function getFilesAndDirectories(aRecursive) {
   directory.getFilesAndDirectories().then(
     function(data) {
       ok(data.length, "We should have some data.");
       var promises = [];
       for (var i = 0; i < data.length; ++i) {
         ok (data[i] instanceof File || data[i] instanceof Directory, "Just Files or Directories");
         if (data[i] instanceof Directory) {
           isnot(data[i].name, '/', "Subdirectory should be called with the leafname");
-          isnot(data[i].path, '/', "Subdirectory path should be called with the leafname");
-          promises.push(checkSubDir(data[i]));
+          is(data[i].path, '/' + data[i].name, "Subdirectory path should be called '/' + leafname");
+
+          if (aRecursive) {
+            promises.push(checkSubDir(data[i]));
+          }
         }
       }
 
       return Promise.all(promises);
     },
     function() {
       ok(false, "Something when wrong");
     }
   ).then(next);
 }
 
 var tests = [
-  create_fileList,
-
+  function() { create_fileList('ProfD') },
   test_basic,
+  function() { getFilesAndDirectories(true) },
 
-  getFilesAndDirectories,
+  function() { create_fileList('root') },
+  test_basic,
+  function() { getFilesAndDirectories(false) },
 ];
 
 function next() {
   if (!tests.length) {
     SimpleTest.finish();
     return;
   }