Add support for directory access to the file system API.

chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc

Index: chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
diff --git a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
index 64c81728b2520a0694dba4b772d2ed684739db2a..becb9d925fe0f28c5497f61a431f3acbe5d92353 100644
--- a/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
+++ b/chrome/browser/extensions/api/file_handlers/app_file_handler_util.cc
@@ -64,6 +64,9 @@ bool DoCheckWritableFile(const base::FilePath& path) {
   if (base::PathExists(path) && file_util::IsLink(path))

[benwells, 2013/08/27 09:05:06]

We need to check what happens to symbolic links contained in folders. Can they
be followed to other folders?

[Sam McNally, 2013/08/28 05:44:25]

The HTML5 filesystem API returns NOT_FOUND_ERR for symlinks, to either files or
directories.

     return false;
 
+  if (base::DirectoryExists(path))
+    return base::PathIsWritable(path);

[benwells, 2013/08/27 09:05:06]

Hmmm. This isn't right. Apps with write permission should still be able to read
read-only directories.

There doesn't appear to be a corresponding check for files (which is good).

I'm not sure if we should auto-create folders like we do files. We shouldn't
auto-create a file if they want a directory which I think this code will do.  
Maybe this will never get reached for directories as the picker ensures the
directory exists, if that's the case I'd still prefer this to handle directories
not existing to avoid it knowing about its callers.

I think that means this needs an is_directory parameter.

[Sam McNally, 2013/08/28 05:44:25]

Done.

+
   // Create the file if it doesn't already exist.
   base::PlatformFileError error = base::PLATFORM_FILE_OK;
   int creation_flags = base::PLATFORM_FILE_CREATE |
@@ -290,7 +293,8 @@ GrantedFileEntry CreateFileEntry(
     Profile* profile,
     const Extension* extension,
     int renderer_id,
-    const base::FilePath& path) {
+    const base::FilePath& path,
+    bool is_directory) {
   GrantedFileEntry result;
   fileapi::IsolatedContext* isolated_context =
       fileapi::IsolatedContext::GetInstance();
@@ -303,8 +307,11 @@ GrantedFileEntry CreateFileEntry(
   content::ChildProcessSecurityPolicy* policy =
       content::ChildProcessSecurityPolicy::GetInstance();
   policy->GrantReadFileSystem(renderer_id, result.filesystem_id);
-  if (HasFileSystemWritePermission(extension))
+  if (HasFileSystemWritePermission(extension)) {
     policy->GrantWriteFileSystem(renderer_id, result.filesystem_id);
+    if (is_directory)
+      policy->GrantCreateFileForFileSystem(renderer_id, result.filesystem_id);
+  }
 
   result.id = result.filesystem_id + ":" + result.registered_name;
   return result;
@@ -320,6 +327,8 @@ void CheckWritableFiles(
   checker->Check();
 }
 
+GrantedFileEntry::GrantedFileEntry() {}

[benwells, 2013/08/27 09:05:06]

I'm sure there is an intelligent reason, but why did you add this?

[Matt Giuca, 2013/08/28 00:37:42]

It's an unrelated change, but I believe it's in the style guide that you should
always put an explicit empty constructor in the .cc file instead of leaving it
implicit, for non-POD classes, to avoid the binary code duplication of having an
implicit constructor generated in every module that constructs an object of that
type.

[Sam McNally, 2013/08/28 05:44:25]

What Matt said. Specifically,
http://dev.chromium.org/developers/coding-style/chromium-style-checker-errors...
started complaining about it.

+
 bool HasFileSystemWritePermission(const Extension* extension) {
   return extension->HasAPIPermission(APIPermission::kFileSystemWrite);
 }