Move OpenFile and CloseFile implementation.

This CL extracts and moves the implementation of OpenFile and NotifyCloseFile
from FileSystemProxy to FileApiWorker (with renaming NotifyCloseFile to
CloseFile).

BUG=254806
TEST=Ran browser_test --gtest_filter="*FileSystemExtensionApiTest*:*FileManagerBrowserTest*"

Review URL: https://chromiumcodereview.appspot.com/18519002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@209900 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 179 insertions, 214 deletions

diff --git a/chrome/browser/chromeos/drive/file_system_proxy.cc b/chrome/browser/chromeos/drive/file_system_proxy.cc
index de775a6..37845ab 100644
--- a/chrome/browser/chromeos/drive/file_system_proxy.cc
+++ b/chrome/browser/chromeos/drive/file_system_proxy.cc
@@ -37,49 +37,15 @@ namespace drive {
 
 namespace {
 
-typedef fileapi::RemoteFileSystemProxyInterface::OpenFileCallback
-    OpenFileCallback;
-
-// Helper function to run reply on results of base::CreatePlatformFile() on
-// IO thread.
-void OnPlatformFileOpened(
-    const OpenFileCallback& callback,
-    base::ProcessHandle peer_handle,
-    base::PlatformFileError* open_error,
-    base::PlatformFile platform_file) {
-  callback.Run(*open_error, platform_file, peer_handle);
-}
-
-// Helper function to run OpenFileCallback from
-// FileSystemProxy::OpenFile().
-void OnGetFileByPathForOpen(
-    const OpenFileCallback& callback,
-    int file_flags,
+// Runs |callback| with |error|, |file| and |peer_handle|.
+void RunOpenFileCallback(
     base::ProcessHandle peer_handle,
-    FileError file_error,
-    const base::FilePath& local_path,
-    scoped_ptr<ResourceEntry> entry) {
-  base::PlatformFileError error =
-      FileErrorToPlatformError(file_error);
-  if (error != base::PLATFORM_FILE_OK) {
-    callback.Run(error, base::kInvalidPlatformFileValue, peer_handle);
-    return;
-  }
+    const fileapi::RemoteFileSystemProxyInterface::OpenFileCallback& callback,
+    base::PlatformFileError error,
+    base::PlatformFile file) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  base::PlatformFileError* open_error =
-      new base::PlatformFileError(base::PLATFORM_FILE_ERROR_FAILED);
-  base::PostTaskAndReplyWithResult(
-      BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(&base::CreatePlatformFile,
-                 local_path,
-                 file_flags,
-                 static_cast<bool*>(NULL),
-                 open_error),
-      base::Bind(&OnPlatformFileOpened,
-                 callback,
-                 peer_handle,
-                 base::Owned(open_error)));
+  callback.Run(error, file, peer_handle);
 }
 
 // Runs |callback| with the arguments based on the given arguments.
@@ -103,12 +69,6 @@ void RunSnapshotFileCallback(
   callback.Run(error, file_info, local_path, file_reference);
 }
 
-// Emits debug log when FileSystem::CloseFile() is complete.
-void EmitDebugLogForCloseFile(const base::FilePath& local_path,
-                              FileError file_error) {
-  DVLOG(1) << "Closed: " << local_path.AsUTF8Unsafe() << ": " << file_error;
-}
-
 }  // namespace
 
 FileSystemProxy::FileSystemProxy(
@@ -295,75 +255,6 @@ void FileSystemProxy::Truncate(
                  google_apis::CreateRelayCallback(callback)));
 }
 
