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

5 files changed, 180 insertions, 197 deletions

diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc
index fe07758..97e2bfe 100644
--- a/webkit/fileapi/file_system_operation.cc
+++ b/webkit/fileapi/file_system_operation.cc
@@ -16,8 +16,7 @@ FileSystemOperation::FileSystemOperation(
     scoped_refptr<base::MessageLoopProxy> proxy)
     : proxy_(proxy),
       dispatcher_(dispatcher),
-      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
-      cancel_operation_(NULL) {
+      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   DCHECK(dispatcher);
 #ifndef NDEBUG
   pending_operation_ = kOperationNone;
@@ -159,6 +158,7 @@ void FileSystemOperation::OnFileOpenedForWrite(
     bool created) {
   if (base::PLATFORM_FILE_OK != rv) {
     dispatcher_->DidFail(rv);
+    delete this;
     return;
   }
   file_writer_delegate_->Start(file.ReleaseValue(), blob_request_.get());
@@ -189,7 +189,8 @@ void FileSystemOperation::TouchFile(const FilePath& path,
 
 // We can only get here on a write or truncate that's not yet completed.
 // We don't support cancelling any other operation at this time.
-void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) {
+void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation_ptr) {
+  scoped_ptr<FileSystemOperation> cancel_operation(cancel_operation_ptr);
   if (file_writer_delegate_.get()) {
 #ifndef NDEBUG
     DCHECK(kOperationWrite == pending_operation_);
@@ -203,9 +204,9 @@ void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) {
       // This halts any calls to file_writer_delegate_ from blob_request_.
       blob_request_->Cancel();
 
-    // This deletes us, and by proxy deletes file_writer_delegate_ if any.
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT);
-    cancel_operation->dispatcher_->DidSucceed();
+    cancel_operation->dispatcher()->DidSucceed();
+    delete this;
   } else {
 #ifndef NDEBUG
     DCHECK(kOperationTruncate == pending_operation_);
@@ -214,15 +215,17 @@ void FileSystemOperation::Cancel(FileSystemOperation* cancel_operation) {
     // since it's been proxied to another thread.  We need to save the
     // cancel_operation so that when the truncate returns, it can see that it's
     // been cancelled, report it, and report that the cancel has succeeded.
-    cancel_operation_ = cancel_operation;
+    DCHECK(!cancel_operation_.get());
+    cancel_operation_.swap(cancel_operation);
   }
 }
 
 void FileSystemOperation::DidEnsureFileExistsExclusive(
     base::PlatformFileError rv, bool created) {
-  if (rv == base::PLATFORM_FILE_OK && !created)
+  if (rv == base::PLATFORM_FILE_OK && !created) {
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_EXISTS);
-  else
+    delete this;
+  } else
     DidFinishFileOperation(rv);
 }
 
@@ -233,19 +236,19 @@ void FileSystemOperation::DidEnsureFileExistsNonExclusive(
 
 void FileSystemOperation::DidFinishFileOperation(
     base::PlatformFileError rv) {
-  if (cancel_operation_) {
+  if (cancel_operation_.get()) {
 #ifndef NDEBUG
     DCHECK(kOperationTruncate == pending_operation_);
 #endif
-    FileSystemOperation *cancel_op = cancel_operation_;
-    // This call deletes us, so we have to extract cancel_op first.
+
     dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_ABORT);
-    cancel_op->dispatcher_->DidSucceed();
+    cancel_operation_->dispatcher()->DidSucceed();
   } else if (rv == base::PLATFORM_FILE_OK) {
     dispatcher_->DidSucceed();
   } else {
     dispatcher_->DidFail(rv);
   }
+  delete this;
 }
 
 void FileSystemOperation::DidDirectoryExists(
@@ -259,6 +262,7 @@ void FileSystemOperation::DidDirectoryExists(
   } else {
     dispatcher_->DidFail(rv);
   }
+  delete this;
 }
 
 void FileSystemOperation::DidFileExists(
@@ -272,6 +276,7 @@ void FileSystemOperation::DidFileExists(
   } else {
     dispatcher_->DidFail(rv);
   }
+  delete this;
 }
 
 void FileSystemOperation::DidGetMetadata(
@@ -281,6 +286,7 @@ void FileSystemOperation::DidGetMetadata(
     dispatcher_->DidReadMetadata(file_info);
   else
     dispatcher_->DidFail(rv);
+  delete this;
 }
 
 void FileSystemOperation::DidReadDirectory(
@@ -290,6 +296,7 @@ void FileSystemOperation::DidReadDirectory(
     dispatcher_->DidReadDirectory(entries, false /* has_more */);
   else
     dispatcher_->DidFail(rv);
+  delete this;
 }
 
 void FileSystemOperation::DidWrite(
@@ -300,6 +307,8 @@ void FileSystemOperation::DidWrite(
     dispatcher_->DidWrite(bytes, complete);
   else
     dispatcher_->DidFail(rv);
+  if (complete || rv != base::PLATFORM_FILE_OK)
+    delete this;
 }
 
 void FileSystemOperation::DidTouchFile(base::PlatformFileError rv) {
@@ -307,6 +316,7 @@ void FileSystemOperation::DidTouchFile(base::PlatformFileError rv) {
     dispatcher_->DidSucceed();
   else
     dispatcher_->DidFail(rv);
+  delete this;
 }
 
 }  // namespace fileapi
diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h
index 6614b5e..e260e24 100644
--- a/webkit/fileapi/file_system_operation.h
+++ b/webkit/fileapi/file_system_operation.h
@@ -36,8 +36,11 @@ class FileWriterDelegate;
 // Only one method(CreateFile, CreateDirectory, Copy, Move, DirectoryExists,
 // GetMetadata, ReadDirectory and Remove) may be called during the lifetime of
 // this object and it should be called no more than once.
+// This class is self-destructed and an instance automatically gets deleted
+// when its operation is finished.
 class FileSystemOperation {
  public:
+  // |dispatcher| will be owned by this class.
   FileSystemOperation(FileSystemCallbackDispatcher* dispatcher,
                       scoped_refptr<base::MessageLoopProxy> proxy);
   virtual ~FileSystemOperation();
@@ -65,15 +68,16 @@ class FileSystemOperation {
 
   virtual void Remove(const FilePath& path, bool recursive);
 
-  virtual void Write(
-      scoped_refptr<URLRequestContext> url_request_context,
-      const FilePath& path, const GURL& blob_url, int64 offset);
+  virtual void Write(scoped_refptr<URLRequestContext> url_request_context,
+                     const FilePath& path,
+                     const GURL& blob_url,
+                     int64 offset);
 
   virtual void Truncate(const FilePath& path, int64 length);
 
   virtual void TouchFile(const FilePath& path,
-                 const base::Time& last_access_time,
-                 const base::Time& last_modified_time);
+                         const base::Time& last_access_time,
+                         const base::Time& last_modified_time);
 
   // Try to cancel the current operation [we support cancelling write or
   // truncate only].  Report failure for the current operation, then tell the
@@ -154,7 +158,7 @@ class FileSystemOperation {
   friend class FileWriterDelegate;
   scoped_ptr<FileWriterDelegate> file_writer_delegate_;
   scoped_ptr<net::URLRequest> blob_request_;
-  FileSystemOperation* cancel_operation_;
+  scoped_ptr<FileSystemOperation> cancel_operation_;
 
   DISALLOW_COPY_AND_ASSIGN(FileSystemOperation);
 };
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));
diff --git a/webkit/fileapi/sandboxed_file_system_operation.cc b/webkit/fileapi/sandboxed_file_system_operation.cc
index dfc2884..7f9dd38 100644
--- a/webkit/fileapi/sandboxed_file_system_operation.cc
+++ b/webkit/fileapi/sandboxed_file_system_operation.cc
@@ -38,15 +38,19 @@ void SandboxedFileSystemOperation::OpenFileSystem(
 
 void SandboxedFileSystemOperation::CreateFile(
     const FilePath& path, bool exclusive) {
-  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0))
+  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) {
+    delete this;
     return;
+  }
   FileSystemOperation::CreateFile(path, exclusive);
 }
 
 void SandboxedFileSystemOperation::CreateDirectory(
     const FilePath& path, bool exclusive, bool recursive) {
-  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0))
+  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) {
+    delete this;
     return;
+  }
   FileSystemOperation::CreateDirectory(path, exclusive, recursive);
 }
 
