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

net/url_request/url_request_job.cc

Index: net/url_request/url_request_job.cc
diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc
index b315664de1ac52eafb49b36360be97e608d87ecc..5cf113e344f96999b04d897171edad98c7855d50 100644
--- a/net/url_request/url_request_job.cc
+++ b/net/url_request/url_request_job.cc
@@ -495,8 +495,9 @@ void URLRequestJob::NotifyHeadersComplete() {
     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) {
@@ -563,7 +564,7 @@ void URLRequestJob::NotifyStartError(const URLRequestStatus &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;
@@ -589,14 +590,16 @@ void URLRequestJob::NotifyDone(const URLRequestStatus &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()));
+  }
 }
 
-void URLRequestJob::CompleteNotifyDone() {
+void URLRequestJob::NotifyDone() {
   // Check if we should notify the delegate that we're done because of an error.

[Randy Smith (Not in Mondays), 2016/12/06 23:44:43]

nit, suggestion, not relevant to this CL: I wince at referring to the request as
"the delegate" given that we're not accessing it through a delegate interface
and there are several delegates at the URLRequest level (network &
URLRequest::Delegate).  Using that language seems like an invitation to
confusion.

[mmenke, 2016/12/07 16:17:15]

It's actually referring to the URLRequest::Delegate, I bet.  Anyhow, I switched
it to URLRequest (The URLRequestJob shouldn't know about the
URLRequest::Delegate).

Could do more cleanup here, but this is something I'd rather just remove, in the
future.

   if (!request_->status().is_success()) {
     // We report the error differently depending on whether we've called
@@ -613,7 +616,7 @@ void URLRequestJob::CompleteNotifyDone() {
 
 void URLRequestJob::NotifyCanceled() {
   if (!done_) {
-    NotifyDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED));
+    OnDone(URLRequestStatus(URLRequestStatus::CANCELED, ERR_ABORTED), true);
   }
 }
 
@@ -669,20 +672,21 @@ void URLRequestJob::SourceStreamReadComplete(bool synchronous, int result) {
   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;
+  } else {
+    DCHECK_EQ(0, result);
+    DoneReading();
+    // In the synchronous case, the caller will notify the URLRequest of
+    // completion. In the async case, the NotifyReadCompleted call will.
+    // TODO(mmenke): Can this be combined with the error case?
+    OnDone(URLRequestStatus(), false);
   }
 
-  DCHECK_EQ(0, result);
-  DoneReading();
-  NotifyDone(URLRequestStatus());
   if (!synchronous)
     request_->NotifyReadCompleted(result);
 }
@@ -713,7 +717,7 @@ int URLRequestJob::ReadRawDataHelper(IOBuffer* buf,
 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) {