Remove legacy from drive::internal::FileCache.

- Remove XXXOnUIThread methods.
- Merge StoreInternal method to Store.

BUG=231221

Review URL: https://codereview.chromium.org/108873005

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

6 files changed, 98 insertions, 186 deletions

diff --git a/chrome/browser/chromeos/drive/file_cache.cc b/chrome/browser/chromeos/drive/file_cache.cc
index 876df69..4ca19db 100644
--- a/chrome/browser/chromeos/drive/file_cache.cc
+++ b/chrome/browser/chromeos/drive/file_cache.cc
@@ -13,7 +13,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
-#include "base/task_runner_util.h"
 #include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/chromeos/drive/resource_metadata_storage.h"
@@ -35,18 +34,6 @@ std::string GetIdFromPath(const base::FilePath& path) {
   return util::UnescapeCacheFileName(path.BaseName().AsUTF8Unsafe());
 }
 
-// Runs callback with pointers dereferenced.
-// Used to implement GetFile, MarkAsMounted.
-void RunGetFileFromCacheCallback(const GetFileFromCacheCallback& callback,
-                                 base::FilePath* file_path,
-                                 FileError error) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
-  DCHECK(file_path);
-
-  callback.Run(error, *file_path);
-}
-
 }  // namespace
 
 FileCache::FileCache(ResourceMetadataStorage* storage,
@@ -74,8 +61,7 @@ base::FilePath FileCache::GetCacheFilePath(const std::string& id) const {
 }
 
 void FileCache::AssertOnSequencedWorkerPool() {
-  DCHECK(!blocking_task_runner_.get() ||
-         blocking_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
 }
 
 bool FileCache::IsUnderFileCacheDirectory(const base::FilePath& path) const {
@@ -150,19 +136,51 @@ FileError FileCache::Store(const std::string& id,
                            const base::FilePath& source_path,
                            FileOperationType file_operation_type) {
   AssertOnSequencedWorkerPool();
-  return StoreInternal(id, md5, source_path, file_operation_type);
-}
 
-void FileCache::PinOnUIThread(const std::string& id,
-                              const FileOperationCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
+  int64 file_size = 0;
+  if (file_operation_type == FILE_OPERATION_COPY) {
+    if (!base::GetFileSize(source_path, &file_size)) {
+      LOG(WARNING) << "Couldn't get file size for: " << source_path.value();
+      return FILE_ERROR_FAILED;
+    }
+  }
+  if (!FreeDiskSpaceIfNeededFor(file_size))
+    return FILE_ERROR_NO_LOCAL_SPACE;
 
-  base::PostTaskAndReplyWithResult(
-      blocking_task_runner_.get(),
-      FROM_HERE,
-      base::Bind(&FileCache::Pin, base::Unretained(this), id),
-      callback);
+  FileCacheEntry cache_entry;
+  storage_->GetCacheEntry(id, &cache_entry);
+
+  // If file is dirty or mounted, return error.
+  if (cache_entry.is_dirty() || mounted_files_.count(id))
+    return FILE_ERROR_IN_USE;
+
+  base::FilePath dest_path = GetCacheFilePath(id);
+  bool success = false;
+  switch (file_operation_type) {
+    case FILE_OPERATION_MOVE:
+      success = base::Move(source_path, dest_path);
+      break;
+    case FILE_OPERATION_COPY:
+      success = base::CopyFile(source_path, dest_path);
+      break;
+    default:
+      NOTREACHED();
+  }
+
+  if (!success) {
+    LOG(ERROR) << "Failed to store: "
+               << "source_path = " << source_path.value() << ", "
+               << "dest_path = " << dest_path.value() << ", "
+               << "file_operation_type = " << file_operation_type;
+    return FILE_ERROR_FAILED;
+  }
+
+  // Now that file operations have completed, update metadata.
+  cache_entry.set_md5(md5);
+  cache_entry.set_is_present(true);
+  cache_entry.set_is_dirty(false);
+  return storage_->PutCacheEntry(id, cache_entry) ?
+      FILE_ERROR_OK : FILE_ERROR_FAILED;
 }
 
 FileError FileCache::Pin(const std::string& id) {
@@ -175,18 +193,6 @@ FileError FileCache::Pin(const std::string& id) {
       FILE_ERROR_OK : FILE_ERROR_FAILED;
 }
 
-void FileCache::UnpinOnUIThread(const std::string& id,
-                                const FileOperationCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
-
-  base::PostTaskAndReplyWithResult(
-      blocking_task_runner_.get(),
-      FROM_HERE,
-      base::Bind(&FileCache::Unpin, base::Unretained(this), id),
-      callback);
-}
-
 FileError FileCache::Unpin(const std::string& id) {
   AssertOnSequencedWorkerPool();
 
@@ -212,24 +218,6 @@ FileError FileCache::Unpin(const std::string& id) {
   return FILE_ERROR_OK;
 }
 
-void FileCache::MarkAsMountedOnUIThread(
-    const std::string& id,
-    const GetFileFromCacheCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
-
-  base::FilePath* cache_file_path = new base::FilePath;
-  base::PostTaskAndReplyWithResult(
-      blocking_task_runner_.get(),
-      FROM_HERE,
-      base::Bind(&FileCache::MarkAsMounted,
-                 base::Unretained(this),
-                 id,
-                 cache_file_path),
-      base::Bind(
-          RunGetFileFromCacheCallback, callback, base::Owned(cache_file_path)));
-}
-
 FileError FileCache::MarkAsMounted(const std::string& id,
                                    base::FilePath* cache_file_path) {
   AssertOnSequencedWorkerPool();
@@ -259,20 +247,6 @@ FileError FileCache::MarkAsMounted(const std::string& id,
   return FILE_ERROR_OK;
 }
 
-void FileCache::MarkAsUnmountedOnUIThread(
-    const base::FilePath& file_path,
-    const FileOperationCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!callback.is_null());
-
-  base::PostTaskAndReplyWithResult(
-      blocking_task_runner_.get(),
-      FROM_HERE,
-      base::Bind(
-          &FileCache::MarkAsUnmounted, base::Unretained(this), file_path),
-      callback);
-}
-
 FileError FileCache::MarkDirty(const std::string& id) {
   AssertOnSequencedWorkerPool();
 
@@ -486,58 +460,6 @@ bool FileCache::RecoverFilesFromCacheDirectory(
   return true;
 }
 
-FileError FileCache::StoreInternal(const std::string& id,
-                                   const std::string& md5,
-                                   const base::FilePath& source_path,
-                                   FileOperationType file_operation_type) {
-  AssertOnSequencedWorkerPool();
-
-  int64 file_size = 0;
-  if (file_operation_type == FILE_OPERATION_COPY) {
-    if (!base::GetFileSize(source_path, &file_size)) {
-      LOG(WARNING) << "Couldn't get file size for: " << source_path.value();
-      return FILE_ERROR_FAILED;
-    }
-  }
-  if (!FreeDiskSpaceIfNeededFor(file_size))
-    return FILE_ERROR_NO_LOCAL_SPACE;
-
-  FileCacheEntry cache_entry;
-  storage_->GetCacheEntry(id, &cache_entry);
-
-  // If file is dirty or mounted, return error.
-  if (cache_entry.is_dirty() || mounted_files_.count(id))
-    return FILE_ERROR_IN_USE;
-
-  base::FilePath dest_path = GetCacheFilePath(id);
-  bool success = false;
-  switch (file_operation_type) {
-    case FILE_OPERATION_MOVE:
-      success = base::Move(source_path, dest_path);
-      break;
-    case FILE_OPERATION_COPY:
-      success = base::CopyFile(source_path, dest_path);
-      break;
-    default:
-      NOTREACHED();
-  }
-
-  if (!success) {
-    LOG(ERROR) << "Failed to store: "
-               << "source_path = " << source_path.value() << ", "
-               << "dest_path = " << dest_path.value() << ", "
-               << "file_operation_type = " << file_operation_type;
-    return FILE_ERROR_FAILED;
-  }
-
-  // Now that file operations have completed, update metadata.
-  cache_entry.set_md5(md5);
-  cache_entry.set_is_present(true);
-  cache_entry.set_is_dirty(false);
-  return storage_->PutCacheEntry(id, cache_entry) ?
-      FILE_ERROR_OK : FILE_ERROR_FAILED;
-}
-
 FileError FileCache::MarkAsUnmounted(const base::FilePath& file_path) {
   AssertOnSequencedWorkerPool();
   DCHECK(IsUnderFileCacheDirectory(file_path));
diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h
index cc59537..c3210cb 100644
--- a/chrome/browser/chromeos/drive/file_cache.h
+++ b/chrome/browser/chromeos/drive/file_cache.h
@@ -7,9 +7,7 @@
 
 #include <set>
 #include <string>
-#include <vector>
 
-#include "base/callback_forward.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -26,11 +24,6 @@ class FileCacheEntry;
 
 namespace internal {
 
-// Callback for GetFileFromCache.
-typedef base::Callback<void(FileError error,
-                            const base::FilePath& cache_file_path)>
-    GetFileFromCacheCallback;
-
 // Interface class used for getting the free disk space. Tests can inject an
 // implementation that reports fake free disk space.
 class FreeDiskSpaceGetterInterface {
@@ -55,8 +48,9 @@ class FileCache {
 
   // |cache_file_directory| stores cached files.
   //
-  // |blocking_task_runner| is used to post a task to the blocking worker
-  // pool for file operations. Must not be null.
+  // |blocking_task_runner| indicates the blocking worker pool for cache
+  // operations. All operations on this FileCache must be run on this runner.
+  // Must not be null.
   //
   // |free_disk_space_getter| is used to inject a custom free disk space
   // getter for testing. NULL must be passed for production code.
@@ -98,42 +92,18 @@ class FileCache {
                   const base::FilePath& source_path,
                   FileOperationType file_operation_type);
 
-  // Runs Pin() on |blocking_task_runner_|, and calls |callback| with the result
-  // asynchronously.
-  // |callback| must not be null.
-  // Must be called on the UI thread.
-  void PinOnUIThread(const std::string& id,
-                     const FileOperationCallback& callback);
-
   // Pins the specified entry.
   FileError Pin(const std::string& id);
 
-  // Runs Unpin() on |blocking_task_runner_|, and calls |callback| with the
-  // result asynchronously.
-  // |callback| must not be null.
-  // Must be called on the UI thread.
-  void UnpinOnUIThread(const std::string& id,
-                       const FileOperationCallback& callback);
-
   // Unpins the specified entry.
   FileError Unpin(const std::string& id);
 
-  // Runs MarkAsMounted() on |blocking_task_runner_|, and calls |callback| with
-  // the result asynchronously.
-  // |callback| must not be null.
-  // Must be called on the UI thread.
-  void MarkAsMountedOnUIThread(const std::string& id,
-                               const GetFileFromCacheCallback& callback);
-
   // Sets the state of the cache entry corresponding to |id| as mounted.
   FileError MarkAsMounted(const std::string& id,
                           base::FilePath* cache_file_path);
 
-  // Set the state of the cache entry corresponding to file_path as unmounted.
-  // |callback| must not be null.
-  // Must be called on the UI thread.
-  void MarkAsUnmountedOnUIThread(const base::FilePath& file_path,
-                                 const FileOperationCallback& callback);
+  // Sets the state of the cache entry corresponding to file_path as unmounted.
+  FileError MarkAsUnmounted(const base::FilePath& file_path);
 
   // Marks the specified entry dirty.
   FileError MarkDirty(const std::string& id);
@@ -155,7 +125,7 @@ class FileCache {
   // Must be called on the UI thread.
   void Destroy();
 
-  // Moves files in the cache directory which are not manged by FileCache to
+  // Moves files in the cache directory which are not managed by FileCache to
   // |dest_directory|.
   // |recovered_cache_info| should contain cache info recovered from the trashed
   // metadata DB. It is used to ignore non-dirty files.
@@ -182,17 +152,6 @@ class FileCache {
   // Destroys the cache on the blocking pool.
   void DestroyOnBlockingPool();
 
-  // Used to implement Store and StoreLocallyModifiedOnUIThread.
-  // TODO(hidehiko): Merge this method with Store(), after
-  // StoreLocallyModifiedOnUIThread is removed.
-  FileError StoreInternal(const std::string& id,
-                          const std::string& md5,
-                          const base::FilePath& source_path,
-                          FileOperationType file_operation_type);
-
-  // Used to implement MarkAsUnmountedOnUIThread.
-  FileError MarkAsUnmounted(const base::FilePath& file_path);
-
   // Returns true if we have sufficient space to store the given number of
   // bytes, while keeping kMinFreeSpace bytes on the disk.
   bool HasEnoughSpaceFor(int64 num_bytes, const base::FilePath& path);
@@ -223,7 +182,7 @@ class FileCache {
 // this value.
 //
 // Copied from cryptohome/homedirs.h.
-// TODO(satorux): Share the constant.
+// TODO(hashimoto): Share the constant.
 const int64 kMinFreeSpace = 512 * 1LL << 20;
 
 }  // namespace internal
diff --git a/chrome/browser/chromeos/drive/file_cache_unittest.cc b/chrome/browser/chromeos/drive/file_cache_unittest.cc
index 41edfaa..6908e5f 100644
--- a/chrome/browser/chromeos/drive/file_cache_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc
@@ -159,8 +159,12 @@ class FileCacheTestOnUIThread : public testing::Test {
     expected_cache_state_ = expected_cache_state;
 
     FileError error = FILE_ERROR_OK;
-    cache_->PinOnUIThread(
-        id,
+    base::PostTaskAndReplyWithResult(
+        blocking_task_runner_,
+        FROM_HERE,
+        base::Bind(&internal::FileCache::Pin,
+                   base::Unretained(cache_.get()),
+                   id),
         google_apis::test_util::CreateCopyResultCallback(&error));
     test_util::RunBlockingPoolTask();
     VerifyCacheFileState(error, id);
@@ -173,8 +177,12 @@ class FileCacheTestOnUIThread : public testing::Test {
     expected_cache_state_ = expected_cache_state;
 
     FileError error = FILE_ERROR_OK;
-    cache_->UnpinOnUIThread(
-        id,
+    base::PostTaskAndReplyWithResult(
+        blocking_task_runner_,
+        FROM_HERE,
+        base::Bind(&internal::FileCache::Unpin,
+                   base::Unretained(cache_.get()),
+                   id),
         google_apis::test_util::CreateCopyResultCallback(&error));
     test_util::RunBlockingPoolTask();
     VerifyCacheFileState(error, id);
@@ -224,7 +232,7 @@ class FileCacheTestOnUIThread : public testing::Test {
     expected_cache_state_ = expected_cache_state;
 
     FileError error = FILE_ERROR_OK;
-    PostTaskAndReplyWithResult(
+    base::PostTaskAndReplyWithResult(
         blocking_task_runner_.get(),
         FROM_HERE,
         base::Bind(&FileCache::ClearDirty,
@@ -254,10 +262,15 @@ class FileCacheTestOnUIThread : public testing::Test {
 
     FileError error = FILE_ERROR_OK;
     base::FilePath cache_file_path;
-    cache_->MarkAsMountedOnUIThread(
-        id,
-        google_apis::test_util::CreateCopyResultCallback(
-            &error, &cache_file_path));
+
+    base::PostTaskAndReplyWithResult(
+        blocking_task_runner_.get(),
+        FROM_HERE,
+        base::Bind(&FileCache::MarkAsMounted,
+                   base::Unretained(cache_.get()),
+                   id,
+                   &cache_file_path),
+        google_apis::test_util::CreateCopyResultCallback(&error));
     test_util::RunBlockingPoolTask();
 
     EXPECT_TRUE(base::PathExists(cache_file_path));
@@ -272,8 +285,12 @@ class FileCacheTestOnUIThread : public testing::Test {
     expected_cache_state_ = expected_cache_state;
 
     FileError error = FILE_ERROR_OK;
-    cache_->MarkAsUnmountedOnUIThread(
-        file_path,
+    base::PostTaskAndReplyWithResult(
+        blocking_task_runner_.get(),
+        FROM_HERE,
+        base::Bind(&FileCache::MarkAsUnmounted,
+                   base::Unretained(cache_.get()),
+                   file_path),
         google_apis::test_util::CreateCopyResultCallback(&error));
     test_util::RunBlockingPoolTask();
 
diff --git a/chrome/browser/chromeos/drive/file_system.cc b/chrome/browser/chromeos/drive/file_system.cc
index 2fdf176..450cc1f 100644
--- a/chrome/browser/chromeos/drive/file_system.cc
+++ b/chrome/browser/chromeos/drive/file_system.cc
@@ -969,7 +969,14 @@ void FileSystem::MarkCacheFileAsUnmounted(
     callback.Run(FILE_ERROR_FAILED);
     return;
   }
-  cache_->MarkAsUnmountedOnUIThread(cache_file_path, callback);
+
+  base::PostTaskAndReplyWithResult(
+      blocking_task_runner_,
+      FROM_HERE,
+      base::Bind(&internal::FileCache::MarkAsUnmounted,
+                 base::Unretained(cache_),
+                 cache_file_path),
+      callback);
 }
 
 void FileSystem::GetCacheEntry(
diff --git a/chrome/browser/chromeos/drive/file_system/update_operation_unittest.cc b/chrome/browser/chromeos/drive/file_system/update_operation_unittest.cc
index 3d398eb..cec98f8 100644
--- a/chrome/browser/chromeos/drive/file_system/update_operation_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_system/update_operation_unittest.cc
@@ -43,8 +43,12 @@ TEST_F(UpdateOperationTest, UpdateFileByLocalId_PersistentFile) {
 
   // Pin the file so it'll be store in "persistent" directory.
   FileError error = FILE_ERROR_FAILED;
-  cache()->PinOnUIThread(
-      local_id,
+  base::PostTaskAndReplyWithResult(
+      blocking_task_runner(),
+      FROM_HERE,
+      base::Bind(&internal::FileCache::Pin,
+                 base::Unretained(cache()),
+                 local_id),
       google_apis::test_util::CreateCopyResultCallback(&error));
   test_util::RunBlockingPoolTask();
   EXPECT_EQ(FILE_ERROR_OK, error);
diff --git a/chrome/browser/chromeos/drive/sync_client.cc b/chrome/browser/chromeos/drive/sync_client.cc
index 1c034ae..4de80b0 100644
--- a/chrome/browser/chromeos/drive/sync_client.cc
+++ b/chrome/browser/chromeos/drive/sync_client.cc
@@ -398,8 +398,11 @@ void SyncClient::OnFetchFileComplete(const std::string& local_id,
       case FILE_ERROR_ABORT:
         // If user cancels download, unpin the file so that we do not sync the
         // file again.
-        cache_->UnpinOnUIThread(local_id,
-                                base::Bind(&util::EmptyFileOperationCallback));
+        base::PostTaskAndReplyWithResult(
+            blocking_task_runner_,
+            FROM_HERE,
+            base::Bind(&FileCache::Unpin, base::Unretained(cache_), local_id),
+            base::Bind(&util::EmptyFileOperationCallback));
         break;
       case FILE_ERROR_NO_CONNECTION:
         // Add the task again so that we'll retry once the connection is back.