Add CreateFileSystemOperation() method to FileSystemContext

content/browser/file_system/file_system_dispatcher_host.cc

Index: content/browser/file_system/file_system_dispatcher_host.cc
diff --git a/content/browser/file_system/file_system_dispatcher_host.cc b/content/browser/file_system/file_system_dispatcher_host.cc
index 3f75e343ad18d66bf1f2fca194ff0c0d650b9528..b3d2ae470952087a9e79f3e5f805257c5b9b5eab 100644
--- a/content/browser/file_system/file_system_dispatcher_host.cc
+++ b/content/browser/file_system/file_system_dispatcher_host.cc
@@ -20,9 +20,9 @@
 #include "webkit/fileapi/file_system_callback_dispatcher.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_operation.h"
-#include "webkit/fileapi/file_system_operation.h"
 #include "webkit/fileapi/file_system_quota_util.h"
 #include "webkit/fileapi/file_system_util.h"
+#include "webkit/fileapi/sandbox_mount_point_provider.h"
 
 using content::BrowserMessageFilter;
 using content::BrowserThread;
@@ -30,6 +30,7 @@ using content::UserMetricsAction;
 using fileapi::FileSystemCallbackDispatcher;
 using fileapi::FileSystemFileUtil;
 using fileapi::FileSystemOperation;
