Bug 1258095 - patch 2/3 - Implement Directory::GetPath() correctly, r=smaug

dom/filesystem/Directory.cpp

@@ -355,12 +355,20 @@ Directory::RemoveInternal(const StringOrFileOrDirectory& aPath, bool aRecursive,
 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

dom/filesystem/Directory.h

@@ -167,6 +167,7 @@ private:
   DirectoryType mType;
 
   nsString mFilters;
+  nsString mPath;
 };
 
 } // namespace dom

dom/filesystem/FileSystemBase.cpp

@@ -99,5 +99,78 @@ 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

dom/filesystem/FileSystemBase.h

@@ -9,12 +9,12 @@
 
 #include "nsAutoPtr.h"
 #include "nsString.h"
+#include "Directory.h"
 
 namespace mozilla {
 namespace dom {
 
 class BlobImpl;
-class Directory;
 
 class FileSystemBase
 {
@@ -47,6 +47,10 @@ public:
   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;

dom/filesystem/tests/script_fileList.js

@@ -1,11 +1,23 @@
 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

dom/filesystem/tests/test_basic.html

@@ -12,7 +12,7 @@
 
 var directory;
 
-function create_fileList() {
+function create_fileList(aPath) {
   var url = SimpleTest.getTestFileURL("script_fileList.js");
   var script = SpecialPowers.loadChromeScript(url);
 
@@ -31,7 +31,7 @@ function create_fileList() {
   }
 
   script.addMessageListener("dir.opened", onOpened);
-  script.sendAsyncMessage("dir.open");
+  script.sendAsyncMessage("dir.open", { path: aPath });
 }
 
 function test_basic() {
@@ -48,15 +48,17 @@ function checkSubDir(dir) {
       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.");
@@ -65,8 +67,11 @@ function getFilesAndDirectories() {
         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]));
+          }
         }
       }
 
@@ -79,11 +84,13 @@ function getFilesAndDirectories() {
 }
 
 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() {