Add support for persistent file access in apps.

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 be3d9bd20df38d2326e754d0c04ee62b5cbdc4ff..adae106fcdc0e0fcec7410236fbf70e2598ac708 100644
--- a/chrome/browser/extensions/api/file_system/file_system_api.cc
+++ b/chrome/browser/extensions/api/file_system/file_system_api.cc
@@ -13,6 +13,7 @@
 #include "base/strings/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/api/file_handlers/app_file_handler_util.h"
+#include "chrome/browser/extensions/api/file_system/saved_files_service.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
@@ -57,6 +58,7 @@ const char kUserCancelled[] = "User cancelled";
 const char kWritableFileError[] = "Invalid file for writing";
 const char kRequiresFileSystemWriteError[] =
     "Operation requires fileSystem.write permission";
+const char kUnknownIdError[] = "Unknown id";
 
 namespace file_system = extensions::api::file_system;
 namespace ChooseEntry = file_system::ChooseEntry;
@@ -309,7 +311,7 @@ void FileSystemEntryFunction::CheckWritableFile(const base::FilePath& path) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   base::Closure on_success =
       base::Bind(&FileSystemEntryFunction::RegisterFileSystemAndSendResponse,
-                 this, path, WRITABLE);
+                 this, path, WRITABLE, "");
   base::Closure on_failure =
       base::Bind(&FileSystemEntryFunction::HandleWritableFileError, this);
 
@@ -325,7 +327,7 @@ void FileSystemEntryFunction::CheckWritableFile(const base::FilePath& path) {
 }
 
 void FileSystemEntryFunction::RegisterFileSystemAndSendResponse(
-    const base::FilePath& path, EntryType entry_type) {
+    const base::FilePath& path, EntryType entry_type, const std::string& id) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   fileapi::IsolatedContext* isolated_context =
@@ -342,7 +344,10 @@ void FileSystemEntryFunction::RegisterFileSystemAndSendResponse(
   SetResult(dict);
   dict->SetString("fileSystemId", file_entry.filesystem_id);
   dict->SetString("baseName", file_entry.registered_name);
-  dict->SetString("id", file_entry.id);
+  if (id.empty())
+    dict->SetString("id", file_entry.id);
+  else
+    dict->SetString("id", id);
 
   SendResponse(true);
 }
@@ -582,7 +587,7 @@ void FileSystemChooseEntryFunction::FileSelected(const base::FilePath& path,
   }
 
   // Don't need to check the file, it's for reading.
-  RegisterFileSystemAndSendResponse(path, READ_ONLY);
+  RegisterFileSystemAndSendResponse(path, READ_ONLY, "");
 }
 
 void FileSystemChooseEntryFunction::FileSelectionCanceled() {
@@ -701,4 +706,83 @@ bool FileSystemChooseEntryFunction::RunImpl() {
   return true;
 }
 
+bool FileSystemRetainEntryFunction::RunImpl() {
+  std::string entry_id;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id));

[Matt Giuca, 2013/05/16 06:40:59]

Comment: // Add the file to the retain list if it is not already on there.

[Sam McNally, 2013/05/17 03:38:52]

Done.

+  SavedFilesService* saved_files_service = SavedFilesService::Get(profile());
+  if (!saved_files_service->IsSaved(extension_->id(), entry_id) &&
+      !AddFileEntry(entry_id)) {
+    return false;
+  }

[Matt Giuca, 2013/05/16 06:40:59]

Comment: // Move the file to the front of the queue.

[Sam McNally, 2013/05/17 03:38:52]

Renamed instead.

+  saved_files_service->RecordFileAccess(extension_->id(), entry_id);
+  return true;
+}
+
+bool FileSystemRetainEntryFunction::AddFileEntry(const std::string& entry_id) {
+  std::string filesystem_name;
+  std::string filesystem_path;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &filesystem_name));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(2, &filesystem_path));

[Matt Giuca, 2013/05/16 06:40:59]

Question: Does GetString guarantee a valid UTF-8 string? You use this later in
FromUTF8Unsafe, which has a precondition of valid UTF-8.

[Sam McNally, 2013/05/17 03:38:52]

Yes.

+  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();
+  if (!policy->CanReadFileSystem(renderer_id, filesystem_id)) {
+    error_ = kSecurityError;
+    return false;
+  }
+
+  IsolatedContext* context = IsolatedContext::GetInstance();
+  base::FilePath relative_path =
+      base::FilePath::FromUTF8Unsafe(filesystem_path);

[Matt Giuca, 2013/05/16 06:40:59]

If the above is true, can you add a comment here that filesystem_path is valid
UTF-8 from GetString.

[Sam McNally, 2013/05/17 03:38:52]

Refactored to remove this duplicate code.

+  base::FilePath virtual_path = context->CreateVirtualRootPath(filesystem_id)
+      .Append(relative_path);
+  base::FilePath real_path;
+  if (!context->CrackVirtualPath(
+          virtual_path, &filesystem_id, NULL, &real_path)) {
+    error_ = kInvalidParameters;
+    return false;
+  }
+  bool is_writable = policy->CanReadWriteFileSystem(renderer_id, filesystem_id);
+  SavedFilesService::Get(profile())->AddFileEntry(
+      extension_->id(), entry_id, real_path, is_writable);
+  return true;
+}
+
+bool FileSystemIsRestorableFunction::RunImpl() {
+  std::string entry_id;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id));
+  SetResult(new base::FundamentalValue(
+      SavedFilesService::Get(profile())->IsSaved(extension_->id(), entry_id)));
+  return true;
+}
+
+bool FileSystemRestoreEntryFunction::RunImpl() {
+  std::string entry_id;
+  bool needs_new_file_entry;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &entry_id));
+  EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(1, &needs_new_file_entry));

[Matt Giuca, 2013/05/16 06:40:59]

What is needs_new_file_entry? Where is it used? It isn't documented in the .idl.

[Sam McNally, 2013/05/17 03:38:52]

It's an implementation detail. Added a comment.

+  SavedFileEntry file_entry;
+  if (!SavedFilesService::Get(profile())->GetFileEntry(
+      extension_->id(), entry_id, &file_entry)) {
+    error_ = kUnknownIdError;
+    return false;
+  }
+  SavedFilesService::Get(profile())->RecordFileAccess(extension_->id(),

[Matt Giuca, 2013/05/16 06:40:59]

// Move the file to the front of the queue.

(Maybe RecordFileAccess needs a better name so we don't need this comment, like
MoveEntryToFrontOfQueue.)

[Sam McNally, 2013/05/17 03:38:52]

Done.

+                                                      entry_id);
+
+  if (needs_new_file_entry) {
+    RegisterFileSystemAndSendResponse(
+        file_entry.path,
+        file_entry.writable ? WRITABLE : READ_ONLY,
+        file_entry.id);
+  }
+  return true;
+}
+
 }  // namespace extensions