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/sequenced_task_runner.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
@@ skipping 272 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));
   set_write_callback(callback);
-  blob_request_.reset(
+  scoped_ptr<net::URLRequest> blob_request(
       new net::URLRequest(blob_url, file_writer_delegate_.get()));
-  blob_request_->set_context(url_request_context);
+  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, weak_factory_.GetWeakPtr(),
+                 base::Passed(&blob_request)),
       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();
+    // so we're guaranteed to be able to shut down atomically.
+    const bool delete_now = 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.
     }
     cancel_callback.Run(base::PLATFORM_FILE_OK);
     write_callback_.Reset();
+
+    if (delete_now) {
+      delete this;
+      return;
+    }
   } 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());
     cancel_callback_ = cancel_callback;
   }
 }
@@ skipping 24 matching lines @@
     const SnapshotFileCallback& callback) {
   GetMetadata(path_url, base::Bind(&GetMetadataForSnapshot, callback));
 }
 
 FileSystemOperation::FileSystemOperation(
     FileSystemContext* file_system_context)
     : operation_context_(file_system_context),
       src_util_(NULL),
       dest_util_(NULL),
       peer_handle_(base::kNullProcessHandle),
-      pending_operation_(kOperationNone) {
+      pending_operation_(kOperationNone),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 void FileSystemOperation::GetUsageAndQuotaThenRunTask(
     const GURL& origin, FileSystemType type,
     const base::Closure& task,
     const base::Closure& error_callback) {
   quota::QuotaManagerProxy* quota_manager_proxy =
       file_system_context()->quota_manager_proxy();
   if (!quota_manager_proxy ||
       !file_system_context()->GetQuotaUtil(type)) {
@@ skipping 69 matching lines @@
 
 void FileSystemOperation::DoMove(const StatusCallback& callback) {
   FileSystemFileUtilProxy::Move(
       &operation_context_,
       src_util_, dest_util_,
       src_path_, dest_path_,
       base::Bind(&FileSystemOperation::DidFinishFileOperation,
                  base::Owned(this), callback));
 }
 
-void FileSystemOperation::DoWrite() {
+void FileSystemOperation::DoWrite(scoped_ptr<net::URLRequest> blob_request) {
   int file_flags = base::PLATFORM_FILE_OPEN |
                    base::PLATFORM_FILE_WRITE |
                    base::PLATFORM_FILE_ASYNC;
 
+  // We may get deleted on the way so allocate a new operation context
+  // to keep it alive.
+  FileSystemOperationContext* write_context = new FileSystemOperationContext(
+      operation_context_);
   FileSystemFileUtilProxy::CreateOrOpen(
-      &operation_context_, src_util_, src_path_, file_flags,
+      write_context, src_util_, src_path_, file_flags,
       base::Bind(&FileSystemOperation::OnFileOpenedForWrite,
-                 base::Unretained(this)));
+                 weak_factory_.GetWeakPtr(),
+                 base::Passed(&blob_request),
+                 base::Owned(write_context)));
 }
 
 void FileSystemOperation::DoTruncate(const StatusCallback& callback,
                                      int64 length) {
   FileSystemFileUtilProxy::Truncate(
       &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(
+    scoped_ptr<net::URLRequest> blob_request,
+    FileSystemOperationContext* unused,
     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());
+  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