ServiceWorker: Strip away fragments from document url

content/browser/service_worker/service_worker_controllee_request_handler.cc

 // Copyright 2014 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 "content/browser/service_worker/service_worker_controllee_request_handler.h"
 
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_provider_host.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_url_request_job.h"
 #include "content/browser/service_worker/service_worker_utils.h"
 #include "content/common/service_worker/service_worker_types.h"
+#include "net/base/net_util.h"
 #include "net/url_request/url_request.h"
 
 namespace content {
 
 ServiceWorkerControlleeRequestHandler::ServiceWorkerControlleeRequestHandler(
     base::WeakPtr<ServiceWorkerContextCore> context,
     base::WeakPtr<ServiceWorkerProviderHost> provider_host,
     ResourceType::Type resource_type)
     : ServiceWorkerRequestHandler(context, provider_host, resource_type),
       weak_factory_(this) {
@@ skipping 22 matching lines @@
   // We've come here by restart, we already have original request and it
   // tells we should fallback to network. (Case B-c)
   if (job_.get() && job_->ShouldFallbackToNetwork()) {
     job_ = NULL;
     return NULL;
   }
 
   // It's for original request (A) or redirect case (B-a or B-b).
   DCHECK(!job_.get() || job_->ShouldForwardToServiceWorker());
 
+  // Strip away reference fragments.
+  const GURL& stripped_url(net::SimplifyUrlForRequest(request->url()));

[michaeln, 2014/05/30 23:38:47]

const GURL&, doesn't look right here

i'd put this line inside the PrepareForMainResource method to be closer to where
its used, that way it would be more clear that this value is not used for the
RequestJob

[nhiroki, 2014/06/02 06:26:08]

Ah, it's a slip of the finger. Good catch!
Done.

+
   job_ = new ServiceWorkerURLRequestJob(request, network_delegate,
                                         provider_host_);
   if (ServiceWorkerUtils::IsMainResourceType(resource_type_))
-    PrepareForMainResource(request->url());
+    PrepareForMainResource(stripped_url);
   else
     PrepareForSubResource();
 
   if (job_->ShouldFallbackToNetwork()) {
     // If we know we can fallback to network at this point (in case
     // the storage lookup returned immediately), just return NULL here to
     // fallback to network.
     job_ = NULL;
     return NULL;
   }
 
   return job_.get();
 }
 
 void ServiceWorkerControlleeRequestHandler::PrepareForMainResource(
     const GURL& url) {
   DCHECK(job_.get());
   DCHECK(context_);
   // The corresponding provider_host may already have associate version in
   // redirect case, unassociate it now.
   provider_host_->SetActiveVersion(NULL);
   provider_host_->SetPendingVersion(NULL);
   provider_host_->set_document_url(url);
   context_->storage()->FindRegistrationForDocument(

[michaeln, 2014/05/30 23:38:47]

maybe add some DCHECKs to set_document_url(url) and FindRegistrationForDocument

[nhiroki, 2014/06/02 06:26:08]

Added.

       url,
       base::Bind(&self::DidLookupRegistrationForMainResource,
                  weak_factory_.GetWeakPtr()));
 }
 
 void
 ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource(
     ServiceWorkerStatusCode status,
     const scoped_refptr<ServiceWorkerRegistration>& registration) {
   DCHECK(job_.get());
@@ skipping 12 matching lines @@
 }
 
 void ServiceWorkerControlleeRequestHandler::PrepareForSubResource() {
   DCHECK(job_.get());
   DCHECK(context_);
   DCHECK(provider_host_->active_version());
   job_->ForwardToServiceWorker();
 }
 
 }  // namespace content