Deprecate FileSystemMountPointProvider::GetFileSystemRootPathOnFileThread

This was initially introduced to get ExternalFileSystem's
local mount root path, but accessing MountPointProvider
from lower-layer FileUtil module looks ugly.

This patch:
- Deprecate FileSystemMountPointProvider::GetFileSystemRootPathOnFileThread
- Add FileSystemOperationContext::root_path() instead for passing root path

BUG=241701
TEST=Moved tests from content_unittests:FileSystemMountPointProviderTest.*
 to content_unittests:SandboxMountPointProviderTest.*

R=bauerb@chromium.org, michaeln@chromium.org, tzik@chromium.org

Review URL: https://codereview.chromium.org/15959006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202776 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/browsing_data/browsing_data_file_system_helper_unittest.cc

@@ -173,9 +173,8 @@ class BrowsingDataFileSystemHelperTest : public testing::Test {
   // specified origin.
   void CreateDirectoryForOriginAndType(const GURL& origin,
                                        fileapi::FileSystemType type) {
-    base::FilePath target = sandbox_->GetFileSystemRootPathOnFileThread(
-        fileapi::FileSystemURL::CreateForTest(origin, type, base::FilePath()),
-        true);
+    base::FilePath target = sandbox_->GetBaseDirectoryForOriginAndType(
+        origin, type, true /* create */);
     EXPECT_TRUE(file_util::DirectoryExists(target));
   }
 

chrome/browser/media_galleries/fileapi/media_file_system_mount_point_provider.cc

@@ -83,15 +83,6 @@ void MediaFileSystemMountPointProvider::ValidateFileSystemRoot(
       base::Bind(callback, base::PLATFORM_FILE_ERROR_SECURITY));
 }
 
-base::FilePath
-MediaFileSystemMountPointProvider::GetFileSystemRootPathOnFileThread(
-    const FileSystemURL& url,
-    bool create) {
-  // This is not supposed to be used.
-  NOTREACHED();
-  return base::FilePath();
-}
-
 fileapi::FileSystemFileUtil* MediaFileSystemMountPointProvider::GetFileUtil(
     fileapi::FileSystemType type) {
   switch (type) {

chrome/browser/media_galleries/fileapi/media_file_system_mount_point_provider.h

@@ -35,9 +35,6 @@ class MediaFileSystemMountPointProvider
       fileapi::FileSystemType type,
       bool create,
       const ValidateFileSystemCallback& callback) OVERRIDE;
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const fileapi::FileSystemURL& url,
-      bool create) OVERRIDE;
   virtual fileapi::FileSystemFileUtil* GetFileUtil(
       fileapi::FileSystemType type) OVERRIDE;
   virtual fileapi::AsyncFileUtil* GetAsyncFileUtil(

webkit/browser/chromeos/fileapi/cros_mount_point_provider.cc

@@ -100,23 +100,6 @@ void CrosMountPointProvider::ValidateFileSystemRoot(
   callback.Run(base::PLATFORM_FILE_OK);
 }
 
-base::FilePath CrosMountPointProvider::GetFileSystemRootPathOnFileThread(
-    const fileapi::FileSystemURL& url,
-    bool create) {
-  DCHECK(fileapi::IsolatedContext::IsIsolatedType(url.mount_type()));
-  if (!url.is_valid())
-    return base::FilePath();
-
-  base::FilePath root_path;
-  std::string mount_name = url.filesystem_id();
-  if (!mount_points_->GetRegisteredPath(mount_name, &root_path) &&
-      !system_mount_points_->GetRegisteredPath(mount_name, &root_path)) {
-    return base::FilePath();
-  }
-
-  return root_path.DirName();
-}
-
 fileapi::FileSystemQuotaUtil* CrosMountPointProvider::GetQuotaUtil() {
   // No quota support.
   return NULL;
@@ -288,6 +271,7 @@ fileapi::FileSystemOperation* CrosMountPointProvider::CreateFileSystemOperation(
          url.type() == fileapi::kFileSystemTypeRestrictedNativeLocal);
   scoped_ptr<fileapi::FileSystemOperationContext> operation_context(
       new fileapi::FileSystemOperationContext(context));
+  operation_context->set_root_path(GetFileSystemRootPath(url));
   return new fileapi::LocalFileSystemOperation(context,
                                                operation_context.Pass());
 }
@@ -355,4 +339,20 @@ fileapi::RemoteFileSystemProxyInterface* CrosMountPointProvider::GetRemoteProxy(
   return system_mount_points_->GetRemoteFileSystemProxy(mount_name);
 }
 
+base::FilePath CrosMountPointProvider::GetFileSystemRootPath(
+    const fileapi::FileSystemURL& url) const {
+  DCHECK(fileapi::IsolatedContext::IsIsolatedType(url.mount_type()));
+  if (!url.is_valid())
+    return base::FilePath();
+
+  base::FilePath root_path;
+  std::string mount_name = url.filesystem_id();
+  if (!mount_points_->GetRegisteredPath(mount_name, &root_path) &&
+      !system_mount_points_->GetRegisteredPath(mount_name, &root_path)) {
+    return base::FilePath();
+  }
+
+  return root_path.DirName();
+}
+
 }  // namespace chromeos

