ResourceLoader: Fix a bunch of double-cancellation/double-error notification cases.

net/url_request/url_request_job.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 "net/url_request/url_request_job.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 477 matching lines @@
       return;
     }
   }
 
   has_handled_response_ = true;
   if (request_->status().is_success()) {
     DCHECK(!source_stream_);
     source_stream_ = SetUpSourceStream();
 
     if (!source_stream_) {
-      NotifyDone(URLRequestStatus(URLRequestStatus::FAILED,
-                                  ERR_CONTENT_DECODING_INIT_FAILED));
+      OnDone(URLRequestStatus(URLRequestStatus::FAILED,
+                              ERR_CONTENT_DECODING_INIT_FAILED),
+             true);
       return;
     }
     if (source_stream_->type() == SourceStream::TYPE_NONE) {
       std::string content_length;
       request_->GetResponseHeaderByName("content-length", &content_length);
       if (!content_length.empty())
         base::StringToInt64(content_length, &expected_content_size_);
     } else {
       request_->net_log().AddEvent(
           NetLogEventType::URL_REQUEST_FILTERS_SET,
@@ skipping 46 matching lines @@
   // There may be relevant information in the response info even in the
   // error case.
   GetResponseInfo(&request_->response_info_);
 
   MaybeNotifyNetworkBytes();
 
   request_->NotifyResponseStarted(status);
   // |this| may have been deleted here.
 }
 
-void URLRequestJob::NotifyDone(const URLRequestStatus &status) {
+void URLRequestJob::OnDone(const URLRequestStatus& status, bool notify_done) {
   DCHECK(!done_) << "Job sending done notification twice";
   if (done_)
     return;
   done_ = true;
 
   // Unless there was an error, we should have at least tried to handle
   // the response before getting here.
   DCHECK(has_handled_response_ || !status.is_success());
 
   request_->set_is_pending(false);
   // With async IO, it's quite possible to have a few outstanding
   // requests.  We could receive a request to Cancel, followed shortly
   // by a successful IO.  For tracking the status(), once there is
   // an error, we do not change the status back to success.  To
   // enforce this, only set the status if the job is so far
   // successful.
   if (request_->status().is_success()) {
     if (status.status() == URLRequestStatus::FAILED)
       request_->net_log().AddEventWithNetErrorCode(NetLogEventType::FAILED,
                                                    status.error());
     request_->set_status(status);
   }
 
   MaybeNotifyNetworkBytes();
 
-  // Complete this notification later.  This prevents us from re-entering the
-  // delegate if we're done because of a synchronous call.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&URLRequestJob::CompleteNotifyDone,
-                            weak_factory_.GetWeakPtr()));
+  if (notify_done) {
+    // Complete this notification later.  This prevents us from re-entering the
+    // delegate if we're done because of a synchronous call.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&URLRequestJob::NotifyDone, weak_factory_.GetWeakPtr()));

[mmenke, 2016/12/02 15:20:08]

Note that I want to get rid of NotifyDone entirely.  I have a CL that does it,
but it depends on the suspend mode stuff, which I still need to work out.  It's
not a coincidence that I'm working on the ResourceHandler stuff and the suspend
stuff at the same time.

+  }
 }
 
-void URLRequestJob::CompleteNotifyDone() {
+void URLRequestJob::NotifyDone() {
   // Check if we should notify the delegate that we're done because of an error.
   if (!request_->status().is_success()) {
     // We report the error differently depending on whether we've called
     // OnResponseStarted yet.
     if (has_handled_response_) {
       // We signal the error by calling OnReadComplete with a bytes_read of -1.
       request_->NotifyReadCompleted(-1);
     } else {
       has_handled_response_ = true;
       request_->NotifyResponseStarted(URLRequestStatus());
     }
   }
 }
 
 void URLRequestJob::NotifyCanceled() {
   if (!done_) {
-    NotifyDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED));
+    OnDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED), true);
   }
 }
 
 void URLRequestJob::NotifyRestartRequired() {
   DCHECK(!has_handled_response_);
   if (GetStatus().status() != URLRequestStatus::CANCELED)
     request_->Restart();
 }
 
 void URLRequestJob::OnCallToDelegate() {
@@ skipping 35 matching lines @@
   DCHECK_NE(ERR_IO_PENDING, result);
 
   if (result > 0 && request()->net_log().IsCapturing()) {
     request()->net_log().AddByteTransferEvent(
         NetLogEventType::URL_REQUEST_JOB_FILTERED_BYTES_READ, result,
         pending_read_buffer_->data());
   }
   pending_read_buffer_ = nullptr;
 
   if (result < 0) {
-    NotifyDone(URLRequestStatus::FromError(result));
+    OnDone(URLRequestStatus::FromError(result), !synchronous);
     return;
   }
 
   if (result > 0) {
     postfilter_bytes_read_ += result;
     if (!synchronous)
       request_->NotifyReadCompleted(result);
     return;
   }
 
   DCHECK_EQ(0, result);
   DoneReading();
-  NotifyDone(URLRequestStatus());
+  OnDone(URLRequestStatus(), !synchronous);
   if (!synchronous)
     request_->NotifyReadCompleted(result);
 }
 
 int URLRequestJob::ReadRawDataHelper(IOBuffer* buf,
                                      int buf_size,
                                      const CompletionCallback& callback) {
   DCHECK(!raw_read_buffer_);
 
   // Keep a pointer to the read buffer, so URLRequestJob::GatherRawReadStats()
@@ skipping 10 matching lines @@
     GatherRawReadStats(result);
   } else {
     read_raw_callback_ = callback;
   }
   return result;
 }
 
 void URLRequestJob::FollowRedirect(const RedirectInfo& redirect_info) {
   int rv = request_->Redirect(redirect_info);
   if (rv != OK)
-    NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, rv));
+    OnDone(URLRequestStatus(URLRequestStatus::FAILED, rv), true);
 }
 
 void URLRequestJob::GatherRawReadStats(int bytes_read) {
   DCHECK(raw_read_buffer_ || bytes_read == 0);
   DCHECK_NE(ERR_IO_PENDING, bytes_read);
 
   if (bytes_read > 0) {
     // If there is a filter, bytes will be logged after the filter is applied.
     if (source_stream_->type() != SourceStream::TYPE_NONE &&
         request()->net_log().IsCapturing()) {
@@ skipping 115 matching lines @@
   int64_t total_sent_bytes = GetTotalSentBytes();
   DCHECK_GE(total_sent_bytes, last_notified_total_sent_bytes_);
   if (total_sent_bytes > last_notified_total_sent_bytes_) {
     network_delegate_->NotifyNetworkBytesSent(
         request_, total_sent_bytes - last_notified_total_sent_bytes_);
   }
   last_notified_total_sent_bytes_ = total_sent_bytes;
 }
 
 }  // namespace net