Refactor FileSystemOperation to take callback for each method.

webkit/fileapi/file_system_operation_write_unittest.cc

Index: webkit/fileapi/file_system_operation_write_unittest.cc
diff --git a/webkit/fileapi/file_system_operation_write_unittest.cc b/webkit/fileapi/file_system_operation_write_unittest.cc
index 28d0298d5041ec06aa87839f9d73b471e97637e2..b45f835eccb3336d684b664442b5cabbcc6fafff 100644
--- a/webkit/fileapi/file_system_operation_write_unittest.cc
+++ b/webkit/fileapi/file_system_operation_write_unittest.cc
@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/message_loop.h"
 #include "base/message_loop_proxy.h"
@@ -23,7 +24,6 @@
 #include "webkit/blob/blob_data.h"
 #include "webkit/blob/blob_storage_controller.h"
 #include "webkit/blob/blob_url_request_job.h"
-#include "webkit/fileapi/file_system_callback_dispatcher.h"
 #include "webkit/fileapi/file_system_context.h"
 #include "webkit/fileapi/file_system_file_util.h"
 #include "webkit/fileapi/file_system_operation.h"
@@ -65,25 +65,22 @@ class MockQuotaManager : public QuotaManager {
 
 }  // namespace (anonymous)
 
-class FileSystemOperationWriteTest : public testing::Test {
+class FileSystemOperationWriteTest
+    : public testing::Test,
+      public base::SupportsWeakPtr<FileSystemOperationWriteTest> {
  public:
   FileSystemOperationWriteTest()
       : local_file_util_(new LocalFileUtil(QuotaFileUtil::CreateDefault())),
         loop_(MessageLoop::TYPE_IO),
         status_(base::PLATFORM_FILE_OK),
+        cancel_status_(base::PLATFORM_FILE_ERROR_FAILED),
         bytes_written_(0),
         complete_(false) {}
 
   FileSystemOperation* operation();
 
-  void set_failure_status(base::PlatformFileError status) {
-    EXPECT_FALSE(complete_);
-    EXPECT_EQ(status_, base::PLATFORM_FILE_OK);
-    EXPECT_NE(status, base::PLATFORM_FILE_OK);
-    complete_ = true;
-    status_ = status;
-  }
   base::PlatformFileError status() const { return status_; }
+  base::PlatformFileError cancel_status() const { return cancel_status_; }
   void add_bytes_written(int64 bytes, bool complete) {
     bytes_written_ += bytes;
     EXPECT_FALSE(complete_);
@@ -100,6 +97,36 @@ class FileSystemOperationWriteTest : public testing::Test {
     return test_helper_.GetURLForPath(path);
   }
 
+  // Callback function for recording test results.
+  FileSystemOperationInterface::WriteCallback RecordWriteCallback() {
+    return base::Bind(&FileSystemOperationWriteTest::DidWrite, AsWeakPtr());
+  }
+
+  FileSystemOperationInterface::StatusCallback RecordCancelCallback() {
+    return base::Bind(&FileSystemOperationWriteTest::DidCancel, AsWeakPtr());
+  }
+
+  void DidWrite(base::PlatformFileError status, int64 bytes, bool complete) {
+    if (status == base::PLATFORM_FILE_OK) {
+      add_bytes_written(bytes, complete);
+      if (complete) {
+        if (MessageLoop::current()->is_running())

[kinuko, 2012/02/13 10:10:21]

This is ok but I guess it will never be false?

[kinaba, 2012/02/14 02:17:47]

Done.

+          MessageLoop::current()->Quit();
+      }
+    } else {
+      EXPECT_FALSE(complete_);
+      EXPECT_EQ(status_, base::PLATFORM_FILE_OK);
+      complete_ = true;
+      status_ = status;
+      if (MessageLoop::current()->is_running())
+        MessageLoop::current()->Quit();
+    }
+  }
+
+  void DidCancel(base::PlatformFileError status) {
+    cancel_status_ = status;
+  }
+
   scoped_ptr<LocalFileUtil> local_file_util_;
   scoped_refptr<MockQuotaManager> quota_manager_;
   FileSystemTestOriginHelper test_helper_;
@@ -113,6 +140,7 @@ class FileSystemOperationWriteTest : public testing::Test {
 
   // For post-operation status.
   base::PlatformFileError status_;
+  base::PlatformFileError cancel_status_;
   int64 bytes_written_;
   bool complete_;
 
@@ -147,45 +175,6 @@ static net::URLRequestJob* BlobURLRequestJobFactory(net::URLRequest* request,
       base::MessageLoopProxy::current());
 }
 
-class MockDispatcher : public FileSystemCallbackDispatcher {
- public:
-  MockDispatcher(FileSystemOperationWriteTest* test) : test_(test) { }
-
-  virtual void DidFail(base::PlatformFileError status) {
-    test_->set_failure_status(status);
-    MessageLoop::current()->Quit();
-  }
-
-  virtual void DidSucceed() {
-    ADD_FAILURE();
-  }
-
-  virtual void DidReadMetadata(
-      const base::PlatformFileInfo& info,
-      const FilePath& platform_path) {
-    ADD_FAILURE();
-  }
-
-  virtual void DidReadDirectory(
-      const std::vector<base::FileUtilProxy::Entry>& entries,
-      bool /* has_more */) {
-    ADD_FAILURE();
-  }
-
-  virtual void DidOpenFileSystem(const std::string&, const GURL&) {
-    ADD_FAILURE();
-  }
-
-  virtual void DidWrite(int64 bytes, bool complete) {
-    test_->add_bytes_written(bytes, complete);
-    if (complete)
-      MessageLoop::current()->Quit();
-  }
-
- private:
-  FileSystemOperationWriteTest* test_;
-};
-
 }  // namespace (anonymous)
 
 void FileSystemOperationWriteTest::SetUp() {
@@ -213,7 +202,7 @@ void FileSystemOperationWriteTest::TearDown() {
 }
 
 FileSystemOperation* FileSystemOperationWriteTest::operation() {
-  return test_helper_.NewOperation(new MockDispatcher(this));
+  return test_helper_.NewOperation();
 }
 
 TEST_F(FileSystemOperationWriteTest, TestWriteSuccess) {
@@ -227,7 +216,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteSuccess) {
       blob_url, blob_data);
 
   operation()->Write(url_request_context, URLForPath(virtual_path_), blob_url,
-                     0);
+                     0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
@@ -248,7 +237,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteZero) {
       blob_url, blob_data);
 
   operation()->Write(url_request_context, URLForPath(virtual_path_),
-                     blob_url, 0);
+                     blob_url, 0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
@@ -263,7 +252,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteInvalidBlobUrl) {
       new TestURLRequestContext());
 
   operation()->Write(url_request_context, URLForPath(virtual_path_),
-      GURL("blob:invalid"), 0);
+      GURL("blob:invalid"), 0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   EXPECT_EQ(0, bytes_written());
@@ -283,7 +272,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteInvalidFile) {
 
   operation()->Write(url_request_context,
                      URLForPath(FilePath(FILE_PATH_LITERAL("nonexist"))),
-                     blob_url, 0);
+                     blob_url, 0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
@@ -310,7 +299,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteDir) {
       blob_url, blob_data);
 
   operation()->Write(url_request_context, URLForPath(virtual_subdir_path),
-                     blob_url, 0);
+                     blob_url, 0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
@@ -332,7 +321,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteFailureByQuota) {
 
   quota_manager_->set_quota(10);
   operation()->Write(url_request_context, URLForPath(virtual_path_), blob_url,
-                     0);
+                     0, RecordWriteCallback());
   MessageLoop::current()->Run();
 
   url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
