Fixed file/directory url resolution for external mount point provider.

webkit/chromeos/fileapi/cros_mount_point_provider.cc

   // Copyright (c) 2011 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/logging.h"
 #include "base/memory/scoped_callback_factory.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
 #include "base/stringprintf.h"
+#include "base/synchronization/lock.h"
 #include "base/utf_string_conversions.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebCString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFileSystem.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityOrigin.h"
 #include "webkit/chromeos/fileapi/file_access_permissions.h"
 #include "webkit/fileapi/file_system_path_manager.h"
 #include "webkit/fileapi/file_system_util.h"
 #include "webkit/glue/webkit_glue.h"
 
 namespace chromeos {
 
 typedef struct {
   const char* local_root_path;
   const char* web_root_path;
 } FixedExposedPaths;
 
 // Top level file system elements exposed in FileAPI in ChromeOS:
 FixedExposedPaths fixed_exposed_paths[] = {
     {"/home/chronos/user/", "Downloads"},
     {"/",                   "media"},
-    {"/",                   "tmp"},
 };
 
 CrosMountPointProvider::CrosMountPointProvider(
     scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy)
     : special_storage_policy_(special_storage_policy),
       file_access_permissions_(new FileAccessPermissions()) {
   for (size_t i = 0; i < arraysize(fixed_exposed_paths); i++) {
     mount_point_map_.insert(std::pair<std::string, FilePath>(
         std::string(fixed_exposed_paths[i].web_root_path),
         FilePath(std::string(fixed_exposed_paths[i].local_root_path))));
   }
 }
 
 CrosMountPointProvider::~CrosMountPointProvider() {
 }
 
 // TODO(zelidrag) share with SandboxMountPointProvider impl.
 std::string GetOriginIdentifierFromURL(
     const GURL& url) {
   WebKit::WebSecurityOrigin web_security_origin =
       WebKit::WebSecurityOrigin::createFromString(UTF8ToUTF16(url.spec()));
   return web_security_origin.databaseIdentifier().utf8();
 }
 
+bool CrosMountPointProvider::GetRootForVirtualPath(
+    const FilePath& virtual_path, FilePath* root_path) {
+  // On the other hand, the root path of a file/dir from the real file system

[ericu, 2011/04/18 20:35:56]

I don't understand this comment.
Also, the start "on the other hand" implies there's some background I'm missing.

[zel, 2011/04/18 20:51:12]

sorry, this got removed from the context of other comments. Done

+  // needs to be mapped based on its first virtual directory segment that
+  // comes just after as in /external/<first-dir-segment>/file.foo
+  std::vector<FilePath::StringType> components;
+  virtual_path.GetComponents(&components);
+  if (components.size() < 1) {
+    return false;
+  }
+
+  base::AutoLock locker(lock_);
+  // Check if this root directory is exposed by this provider.
+  MountPointMap::iterator iter = mount_point_map_.find(components[0]);
+  if (iter == mount_point_map_.end()) {
+    return false;
+  }
+  *root_path = iter->second;
+  return true;
+}
+
 void CrosMountPointProvider::GetFileSystemRootPath(
     const GURL& origin_url,
     fileapi::FileSystemType type,
     bool create,
+    bool is_file_system,

[ericu, 2011/04/18 20:35:56]

is_file_system seems poorly named.  If this flag is intended to mean "this path
isn't actually in the filesystem at all", then it seems like you shouldn't
really be calling this method at all.  Is the whole point of this change just to
add a hack so that you can test some other class?  Is there another way to do
that?

[zel, 2011/04/18 20:51:12]

I agree about the name, any suggestion is welcomed.

It's actually not a test hack, it's little bit more complicated than that. The
external mount point provider exposes first segment of virtual path that does
not have a corresponding directory entry in the real file system - /external/.
When we want to get root path to construct DOMFileSystem, then /external/ is
acceptable, but when I need to map a file/dir URL to the real file path then I
need to strip /external/ out of the virtual path. This flag differentiates
between these two cases.

+    const FilePath& virtual_path,
     fileapi::FileSystemPathManager::GetRootPathCallback* callback_ptr) {
   DCHECK(type == fileapi::kFileSystemTypeExternal);
-
   std::string name(GetOriginIdentifierFromURL(origin_url));
   name += ':';
   name += fileapi::kExternalName;
-
-  FilePath root_path = FilePath(fileapi::kExternalDir);
+  // File system root path is a non-existent path from the file system...

[ericu, 2011/04/18 20:35:56]

Why are we looking up a nonexistent path?

[zel, 2011/04/18 20:51:12]

see above

+  FilePath root_path;
+  if (is_file_system) {
+    root_path = FilePath(fileapi::kExternalDir);

[ericu, 2011/04/18 20:35:56]

This is kind of weird.  '/external/' is in the namespace of FileSystem URLs, and
now you're using it in a FilePath that's intended to be in the namespace of real
paths.  Why do this?

[zel, 2011/04/18 20:51:12]

see my first comment on this one as well

+  } else {
+    if (!GetRootForVirtualPath(virtual_path, &root_path)) {
+      callback_ptr->Run(false, FilePath(), std::string());
+      return;
+    }
+  }
   callback_ptr->Run(!root_path.empty(), root_path, name);
 }
 
 // Like GetFileSystemRootPath, but synchronous, and can be called only while
 // running on the file thread.
 FilePath CrosMountPointProvider::GetFileSystemRootPathOnFileThread(
     const GURL& origin_url,
     fileapi::FileSystemType type,
     const FilePath& virtual_path,
     bool create) {
   DCHECK(type == fileapi::kFileSystemTypeExternal);
+  FilePath root_path;
+  if (!GetRootForVirtualPath(virtual_path, &root_path))
+    return FilePath();
 
-  std::vector<FilePath::StringType> components;
-  virtual_path.GetComponents(&components);
-  if (components.size() < 1) {
-    return FilePath();
-  }
-
-  // Check if this root directory is exposed by this provider.
-  MountPointMap::iterator iter = mount_point_map_.find(components[0]);
-  if (iter == mount_point_map_.end()) {
-    return FilePath();
-  }
-
-  return iter->second;
+  return root_path;
 }
 
 // TODO(zelidrag): Share this code with SandboxMountPointProvider impl.
 bool CrosMountPointProvider::IsRestrictedFileName(const FilePath& path) const {
   return false;
 }
 
 bool CrosMountPointProvider::IsAccessAllowed(const GURL& origin_url,
                                              fileapi::FileSystemType type,
                                              const FilePath& virtual_path) {
   if (type != fileapi::kFileSystemTypeExternal)
     return false;
   std::string extension_id = origin_url.host();
   // Check first to make sure this extension has fileBrowserHander permissions.
   if (!special_storage_policy_->IsFileHandler(extension_id))
     return false;
   return file_access_permissions_->HasAccessPermission(extension_id,
                                                        virtual_path);
 }
 
+void CrosMountPointProvider::AddMountPoint(FilePath mount_point) {
+  base::AutoLock locker(lock_);
+  mount_point_map_.insert(std::pair<std::string, FilePath>(
+      mount_point.BaseName().value(), mount_point.DirName()));
+}
+
+void CrosMountPointProvider::RemoveMountPoint(FilePath mount_point) {
+  base::AutoLock locker(lock_);
+  mount_point_map_.erase(mount_point.BaseName().value());
+}
+
 void CrosMountPointProvider::GrantFullAccessToExtension(
     const std::string& extension_id) {
   DCHECK(special_storage_policy_->IsFileHandler(extension_id));
   if (!special_storage_policy_->IsFileHandler(extension_id))
     return;
   for (MountPointMap::const_iterator iter = mount_point_map_.begin();
        iter != mount_point_map_.end();
        ++iter) {
     GrantFileAccessToExtension(extension_id, FilePath(iter->first));
   }
@@ skipping 18 matching lines @@
   for (MountPointMap::const_iterator iter = mount_point_map_.begin();
        iter != mount_point_map_.end();
        ++iter) {
     root_dirs.push_back(iter->second.Append(iter->first));
   }
   return root_dirs;
 }
 
 }  // namespace chromeos