Extract external file systems handling from isolated context.

webkit/chromeos/fileapi/cros_mount_point_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/chromeos/fileapi/cros_mount_point_provider.h"
 
 #include "base/chromeos/chromeos_version.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
-#include "base/path_service.h"
 #include "base/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/utf_string_conversions.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebCString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebFileSystem.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebString.h"
 #include "webkit/chromeos/fileapi/file_access_permissions.h"
 #include "webkit/chromeos/fileapi/remote_file_stream_writer.h"
 #include "webkit/chromeos/fileapi/remote_file_system_operation.h"
+#include "webkit/fileapi/external_mount_points.h"
 #include "webkit/fileapi/file_system_file_stream_reader.h"
 #include "webkit/fileapi/file_system_operation_context.h"
 #include "webkit/fileapi/file_system_url.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/fileapi/isolated_context.h"
 #include "webkit/fileapi/isolated_file_util.h"
 #include "webkit/fileapi/local_file_stream_writer.h"
 #include "webkit/fileapi/local_file_system_operation.h"
 #include "webkit/glue/webkit_glue.h"
 
 namespace {
 
 const char kChromeUIScheme[] = "chrome";
 
 }  // namespace
 
 namespace chromeos {
 
 // static
 bool CrosMountPointProvider::CanHandleURL(const fileapi::FileSystemURL& url) {
   if (!url.is_valid())
     return false;
   return url.type() == fileapi::kFileSystemTypeNativeLocal ||
          url.type() == fileapi::kFileSystemTypeRestrictedNativeLocal ||
          url.type() == fileapi::kFileSystemTypeDrive;
 }
 
 CrosMountPointProvider::CrosMountPointProvider(
-    scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy)
+    scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy,
+    scoped_refptr<fileapi::ExternalMountPoints> mount_points,
+    fileapi::ExternalMountPoints* system_mount_points)

[kinuko, 2013/01/08 12:22:43]

Once we feel more comfortable about the basic architecture can we drop changes
related to separating the external mount points code into system and per-context
to make the final cl clearer?  (Having them is helpful for now as we can see how
the final code should look)

[tbarzic, 2013/01/09 01:26:34]

I think we'll still have to do this somewhere, but yeah, I think it would be
nicer not to do it here (maybe ExternalMountPoints?).

I'm keeping the logic here for now because I feel things could get little
subtler..

     : special_storage_policy_(special_storage_policy),
       file_access_permissions_(new FileAccessPermissions()),
