Add isWritableFileEntry to the fileSystem API

chrome/browser/extensions/api/file_system/file_system_api.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 "chrome/browser/extensions/api/file_system/file_system_api.h"
 
 #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"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/ui/chrome_select_file_policy.h"
 #include "chrome/browser/ui/extensions/shell_window.h"
 #include "chrome/common/extensions/api/file_system.h"
 #include "chrome/common/extensions/permissions/api_permission.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/render_view_host.h"
@@ skipping 105 matching lines @@
   // Create the file if it doesn't already exist.
   base::PlatformFileError error = base::PLATFORM_FILE_OK;
   int creation_flags = base::PLATFORM_FILE_CREATE |
                        base::PLATFORM_FILE_READ |
                        base::PLATFORM_FILE_WRITE;
   base::CreatePlatformFile(path, creation_flags, NULL, &error);
   return error == base::PLATFORM_FILE_OK ||
          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 {
 
 bool FileSystemGetDisplayPathFunction::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));
 
   FilePath file_path;
   if (!GetFilePathOfFileEntry(filesystem_name, filesystem_path,
                               render_view_host_, &file_path, &error_))
     return false;
 
   file_path = PrettifyPath(file_path);
   result_.reset(base::Value::CreateStringValue(file_path.value()));
   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)) {
     content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
         base::Bind(&FileSystemEntryFunction::RegisterFileSystemAndSendResponse,
             this, path, WRITABLE));
     return;
   }
 
   content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
@@ skipping 38 matching lines @@
   error_ = kWritableFileError;
   SendResponse(false);
 }
 
 bool FileSystemGetWritableFileEntryFunction::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()) {
+  if (!HasFileSystemWritePermission(GetExtension())) {
     error_ = kRequiresFileSystemWriteError;
     return false;
   }
 
   FilePath path;
   if (!GetFilePathOfFileEntry(filesystem_name, filesystem_path,
                               render_view_host_, &path, &error_))
     return false;
 
   content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
       base::Bind(&FileSystemGetWritableFileEntryFunction::CheckWritableFile,
           this, path));
   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
     : public SelectFileDialog::Listener {
  public:
   FilePicker(FileSystemChooseFileFunction* function,
              content::WebContents* web_contents,
              const FilePath& suggested_path,
              SelectFileDialog::Type picker_type,
              EntryType entry_type)
@@ skipping 133 matching lines @@
       entry_type = WRITABLE;
     } else if (*options->type == kSaveFileOption) {
       entry_type = WRITABLE;
       picker_type = SelectFileDialog::SELECT_SAVEAS_FILE;
     } else if (*options->type != kOpenFileOption) {
       error_ = kUnknownChooseFileType;
       return false;
     }
   }
 
-  if (entry_type == WRITABLE && !HasFileSystemWritePermission()) {
+  if (entry_type == WRITABLE &&
+      !HasFileSystemWritePermission(GetExtension())) {
     error_ = kRequiresFileSystemWriteError;
     return false;
   }
 
   return ShowPicker(FilePath(), picker_type, entry_type);
 }
 
 }  // namespace extensions