Make FileSystemOperation's lifetime more explicit.

In the current code calling dispatcher->DidXxx in an operation's DidXxx method MAY indirectly delete the operation itself depending on the dispatcher's implementation.  I was confused by this several times and I want to make this flow more explicit.

This patch lets FileSystemOperation control its lifetime by itself so that each callback dispatcher implementation does not need to take care of it.

Also moved BrowserFileSystemCallbackDispatcher into file_system_dispatcher_host.cc as it's only used in it and its implementation is tightly coupled with the DispatcherHost.

BUG=60243
TEST=none

Review URL: http://codereview.chromium.org/4821005

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

1 files changed, 107 insertions, 167 deletions

diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc
index 370d95c..dffcc33 100644
--- a/webkit/fileapi/file_system_operation_unittest.cc
+++ b/webkit/fileapi/file_system_operation_unittest.cc
@@ -14,40 +14,69 @@
 
 namespace fileapi {
 
-const int kInvalidRequestId = -1;
-const int kFileOperationStatusNotSet = 0;
-const int kFileOperationSucceeded = 1;
-
-static int last_request_id = -1;
+const int kFileOperationStatusNotSet = 1;
+const int kFileOperationSucceeded = 0;
 
 static bool FileExists(FilePath path) {
   return file_util::PathExists(path) && !file_util::DirectoryExists(path);
 }
 
-class MockDispatcher : public FileSystemCallbackDispatcher {
+class MockDispatcher;
+
+class FileSystemOperationTest : public testing::Test {
  public:
-  MockDispatcher(int request_id)
-      : status_(kFileOperationStatusNotSet),
-        request_id_(request_id) {
+  FileSystemOperationTest()
+      : status_(kFileOperationStatusNotSet) {
+    base_.CreateUniqueTempDir();
+    EXPECT_TRUE(base_.IsValid());
   }
 
+  FileSystemOperation* operation();
+
+  void set_status(int status) { status_ = status; }
+  int status() const { return status_; }
+  void set_info(const base::PlatformFileInfo& info) { info_ = info; }
+  const base::PlatformFileInfo& info() const { return info_; }
+  void set_entries(const std::vector<base::FileUtilProxy::Entry>& entries) {
+    entries_ = entries;
+  }
+  const std::vector<base::FileUtilProxy::Entry>& entries() const {
+    return entries_;
+  }
+
+ protected:
+  // Common temp base for nondestructive uses.
+  ScopedTempDir base_;
+
+  // For post-operation status.
+  int status_;
+  base::PlatformFileInfo info_;
+  std::vector<base::FileUtilProxy::Entry> entries_;
+
+  DISALLOW_COPY_AND_ASSIGN(FileSystemOperationTest);
+};
+
+class MockDispatcher : public FileSystemCallbackDispatcher {
+ public:
+  MockDispatcher(FileSystemOperationTest* test) : test_(test) { }
+
   virtual void DidFail(base::PlatformFileError status) {
-    status_ = status;
+    test_->set_status(status);
   }
 
   virtual void DidSucceed() {
-    status_ = kFileOperationSucceeded;
+    test_->set_status(kFileOperationSucceeded);
   }
 
   virtual void DidReadMetadata(const base::PlatformFileInfo& info) {
-    info_ = info;
-    status_ = kFileOperationSucceeded;
+    test_->set_info(info);
+    test_->set_status(kFileOperationSucceeded);
   }
 
   virtual void DidReadDirectory(
       const std::vector<base::FileUtilProxy::Entry>& entries,
       bool /* has_more */) {
-    entries_ = entries;
+    test_->set_entries(entries);
   }
 
   virtual void DidOpenFileSystem(const std::string&, const FilePath&) {
@@ -58,58 +87,22 @@ class MockDispatcher : public FileSystemCallbackDispatcher {
     NOTREACHED();
   }
 
-  // Helpers for testing.
-  int status() const { return status_; }
-  int request_id() const { return request_id_; }
-  const base::PlatformFileInfo& info() const { return info_; }
-  const std::vector<base::FileUtilProxy::Entry>& entries() const {
-    return entries_;
-  }
-
  private:
-  int status_;
-  int request_id_;
-  base::PlatformFileInfo info_;
-  std::vector<base::FileUtilProxy::Entry> entries_;
+  FileSystemOperationTest* test_;
 };
 
-class FileSystemOperationTest : public testing::Test {
- public:
-  FileSystemOperationTest()
-      : request_id_(kInvalidRequestId),
-        operation_(NULL) {
-    base_.CreateUniqueTempDir();
-    EXPECT_TRUE(base_.IsValid());
-  }
-
-  FileSystemOperation* operation() {
-    request_id_ = ++last_request_id;
-    mock_dispatcher_ = new MockDispatcher(request_id_);
-    operation_.reset(new FileSystemOperation(
-        mock_dispatcher_, base::MessageLoopProxy::CreateForCurrentThread()));
-    return operation_.get();
-  }
-
- protected:
-  // Common temp base for nondestructive uses.
-  ScopedTempDir base_;
-
-  int request_id_;
-  scoped_ptr<FileSystemOperation> operation_;
-
-  // Owned by |operation_|.
-  MockDispatcher* mock_dispatcher_;
-
-  DISALLOW_COPY_AND_ASSIGN(FileSystemOperationTest);
-};
+FileSystemOperation* FileSystemOperationTest::operation() {
+  return new FileSystemOperation(
+      new MockDispatcher(this),
+      base::MessageLoopProxy::CreateForCurrentThread());
+}
 
 TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) {
   FilePath src(base_.path().Append(FILE_PATH_LITERAL("a")));
   FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b")));
   operation()->Move(src, dest);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) {
@@ -121,9 +114,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) {
                                                  &dest_dir_path));
   operation()->Move(src_dir.path(), dest_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) {
@@ -138,9 +129,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) {
 
   operation()->Move(src_dir.path(), dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) {
@@ -155,8 +144,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) {
 
   operation()->Move(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) {
@@ -171,9 +159,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) {
 
   operation()->Move(src_file, dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) {
@@ -186,8 +172,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) {
 
   operation()->Move(src_dir.path(), nonexisting_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) {
@@ -203,9 +188,8 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) {
 
   operation()->Move(src_file, dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) {
@@ -220,9 +204,8 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) {
 
   operation()->Move(src_file, dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) {
@@ -234,8 +217,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) {
 
   operation()->Move(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_FALSE(file_util::DirectoryExists(src_dir.path()));
 
   // Make sure we've overwritten but not moved the source under the |dest_dir|.
@@ -254,8 +236,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndNew) {
 
   operation()->Move(src_dir.path(), dest_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_FALSE(file_util::DirectoryExists(src_dir.path()));
   EXPECT_TRUE(file_util::DirectoryExists(dest_dir_path));
 }
@@ -271,8 +252,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) {
 
   operation()->Move(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName())));
 }
 
@@ -281,8 +261,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) {
   FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b")));
   operation()->Copy(src, dest);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) {
@@ -294,9 +273,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) {
                                                  &dest_dir_path));
   operation()->Copy(src_dir.path(), dest_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) {
@@ -311,9 +288,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) {
 
   operation()->Copy(src_dir.path(), dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) {
@@ -328,8 +303,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) {
 
   operation()->Copy(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) {
@@ -344,9 +318,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) {
 
   operation()->Copy(src_file, dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) {
@@ -362,8 +334,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) {
 
   operation()->Copy(src_dir, nonexisting_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) {
@@ -379,9 +350,8 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) {
 
   operation()->Copy(src_file, dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) {
@@ -396,9 +366,8 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) {
 
   operation()->Copy(src_file, dest_file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) {
@@ -410,8 +379,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) {
 
   operation()->Copy(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   // Make sure we've overwritten but not copied the source under the |dest_dir|.
   EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path()));
@@ -429,8 +397,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) {
 
   operation()->Copy(src_dir.path(), dest_dir);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(file_util::DirectoryExists(dest_dir));
 }
 
@@ -445,8 +412,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirRecursive) {
 
   operation()->Copy(src_dir.path(), dest_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(dest_dir.path().Append(child_file.BaseName())));
 }
 
@@ -459,8 +425,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileFailure) {
   file_util::CreateTemporaryFileInDir(dir.path(), &file);
   operation()->CreateFile(file, true);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) {
@@ -472,9 +437,8 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) {
 
   operation()->CreateFile(file, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) {
@@ -484,9 +448,8 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) {
   FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist"));
   operation()->CreateFile(file, true);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(FileExists(file));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) {
@@ -496,8 +459,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) {
   FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist"));
   operation()->CreateFile(file, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 }
 
 TEST_F(FileSystemOperationTest,
@@ -509,8 +471,7 @@ TEST_F(FileSystemOperationTest,
       FILE_PATH_LITERAL("FileDoesntExist"));
   operation()->CreateDirectory(nonexisting_file, false, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) {
@@ -519,8 +480,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) {
   ASSERT_TRUE(src_dir.CreateUniqueTempDir());
   operation()->CreateDirectory(src_dir.path(), true, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) {
@@ -531,8 +491,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) {
   file_util::CreateTemporaryFileInDir(dir.path(), &file);
   operation()->CreateDirectory(file, true, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateDirSuccess) {
@@ -541,17 +500,15 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) {
   ASSERT_TRUE(dir.CreateUniqueTempDir());
   operation()->CreateDirectory(dir.path(), false, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   // Dir doesn't exist.
   FilePath nonexisting_dir_path(base_.path().Append(
       FILE_PATH_LITERAL("nonexistingdir")));
   operation()->CreateDirectory(nonexisting_dir_path, false, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) {
@@ -561,9 +518,8 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) {
 
   operation()->CreateDirectory(nonexisting_dir_path, true, false);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) {
@@ -571,18 +527,16 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) {
       FILE_PATH_LITERAL("nonexistingdir")));
   operation()->GetMetadata(nonexisting_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 
   operation()->FileExists(nonexisting_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 
   file_util::EnsureEndsWithSeparator(&nonexisting_dir_path);
   operation()->DirectoryExists(nonexisting_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) {
@@ -591,27 +545,23 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) {
 
   operation()->DirectoryExists(dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   operation()->GetMetadata(dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_TRUE(mock_dispatcher_->info().is_directory);
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
+  EXPECT_TRUE(info().is_directory);
 
   FilePath file;
   file_util::CreateTemporaryFileInDir(dir.path(), &file);
   operation()->FileExists(file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   operation()->GetMetadata(file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_FALSE(mock_dispatcher_->info().is_directory);
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
+  EXPECT_FALSE(info().is_directory);
 }
 
 TEST_F(FileSystemOperationTest, TestReadDirFailure) {
@@ -621,8 +571,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) {
   file_util::EnsureEndsWithSeparator(&nonexisting_dir_path);
   operation()->ReadDirectory(nonexisting_dir_path);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 
   // File exists.
   ScopedTempDir dir;
@@ -632,8 +581,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) {
   operation()->ReadDirectory(file);
   MessageLoop::current()->RunAllPending();
   // TODO(kkanetkar) crbug.com/54309 to change the error code.
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 }
 
 TEST_F(FileSystemOperationTest, TestReadDirSuccess) {
@@ -651,17 +599,16 @@ TEST_F(FileSystemOperationTest, TestReadDirSuccess) {
 
   operation()->ReadDirectory(parent_dir.path());
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationStatusNotSet, mock_dispatcher_->status());
-  EXPECT_EQ(2u, mock_dispatcher_->entries().size());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationStatusNotSet, status());
+  EXPECT_EQ(2u, entries().size());
 
-  for (size_t i = 0; i < mock_dispatcher_->entries().size(); ++i) {
-    if (mock_dispatcher_->entries()[i].is_directory) {
+  for (size_t i = 0; i < entries().size(); ++i) {
+    if (entries()[i].is_directory) {
       EXPECT_EQ(child_dir.BaseName().value(),
-                mock_dispatcher_->entries()[i].name);
+                entries()[i].name);
     } else {
       EXPECT_EQ(child_file.BaseName().value(),
-                mock_dispatcher_->entries()[i].name);
+                entries()[i].name);
     }
   }
 }
@@ -674,8 +621,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) {
 
   operation()->Remove(nonexisting, false /* recursive */);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status());
 
   // It's an error to try to remove a non-empty directory if recursive flag
   // is false.
@@ -694,8 +640,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) {
   operation()->Remove(parent_dir.path(), false /* recursive */);
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY,
-            mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+            status());
 }
 
 TEST_F(FileSystemOperationTest, TestRemoveSuccess) {
@@ -705,9 +650,8 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) {
 
   operation()->Remove(empty_dir.path(), false /* recursive */);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path()));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 
   // Removing a non-empty directory with recursive flag == true should be ok.
   //      parent_dir
@@ -724,9 +668,8 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) {
 
   operation()->Remove(parent_dir.path(), true /* recursive */);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
+  EXPECT_EQ(kFileOperationSucceeded, status());
   EXPECT_FALSE(file_util::DirectoryExists(parent_dir.path()));
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
 }
 
 // TODO(ericu): Add tests for Write, Cancel.
@@ -745,17 +688,15 @@ TEST_F(FileSystemOperationTest, TestTruncate) {
   // Check that its length is the size of the data written.
   operation()->GetMetadata(file);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_FALSE(mock_dispatcher_->info().is_directory);
-  EXPECT_EQ(data_size, mock_dispatcher_->info().size);
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
+  EXPECT_FALSE(info().is_directory);
+  EXPECT_EQ(data_size, info().size);
 
   // Extend the file by truncating it.
   int length = 17;
   operation()->Truncate(file, length);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   // Check that its length is now 17 and that it's all zeroes after the test
   // data.
@@ -776,8 +717,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) {
   length = 3;
   operation()->Truncate(file, length);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kFileOperationSucceeded, mock_dispatcher_->status());
-  EXPECT_EQ(request_id_, mock_dispatcher_->request_id());
+  EXPECT_EQ(kFileOperationSucceeded, status());
 
   // Check that its length is now 3 and that it contains only bits of test data.
   EXPECT_TRUE(file_util::GetFileInfo(file, &info));