-void FileSystemProxy::OnOpenFileForWriting(
-    int file_flags,
-    base::ProcessHandle peer_handle,
-    const OpenFileCallback& callback,
-    FileError file_error,
-    const base::FilePath& local_cache_path) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  base::PlatformFileError error =
-      FileErrorToPlatformError(file_error);
-
-  if (error != base::PLATFORM_FILE_OK) {
-    callback.Run(error, base::kInvalidPlatformFileValue, peer_handle);
-    return;
-  }
-
-  // Cache file prepared for modification is available. Truncate it.
-  base::PlatformFileError* result =
-      new base::PlatformFileError(base::PLATFORM_FILE_ERROR_FAILED);
-  bool posted = base::PostTaskAndReplyWithResult(
-      BrowserThread::GetBlockingPool(), FROM_HERE,
-      base::Bind(&base::CreatePlatformFile,
-                 local_cache_path,
-                 file_flags,
-                 static_cast<bool*>(NULL),
-                 result),
-      base::Bind(&OnPlatformFileOpened,
-                 callback,
-                 peer_handle,
-                 base::Owned(result)));
-  DCHECK(posted);
-}
-
-void FileSystemProxy::OnCreateFileForOpen(
-    const base::FilePath& file_path,
-    int file_flags,
-    base::ProcessHandle peer_handle,
-    const OpenFileCallback& callback,
-    FileError file_error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  base::PlatformFileError create_result =
-      FileErrorToPlatformError(file_error);
-
-  if ((create_result == base::PLATFORM_FILE_OK) ||
-      ((create_result == base::PLATFORM_FILE_ERROR_EXISTS) &&
-       (file_flags & base::PLATFORM_FILE_CREATE_ALWAYS))) {
-    // If we are trying to always create an existing file, then
-    // if it really exists open it as truncated.
-    file_flags &= ~base::PLATFORM_FILE_CREATE;
-    file_flags &= ~base::PLATFORM_FILE_CREATE_ALWAYS;
-    file_flags |= base::PLATFORM_FILE_OPEN_TRUNCATED;
-  } else {
-    callback.Run(create_result, base::kInvalidPlatformFileValue, peer_handle);
-    return;
-  }
-
-  // Open created (or existing) file for writing.
-  CallFileSystemMethodOnUIThread(
-      base::Bind(&FileSystemInterface::OpenFile,
-                 base::Unretained(file_system_),
-                 file_path,
-                 google_apis::CreateRelayCallback(
-                     base::Bind(&FileSystemProxy::OnOpenFileForWriting,
-                                this,
-                                file_flags,
-                                peer_handle,
-                                callback))));
-}
-
 void FileSystemProxy::OpenFile(
     const FileSystemURL& file_url,
     int file_flags,
@@ -382,73 +273,12 @@ void FileSystemProxy::OpenFile(
     return;
   }
 
-  // TODO(zelidrag): Wire all other file open operations.
-  if ((file_flags & base::PLATFORM_FILE_DELETE_ON_CLOSE)) {
-    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_ALWAYS) ||
-      (file_flags & base::PLATFORM_FILE_OPEN_TRUNCATED)) {
-    if ((file_flags & base::PLATFORM_FILE_OPEN_TRUNCATED) ||
-        (file_flags & base::PLATFORM_FILE_OPEN_ALWAYS) ||
-        (file_flags & base::PLATFORM_FILE_WRITE) ||
-        (file_flags & base::PLATFORM_FILE_EXCLUSIVE_WRITE)) {
-      // Open existing file for writing.
-      CallFileSystemMethodOnUIThread(
-          base::Bind(&FileSystemInterface::OpenFile,
-                     base::Unretained(file_system_),
-                     file_path,
-                     google_apis::CreateRelayCallback(
-                         base::Bind(&FileSystemProxy::OnOpenFileForWriting,
-                                    this,
-                                    file_flags,
-                                    peer_handle,
-                                    callback))));
-    } else {
-      // Read-only file open.
-      CallFileSystemMethodOnUIThread(
-          base::Bind(&FileSystemInterface::GetFileByPath,
-                     base::Unretained(file_system_),
-                     file_path,
-                     google_apis::CreateRelayCallback(
-                         base::Bind(&OnGetFileByPathForOpen,
-                                    callback,
-                                    file_flags,
-                                    peer_handle))));
-    }
-  } else if ((file_flags & base::PLATFORM_FILE_CREATE) ||
-             (file_flags & base::PLATFORM_FILE_CREATE_ALWAYS)) {
-    // Open existing file for writing.
-    CallFileSystemMethodOnUIThread(
-        base::Bind(&FileSystemInterface::CreateFile,
-                   base::Unretained(file_system_),
-                   file_path,
-                   file_flags & base::PLATFORM_FILE_EXCLUSIVE_WRITE,
-                   google_apis::CreateRelayCallback(
-                       base::Bind(&FileSystemProxy::OnCreateFileForOpen,
-                                  this,
-                                  file_path,
-                                  file_flags,
-                                  peer_handle,
-                                  callback))));
-  } else {
-    NOTREACHED() << "Unhandled file flags combination " << file_flags;
-    MessageLoopProxy::current()->PostTask(
-        FROM_HERE,
-        base::Bind(callback,
-                   base::PLATFORM_FILE_ERROR_FAILED,
-                   base::kInvalidPlatformFileValue,
-                   peer_handle));
-  }
+  CallFileSystemMethodOnUIThread(
+      base::Bind(&internal::FileApiWorker::OpenFile,
+                 base::Unretained(worker_.get()),
+                 file_path, file_flags,
+                 google_apis::CreateRelayCallback(
+                     base::Bind(&RunOpenFileCallback, peer_handle, callback))));
 }
 
 void FileSystemProxy::NotifyCloseFile(const FileSystemURL& url) {
@@ -457,12 +287,8 @@ void FileSystemProxy::NotifyCloseFile(const FileSystemURL& url) {
     return;
 
   CallFileSystemMethodOnUIThread(
-      base::Bind(&FileSystemInterface::CloseFile,
-                 base::Unretained(file_system_),
-                 file_path,
-                 google_apis::CreateRelayCallback(
-                     base::Bind(&EmitDebugLogForCloseFile,
-                                file_path))));
+      base::Bind(&internal::FileApiWorker::CloseFile,
+                 base::Unretained(worker_.get()), file_path));
 }
 
 void FileSystemProxy::TouchFile(
@@ -617,12 +443,8 @@ void FileSystemProxy::CloseWritableSnapshotFile(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   CallFileSystemMethodOnUIThread(
-      base::Bind(&FileSystemInterface::CloseFile,
-                 base::Unretained(file_system_),
-                 virtual_path,
-                 google_apis::CreateRelayCallback(
-                     base::Bind(&EmitDebugLogForCloseFile,
-                                virtual_path))));
+      base::Bind(&internal::FileApiWorker::CloseFile,
+                 base::Unretained(worker_.get()), virtual_path));
 }
 
 FileSystemInterface* FileSystemProxy::GetFileSystemOnUIThread() {
diff --git a/chrome/browser/chromeos/drive/file_system_proxy.h b/chrome/browser/chromeos/drive/file_system_proxy.h
index 1776976..f4dcaca 100644
--- a/chrome/browser/chromeos/drive/file_system_proxy.h
+++ b/chrome/browser/chromeos/drive/file_system_proxy.h
@@ -132,24 +132,6 @@ class FileSystemProxy : public fileapi::RemoteFileSystemProxyInterface {
       const base::FilePath& virtual_path,
       const base::FilePath& local_path);
 
-  // Invoked during OpenFile() operation when truncate or write flags are set.
-  // This is called when a local modifiable cached file is ready for such
-  // operation.
-  void OnOpenFileForWriting(
-      int file_flags,
-      base::ProcessHandle peer_handle,
-      const OpenFileCallback& callback,
-      FileError file_error,
-      const base::FilePath& local_cache_path);
-
-  // Invoked during OpenFile() operation when file create flags are set.
-  void OnCreateFileForOpen(
-      const base::FilePath& file_path,
-      int file_flags,
-      base::ProcessHandle peer_handle,
-      const OpenFileCallback& callback,
-      FileError file_error);
-
   // Returns |file_system_| on UI thread.
   FileSystemInterface* GetFileSystemOnUIThread();
 
diff --git a/chrome/browser/chromeos/drive/fileapi_worker.cc b/chrome/browser/chromeos/drive/fileapi_worker.cc
index 0d282a3d..e69a386 100644
--- a/chrome/browser/chromeos/drive/fileapi_worker.cc
+++ b/chrome/browser/chromeos/drive/fileapi_worker.cc
@@ -6,6 +6,8 @@
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/task_runner_util.h"
+#include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/file_errors.h"
 #include "chrome/browser/chromeos/drive/file_system_interface.h"
@@ -110,10 +112,62 @@ void RunCreateSnapshotFileCallback(
   callback.Run(base::PLATFORM_FILE_OK, file_info, local_path, scope_out_policy);
 }
 
+// Runs |callback| with |error| and |platform_file|.
+void RunOpenFileCallback(
+    const FileApiWorker::OpenFileCallback& callback,
+    base::PlatformFileError* error,
+    base::PlatformFile platform_file) {
+  callback.Run(*error, platform_file);
+}
+
+// Part of FileApiWorker::OpenFile(). Called after FileSystem::OpenFile().
+void OpenFileAfterFileSystemOpenFile(
+    int file_flags,
+    const FileApiWorker::OpenFileCallback& callback,
+    FileError error,
+    const base::FilePath& local_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (error != FILE_ERROR_OK) {
+    callback.Run(FileErrorToPlatformError(error),
+                 base::kInvalidPlatformFileValue);
+    return;
+  }
+
+  // Cache file prepared for modification is available. Open it locally.
+  base::PlatformFileError* result =
+      new base::PlatformFileError(base::PLATFORM_FILE_ERROR_FAILED);
+  bool posted = base::PostTaskAndReplyWithResult(
+      BrowserThread::GetBlockingPool(), FROM_HERE,
+      base::Bind(&base::CreatePlatformFile,
+                 local_path, file_flags, static_cast<bool*>(NULL), result),
+      base::Bind(&RunOpenFileCallback, callback, base::Owned(result)));
+  DCHECK(posted);
+}
+
+// Part of FileApiWorker::OpenFile(). Called after FileSystem::GetFileByPath().
+void OpenFileAfterGetFileByPath(int file_flags,
+                                const FileApiWorker::OpenFileCallback& callback,
+                                FileError error,
+                                const base::FilePath& local_path,
+                                scoped_ptr<ResourceEntry> entry) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Just redirect to OpenFileAfterFileSystemOpenFile() with ignoring |entry|.
+  OpenFileAfterFileSystemOpenFile(file_flags, callback, error, local_path);
+}
+
+// Emits debug log when FileSystem::CloseFile() is complete.
+void EmitDebugLogForCloseFile(const base::FilePath& local_path,
+                              FileError file_error) {
+  DVLOG(1) << "Closed: " << local_path.AsUTF8Unsafe() << ": " << file_error;
+}
+
 }  // namespace
 
 FileApiWorker::FileApiWorker(FileSystemInterface* file_system)