webkit/browser/chromeos/fileapi/cros_mount_point_provider.h

@@ -59,9 +59,6 @@ class WEBKIT_STORAGE_EXPORT CrosMountPointProvider
       fileapi::FileSystemType type,
       bool create,
       const ValidateFileSystemCallback& callback) OVERRIDE;
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const fileapi::FileSystemURL& url,
-      bool create) OVERRIDE;
   virtual fileapi::FileSystemFileUtil* GetFileUtil(
       fileapi::FileSystemType type) OVERRIDE;
   virtual fileapi::AsyncFileUtil* GetAsyncFileUtil(
@@ -113,6 +110,7 @@ class WEBKIT_STORAGE_EXPORT CrosMountPointProvider
  private:
   fileapi::RemoteFileSystemProxyInterface* GetRemoteProxy(
       const std::string& mount_name) const;
+  base::FilePath GetFileSystemRootPath(const fileapi::FileSystemURL& url) const;
 
   scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy_;
   scoped_ptr<FileAccessPermissions> file_access_permissions_;

webkit/browser/fileapi/copy_or_move_file_validator_unittest.cc

@@ -27,6 +27,10 @@ namespace {
 const FileSystemType kNoValidatorType = kFileSystemTypeTemporary;
 const FileSystemType kWithValidatorType = kFileSystemTypeTest;
 
+void ExpectOk(base::PlatformFileError error) {
+  ASSERT_EQ(base::PLATFORM_FILE_OK, error);
+}
+
 class CopyOrMoveFileValidatorTestHelper {
  public:
   CopyOrMoveFileValidatorTestHelper(
@@ -57,8 +61,9 @@ class CopyOrMoveFileValidatorTestHelper {
     // Sets up source.
     FileSystemMountPointProvider* src_mount_point_provider =
         file_system_context_->GetMountPointProvider(src_type_);
-    src_mount_point_provider->GetFileSystemRootPathOnFileThread(
-        SourceURL(std::string()), true /* create */);
+    src_mount_point_provider->ValidateFileSystemRoot(
+        origin_, src_type_, true /* create */, base::Bind(&ExpectOk));
+    base::MessageLoop::current()->RunUntilIdle();
     ASSERT_EQ(base::PLATFORM_FILE_OK, CreateDirectory(SourceURL("")));
 
     // Sets up dest.

webkit/browser/fileapi/file_system_mount_point_provider.h

@@ -62,14 +62,6 @@ class WEBKIT_STORAGE_EXPORT FileSystemMountPointProvider {
       bool create,
       const ValidateFileSystemCallback& callback) = 0;
 
-  // Retrieves the root path of the filesystem specified by the given
-  // file system url on the file thread.
-  // If |create| is true this may also create the root directory for
-  // the filesystem if it doesn't exist.
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const FileSystemURL& url,
-      bool create) = 0;
-
   // Returns the specialized FileSystemFileUtil for this mount point.
   // It is ok to return NULL if the filesystem doesn't support synchronous
   // version of FileUtil.

webkit/browser/fileapi/file_system_operation_context.h

@@ -5,6 +5,7 @@
 #ifndef WEBKIT_BROWSER_FILEAPI_FILE_SYSTEM_OPERATION_CONTEXT_H_
 #define WEBKIT_BROWSER_FILEAPI_FILE_SYSTEM_OPERATION_CONTEXT_H_
 
+#include "base/files/file_path.h"
 #include "base/supports_user_data.h"
 #include "base/threading/thread_checker.h"
 #include "webkit/browser/fileapi/task_runner_bound_observer_list.h"
@@ -48,15 +49,9 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext
 
   // Returns the current remaining quota.
   int64 allowed_bytes_growth() const { return allowed_bytes_growth_; }
-
-  quota::QuotaLimitType quota_limit_type() const {
-    return quota_limit_type_;
-  }
-
-  // Returns TaskRunner which the operation is performed on.
-  base::SequencedTaskRunner* task_runner() const {
-    return task_runner_.get();
-  }
+  quota::QuotaLimitType quota_limit_type() const { return quota_limit_type_; }
+  base::SequencedTaskRunner* task_runner() const { return task_runner_.get(); }
+  const base::FilePath& root_path() const { return root_path_; }
 
   ChangeObserverList* change_observers() { return &change_observers_; }
   AccessObserverList* access_observers() { return &access_observers_; }
@@ -81,6 +76,10 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext
     DCHECK(setter_thread_checker_.CalledOnValidThread());
     quota_limit_type_ = limit_type;
   }
+  void set_root_path(const base::FilePath& root_path) {
+    DCHECK(setter_thread_checker_.CalledOnValidThread());
+    root_path_ = root_path;
+  }
 
   // Gets and sets value-type (or not-owned) variable as UserData.
   // (SetUserValue can be called only on the same thread as this context
@@ -109,13 +108,20 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE FileSystemOperationContext
   FileSystemContext* file_system_context_;
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
+  // The current remaining quota, used by ObfuscatedFileUtil.
   int64 allowed_bytes_growth_;
+
+  // The current quota limit type, used by ObfuscatedFileUtil.
   quota::QuotaLimitType quota_limit_type_;
 
+  // Observers attached to this context.
   AccessObserverList access_observers_;
   ChangeObserverList change_observers_;
   UpdateObserverList update_observers_;
 
+  // Root path for the operation, used by LocalFileUtil.
+  base::FilePath root_path_;
+
   // Used to check its setters are not called on arbitrary thread.
   base::ThreadChecker setter_thread_checker_;
 

webkit/browser/fileapi/file_writer_delegate_unittest.cc

@@ -117,6 +117,7 @@ class FileWriterDelegateTest : public PlatformTest {
       new FileSystemOperationContext(file_system_context_);
     context->set_update_observers(
         *file_system_context_->GetUpdateObservers(kFileSystemType));
+    context->set_root_path(dir_.path());
     return make_scoped_ptr(context);
   }
 

webkit/browser/fileapi/isolated_mount_point_provider.cc

@@ -67,14 +67,6 @@ void IsolatedMountPointProvider::ValidateFileSystemRoot(
       base::Bind(callback, base::PLATFORM_FILE_ERROR_SECURITY));
 }
 
-base::FilePath IsolatedMountPointProvider::GetFileSystemRootPathOnFileThread(
-    const FileSystemURL& url,
-    bool create) {
-  // This is not supposed to be used.
-  NOTREACHED();
-  return base::FilePath();
-}
-
 FileSystemFileUtil* IsolatedMountPointProvider::GetFileUtil(
     FileSystemType type) {
   switch (type) {

webkit/browser/fileapi/isolated_mount_point_provider.h

@@ -24,9 +24,6 @@ class IsolatedMountPointProvider : public FileSystemMountPointProvider {
       FileSystemType type,
       bool create,
       const ValidateFileSystemCallback& callback) OVERRIDE;
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const FileSystemURL& url,
-      bool create) OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
   virtual AsyncFileUtil* GetAsyncFileUtil(FileSystemType type) OVERRIDE;
   virtual CopyOrMoveFileValidatorFactory* GetCopyOrMoveFileValidatorFactory(

webkit/browser/fileapi/local_file_system_cross_operation_unittest.cc

@@ -27,6 +27,14 @@ namespace fileapi {
 
 typedef FileSystemOperation::FileEntryList FileEntryList;
 
+namespace {
+
+void ExpectOk(base::PlatformFileError error) {
+  ASSERT_EQ(base::PLATFORM_FILE_OK, error);
+}
+
+}  // namespace
+
 class CrossOperationTestHelper {
  public:
   CrossOperationTestHelper(
@@ -63,12 +71,13 @@ class CrossOperationTestHelper {
     // Prepare the origin's root directory.
     FileSystemMountPointProvider* mount_point_provider =
         file_system_context_->GetMountPointProvider(src_type_);
-    mount_point_provider->GetFileSystemRootPathOnFileThread(
-        SourceURL(std::string()), true /* create */);
+    mount_point_provider->ValidateFileSystemRoot(
+        origin_, src_type_, true /* create */, base::Bind(&ExpectOk));
     mount_point_provider =
         file_system_context_->GetMountPointProvider(dest_type_);
-    mount_point_provider->GetFileSystemRootPathOnFileThread(
-        DestURL(std::string()), true /* create */);
+    mount_point_provider->ValidateFileSystemRoot(
+        origin_, dest_type_, true /* create */, base::Bind(&ExpectOk));
+    base::MessageLoop::current()->RunUntilIdle();
 
     // Grant relatively big quota initially.
     quota_manager_->SetQuota(origin_,

webkit/browser/fileapi/local_file_util.cc

@@ -164,7 +164,7 @@ PlatformFileError LocalFileUtil::GetLocalFilePath(
   FileSystemMountPointProvider* provider =
       context->file_system_context()->GetMountPointProvider(url.type());
   DCHECK(provider);
-  base::FilePath root = provider->GetFileSystemRootPathOnFileThread(url, false);
+  base::FilePath root = context->root_path();
   if (root.empty())
     return base::PLATFORM_FILE_ERROR_NOT_FOUND;
   *local_file_path = root.Append(url.path());

webkit/browser/fileapi/local_file_util_unittest.cc

@@ -52,6 +52,7 @@ class LocalFileUtilTest : public testing::Test {
       new FileSystemOperationContext(file_system_context_);
     context->set_update_observers(
         *file_system_context_->GetUpdateObservers(kFileSystemType));
+    context->set_root_path(data_dir_.path());
     return context;
   }
 

webkit/browser/fileapi/sandbox_file_system_test_helper.cc

@@ -61,7 +61,7 @@ void SandboxFileSystemTestHelper::TearDown() {
   base::MessageLoop::current()->RunUntilIdle();
 }
 
-base::FilePath SandboxFileSystemTestHelper::GetOriginRootPath() const {
+base::FilePath SandboxFileSystemTestHelper::GetOriginRootPath() {
   return file_system_context_->sandbox_provider()->
       GetBaseDirectoryForOriginAndType(origin_, type_, false);
 }
@@ -104,7 +104,7 @@ int64 SandboxFileSystemTestHelper::ComputeCurrentOriginUsage() {
 }
 
 int64
-SandboxFileSystemTestHelper::ComputeCurrentDirectoryDatabaseUsage() const {
+SandboxFileSystemTestHelper::ComputeCurrentDirectoryDatabaseUsage() {
   return file_util::ComputeDirectorySize(
       GetOriginRootPath().AppendASCII("Paths"));
 }
@@ -141,8 +141,7 @@ void SandboxFileSystemTestHelper::SetUpFileSystem() {
 
   // Prepare the origin's root directory.
   file_system_context_->sandbox_provider()->
-      GetFileSystemRootPathOnFileThread(CreateURL(base::FilePath()),
-                                        true /* create */);
+      GetBaseDirectoryForOriginAndType(origin_, type_, true /* create */);
 
   // Initialize the usage cache file.
   base::FilePath usage_cache_path = GetUsageCachePath();

webkit/browser/fileapi/sandbox_file_system_test_helper.h

@@ -50,7 +50,7 @@ class SandboxFileSystemTestHelper {
              quota::QuotaManagerProxy* quota_manager_proxy);
   void TearDown();
 
-  base::FilePath GetOriginRootPath() const;
+  base::FilePath GetOriginRootPath();
   base::FilePath GetLocalPath(const base::FilePath& path);
   base::FilePath GetLocalPathFromASCII(const std::string& path);
 
@@ -68,7 +68,7 @@ class SandboxFileSystemTestHelper {
   // This doesn't work with OFSFU.
   int64 ComputeCurrentOriginUsage();
 
-  int64 ComputeCurrentDirectoryDatabaseUsage() const;
+  int64 ComputeCurrentDirectoryDatabaseUsage();
 
   LocalFileSystemOperation* NewOperation();
   FileSystemOperationContext* NewOperationContext();

webkit/browser/fileapi/sandbox_mount_point_provider.cc

@@ -242,23 +242,6 @@ void SandboxMountPointProvider::ValidateFileSystemRoot(
                  file_system_usage_cache_.get()));
 };
 
-base::FilePath
-SandboxMountPointProvider::GetFileSystemRootPathOnFileThread(
-    const FileSystemURL& url,
-    bool create) {
-  if (file_system_options_.is_incognito() &&
-      !(enable_temporary_file_system_in_incognito_ &&
-        url.type() == kFileSystemTypeTemporary)) {
-    // TODO(kinuko): return an isolated temporary directory.
-    return base::FilePath();
-  }
-
-  if (!IsAllowedScheme(url.origin()))
-    return base::FilePath();
-
-  return GetBaseDirectoryForOriginAndType(url.origin(), url.type(), create);
-}
-
 FileSystemFileUtil* SandboxMountPointProvider::GetFileUtil(
     FileSystemType type) {
   DCHECK(sandbox_file_util_.get());

webkit/browser/fileapi/sandbox_mount_point_provider.h

@@ -88,9 +88,6 @@ class WEBKIT_STORAGE_EXPORT SandboxMountPointProvider
       FileSystemType type,
       bool create,
       const ValidateFileSystemCallback& callback) OVERRIDE;
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const FileSystemURL& url,
-      bool create) OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
   virtual AsyncFileUtil* GetAsyncFileUtil(FileSystemType type) OVERRIDE;
   virtual CopyOrMoveFileValidatorFactory* GetCopyOrMoveFileValidatorFactory(

webkit/browser/fileapi/test_mount_point_provider.cc

@@ -96,21 +96,7 @@ void TestMountPointProvider::ValidateFileSystemRoot(
     FileSystemType type,
     bool create,
     const ValidateFileSystemCallback& callback) {
-  // This won't be called unless we add test code that opens a test
-  // filesystem by OpenFileSystem.
-  NOTREACHED();
-}
-
-base::FilePath TestMountPointProvider::GetFileSystemRootPathOnFileThread(
-    const FileSystemURL& url,
-    bool create) {
-  DCHECK_EQ(kFileSystemTypeTest, url.type());
-  bool success = true;
-  if (create)
-    success = file_util::CreateDirectory(base_path_);
-  else
-    success = file_util::DirectoryExists(base_path_);
-  return success ? base_path_ : base::FilePath();
+  callback.Run(base::PLATFORM_FILE_OK);
 }
 
 FileSystemFileUtil* TestMountPointProvider::GetFileUtil(FileSystemType type) {
@@ -157,6 +143,7 @@ FileSystemOperation* TestMountPointProvider::CreateFileSystemOperation(
   scoped_ptr<FileSystemOperationContext> operation_context(
       new FileSystemOperationContext(context));
   operation_context->set_update_observers(observers_);
+  operation_context->set_root_path(base_path_);
   return new LocalFileSystemOperation(context, operation_context.Pass());
 }
 

webkit/browser/fileapi/test_mount_point_provider.h

@@ -40,9 +40,6 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE TestMountPointProvider
       FileSystemType type,
       bool create,
       const ValidateFileSystemCallback& callback) OVERRIDE;
-  virtual base::FilePath GetFileSystemRootPathOnFileThread(
-      const FileSystemURL& url,
-      bool create) OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
   virtual AsyncFileUtil* GetAsyncFileUtil(FileSystemType type) OVERRIDE;
   virtual CopyOrMoveFileValidatorFactory* GetCopyOrMoveFileValidatorFactory(