File API changes needed for safely passing user selected file entities from the file browser comp...

webkit/chromeos/fileapi/cros_mount_point_provider.cc

Index: webkit/chromeos/fileapi/cros_mount_point_provider.cc
===================================================================
--- webkit/chromeos/fileapi/cros_mount_point_provider.cc	(revision 81576)
+++ webkit/chromeos/fileapi/cros_mount_point_provider.cc	(working copy)
@@ -14,14 +14,12 @@
 #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/glue/webkit_glue.h"
 
 namespace chromeos {
 
-const char CrosMountPointProvider::kLocalName[] = "Local";
-const char CrosMountPointProvider::kLocalDirName[] = "/local/";
-
 typedef struct {
   const char* local_root_path;
   const char* web_root_path;
@@ -34,30 +32,17 @@
     {"/",                   "tmp"},
 };
 
+const char CrosMountPointProvider::kExternalName[] = "External";

[michaeln, 2011/04/14 21:25:34]

should this string value be specific to CROS some how... prefixed?

[zel, 2011/04/14 21:46:20]

Darin said that 'external' is already part of Pepper and that we should have in
on WebCore side as well. So, I don't think that this part is ChromeOS-specific
since all parsing and url creation methods need to understand it as well. See
https://bugs.webkit.org/show_bug.cgi?id=58456 for details.
  

[michaeln, 2011/04/14 22:36:58]

ok... i'm not so familiar with the plan for extending this to 'external' file
systems, it just caught my eye that cros specific .cc files held generic looking
string identifiers.

Does kExternalDirName[] need to match the value defined in file_system_util.cc
   static const char kExternalDir[] = "/external/";
if so, we should probably have one source for this value in chrome code. And is
there a similar redifinition of CrosMountPointProvider::kExternalName somewhere
else in chrome sources that needs to be kept in sync?

[zel, 2011/04/14 22:53:04]

I have moved all these constants to file_system_util.h/cc where most of the
others were defined.

+const char CrosMountPointProvider::kExternalDirName[] = "/external/";
+
 CrosMountPointProvider::CrosMountPointProvider(
     scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy)
-    : local_dir_name_(kLocalName),
-      special_storage_policy_(special_storage_policy) {
-
-  // TODO(zelidrag): There's got to be a better way to generate UUID.
-  srand(time(NULL));
-  std::string virtual_root;
-  virtual_root.append(base::StringPrintf("%hx%hx-%hx-%hx-%hx-%hx%hx%hx",
-      static_cast<unsigned short>(rand()), static_cast<unsigned short>(rand()),
-      static_cast<unsigned short>(rand()),
-      static_cast<unsigned short>(rand()),
-      static_cast<unsigned short>(rand()),
-      static_cast<unsigned short>(rand()), static_cast<unsigned short>(rand()),
-          static_cast<unsigned short>(rand())));
-
-  // Create virtual root node.
-  virtual_root_path_ = FilePath(virtual_root);
-  base_path_ = virtual_root_path_.Append(local_dir_name_);
-
+    : 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, std::string>(
+    mount_point_map_.insert(std::pair<std::string, FilePath>(
         std::string(fixed_exposed_paths[i].web_root_path),
-        std::string(fixed_exposed_paths[i].local_root_path)));
+        FilePath(std::string(fixed_exposed_paths[i].local_root_path))));
   }
 }
 
@@ -77,13 +62,13 @@
     fileapi::FileSystemType type,
     bool create,
     fileapi::FileSystemPathManager::GetRootPathCallback* callback_ptr) {
-  DCHECK(type == fileapi::kFileSystemTypeLocal);
+  DCHECK(type == fileapi::kFileSystemTypeExternal);
 
   std::string name(GetOriginIdentifierFromURL(origin_url));
   name += ':';
-  name += CrosMountPointProvider::kLocalName;
+  name += CrosMountPointProvider::kExternalName;
 
-  FilePath root_path = FilePath(CrosMountPointProvider::kLocalDirName);
+  FilePath root_path = FilePath(CrosMountPointProvider::kExternalDirName);
   callback_ptr->Run(!root_path.empty(), root_path, name);
 }
 
@@ -94,7 +79,7 @@
     fileapi::FileSystemType type,
     const FilePath& virtual_path,
     bool create) {
-  DCHECK(type == fileapi::kFileSystemTypeLocal);
+  DCHECK(type == fileapi::kFileSystemTypeExternal);
 
   std::vector<FilePath::StringType> components;
   virtual_path.GetComponents(&components);
@@ -108,7 +93,7 @@
     return FilePath();
   }
 
-  return FilePath(iter->second);
+  return iter->second;
 }
 
 // TODO(zelidrag): Share this code with SandboxMountPointProvider impl.
@@ -116,9 +101,45 @@
   return false;
 }
 
-bool CrosMountPointProvider::IsAccessAllowed(const GURL& origin_url) {
-  return special_storage_policy_->IsLocalFileSystemAccessAllowed(origin_url);
+bool CrosMountPointProvider::IsAccessAllowed(const GURL& origin_url,
+                                             const FilePath& virtual_path) {
+  std::string extension_id = origin_url.host();
+  // Check first to make sure this extension has fileBrowserHander permissions.
+  if (!special_storage_policy_->IsFileHanlder(extension_id))
+    return false;
+  return file_access_permissions_->HasAccessPermission(extension_id,
+                                                       virtual_path);
 }
 
+void CrosMountPointProvider::GrantFullAccessToExtension(
+    const std::string& extension_id) {
+  for (MountPointMap::const_iterator iter = mount_point_map_.begin();
+       iter != mount_point_map_.end();
+       ++iter) {
+    GrantFileAccessToExtension(extension_id, iter->second);
+  }
+}
+
+void CrosMountPointProvider::GrantFileAccessToExtension(
+    const std::string& extension_id, const FilePath& virtual_path) {

[michaeln, 2011/04/14 21:25:34]

would it make sense to test IsFileHandler() here too?

[zel, 2011/04/14 21:46:20]

The code won't let you get here unless the extension is a handler, but DCHECK is
surely appropriate. Done.

+  // All we care about here is access from extensions access for now.
+  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<FilePath> root_dirs;
+  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