From 782fdaa5efa0334cc784f0791efe50109f726221 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Wed, 30 Mar 2016 07:17:56 +0100 Subject: [PATCH] Bug 1258095 - patch 2/3 - Implement Directory::GetPath() correctly, r=smaug --- dom/filesystem/Directory.cpp | 18 ++++-- dom/filesystem/Directory.h | 1 + dom/filesystem/FileSystemBase.cpp | 73 +++++++++++++++++++++++++ dom/filesystem/FileSystemBase.h | 6 +- dom/filesystem/tests/script_fileList.js | 16 +++++- dom/filesystem/tests/test_basic.html | 27 +++++---- 6 files changed, 123 insertions(+), 18 deletions(-) diff --git a/dom/filesystem/Directory.cpp b/dom/filesystem/Directory.cpp index f4fd5bd4c150..db824051edd8 100644 --- a/dom/filesystem/Directory.cpp +++ b/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 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 diff --git a/dom/filesystem/Directory.h b/dom/filesystem/Directory.h index 5e6ac2deef62..99cc6236cb06 100644 --- a/dom/filesystem/Directory.h +++ b/dom/filesystem/Directory.h @@ -167,6 +167,7 @@ private: DirectoryType mType; nsString mFilters; + nsString mPath; }; } // namespace dom diff --git a/dom/filesystem/FileSystemBase.cpp b/dom/filesystem/FileSystemBase.cpp index 3d6e4529ae39..6f45ea10f6cd 100644 --- a/dom/filesystem/FileSystemBase.cpp +++ b/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 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 path; + aRv = aFile->Clone(getter_AddRefs(path)); + if (NS_WARN_IF(aRv.Failed())) { + return; + } + + nsTArray 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 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 diff --git a/dom/filesystem/FileSystemBase.h b/dom/filesystem/FileSystemBase.h index 34725fc8fdec..a3cbfcecb0f3 100644 --- a/dom/filesystem/FileSystemBase.h +++ b/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; diff --git a/dom/filesystem/tests/script_fileList.js b/dom/filesystem/tests/script_fileList.js index 81fd6b38d1ad..f7fbca2e9d39 100644 --- a/dom/filesystem/tests/script_fileList.js +++ b/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 diff --git a/dom/filesystem/tests/test_basic.html b/dom/filesystem/tests/test_basic.html index 1b9f355b81a9..1a929f56a821 100644 --- a/dom/filesystem/tests/test_basic.html +++ b/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() {