+using fileapi::FileSystemOperationInterface;
 
 class BrowserFileSystemCallbackDispatcher
     : public FileSystemCallbackDispatcher {
@@ -177,44 +178,45 @@ void FileSystemDispatcherHost::OnOpen(
 
 void FileSystemDispatcherHost::OnMove(
     int request_id, const GURL& src_path, const GURL& dest_path) {

[ericu, 2012/01/10 00:37:04]

Where are we going to handle issues involving moves/copies that involve two
different types of FileSystemOperations?

[kinuko, 2012/01/13 08:28:35]

As we talked before I'm putting it off for now.  I'm vaguely assuming that we
will need to do something like 1) change the per-operation CallbackDispatcher
architecture (e.g. replacing it with per-method regular callbacks) and 2)
implement cross-FSO operations above the individual FSO layer using lower FSO
routines like readDirectory, createOrOpenFile, createDirectory and etc.

(Filed a bug: http://codereview.chromium.org/8999017/)

-  GetNewOperation(request_id)->Move(src_path, dest_path);
+  GetNewOperation(src_path, request_id)->Move(src_path, dest_path);
 }
 
 void FileSystemDispatcherHost::OnCopy(
     int request_id, const GURL& src_path, const GURL& dest_path) {
-  GetNewOperation(request_id)->Copy(src_path, dest_path);
+  GetNewOperation(src_path, request_id)->Copy(src_path, dest_path);
 }
 
 void FileSystemDispatcherHost::OnRemove(
     int request_id, const GURL& path, bool recursive) {
-  GetNewOperation(request_id)->Remove(path, recursive);
+  GetNewOperation(path, request_id)->Remove(path, recursive);
 }
 
 void FileSystemDispatcherHost::OnReadMetadata(
     int request_id, const GURL& path) {
-  GetNewOperation(request_id)->GetMetadata(path);
+  GetNewOperation(path, request_id)->GetMetadata(path);
 }
 
 void FileSystemDispatcherHost::OnCreate(
     int request_id, const GURL& path, bool exclusive,
     bool is_directory, bool recursive) {
   if (is_directory)
-    GetNewOperation(request_id)->CreateDirectory(path, exclusive, recursive);
+    GetNewOperation(path, request_id)->CreateDirectory(
+        path, exclusive, recursive);
   else
-    GetNewOperation(request_id)->CreateFile(path, exclusive);
+    GetNewOperation(path, request_id)->CreateFile(path, exclusive);
 }
 
 void FileSystemDispatcherHost::OnExists(
     int request_id, const GURL& path, bool is_directory) {
   if (is_directory)
-    GetNewOperation(request_id)->DirectoryExists(path);
+    GetNewOperation(path, request_id)->DirectoryExists(path);
   else
-    GetNewOperation(request_id)->FileExists(path);
+    GetNewOperation(path, request_id)->FileExists(path);
 }
 
 void FileSystemDispatcherHost::OnReadDirectory(
     int request_id, const GURL& path) {
-  GetNewOperation(request_id)->ReadDirectory(path);
+  GetNewOperation(path, request_id)->ReadDirectory(path);
 }
 
 void FileSystemDispatcherHost::OnWrite(
@@ -227,7 +229,7 @@ void FileSystemDispatcherHost::OnWrite(
     NOTREACHED();
     return;
   }
-  GetNewOperation(request_id)->Write(
+  GetNewOperation(path, request_id)->Write(
       request_context_, path, blob_url, offset);
 }
 
@@ -235,7 +237,7 @@ void FileSystemDispatcherHost::OnTruncate(
     int request_id,
     const GURL& path,
     int64 length) {
-  GetNewOperation(request_id)->Truncate(path, length);
+  GetNewOperation(path, request_id)->Truncate(path, length);
 }
 
 void FileSystemDispatcherHost::OnTouchFile(
@@ -243,14 +245,14 @@ void FileSystemDispatcherHost::OnTouchFile(
     const GURL& path,
     const base::Time& last_access_time,
     const base::Time& last_modified_time) {
-  GetNewOperation(request_id)->TouchFile(
+  GetNewOperation(path, request_id)->TouchFile(
       path, last_access_time, last_modified_time);
 }
 
 void FileSystemDispatcherHost::OnCancel(
     int request_id,
     int request_id_to_cancel) {
-  FileSystemOperation* write = operations_.Lookup(
+  FileSystemOperationInterface* write = operations_.Lookup(
       request_id_to_cancel);
   if (write) {
     // The cancel will eventually send both the write failure and the cancel
@@ -265,7 +267,7 @@ void FileSystemDispatcherHost::OnCancel(
 
 void FileSystemDispatcherHost::OnOpenFile(
     int request_id, const GURL& path, int file_flags) {
-  GetNewOperation(request_id)->OpenFile(path, file_flags, peer_handle());
+  GetNewOperation(path, request_id)->OpenFile(path, file_flags, peer_handle());
 }
 
 void FileSystemDispatcherHost::OnWillUpdate(const GURL& path) {
@@ -298,22 +300,36 @@ void FileSystemDispatcherHost::OnSyncGetPlatformPath(
   DCHECK(platform_path);
   *platform_path = FilePath();
 
-  FileSystemOperation* operation = new FileSystemOperation(
-      NULL,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE),
-      context_);
+  GURL origin_url;
+  fileapi::FileSystemType file_system_type = fileapi::kFileSystemTypeUnknown;
+  FilePath file_path;
+  if (!fileapi::CrackFileSystemURL(
+          path, &origin_url, &file_system_type, &file_path)) {
+    return;
+  }
+
+  // We support sync operations only on TEMPORARY or PERSISTENT filesystems.
+  // (This is called only by pepper plugin as of writing.)
+  DCHECK(file_system_type == fileapi::kFileSystemTypeTemporary ||
+         file_system_type == fileapi::kFileSystemTypePersistent);
 
+  FileSystemOperation* operation = static_cast<FileSystemOperation*>(

[satorux1, 2011/12/27 20:55:34]

Is this a down cast? Can we avoid it? Otherwise, it'd be nice to add some
comment about why we need to do this here.

[kinuko, 2012/01/13 08:28:35]

Once we support asynchronous uploading for arbitrary filesystem files we should
be able to get rid of this dirty hack.  I've added comments about that and added
AsFileSystemOperation() virtual method for explicit downcasting.

+      context_->CreateFileSystemOperation(
+          path, NULL,
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)));
   operation->SyncGetPlatformPath(path, platform_path);
 }
 
-FileSystemOperation* FileSystemDispatcherHost::GetNewOperation(
+FileSystemOperationInterface* FileSystemDispatcherHost::GetNewOperation(
+    const GURL& target_path,
     int request_id) {
   BrowserFileSystemCallbackDispatcher* dispatcher =
       new BrowserFileSystemCallbackDispatcher(this, request_id);
-  FileSystemOperation* operation = new FileSystemOperation(
-      dispatcher,
-      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE),
-      context_);
+  FileSystemOperationInterface* operation =
+      context_->CreateFileSystemOperation(
+          target_path,
+          dispatcher,
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
   operations_.AddWithID(operation, request_id);
   return operation;
 }