@@ -54,8 +58,10 @@ void SandboxedFileSystemOperation::Copy(
     const FilePath& src_path, const FilePath& dest_path) {
   if (!VerifyFileSystemPathForRead(src_path) ||
       !VerifyFileSystemPathForWrite(dest_path, true /* create */,
-                                    FileSystemQuotaManager::kUnknownSize))
+                                    FileSystemQuotaManager::kUnknownSize)) {
+    delete this;
     return;
+  }
   FileSystemOperation::Copy(src_path, dest_path);
 }
 
@@ -63,39 +69,51 @@ void SandboxedFileSystemOperation::Move(
     const FilePath& src_path, const FilePath& dest_path) {
   if (!VerifyFileSystemPathForRead(src_path) ||
       !VerifyFileSystemPathForWrite(dest_path, true /* create */,
-                                    FileSystemQuotaManager::kUnknownSize))
+                                    FileSystemQuotaManager::kUnknownSize)) {
+    delete this;
     return;
+  }
   FileSystemOperation::Move(src_path, dest_path);
 }
 
 void SandboxedFileSystemOperation::DirectoryExists(const FilePath& path) {
-  if (!VerifyFileSystemPathForRead(path))
+  if (!VerifyFileSystemPathForRead(path)) {
+    delete this;
     return;
+  }
   FileSystemOperation::DirectoryExists(path);
 }
 
 void SandboxedFileSystemOperation::FileExists(const FilePath& path) {
-  if (!VerifyFileSystemPathForRead(path))
+  if (!VerifyFileSystemPathForRead(path)) {
+    delete this;
     return;
+  }
   FileSystemOperation::FileExists(path);
 }
 
 void SandboxedFileSystemOperation::GetMetadata(const FilePath& path) {
-  if (!VerifyFileSystemPathForRead(path))
+  if (!VerifyFileSystemPathForRead(path)) {
+    delete this;
     return;
+  }
   FileSystemOperation::GetMetadata(path);
 }
 
 void SandboxedFileSystemOperation::ReadDirectory(const FilePath& path) {
-  if (!VerifyFileSystemPathForRead(path))
+  if (!VerifyFileSystemPathForRead(path)) {
+    delete this;
     return;
+  }
   FileSystemOperation::ReadDirectory(path);
 }
 
 void SandboxedFileSystemOperation::Remove(
     const FilePath& path, bool recursive) {
-  if (!VerifyFileSystemPathForWrite(path, false /* create */, 0))
+  if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) {
+    delete this;
     return;
+  }
   FileSystemOperation::Remove(path, recursive);
 }
 
