[fsp] Simplify aborting logic.

chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc

Index: chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
diff --git a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
index 227ddaa604acb96d04e584fe9424b16b07ef3afe..67620b589c4e7e9dd83f3644e2b1e671ce840751 100644
--- a/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
+++ b/chrome/browser/chromeos/file_system_provider/fileapi/file_stream_writer.cc
@@ -38,6 +38,7 @@ class FileStreamWriter::OperationRunner
       const storage::FileSystemURL& url,
       const storage::AsyncFileUtil::StatusCallback& callback) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    DCHECK(abort_callback_.is_null());
 
     util::FileSystemURLParser parser(url);
     if (!parser.Parse()) {
@@ -60,6 +61,8 @@ class FileStreamWriter::OperationRunner
   // UI thread.
   void CloseFileOnUIThread() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    DCHECK(abort_callback_.is_null());
+
     if (file_system_.get() && file_handle_ != -1) {
       // Closing a file must not be aborted, since we could end up on files
       // which are never closed.
@@ -75,6 +78,7 @@ class FileStreamWriter::OperationRunner
       int length,
       const storage::AsyncFileUtil::StatusCallback& callback) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    DCHECK(abort_callback_.is_null());
 
     // If the file system got unmounted, then abort the writing operation.
     if (!file_system_.get()) {
@@ -94,23 +98,15 @@ class FileStreamWriter::OperationRunner
             &OperationRunner::OnWriteFileCompletedOnUIThread, this, callback));
   }
 
-  // Aborts the most recent operation (if exists), and calls the callback.
-  void AbortOnUIThread(const storage::AsyncFileUtil::StatusCallback& callback) {
+  // Aborts the most recent operation (if exists).
+  void AbortOnUIThread() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-    if (abort_callback_.is_null()) {
-      // No operation to be cancelled. At most a callback call, which will be
-      // discarded.
-      BrowserThread::PostTask(BrowserThread::IO,
-                              FROM_HERE,
-                              base::Bind(callback, base::File::FILE_OK));
+    if (abort_callback_.is_null())
       return;
-    }
 
     const AbortCallback last_abort_callback = abort_callback_;
     abort_callback_ = AbortCallback();
-    last_abort_callback.Run(base::Bind(
-        &OperationRunner::OnAbortCompletedOnUIThread, this, callback));
+    last_abort_callback.Run();
   }
 
  private:
@@ -126,6 +122,7 @@ class FileStreamWriter::OperationRunner
       base::File::Error result) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+    abort_callback_ = AbortCallback();
     if (result == base::File::FILE_OK)
       file_handle_ = file_handle;
 
@@ -138,15 +135,8 @@ class FileStreamWriter::OperationRunner
       const storage::AsyncFileUtil::StatusCallback& callback,
       base::File::Error result) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE, base::Bind(callback, result));
-  }
 
-  // Forwards a response of aborting an operation to the IO thread.
-  void OnAbortCompletedOnUIThread(
-      const storage::AsyncFileUtil::StatusCallback& callback,
-      base::File::Error result) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    abort_callback_ = AbortCallback();
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE, base::Bind(callback, result));
   }
@@ -252,6 +242,7 @@ int FileStreamWriter::Write(net::IOBuffer* buffer,
                                        callback));
       break;
 
+    case EXECUTING:
     case FAILED:
       NOTREACHED();
       break;
@@ -263,18 +254,25 @@ int FileStreamWriter::Write(net::IOBuffer* buffer,
 int FileStreamWriter::Cancel(const net::CompletionCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  if (state_ != INITIALIZING && state_ != EXECUTING) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(

[hirono, 2015/01/14 05:36:06]

Could you explain why it does not return net::ERR_UNEXPECTED directly?

[mtomasz, 2015/01/14 08:55:09]

Not a strong reason, but for simplicity both FileStreamWriter and
FileStreamReader return all values asynchronously. Additionally,
fsp::BufferingFileStreamReader has an assumption that FileStreamReader never
completes synchronously. For BufferingFileStreamWriter there is no such
assumption.

I changed to return UNEXPECTED synchronously. Done.

+        FROM_HERE, base::Bind(callback, net::ERR_UNEXPECTED));
+    return net::ERR_IO_PENDING;
+  }
+
+  // Abort and Optimistically return an OK result code, as the aborting
+  // operation is always forced and can't be cancelled.
   BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&OperationRunner::AbortOnUIThread,
-                 runner_,
-                 base::Bind(&FileStreamWriter::OnAbortCompleted,
-                            weak_ptr_factory_.GetWeakPtr(),
-                            callback)));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&OperationRunner::AbortOnUIThread, runner_));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                base::Bind(callback, net::OK));
+
   return net::ERR_IO_PENDING;
 }
 
 int FileStreamWriter::Flush(const net::CompletionCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(callback, state_ == INITIALIZED ? net::OK : net::ERR_FAILED));
@@ -287,7 +285,8 @@ void FileStreamWriter::OnWriteFileCompleted(
     const net::CompletionCallback& callback,
     base::File::Error result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK_EQ(INITIALIZED, state_);
+  DCHECK_EQ(EXECUTING, state_);
+  state_ = INITIALIZED;
 
   if (result != base::File::FILE_OK) {
     state_ = FAILED;
@@ -307,22 +306,13 @@ void FileStreamWriter::OnWriteCompleted(net::CompletionCallback callback,
       "file_system_provider", "FileStreamWriter::Write", this);
 }
 
-void FileStreamWriter::OnAbortCompleted(const net::CompletionCallback& callback,
-                                        base::File::Error result) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  if (result != base::File::FILE_OK)
-    state_ = FAILED;
-
-  callback.Run(net::FileErrorToNetError(result));
-}
-
 void FileStreamWriter::WriteAfterInitialized(
     scoped_refptr<net::IOBuffer> buffer,
     int buffer_length,
     const net::CompletionCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK_EQ(INITIALIZED, state_);
+  state_ = EXECUTING;
 
   BrowserThread::PostTask(
       BrowserThread::UI,