Add isWritableFileEntry to the fileSystem API

chrome/browser/extensions/api/file_system/file_system_api.cc

Index: chrome/browser/extensions/api/file_system/file_system_api.cc
diff --git a/chrome/browser/extensions/api/file_system/file_system_api.cc b/chrome/browser/extensions/api/file_system/file_system_api.cc
index 871125e0030030d2aab9d8263ef184ef5e896dbb..bc727c665b19342194d1d1b68a68798ddfc59e95 100644
--- a/chrome/browser/extensions/api/file_system/file_system_api.cc
+++ b/chrome/browser/extensions/api/file_system/file_system_api.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/logging.h"
 #include "base/path_service.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
@@ -132,6 +133,13 @@ bool DoCheckWritableFile(const FilePath& path) {
          error == base::PLATFORM_FILE_ERROR_EXISTS;
 }
 
+bool HasFileSystemWritePermission(const extensions::Extension* extension) {
+  if (!extension)
+    return false;

[benwells, 2012/07/10 06:40:23]

Nit: add blank line after early return.

[thorogood, 2012/07/10 06:49:22]

Done.

+  return extension->HasAPIPermission(
+      extensions::APIPermission::kFileSystemWrite);
+}
+
 }  // namespace
 
 namespace extensions {
@@ -152,14 +160,6 @@ bool FileSystemGetDisplayPathFunction::RunImpl() {
   return true;
 }
 
-bool FileSystemEntryFunction::HasFileSystemWritePermission() {
-  const extensions::Extension* extension = GetExtension();
-  if (!extension)
-    return false;
-
-  return extension->HasAPIPermission(APIPermission::kFileSystemWrite);
-}
-
 void FileSystemEntryFunction::CheckWritableFile(const FilePath& path) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
   if (DoCheckWritableFile(path)) {
@@ -218,7 +218,7 @@ bool FileSystemGetWritableFileEntryFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &filesystem_name));
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &filesystem_path));
 
-  if (!HasFileSystemWritePermission()) {
+  if (!HasFileSystemWritePermission(GetExtension())) {
     error_ = kRequiresFileSystemWriteError;
     return false;
   }
@@ -234,6 +234,33 @@ bool FileSystemGetWritableFileEntryFunction::RunImpl() {
   return true;
 }
 
+bool FileSystemIsWritableFileEntryFunction::RunImpl() {
+  std::string filesystem_name;
+  std::string filesystem_path;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &filesystem_name));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &filesystem_path));
+
+  if (!HasFileSystemWritePermission(GetExtension())) {

[benwells, 2012/07/10 06:40:23]

I'm not sure about this. Calls to IsWritableFileEntry should not fail just
because you don't have this permission.

However, unless there is a bug, they should always return false.

We should take out this check, and just ask the security policy what rights
there are.

Maybe we should also add this call to all the file system API tests for
chooseFile and getWritableFileEntry.

[thorogood, 2012/07/10 06:49:22]

True. We would never grant the policy if they do not have this extension
permission.
I've removed the check.
So, we make a call to HasFileSystemWritePermission inside chooseFile and
getWritableFileEntry already. I'm not sure what you mean by add a call to their
tests.

[thorogood, 2012/07/10 08:06:16]

I've added a call to isWritableFileEntry to checkEntry, which is used by many of
the fileSystem tests (it's inside test_util.js). PTAL!

+    error_ = kRequiresFileSystemWriteError;
+    return false;
+  }
+
+  std::string filesystem_id;
+  if (!fileapi::CrackIsolatedFileSystemName(filesystem_name, &filesystem_id)) {
+    error_ = kInvalidParameters;
+    return false;
+  }
+
+  content::ChildProcessSecurityPolicy* policy =
+      content::ChildProcessSecurityPolicy::GetInstance();
+  int renderer_id = render_view_host_->GetProcess()->GetID();
+  bool is_writable = policy->CanReadWriteFileSystem(renderer_id,
+                                                    filesystem_id);
+
+  result_.reset(base::Value::CreateBooleanValue(is_writable));
+  return true;
+}
+
 // Handles showing a dialog to the user to ask for the filename for a file to
 // save or open.
 class FileSystemChooseFileFunction::FilePicker
@@ -387,7 +414,8 @@ bool FileSystemChooseFileFunction::RunImpl() {
     }
   }
 
-  if (entry_type == WRITABLE && !HasFileSystemWritePermission()) {
+  if (entry_type == WRITABLE &&
+      !HasFileSystemWritePermission(GetExtension())) {
     error_ = kRequiresFileSystemWriteError;
     return false;
   }