Fix task execution order between URLFetcherCore::DisownFile and Stop.

URLFetcher cancellation is currently implemented as:

if(now on IO thread) CancelURLRequest(); else PostTask(&CancelURLRequest);

and due to that, the order of execution may be reversed with another
operation always PostTask'ing. This is particularly problematic when
Cancel gets ahead of DisownFile(). In that case, disowning never
happens (actually even worse, current implementation just crashes).

This patch changes DisownFile *not* to post, in order not to
rely on any task scheduling to ensure correctness.

BUG=256109

Review URL: https://chromiumcodereview.appspot.com/18033003

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

1 files changed, 9 insertions, 8 deletions

diff --git a/net/url_request/url_fetcher_core.cc b/net/url_request/url_fetcher_core.cc
index 24d03dd..87cce4f 100644
--- a/net/url_request/url_fetcher_core.cc
+++ b/net/url_request/url_fetcher_core.cc
@@ -358,9 +358,15 @@ bool URLFetcherCore::GetResponseAsFilePath(bool take_ownership,
   *out_response_path = file_writer_->file_path();
 
   if (take_ownership) {
-    network_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&URLFetcherCore::DisownFile, this));
+    // Intentionally calling file_writer_ method directly without posting the
+    // task to network_task_runner_.
+    //
+    // This is for correctly handling the case when DisownFile() is soon
+    // followed by Stop(). We have to make sure that DisownFile takes effect
+    // before Stop is executed and thus file_writer_ is deleted.
+    // This should be thread-safe since file_writer_->DisownFile itself does no
+    // file operation, it just flips the state to be referred in destruction.
+    file_writer_->DisownFile();
   }
   return true;
 }
@@ -864,11 +870,6 @@ void URLFetcherCore::ReadResponse() {
   OnReadCompleted(request_.get(), bytes_read);
 }
 
-void URLFetcherCore::DisownFile() {
-  DCHECK(file_writer_);
-  file_writer_->DisownFile();
-}
-
 void URLFetcherCore::InformDelegateUploadProgress() {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
   if (request_.get()) {