-    : file_system_(file_system) {
+    : file_system_(file_system),
+      weak_ptr_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(file_system);
 }
@@ -197,6 +251,62 @@ void FileApiWorker::CreateSnapshotFile(
       base::Bind(&RunCreateSnapshotFileCallback, callback));
 }
 
+void FileApiWorker::OpenFile(const base::FilePath& file_path,
+                             int file_flags,
+                             const OpenFileCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // TODO(zelidrag): Wire all other file open operations.
+  if (file_flags & base::PLATFORM_FILE_DELETE_ON_CLOSE) {
+    NOTIMPLEMENTED() << "File create/write operations not yet supported "
+                     << file_path.value();
+    callback.Run(base::PLATFORM_FILE_ERROR_FAILED,
+                 base::kInvalidPlatformFileValue);
+    return;
+  }
+
+  // TODO(hidehiko): The opening logic should be moved to FileSystem.
+  //   crbug.com/256583.
+  if (file_flags & (base::PLATFORM_FILE_OPEN |
+                    base::PLATFORM_FILE_OPEN_ALWAYS |
+                    base::PLATFORM_FILE_OPEN_TRUNCATED)) {
+    if (file_flags & (base::PLATFORM_FILE_OPEN_TRUNCATED |
+                      base::PLATFORM_FILE_OPEN_ALWAYS |
+                      base::PLATFORM_FILE_WRITE |
+                      base::PLATFORM_FILE_EXCLUSIVE_WRITE)) {
+      // Open existing file for writing.
+      file_system_->OpenFile(
+          file_path,
+          base::Bind(&OpenFileAfterFileSystemOpenFile, file_flags, callback));
+    } else {
+      // Read-only file open.
+      file_system_->GetFileByPath(
+          file_path,
+          base::Bind(&OpenFileAfterGetFileByPath, file_flags, callback));
+    }
+  } else if (file_flags & (base::PLATFORM_FILE_CREATE |
+                           base::PLATFORM_FILE_CREATE_ALWAYS)) {
+    // Create a new file.
+    file_system_->CreateFile(
+        file_path,
+        file_flags & base::PLATFORM_FILE_EXCLUSIVE_WRITE,
+        base::Bind(&FileApiWorker::OpenFileAfterCreateFile,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   file_path, file_flags, callback));
+  } else {
+    NOTREACHED() << "Unhandled file flags combination " << file_flags;
+    callback.Run(base::PLATFORM_FILE_ERROR_FAILED,
+                 base::kInvalidPlatformFileValue);
+    return;
+  }
+}
+
+void FileApiWorker::CloseFile(const base::FilePath& file_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  file_system_->CloseFile(file_path,
+                          base::Bind(&EmitDebugLogForCloseFile, file_path));
+}
+
 void FileApiWorker::TouchFile(const base::FilePath& file_path,
                               const base::Time& last_access_time,
                               const base::Time& last_modified_time,
@@ -207,5 +317,30 @@ void FileApiWorker::TouchFile(const base::FilePath& file_path,
 
 }
 
+void FileApiWorker::OpenFileAfterCreateFile(const base::FilePath& file_path,
+                                            int file_flags,
+                                            const OpenFileCallback& callback,
+                                            FileError error) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (error != FILE_ERROR_OK &&
+      (error != FILE_ERROR_EXISTS ||
+       (file_flags & base::PLATFORM_FILE_CREATE))) {
+    callback.Run(FileErrorToPlatformError(error),
+                 base::kInvalidPlatformFileValue);
+    return;
+  }
+
+  // If we are trying to always create an existing file, then
+  // if it really exists open it as truncated.
+  file_flags &=
+      ~(base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_CREATE_ALWAYS);
+  file_flags |= base::PLATFORM_FILE_OPEN_TRUNCATED;
+
+  // Open created (or existing) file for writing.
+  file_system_->OpenFile(
+      file_path,
+      base::Bind(&OpenFileAfterFileSystemOpenFile, file_flags, callback));
+}
+
 }  // namespace internal
 }  // namespace drive