@@ -342,6 +331,32 @@ TEST_F(FileSystemOperationWriteTest, TestWriteFailureByQuota) {
   EXPECT_TRUE(complete());
 }
 
-// TODO(ericu,dmikurube): Add tests for Cancel.
+TEST_F(FileSystemOperationWriteTest, TestImmediateCancel) {
+  GURL blob_url("blob:success");
+  scoped_refptr<webkit_blob::BlobData> blob_data(new webkit_blob::BlobData());
+  blob_data->AppendData("Hello, world!\n");
+
+  scoped_refptr<TestURLRequestContext> url_request_context(
+      new TestURLRequestContext());
+  url_request_context->blob_storage_controller()->AddFinishedBlob(
+      blob_url, blob_data);
+
+  FileSystemOperationInterface* write_operation = operation();
+  write_operation->Write(url_request_context, URLForPath(virtual_path_),
+                         blob_url, 0, RecordWriteCallback());
+  write_operation->Cancel(RecordCancelCallback());
+  MessageLoop::current()->RunAllPending();

[kinuko, 2012/02/13 10:10:21]

Could we add a comment why we call RunAllPending() here instead of Run()?  (E.g.
We won't dispatch callbacks after Cancel() is issued (so no chance to Quit) nor
do we need to another run write cycle)

[kinaba, 2012/02/14 02:17:47]

Done.

+
+  url_request_context->blob_storage_controller()->RemoveBlob(blob_url);
+
+  // Issued Cancel() before receiving any response from Write(),
+  // so nothing should have happen.
+  EXPECT_EQ(0, bytes_written());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_ABORT, status());
+  EXPECT_EQ(base::PLATFORM_FILE_OK, cancel_status());
+  EXPECT_TRUE(complete());
+}
+
+// TODO(ericu,dmikurube): Add more tests for Cancel.
 
 }  // namespace fileapi