Remove CloseFile from FileSystem.

chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc

Index: chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc
diff --git a/chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc b/chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc
index 4e22efec2a2887e5dda60e68ed3dd2921e081834..87bce4d2a4d44805a4c1fc18dedd53fd411265c2 100644
--- a/chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_system/open_file_operation_unittest.cc
@@ -25,10 +25,9 @@ class OpenFileOperationTest : public OperationTestBase {
 
     operation_.reset(new OpenFileOperation(
         blocking_task_runner(), observer(), scheduler(), metadata(), cache(),
-        temp_dir(), &open_files_));
+        temp_dir()));
   }
 
-  std::map<base::FilePath, int> open_files_;
   scoped_ptr<OpenFileOperation> operation_;
 };
 
@@ -41,10 +40,12 @@ TEST_F(OpenFileOperationTest, OpenExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       OPEN_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -52,9 +53,6 @@ TEST_F(OpenFileOperationTest, OpenExistingFile) {
   int64 local_file_size;
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(file_size, local_file_size);

[hashimoto, 2013/07/23 11:24:58]

How about having EXPECT_FALSE(on_close_callback.is_null()) for all appropriate
cases?
Optionally, actually calling the callback and check the state of
observer()->upload_needed_resource_ids().

[hidehiko, 2013/07/23 14:32:24]

Done.

-
-  // The file_path should be added into the set.
-  EXPECT_EQ(1, open_files_[file_in_root]);
 }
 
 TEST_F(OpenFileOperationTest, OpenNonExistingFile) {
@@ -63,15 +61,14 @@ TEST_F(OpenFileOperationTest, OpenNonExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       OPEN_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
   EXPECT_EQ(FILE_ERROR_NOT_FOUND, error);
-
-  // The file shouldn't be in the set of opened files.
-  EXPECT_EQ(0U, open_files_.count(file_in_root));
 }
 
 TEST_F(OpenFileOperationTest, CreateExistingFile) {
@@ -82,16 +79,15 @@ TEST_F(OpenFileOperationTest, CreateExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       CREATE_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_EXISTS, error);
-
-  // The file shouldn't be in the set of opened files.
-  EXPECT_EQ(0U, open_files_.count(file_in_root));
 }
 
 TEST_F(OpenFileOperationTest, CreateNonExistingFile) {
@@ -100,10 +96,12 @@ TEST_F(OpenFileOperationTest, CreateNonExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       CREATE_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -111,9 +109,6 @@ TEST_F(OpenFileOperationTest, CreateNonExistingFile) {
   int64 local_file_size;
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(0, local_file_size);  // Should be an empty file.
-
-  // The file_path should be added into the set.
-  EXPECT_EQ(1, open_files_[file_in_root]);
 }
 
 TEST_F(OpenFileOperationTest, OpenOrCreateExistingFile) {
@@ -125,10 +120,12 @@ TEST_F(OpenFileOperationTest, OpenOrCreateExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       OPEN_OR_CREATE_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -136,9 +133,6 @@ TEST_F(OpenFileOperationTest, OpenOrCreateExistingFile) {
   int64 local_file_size;
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(file_size, local_file_size);
-
-  // The file_path should be added into the set.
-  EXPECT_EQ(1, open_files_[file_in_root]);
 }
 
 TEST_F(OpenFileOperationTest, OpenOrCreateNonExistingFile) {
@@ -147,10 +141,12 @@ TEST_F(OpenFileOperationTest, OpenOrCreateNonExistingFile) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       OPEN_OR_CREATE_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -158,9 +154,6 @@ TEST_F(OpenFileOperationTest, OpenOrCreateNonExistingFile) {
   int64 local_file_size;
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(0, local_file_size);  // Should be an empty file.
-
-  // The file_path should be added into the set.
-  EXPECT_EQ(1, open_files_[file_in_root]);
 }
 
 TEST_F(OpenFileOperationTest, OpenFileTwice) {
@@ -172,10 +165,12 @@ TEST_F(OpenFileOperationTest, OpenFileTwice) {
 
   FileError error = FILE_ERROR_FAILED;
   base::FilePath file_path;
+  base::Closure on_close_callback;
   operation_->OpenFile(
       file_in_root,
       OPEN_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -184,15 +179,14 @@ TEST_F(OpenFileOperationTest, OpenFileTwice) {
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(file_size, local_file_size);
 
-  // The file_path should be added into the set.
-  EXPECT_EQ(1, open_files_[file_in_root]);
-
   // Open again.
   error = FILE_ERROR_FAILED;
+  base::Closure on_close_callback2;
   operation_->OpenFile(
       file_in_root,
       OPEN_FILE,
-      google_apis::test_util::CreateCopyResultCallback(&error, &file_path));
+      google_apis::test_util::CreateCopyResultCallback(
+          &error, &file_path, &on_close_callback2));
   test_util::RunBlockingPoolTask();
 
   EXPECT_EQ(FILE_ERROR_OK, error);
@@ -200,8 +194,21 @@ TEST_F(OpenFileOperationTest, OpenFileTwice) {
   ASSERT_TRUE(file_util::GetFileSize(file_path, &local_file_size));
   EXPECT_EQ(file_size, local_file_size);
 
-  // The file_path should be added into the set.
-  EXPECT_EQ(2, open_files_[file_in_root]);
+  ASSERT_FALSE(on_close_callback.is_null());
+  ASSERT_FALSE(on_close_callback2.is_null());
+
+  on_close_callback.Run();
+
+  // There still remains a client opening the file, so it shouldn't be
+  // uploaded yet.
+  EXPECT_TRUE(observer()->upload_needed_resource_ids().empty());
+
+  on_close_callback2.Run();
+
+  // Here, all the clients close the file, so it should be uploaded then.
+  EXPECT_EQ(
+      1U,
+      observer()->upload_needed_resource_ids().count(src_entry.resource_id()));
 }
 
 }  // namespace file_system