diff --git a/chrome/browser/chromeos/drive/fileapi_worker.h b/chrome/browser/chromeos/drive/fileapi_worker.h
index 284ad8b..caac16c 100644
--- a/chrome/browser/chromeos/drive/fileapi_worker.h
+++ b/chrome/browser/chromeos/drive/fileapi_worker.h
@@ -9,7 +9,9 @@
 
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
+#include "base/memory/weak_ptr.h"
 #include "base/platform_file.h"
+#include "chrome/browser/chromeos/drive/file_errors.h"
 #include "webkit/common/blob/scoped_file.h"
 
 namespace base {
@@ -46,6 +48,9 @@ class FileApiWorker {
            const base::FilePath& snapshot_file_path,
            webkit_blob::ScopedFile::ScopeOutPolicy scope_out_policy)>
       CreateSnapshotFileCallback;
+  typedef base::Callback<
+      void(base::PlatformFileError result,
+           base::PlatformFile platform_file)> OpenFileCallback;
 
   // |file_system| must not be NULL.
   explicit FileApiWorker(FileSystemInterface* file_system);
@@ -104,6 +109,17 @@ class FileApiWorker {
   void CreateSnapshotFile(const base::FilePath& file_path,
                           const CreateSnapshotFileCallback& callback);
 
+  // Opens the file at |file_path| with options |file_flags|.
+  // Called from FileSystemProxy::OpenFile.
+  void OpenFile(const base::FilePath& file_path,
+                int file_flags,
+                const OpenFileCallback& callback);
+
+  // Closes the file at |file_path|.
+  // Called from FileSystemProxy::NotifyCloseFile and
+  // FileSystemProxy::CloseWRitableSnapshotFile.
+  void CloseFile(const base::FilePath& file_path);
+
   // Changes timestamp of the file at |file_path| to |last_access_time| and
   // |last_modified_time|. Called from FileSystemProxy::TouchFile().
   void TouchFile(const base::FilePath& file_path,
@@ -112,8 +128,18 @@ class FileApiWorker {
                  const StatusCallback& callback);
 
  private:
+  // Part of OpenFile(). Called after FileSystem::CreateFile().
+  void OpenFileAfterCreateFile(const base::FilePath& file_path,
+                               int file_flags,
+                               const OpenFileCallback& callback,
+                               FileError error);
+
   FileSystemInterface* file_system_;
 
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate the weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<FileApiWorker> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(FileApiWorker);
 };