Remove URLRequest::OrphanJob().

This is a relic of a bygone era, when URLRequestJob was refcounted, and
could outlive its URLRequest. Now we can just delete the URLRequestJob.

Also remove a couple checks in URLRequestHttpJob and
ServiceWorkerURLRequestJob that their request is not NULL, as the checks
are no longer needed, for the same reason.

BUG=NONE

Review-Url: https://codereview.chromium.org/2480563002
Cr-Commit-Position: refs/heads/master@{#430301}

content/browser/service_worker/service_worker_url_request_job.cc

@@ -366,9 +366,7 @@ void ServiceWorkerURLRequestJob::RecordResult(
   }
   did_record_result_ = true;
   ServiceWorkerMetrics::RecordURLRequestJobResult(IsMainResourceLoad(), result);
-  if (request()) {
-    request()->net_log().AddEvent(RequestJobResultToNetEventType(result));
-  }
+  request()->net_log().AddEvent(RequestJobResultToNetEventType(result));
 }
 
 base::WeakPtr<ServiceWorkerURLRequestJob>
@@ -584,12 +582,6 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent(
   fetch_dispatcher_.reset();
   ServiceWorkerMetrics::RecordFetchEventStatus(IsMainResourceLoad(), status);
 
-  // Check if we're not orphaned.
-  if (!request()) {
-    RecordResult(ServiceWorkerMetrics::REQUEST_JOB_ERROR_NO_REQUEST);
-    return;
-  }
-
   ServiceWorkerMetrics::URLRequestJobResult result =
       ServiceWorkerMetrics::REQUEST_JOB_ERROR_BAD_DELEGATE;
   if (!delegate_->RequestStillValid(&result)) {

content/browser/service_worker/service_worker_write_to_cache_job.cc

@@ -71,6 +71,7 @@ ServiceWorkerWriteToCacheJob::ServiceWorkerWriteToCacheJob(
 }
 
 ServiceWorkerWriteToCacheJob::~ServiceWorkerWriteToCacheJob() {
+  Kill();
   DCHECK_EQ(did_notify_started_, did_notify_finished_);
 }
 

net/url_request/url_request.cc

@@ -182,11 +182,12 @@ URLRequest::~URLRequest() {
       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);
 
   int net_error = OK;
   // Log error only on failure, not cancellation, as even successful requests
@@ -897,7 +898,7 @@ void URLRequest::PrepareToRestart() {
   // one.
   net_log_.EndEvent(NetLogEventType::URL_REQUEST_START_JOB);
 
-  OrphanJob();
+  job_.reset();
 
   response_info_ = HttpResponseInfo();
   response_info_.request_time = base::Time::Now();
@@ -911,19 +912,6 @@ void URLRequest::PrepareToRestart() {
   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();

net/url_request/url_request.h

@@ -707,11 +707,6 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   void RestartWithJob(URLRequestJob* job);
   void PrepareToRestart();
 
-  // Detaches the job from this request in preparation for this object going
-  // away or the job being replaced. The job will not call us back when it has
-  // been orphaned.
-  void OrphanJob();
-
   // Cancels the request and set the error and ssl info for this request to the
   // passed values. Returns the error that was set.
   int DoCancel(int error, const SSLInfo& ssl_info);

net/url_request/url_request_http_job.cc

@@ -1107,7 +1107,6 @@ std::unique_ptr<SourceStream> URLRequestHttpJob::SetUpSourceStream() {
   std::string mime_type;
   bool success = GetMimeType(&mime_type);
   DCHECK(success || mime_type.empty());
-  DCHECK(request());
   SdchPolicyDelegate::FixUpSdchContentEncodings(
       request()->net_log(), mime_type, dictionaries_advertised_.get(), &types);
 
@@ -1539,17 +1538,15 @@ void URLRequestHttpJob::DoneWithRequest(CompletionCause reason) {
   done_ = true;
 
   // Notify NetworkQualityEstimator.
-  if (request()) {
-    NetworkQualityEstimator* network_quality_estimator =
-        request()->context()->network_quality_estimator();
-    if (network_quality_estimator)
-      network_quality_estimator->NotifyRequestCompleted(
-          *request(), request_->status().error());
+  NetworkQualityEstimator* network_quality_estimator =
+      request()->context()->network_quality_estimator();
+  if (network_quality_estimator) {
+    network_quality_estimator->NotifyRequestCompleted(
+        *request(), request_->status().error());
   }
 
   RecordPerfHistograms(reason);
-  if (request_)
-    request_->set_received_response_content_length(prefilter_bytes_read());
+  request()->set_received_response_content_length(prefilter_bytes_read());
 }
 
 HttpResponseHeaders* URLRequestHttpJob::GetResponseHeaders() const {
@@ -1564,12 +1561,10 @@ void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 
   // Notify NetworkQualityEstimator.
-  if (request()) {
-    NetworkQualityEstimator* network_quality_estimator =
-        request()->context()->network_quality_estimator();
-    if (network_quality_estimator)
-      network_quality_estimator->NotifyURLRequestDestroyed(*request());
-  }
+  NetworkQualityEstimator* network_quality_estimator =
+      request()->context()->network_quality_estimator();
+  if (network_quality_estimator)
+    network_quality_estimator->NotifyURLRequestDestroyed(*request());
 }
 
 }  // namespace net