@@ -103,15 +121,19 @@ void SandboxedFileSystemOperation::Write(
     scoped_refptr<URLRequestContext> url_request_context,
     const FilePath& path, const GURL& blob_url, int64 offset) {
   if (!VerifyFileSystemPathForWrite(path, true /* create */,
-                                    FileSystemQuotaManager::kUnknownSize))
+                                    FileSystemQuotaManager::kUnknownSize)) {
+    delete this;
     return;
+  }
   FileSystemOperation::Write(url_request_context, path, blob_url, offset);
 }
 
 void SandboxedFileSystemOperation::Truncate(
     const FilePath& path, int64 length) {
-  if (!VerifyFileSystemPathForWrite(path, false /* create */, 0))
+  if (!VerifyFileSystemPathForWrite(path, false /* create */, 0)) {
+    delete this;
     return;
+  }
   FileSystemOperation::Truncate(path, length);
 }
 
@@ -119,8 +141,10 @@ void SandboxedFileSystemOperation::TouchFile(
     const FilePath& path,
     const base::Time& last_access_time,
     const base::Time& last_modified_time) {
-  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0))
+  if (!VerifyFileSystemPathForWrite(path, true /* create */, 0)) {
+    delete this;
     return;
+  }
   FileSystemOperation::TouchFile(path, last_access_time, last_modified_time);
 }
 
@@ -128,6 +152,7 @@ void SandboxedFileSystemOperation::DidGetRootPath(
     bool success, const FilePath& path, const std::string& name) {
   DCHECK(success || path.empty());
   dispatcher()->DidOpenFileSystem(name, path);
+  delete this;
 }
 
 bool SandboxedFileSystemOperation::VerifyFileSystemPathForRead(
diff --git a/webkit/fileapi/sandboxed_file_system_operation.h b/webkit/fileapi/sandboxed_file_system_operation.h
index 18d47f7..8794a9d 100644
--- a/webkit/fileapi/sandboxed_file_system_operation.h
+++ b/webkit/fileapi/sandboxed_file_system_operation.h
@@ -71,6 +71,8 @@ class SandboxedFileSystemOperation : public FileSystemOperation {
   // Returns true if the given |path| is a valid FileSystem path.
   // Otherwise it calls dispatcher's DidFail method with
   // PLATFORM_FILE_ERROR_SECURITY and returns false.
+  // (Note: this doesn't delete this when it calls DidFail and returns false;
+  // it's the caller's responsibility.)
   bool VerifyFileSystemPathForRead(const FilePath& path);
 
   // Checks the validity of a given |path| for writing.
@@ -85,6 +87,8 @@ class SandboxedFileSystemOperation : public FileSystemOperation {
   // If |create| flag is true this also checks if the |path| contains
   // any restricted names and chars. If it does, the call fires dispatcher's
   // DidFail with PLATFORM_FILE_ERROR_SECURITY and returns false.
+  // (Note: this doesn't delete this when it calls DidFail and returns false;
+  // it's the caller's responsibility.)
   bool VerifyFileSystemPathForWrite(const FilePath& path,
                                     bool create,
                                     int64 growth);