FileWriterDelegate should not call URLRequest::Start() after Cancel().

webkit/fileapi/file_system_operation.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/fileapi/file_system_operation.h"
 
 #include "base/bind.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/escape.h"
@@ skipping 271 matching lines @@
       path_url, &src_path_, &src_util_, PATH_FOR_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
     callback.Run(result, 0, false);
     delete this;
     return;
   }
   DCHECK(blob_url.is_valid());
   file_writer_delegate_.reset(new FileWriterDelegate(
           this, src_path_, offset, proxy_));
   set_write_callback(callback);
-  blob_request_.reset(
-      new net::URLRequest(blob_url, file_writer_delegate_.get()));
-  blob_request_->set_context(url_request_context);
 
   GetUsageAndQuotaThenRunTask(
       src_path_.origin(), src_path_.type(),
-      base::Bind(&FileSystemOperation::DoWrite, base::Unretained(this)),
+      base::Bind(&FileSystemOperation::DoWrite, base::Unretained(this),
+                 blob_url, make_scoped_refptr(url_request_context)),
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED, 0, true));
 }
 
 void FileSystemOperation::Truncate(const GURL& path_url, int64 length,
                                    const StatusCallback& callback) {
   DCHECK(SetPendingOperationType(kOperationTruncate));
 
   base::PlatformFileError result = SetUpFileSystemPath(
       path_url, &src_path_, &src_util_, PATH_FOR_WRITE);
   if (result != base::PLATFORM_FILE_OK) {
@@ skipping 72 matching lines @@
       base::Bind(callback, base::PLATFORM_FILE_ERROR_FAILED,
                  base::kInvalidPlatformFileValue,
                  base::kNullProcessHandle));
 }
 
 // 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(const StatusCallback& cancel_callback) {
   if (file_writer_delegate_.get()) {
     DCHECK_EQ(kOperationWrite, pending_operation_);
+
     // Writes are done without proxying through FileUtilProxy after the initial
     // opening of the PlatformFile.  All state changes are done on this thread,
     // so we're guaranteed to be able to shut down atomically.  We do need to
     // check that the file has been opened [which means the blob_request_ has
     // been created], so we know how much we need to do.
-    if (blob_request_.get())
-      // This halts any calls to file_writer_delegate_ from blob_request_.
-      blob_request_->Cancel();
+    file_writer_delegate_->Cancel();
 
     if (!write_callback_.is_null()) {
       // Notify the failure status to the ongoing operation's callback.
       write_callback_.Run(base::PLATFORM_FILE_ERROR_ABORT, 0, false);
       // Do not delete this FileSystemOperation object yet. As a result of
       // abort, this->DidWrite is called later and there we delete this object.

[ericu, 2012/04/10 22:10:32]

I'm not sure this is still true; see comments in file_writer_delegate.cc

     }
     cancel_callback.Run(base::PLATFORM_FILE_OK);
     write_callback_.Reset();
   } else {
     DCHECK_EQ(kOperationTruncate, pending_operation_);
     // We're cancelling a truncate operation, but we can't actually stop it
     // since it's been proxied to another thread.  We need to save the
     // cancel_callback so that when the truncate returns, it can see that it's
     // been cancelled, report it, and report that the cancel has succeeded.
     DCHECK(cancel_callback_.is_null());
@@ skipping 117 matching lines @@
 
 void FileSystemOperation::DoMove(const StatusCallback& callback) {
   FileSystemFileUtilProxy::Move(
       proxy_, &operation_context_,
       src_util_, dest_util_,
       src_path_, dest_path_,
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
 
-void FileSystemOperation::DoWrite() {
+void FileSystemOperation::DoWrite(
+    const GURL& blob_url,
+    const net::URLRequestContext* request_context) {
   int file_flags = base::PLATFORM_FILE_OPEN |
                    base::PLATFORM_FILE_WRITE |
                    base::PLATFORM_FILE_ASYNC;
 
   FileSystemFileUtilProxy::CreateOrOpen(
       proxy_, &operation_context_, src_util_, src_path_, file_flags,
       base::Bind(&FileSystemOperation::OnFileOpenedForWrite,
-                 base::Unretained(this)));
+                 base::Unretained(this),
+                 blob_url, make_scoped_refptr(request_context)));
 }
 
 void FileSystemOperation::DoTruncate(const StatusCallback& callback,
                                      int64 length) {
   FileSystemFileUtilProxy::Truncate(
       proxy_, &operation_context_, src_util_, src_path_, length,
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
 
@@ skipping 95 matching lines @@
     const OpenFileCallback& callback,
     base::PlatformFileError rv,
     base::PassPlatformFile file,
     bool unused) {
   if (rv == base::PLATFORM_FILE_OK)
     CHECK_NE(base::kNullProcessHandle, peer_handle_);
   callback.Run(rv, file.ReleaseValue(), peer_handle_);
 }
 
 void FileSystemOperation::OnFileOpenedForWrite(
+    const GURL& blob_url,
+    const net::URLRequestContext* request_context,
     base::PlatformFileError rv,
     base::PassPlatformFile file,
     bool created) {
   if (rv != base::PLATFORM_FILE_OK) {
     if (!write_callback_.is_null())
       write_callback_.Run(rv, 0, false);
     delete this;
     return;
   }
-  file_writer_delegate_->Start(file.ReleaseValue(), blob_request_.get());
+  if (write_callback_.is_null()) {
+    // If cancelled, callback is already invoked and set to null in Cancel().
+    // We must not call it twice. Just shut down this operation object.
+    delete this;
+    return;
+  }
+  scoped_ptr<net::URLRequest> blob_request(
+      new net::URLRequest(blob_url, file_writer_delegate_.get()));
+  blob_request->set_context(request_context);
+  file_writer_delegate_->Start(file.ReleaseValue(), blob_request.Pass());
 }
 
 base::PlatformFileError FileSystemOperation::SetUpFileSystemPath(
     const GURL& path_url,
     FileSystemPath* file_system_path,
     FileSystemFileUtil** file_util,
     SetUpPathMode mode) {
   DCHECK(file_system_path);
   GURL origin_url;
   FileSystemType type;
@@ skipping 49 matching lines @@
 }
 
 bool FileSystemOperation::SetPendingOperationType(OperationType type) {
   if (pending_operation_ != kOperationNone)
     return false;
   pending_operation_ = type;
   return true;
 }
 
 }  // namespace fileapi