Remove URLRequest::OrphanJob().

net/url_request/url_request.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.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 164 matching lines @@
 
 URLRequest::~URLRequest() {
   Cancel();
 
   if (network_delegate_) {
     network_delegate_->NotifyURLRequestDestroyed(this);
     if (job_.get())
       job_->NotifyURLRequestDestroyed();
   }
 
-  if (job_.get())
-    OrphanJob();
+  // Delete job before |this|, since subclasses may do weird things, like depend
+  // on UserData associated with |this| and poke at it during teardown.
+  job_.reset();
 
-  int deleted = context_->url_requests()->erase(this);
-  CHECK_EQ(1, deleted);
+  DCHECK_EQ(1u, context_->url_requests()->count(this));
+  context_->url_requests()->erase(this);

[mmenke, 2016/11/03 20:31:06]

This admittedly isn't really related to the CL, just seems like we shouldn't be
CHECKing here, unless we have a reason to believe this isn't working, or it's
particularly important for some reason.

 
   int net_error = OK;
   // Log error only on failure, not cancellation, as even successful requests
   // are "cancelled" on destruction.
   if (status_.status() == URLRequestStatus::FAILED)
     net_error = status_.error();
   net_log_.EndEventWithNetErrorCode(NetLogEventType::REQUEST_ALIVE, net_error);
 }
 
 void URLRequest::set_upload(std::unique_ptr<UploadDataStream> upload) {
@@ skipping 688 matching lines @@
   job_->ContinueDespiteLastError();
 }
 
 void URLRequest::PrepareToRestart() {
   DCHECK(job_.get());
 
   // Close the current URL_REQUEST_START_JOB, since we will be starting a new
   // one.
   net_log_.EndEvent(NetLogEventType::URL_REQUEST_START_JOB);
 
-  OrphanJob();
+  job_.reset();
 
   response_info_ = HttpResponseInfo();
   response_info_.request_time = base::Time::Now();
 
   load_timing_info_ = LoadTimingInfo();
   load_timing_info_.request_start_time = response_info_.request_time;
   load_timing_info_.request_start = base::TimeTicks::Now();
 
   status_ = URLRequestStatus();
   is_pending_ = false;
   proxy_server_ = ProxyServer();
 }
 
-void URLRequest::OrphanJob() {
-  // When calling this function, please check that URLRequestHttpJob is
-  // not in between calling NetworkDelegate::NotifyHeadersReceived receiving
-  // the call back. This is currently guaranteed by the following strategies:
-  // - OrphanJob is called on JobRestart, in this case the URLRequestJob cannot
-  //   be receiving any headers at that time.
-  // - OrphanJob is called in ~URLRequest, in this case
-  //   NetworkDelegate::NotifyURLRequestDestroyed notifies the NetworkDelegate
-  //   that the callback becomes invalid.
-  job_->Kill();
-  job_ = NULL;
-}
-
 int URLRequest::Redirect(const RedirectInfo& redirect_info) {
   // Matches call in NotifyReceivedRedirect.
   OnCallToDelegateComplete();
   if (net_log_.IsCapturing()) {
     net_log_.AddEvent(
         NetLogEventType::URL_REQUEST_REDIRECTED,
         NetLog::StringCallback("location",
                                &redirect_info.new_url.possibly_invalid_spec()));
   }
 
@@ skipping 288 matching lines @@
     out->clear();
 }
 
 void URLRequest::set_status(URLRequestStatus status) {
   DCHECK(status_.is_io_pending() || status_.is_success() ||
          (!status.is_success() && !status.is_io_pending()));
   status_ = status;
 }
 
 }  // namespace net