Chromium Code Reviews| Index: content/browser/service_worker/service_worker_controllee_request_handler.cc | 
| diff --git a/content/browser/service_worker/service_worker_controllee_request_handler.cc b/content/browser/service_worker/service_worker_controllee_request_handler.cc | 
| index 76e9c2d3139b242ea1acf4b0bc601f4edd6c3a0c..872308271933aa8c15ee42abca14208f7fdba2e5 100644 | 
| --- a/content/browser/service_worker/service_worker_controllee_request_handler.cc | 
| +++ b/content/browser/service_worker/service_worker_controllee_request_handler.cc | 
| @@ -10,6 +10,7 @@ | 
| #include "base/trace_event/trace_event.h" | 
| #include "content/browser/service_worker/service_worker_context_core.h" | 
| +#include "content/browser/service_worker/service_worker_job_wrapper.h" | 
| #include "content/browser/service_worker/service_worker_metrics.h" | 
| #include "content/browser/service_worker/service_worker_provider_host.h" | 
| #include "content/browser/service_worker/service_worker_registration.h" | 
| @@ -34,7 +35,7 @@ namespace content { | 
| namespace { | 
| -bool MaybeForwardToServiceWorker(ServiceWorkerURLRequestJob* job, | 
| +bool MaybeForwardToServiceWorker(ServiceWorkerJobWrapper* job, | 
| const ServiceWorkerVersion* version) { | 
| DCHECK(job); | 
| DCHECK(version); | 
| @@ -50,14 +51,6 @@ bool MaybeForwardToServiceWorker(ServiceWorkerURLRequestJob* job, | 
| return false; | 
| } | 
| -ui::PageTransition GetPageTransition(net::URLRequest* request) { | 
| - const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request); | 
| - // ResourceRequestInfo may not be set in some tests. | 
| - if (!info) | 
| - return ui::PAGE_TRANSITION_LINK; | 
| - return info->GetPageTransition(); | 
| -} | 
| - | 
| } // namespace | 
| ServiceWorkerControlleeRequestHandler::ServiceWorkerControlleeRequestHandler( | 
| @@ -140,12 +133,12 @@ net::URLRequestJob* ServiceWorkerControlleeRequestHandler::MaybeCreateJob( | 
| credentials_mode_, redirect_mode_, resource_type_, | 
| request_context_type_, frame_type_, body_, | 
| ServiceWorkerFetchType::FETCH, base::nullopt, this)); | 
| - job_ = job->GetWeakPtr(); | 
| + job_ = base::MakeUnique<ServiceWorkerJobWrapper>(job->GetWeakPtr()); | 
| resource_context_ = resource_context; | 
| if (is_main_resource_load_) | 
| - PrepareForMainResource(request); | 
| + PrepareForMainResource(request->url(), request->first_party_for_cookies()); | 
| else | 
| PrepareForSubResource(); | 
| @@ -167,15 +160,15 @@ net::URLRequestJob* ServiceWorkerControlleeRequestHandler::MaybeCreateJob( | 
| } | 
| void ServiceWorkerControlleeRequestHandler::PrepareForMainResource( | 
| - const net::URLRequest* request) { | 
| + const GURL& url, | 
| + const GURL& first_party_for_cookies) { | 
| DCHECK(job_.get()); | 
| DCHECK(context_); | 
| DCHECK(provider_host_); | 
| TRACE_EVENT_ASYNC_BEGIN1( | 
| "ServiceWorker", | 
| "ServiceWorkerControlleeRequestHandler::PrepareForMainResource", | 
| - job_.get(), | 
| - "URL", request->url().spec()); | 
| + job_.get(), "URL", url.spec()); | 
| // The corresponding provider_host may already have associated a registration | 
| // in redirect case, unassociate it now. | 
| provider_host_->DisassociateRegistration(); | 
| @@ -184,9 +177,9 @@ void ServiceWorkerControlleeRequestHandler::PrepareForMainResource( | 
| // registration while we're finding an existing registration. | 
| provider_host_->SetAllowAssociation(false); | 
| - stripped_url_ = net::SimplifyUrlForRequest(request->url()); | 
| + stripped_url_ = net::SimplifyUrlForRequest(url); | 
| provider_host_->SetDocumentUrl(stripped_url_); | 
| - provider_host_->SetTopmostFrameUrl(request->first_party_for_cookies()); | 
| + provider_host_->SetTopmostFrameUrl(first_party_for_cookies); | 
| context_->storage()->FindRegistrationForDocument( | 
| stripped_url_, base::Bind(&self::DidLookupRegistrationForMainResource, | 
| weak_factory_.GetWeakPtr())); | 
| @@ -197,7 +190,7 @@ void ServiceWorkerControlleeRequestHandler:: | 
| ServiceWorkerStatusCode status, | 
| scoped_refptr<ServiceWorkerRegistration> registration) { | 
| // The job may have been canceled and then destroyed before this was invoked. | 
| 
 
falken
2017/05/17 01:29:05
I guess "and then destroyed" is no longer needed o
 
scottmg
2017/05/17 18:27:59
Done.
 
 | 
| - if (!job_) | 
| + if (JobWasCanceled()) | 
| return; | 
| const bool need_to_update = !force_update_started_ && registration && | 
| @@ -312,7 +305,7 @@ void ServiceWorkerControlleeRequestHandler:: | 
| ServiceWorkerVersion::FetchHandlerExistence::UNKNOWN); | 
| ServiceWorkerMetrics::CountControlledPageLoad( | 
| active_version->site_for_uma(), stripped_url_, is_main_frame_load_, | 
| - GetPageTransition(job_->request()), job_->request()->url_chain().size()); | 
| + job_->GetPageTransition(), job_->GetURLChainSize()); | 
| bool is_forwarded = | 
| MaybeForwardToServiceWorker(job_.get(), active_version.get()); | 
| @@ -329,7 +322,7 @@ void ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged( | 
| ServiceWorkerRegistration* registration, | 
| ServiceWorkerVersion* version) { | 
| // The job may have been canceled and then destroyed before this was invoked. | 
| - if (!job_) | 
| + if (JobWasCanceled()) | 
| return; | 
| if (provider_host_) | 
| @@ -345,7 +338,7 @@ void ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged( | 
| ServiceWorkerVersion::FetchHandlerExistence::UNKNOWN); | 
| ServiceWorkerMetrics::CountControlledPageLoad( | 
| version->site_for_uma(), stripped_url_, is_main_frame_load_, | 
| - GetPageTransition(job_->request()), job_->request()->url_chain().size()); | 
| + job_->GetPageTransition(), job_->GetURLChainSize()); | 
| provider_host_->AssociateRegistration(registration, | 
| false /* notify_controllerchange */); | 
| @@ -361,7 +354,7 @@ void ServiceWorkerControlleeRequestHandler::DidUpdateRegistration( | 
| DCHECK(force_update_started_); | 
| // The job may have been canceled and then destroyed before this was invoked. | 
| - if (!job_) | 
| + if (JobWasCanceled()) | 
| return; | 
| if (!context_) { | 
| @@ -391,7 +384,7 @@ void ServiceWorkerControlleeRequestHandler::OnUpdatedVersionStatusChanged( | 
| const scoped_refptr<ServiceWorkerRegistration>& registration, | 
| const scoped_refptr<ServiceWorkerVersion>& version) { | 
| // The job may have been canceled and then destroyed before this was invoked. | 
| - if (!job_) | 
| + if (JobWasCanceled()) | 
| return; | 
| if (!context_) { | 
| @@ -474,4 +467,8 @@ void ServiceWorkerControlleeRequestHandler::ClearJob() { | 
| job_.reset(); | 
| } | 
| +bool ServiceWorkerControlleeRequestHandler::JobWasCanceled() const { | 
| + return !job_ || job_->WasCanceled(); | 
| +} | 
| + | 
| } // namespace content |