-      local_file_util_(new fileapi::IsolatedFileUtil()) {
-  FilePath home_path;
-  if (PathService::Get(base::DIR_HOME, &home_path))
-    AddLocalMountPoint(home_path.AppendASCII("Downloads"));
-  AddLocalMountPoint(FilePath(FILE_PATH_LITERAL("/media/archive")));
-  AddLocalMountPoint(FilePath(FILE_PATH_LITERAL("/media/removable")));
-  AddRestrictedLocalMountPoint(FilePath(FILE_PATH_LITERAL("/usr/share/oem")));
+      local_file_util_(new fileapi::IsolatedFileUtil()),
+      mount_points_(mount_points),
+      system_mount_points_(system_mount_points) {
 }
 
 CrosMountPointProvider::~CrosMountPointProvider() {
 }
 
 void CrosMountPointProvider::ValidateFileSystemRoot(
     const GURL& origin_url,
     fileapi::FileSystemType type,
     bool create,
     const ValidateFileSystemCallback& callback) {
   DCHECK(fileapi::IsolatedContext::IsIsolatedType(type));
   // Nothing to validate for external filesystem.
   callback.Run(base::PLATFORM_FILE_OK);
 }
 
 FilePath CrosMountPointProvider::GetFileSystemRootPathOnFileThread(
     const fileapi::FileSystemURL& url,
     bool create) {
   DCHECK(fileapi::IsolatedContext::IsIsolatedType(url.mount_type()));
   if (!url.is_valid())
     return FilePath();
 
   FilePath root_path;
-  if (!isolated_context()->GetRegisteredPath(url.filesystem_id(), &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 FilePath();
+  }
 
   return root_path.DirName();
 }
 
 bool CrosMountPointProvider::IsAccessAllowed(
     const fileapi::FileSystemURL& url) {
   if (!CanHandleURL(url))
     return false;
 
   // No extra check is needed for isolated file systems.
@@ skipping 26 matching lines @@
 
 void CrosMountPointProvider::DeleteFileSystem(
     const GURL& origin_url,
     fileapi::FileSystemType type,
     fileapi::FileSystemContext* context,
     const DeleteFileSystemCallback& callback) {
   NOTREACHED();
   callback.Run(base::PLATFORM_FILE_ERROR_INVALID_OPERATION);
 }
 
-bool CrosMountPointProvider::HasMountPoint(const FilePath& mount_point) {
+bool CrosMountPointProvider::HasMountPoint(const FilePath& mount_point) const {
   std::string mount_name = mount_point.BaseName().AsUTF8Unsafe();
   FilePath path;
-  const bool valid = isolated_context()->GetRegisteredPath(mount_name, &path);
+
+  const bool valid = mount_points_->GetRegisteredPath(mount_name, &path);
   return valid && path == mount_point;
 }
 
-void CrosMountPointProvider::AddLocalMountPoint(const FilePath& mount_point) {
+bool CrosMountPointProvider::AddLocalMountPoint(const FilePath& mount_point) {
   std::string mount_name = mount_point.BaseName().AsUTF8Unsafe();
-  isolated_context()->RevokeFileSystem(mount_name);
-  isolated_context()->RegisterExternalFileSystem(
-      mount_name,
-      fileapi::kFileSystemTypeNativeLocal,
-      mount_point);
-  base::AutoLock locker(mount_point_map_lock_);
-  local_to_virtual_map_[mount_point] = mount_point.BaseName();
+  return mount_points_->RegisterFileSystem(
+             mount_name,
+             fileapi::kFileSystemTypeNativeLocal,
+             mount_point);
 }
 
-void CrosMountPointProvider::AddRestrictedLocalMountPoint(
+bool CrosMountPointProvider::AddRestrictedLocalMountPoint(
     const FilePath& mount_point) {
   std::string mount_name = mount_point.BaseName().AsUTF8Unsafe();
-  isolated_context()->RevokeFileSystem(mount_name);
-  isolated_context()->RegisterExternalFileSystem(
-      mount_name,
-      fileapi::kFileSystemTypeRestrictedNativeLocal,
-      mount_point);
-  base::AutoLock locker(mount_point_map_lock_);
-  local_to_virtual_map_[mount_point] = mount_point.BaseName();
+  return mount_points_->RegisterFileSystem(
+             mount_name,
+             fileapi::kFileSystemTypeRestrictedNativeLocal,
+             mount_point);
 }
 
-void CrosMountPointProvider::AddRemoteMountPoint(
+bool CrosMountPointProvider::AddRemoteMountPoint(
     const FilePath& mount_point,
     fileapi::RemoteFileSystemProxyInterface* remote_proxy) {
   DCHECK(remote_proxy);
   std::string mount_name = mount_point.BaseName().AsUTF8Unsafe();
-  isolated_context()->RevokeFileSystem(mount_name);
-  isolated_context()->RegisterExternalFileSystem(mount_name,
+  return mount_points_->RegisterRemoteFileSystem(mount_name,
                                                  fileapi::kFileSystemTypeDrive,
+                                                 remote_proxy,
                                                  mount_point);
-  base::AutoLock locker(mount_point_map_lock_);
-  remote_proxy_map_[mount_name] = remote_proxy;
-  local_to_virtual_map_[mount_point] = mount_point.BaseName();
 }
 
 void CrosMountPointProvider::RemoveMountPoint(const FilePath& mount_point) {
+  if (!HasMountPoint(mount_point))
+    return;
   std::string mount_name = mount_point.BaseName().AsUTF8Unsafe();
-  isolated_context()->RevokeFileSystem(mount_name);
-  base::AutoLock locker(mount_point_map_lock_);
-  remote_proxy_map_.erase(mount_name);
-  local_to_virtual_map_.erase(mount_point);
+  mount_points_->RevokeFileSystem(mount_name);
 }
 
 void CrosMountPointProvider::GrantFullAccessToExtension(
     const std::string& extension_id) {
   DCHECK(special_storage_policy_->IsFileHandler(extension_id));
   if (!special_storage_policy_->IsFileHandler(extension_id))
     return;
-  std::vector<fileapi::IsolatedContext::FileInfo> files =
-      isolated_context()->GetExternalMountPoints();
+
+  std::vector<fileapi::MountPoints::MountPointInfo> files;
+  mount_points_->AppendMountPoints(&files);
+  system_mount_points_->AppendMountPoints(&files);

[kinuko, 2013/01/08 12:22:43]

This method name AppendMountPoints looks like it's adding mount points to the
target object. Hmm... GetMountPointInfoAndAppendTo() maybe?

[tbarzic, 2013/01/09 01:26:34]

Done.

+
   for (size_t i = 0; i < files.size(); ++i) {
     file_access_permissions_->GrantAccessPermission(
         extension_id,
         FilePath::FromUTF8Unsafe(files[i].name));
   }
 }
 
 void CrosMountPointProvider::GrantFileAccessToExtension(
     const std::string& extension_id, const FilePath& virtual_path) {
   // All we care about here is access from extensions for now.
   DCHECK(special_storage_policy_->IsFileHandler(extension_id));
   if (!special_storage_policy_->IsFileHandler(extension_id))
     return;
 
   std::string id;
   fileapi::FileSystemType type;
   FilePath path;
-  isolated_context()->CrackIsolatedPath(virtual_path, &id, &type, &path);
+  if (!mount_points_->CrackExternalPath(virtual_path, &id, &type, &path) &&
+      !system_mount_points_->CrackExternalPath(virtual_path,
+                                               &id, &type, &path)) {

[kinuko, 2013/01/08 12:22:43]

Does it make sense if we also return |type| in GetVirtualPath (both CrosMPP and
ExternalMP ones) and make this method take |type| so that we can remove this
extra Crack?

(I'm asking this w/o deep understanding of the cros/drive code, so please just
let me know if I'm wrong)

[tbarzic, 2013/01/09 01:26:34]

I guess we could, but I'd still prefer to crack the path here, to verify that
virtual_path is valid for current context.
An alternative would be to pass FileSystemURL here, but that would make file
browser code a bit more awkward.

+    return;
+  }
 
   if (type == fileapi::kFileSystemTypeRestrictedNativeLocal) {
     LOG(ERROR) << "Can't grant access for restricted mount point";
     return;
   }
 
   file_access_permissions_->GrantAccessPermission(extension_id, virtual_path);
 }
 
 void CrosMountPointProvider::RevokeAccessForExtension(
       const std::string& extension_id) {
   file_access_permissions_->RevokePermissions(extension_id);
 }
 
-std::vector<FilePath> CrosMountPointProvider::GetRootDirectories() const {
-  std::vector<fileapi::IsolatedContext::FileInfo> files =
-      isolated_context()->GetExternalMountPoints();
+std::vector<FilePath> CrosMountPointProvider::GetRootDirectories(
+    bool include_system_mount_points) const {
+  std::vector<fileapi::MountPoints::MountPointInfo> files;
+  mount_points_->AppendMountPoints(&files);
+  if (include_system_mount_points)
+    system_mount_points_->AppendMountPoints(&files);
+
   std::vector<FilePath> root_dirs;
   for (size_t i = 0; i < files.size(); ++i)
     root_dirs.push_back(files[i].path);
   return root_dirs;
 }
 
 fileapi::FileSystemFileUtil* CrosMountPointProvider::GetFileUtil(
     fileapi::FileSystemType type) {
   DCHECK(type == fileapi::kFileSystemTypeNativeLocal ||
          type == fileapi::kFileSystemTypeRestrictedNativeLocal);
   return local_file_util_.get();
 }
 
 FilePath CrosMountPointProvider::GetPathForPermissionsCheck(
     const FilePath& virtual_path) const {
   return virtual_path;
 }
 
 fileapi::FileSystemOperation* CrosMountPointProvider::CreateFileSystemOperation(
     const fileapi::FileSystemURL& url,
     fileapi::FileSystemContext* context,
     base::PlatformFileError* error_code) const {
+  if (!url.is_valid())
+    return NULL;
   if (url.type() == fileapi::kFileSystemTypeDrive) {
-    base::AutoLock locker(mount_point_map_lock_);
-    RemoteProxyMap::const_iterator found = remote_proxy_map_.find(
-        url.filesystem_id());
-    if (found != remote_proxy_map_.end()) {
-      return new chromeos::RemoteFileSystemOperation(found->second);
-    } else {
-      *error_code = base::PLATFORM_FILE_ERROR_NOT_FOUND;
+    fileapi::RemoteFileSystemProxyInterface* remote_proxy =
+        GetRemoteProxy(url.filesystem_id());
+    if (!remote_proxy)
       return NULL;
-    }
+    return new chromeos::RemoteFileSystemOperation(remote_proxy);
   }
 
   DCHECK(url.type() == fileapi::kFileSystemTypeNativeLocal ||
          url.type() == fileapi::kFileSystemTypeRestrictedNativeLocal);
   scoped_ptr<fileapi::FileSystemOperationContext> operation_context(
       new fileapi::FileSystemOperationContext(context));
   return new fileapi::LocalFileSystemOperation(context,
                                                operation_context.Pass());
 }
 
 webkit_blob::FileStreamReader* CrosMountPointProvider::CreateFileStreamReader(
     const fileapi::FileSystemURL& url,
     int64 offset,
     const base::Time& expected_modification_time,
     fileapi::FileSystemContext* context) const {
   // For now we return a generic Reader implementation which utilizes
   // CreateSnapshotFile internally (i.e. will download everything first).
   // TODO(satorux,zel): implement more efficient reader for remote cases.
   return new fileapi::FileSystemFileStreamReader(
       context, url, offset, expected_modification_time);
 }
 
 fileapi::FileStreamWriter* CrosMountPointProvider::CreateFileStreamWriter(
     const fileapi::FileSystemURL& url,
     int64 offset,
     fileapi::FileSystemContext* context) const {
   if (!url.is_valid())
     return NULL;
+
   if (url.type() == fileapi::kFileSystemTypeDrive) {
-    base::AutoLock locker(mount_point_map_lock_);
-    RemoteProxyMap::const_iterator found = remote_proxy_map_.find(
-        url.filesystem_id());
-    if (found == remote_proxy_map_.end())
+    fileapi::RemoteFileSystemProxyInterface* remote_proxy =
+        GetRemoteProxy(url.filesystem_id());
+    if (!remote_proxy)
       return NULL;
-    return new fileapi::RemoteFileStreamWriter(found->second, url, offset);
+    return new fileapi::RemoteFileStreamWriter(remote_proxy, url, offset);
   }
 
   if (url.type() == fileapi::kFileSystemTypeRestrictedNativeLocal)
     return NULL;
 
   DCHECK(url.type() == fileapi::kFileSystemTypeNativeLocal);
   return new fileapi::LocalFileStreamWriter(url.path(), offset);
 }
 
 bool CrosMountPointProvider::GetVirtualPath(const FilePath& filesystem_path,
                                            FilePath* virtual_path) {
-  base::AutoLock locker(mount_point_map_lock_);
-  std::map<FilePath, FilePath>::reverse_iterator iter(
-      local_to_virtual_map_.upper_bound(filesystem_path));
-  if (iter == local_to_virtual_map_.rend())
-    return false;
-  if (iter->first == filesystem_path) {
-    *virtual_path = iter->second;
-    return true;
-  }
-  return iter->first.DirName().AppendRelativePath(
-      filesystem_path, virtual_path);
+  return mount_points_->GetVirtualPath(filesystem_path, virtual_path) ||
+         system_mount_points_->GetVirtualPath(filesystem_path, virtual_path);
 }
 
-fileapi::IsolatedContext* CrosMountPointProvider::isolated_context() const {
-  return fileapi::IsolatedContext::GetInstance();
+fileapi::RemoteFileSystemProxyInterface* CrosMountPointProvider::GetRemoteProxy(
+    const std::string& mount_name) const {
+  fileapi::RemoteFileSystemProxyInterface* proxy =
+      mount_points_->GetRemoteFileSystemProxy(mount_name);
+  if (proxy)
+    return proxy;
+  return system_mount_points_->GetRemoteFileSystemProxy(mount_name);
 }
 
 }  // namespace chromeos