Wired support for file truncating with RemoteFileSystemOperation::OpenFile() method (case when base…

chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc b/chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc
index 7efcb59aa31efd674b3702267b4505e42c71600f..b14f6a12918fe9f2f08816b2d089bcc2c65a5ea4 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc
@@ -32,7 +32,7 @@ const char kFeedField[] = "feed";
 
 
 // Helper function that creates platform file on bocking IO thread pool.
-void CreatePlatformFileOnIOPool(const FilePath& local_path,
+void OpenPlatformFileOnIOPool(const FilePath& local_path,
                                 int file_flags,
                                 base::PlatformFile* platform_file,
                                 base::PlatformFileError* open_error) {
@@ -43,9 +43,9 @@ void CreatePlatformFileOnIOPool(const FilePath& local_path,
                                             open_error);
 }
 
-// Helper function to run reply on results of CreatePlatformFileOnIOPool() on
+// Helper function to run reply on results of OpenPlatformFileOnIOPool() on
 // IO thread.
-void OnPlatformFileCreated(
+void OnPlatformFileOpened(
     const FileSystemOperationInterface::OpenFileCallback& callback,
     base::ProcessHandle peer_handle,
     base::PlatformFile* platform_file,
@@ -73,12 +73,12 @@ void OnGetFileByPathForOpen(
   base::PlatformFileError* open_error =
       new base::PlatformFileError(base::PLATFORM_FILE_ERROR_FAILED);
   BrowserThread::GetBlockingPool()->PostTaskAndReply(FROM_HERE,
-      base::Bind(&CreatePlatformFileOnIOPool,
+      base::Bind(&OpenPlatformFileOnIOPool,
                  local_path,
                  file_flags,
                  platform_file,
                  open_error),
-      base::Bind(&OnPlatformFileCreated,
+      base::Bind(&OnPlatformFileOpened,
                  callback,
                  peer_handle,
                  base::Owned(platform_file),
@@ -122,6 +122,26 @@ void OnClose(const FilePath& local_path, base::PlatformFileError error_code) {
   DVLOG(1) << "Closed: " << local_path.AsUTF8Unsafe() << ": " << error_code;
 }
 
+void OpenAndTruncateOnIOPool(

[kinaba, 2012/06/27 09:17:21]

I think we can simply use OpenPlatformFileOnIOPool() also for truncation case.
Passing base::PLATFORM_FILE_OPEN_TRUNCATED to base::CreatePlatformFile should
automatically truncate the file to size-0.

The reason I needed to call base::TruncatePlatformFile in DoTruncateOnFileThread
is for handling non-zero truncation.

[zel, 2012/06/27 22:20:45]

I see. Done.

+    const FilePath& local_cache_path,
+    base::PlatformFile* platform_file,
+    base::PlatformFileError* result) {
+  base::PlatformFile file = base::CreatePlatformFile(
+      local_cache_path,
+      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_WRITE,
+      NULL,
+      result);
+  if (*result == base::PLATFORM_FILE_OK) {
+    DCHECK_NE(base::kInvalidPlatformFileValue, file);
+    if (!base::TruncatePlatformFile(file, 0)) {
+      base::ClosePlatformFile(file);
+      *result = base::PLATFORM_FILE_ERROR_FAILED;
+      file = base::kInvalidPlatformFileValue;
+    }
+  }
+  *platform_file = file;
+}
+
 void DoTruncateOnFileThread(
     const FilePath& local_cache_path,
     int64 length,
@@ -142,7 +162,7 @@ void DoTruncateOnFileThread(
 }
 
 void DidCloseFileForTruncate(
-    const fileapi::FileSystemOperationInterface::StatusCallback& callback,
+    const FileSystemOperationInterface::StatusCallback& callback,
     base::PlatformFileError truncate_result,
     base::PlatformFileError close_result) {
   // Reports the first error.
@@ -180,15 +200,13 @@ GDataFileSystemProxy::GDataFileSystemProxy(
 void GDataFileSystemProxy::GetFileInfo(const GURL& file_url,
     const FileSystemOperationInterface::GetMetadataCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
   FilePath file_path;
   if (!ValidateUrl(file_url, &file_path)) {
-    base::MessageLoopProxy::current()->PostTask(
-        FROM_HERE,
-        base::Bind(callback,
-                   base::PLATFORM_FILE_ERROR_NOT_FOUND,
-                   base::PlatformFileInfo(),
-                   FilePath()));
+    MessageLoopProxy::current()->PostTask(FROM_HERE,
+         base::Bind(callback,
+                    base::PLATFORM_FILE_ERROR_NOT_FOUND,
+                    base::PlatformFileInfo(),
+                    FilePath()));
     return;
   }
 
@@ -286,7 +304,7 @@ void GDataFileSystemProxy::CreateDirectory(
 }
 
 void GDataFileSystemProxy::Truncate(const GURL& file_url, int64 length,
-    const fileapi::FileSystemOperationInterface::StatusCallback& callback) {
+    const FileSystemOperationInterface::StatusCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (length < 0) {
@@ -314,10 +332,61 @@ void GDataFileSystemProxy::Truncate(const GURL& file_url, int64 length,
                  callback));
 }
 
+void GDataFileSystemProxy::OnOpenFileForWriting(
+    int file_flags,
+    base::ProcessHandle peer_handle,
+    const FileSystemOperationInterface::OpenFileCallback& callback,
+    base::PlatformFileError open_result,
+    const FilePath& local_cache_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (open_result != base::PLATFORM_FILE_OK) {
+    callback.Run(open_result, base::kInvalidPlatformFileValue, peer_handle);
+    return;
+  }
+
+  // Cache file prepared for modification is available. Truncate it.
+  // File operation must be done on FILE thread, so relay the operation.
+  base::PlatformFileError* result =
+      new base::PlatformFileError(base::PLATFORM_FILE_ERROR_FAILED);
+  base::PlatformFile* platform_file = new base::PlatformFile(
+      base::kInvalidPlatformFileValue);
+  bool posted = false;
+  if (file_flags & base::PLATFORM_FILE_OPEN_TRUNCATED) {

[kinaba, 2012/06/27 09:17:21]

As noted above, I don't think this branching is necessary.

[zel, 2012/06/27 22:20:45]

Done.

+    // Perform file truncate operation.
+    posted = BrowserThread::GetBlockingPool()->PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&OpenAndTruncateOnIOPool,
+                   local_cache_path,
+                   platform_file,
+                   result),
+        base::Bind(&GDataFileSystemProxy::OnOpenAndTruncate,
+                   this,
+                   peer_handle,
+                   callback,
+                   base::Owned(platform_file),
+                   base::Owned(result)));
+  } else {
+    // Perform file open operation.
+    BrowserThread::GetBlockingPool()->PostTaskAndReply(FROM_HERE,
+        base::Bind(&OpenPlatformFileOnIOPool,
+                   local_cache_path,
+                   file_flags,
+                   platform_file,
+                   result),
+        base::Bind(&OnPlatformFileOpened,
+                   callback,
+                   peer_handle,
+                   base::Owned(platform_file),
+                   base::Owned(result)));
+  }
+  DCHECK(posted);
+}
+
 void GDataFileSystemProxy::OnFileOpenedForTruncate(
     const FilePath& virtual_path,
     int64 length,
-    const fileapi::FileSystemOperationInterface::StatusCallback& callback,
+    const FileSystemOperationInterface::StatusCallback& callback,
     base::PlatformFileError open_result,
     const FilePath& local_cache_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -348,7 +417,7 @@ void GDataFileSystemProxy::OnFileOpenedForTruncate(
 
 void GDataFileSystemProxy::DidTruncate(
     const FilePath& virtual_path,
-    const fileapi::FileSystemOperationInterface::StatusCallback& callback,
+    const FileSystemOperationInterface::StatusCallback& callback,
     base::PlatformFileError* truncate_result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
@@ -359,6 +428,16 @@ void GDataFileSystemProxy::DidTruncate(
                                                    *truncate_result));
 }
 
+
+void GDataFileSystemProxy::OnOpenAndTruncate(
+    base::ProcessHandle peer_handle,
+    const FileSystemOperationInterface::OpenFileCallback& callback,
+    base::PlatformFile* platform_file,
+    base::PlatformFileError* truncate_result) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  callback.Run(*truncate_result, *platform_file, peer_handle);
+}
+
 void GDataFileSystemProxy::OpenFile(
     const GURL& file_url,
     int file_flags,
@@ -376,12 +455,56 @@ void GDataFileSystemProxy::OpenFile(
     return;
   }
 
-  file_system_->GetFileByPath(file_path,
-                              base::Bind(&OnGetFileByPathForOpen,
-                                         callback,
-                                         file_flags,
-                                         peer_handle),
-                              GetDownloadDataCallback());
+  // TODO(zelidrag): Wire all other file open operations.
+  if ((file_flags & base::PLATFORM_FILE_CREATE) ||
+      (file_flags & base::PLATFORM_FILE_CREATE_ALWAYS) ||
+      (file_flags & base::PLATFORM_FILE_DELETE_ON_CLOSE)) {

[kinaba, 2012/06/27 09:17:21]

base::PLATFORM_FILE_OPEN_ALWAYS (if exist=>open else=>create) is an error, too.

[zel, 2012/06/27 22:20:45]

Done.

+    NOTIMPLEMENTED() << "File create/write operations not yet supported "
+                     << file_path.value();
+    MessageLoopProxy::current()->PostTask(FROM_HERE,
+         base::Bind(callback,
+                    base::PLATFORM_FILE_ERROR_FAILED,
+                    base::kInvalidPlatformFileValue,
+                    peer_handle));
+    return;
+  }
+
+  if ((file_flags & base::PLATFORM_FILE_OPEN) ||
+      (file_flags & base::PLATFORM_FILE_OPEN_TRUNCATED)) {
+    if ((file_flags & base::PLATFORM_FILE_OPEN_TRUNCATED) ||
+        (file_flags & base::PLATFORM_FILE_WRITE) ||
+        (file_flags & base::PLATFORM_FILE_EXCLUSIVE_WRITE)) {
+      // Open existing file for writing.
+      file_system_->OpenFile(
+          file_path,
+          base::Bind(&GDataFileSystemProxy::OnOpenFileForWriting,
+                     this,
+                     file_flags,
+                     peer_handle,
+                     callback));
+    } else {
+      // Read-only file open.
+      file_system_->GetFileByPath(file_path,
+                                  base::Bind(&OnGetFileByPathForOpen,
+                                             callback,
+                                             file_flags,
+                                             peer_handle),
+                                  GetDownloadDataCallback());
+    }
+  }

[kinaba, 2012/06/27 09:17:21]

It might be slightly safer to callback failure when none of the recognized
file_flags are set. Currently, say, if file_flags==0, then no feedback is
returned to the caller. (Though it should never happen if callers are correctly
implemented...)

[zel, 2012/06/27 22:20:45]

Done.

+  // TODO(zelidrag): Wire file create operation.
+}
+
+void GDataFileSystemProxy::NotifyFileClosed(
+    const GURL& file_url,
+    const FileSystemOperationInterface::StatusCallback& callback) {
+  FilePath file_path;
+  if (!ValidateUrl(file_url, &file_path)) {
+    MessageLoopProxy::current()->PostTask(FROM_HERE,
+         base::Bind(callback, base::PLATFORM_FILE_ERROR_NOT_FOUND));
+    return;
+  }
+  file_system_->CloseFile(file_path, callback);
 }
 
 void GDataFileSystemProxy::CreateSnapshotFile(