[ServiceWorker] Keep ServiceWorker alive while streaming the response for FetchEvent.

content/browser/service_worker/service_worker_url_request_job.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_url_request_job.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
@@ skipping 65 matching lines @@
   MaybeStartRequest();
 }
 
 void ServiceWorkerURLRequestJob::Start() {
   is_started_ = true;
   MaybeStartRequest();
 }
 
 void ServiceWorkerURLRequestJob::Kill() {
   net::URLRequestJob::Kill();
+  if (stream_ || !waiting_stream_url_.is_empty()) {
+    if (ServiceWorkerVersion* active_version = provider_host_->active_version())

[michaeln, 2015/01/07 21:48:46]

couple of things i wonder about?

provider_host_ is a weakptr, i'm not sure there's anything preventing the host
from being deleted prior to this being called

is Kill() called in all cases? this class does not itself do anything to ensure
that it is.

[horo, 2015/01/08 04:04:02]

ServiceWorkerURLRequestJob::Kill() must be called in the destructor of the
URLRequest.
 URLRequest::~URLRequest() calls URLRequest::OrphanJob().
 URLRequest::OrphanJob() calls ServiceWorkerURLRequestJob::Kill().


ServiceWorkerProviderHost is deleted when:
 1. the browser process recieves ServiceWorkerHostMsg_ProviderDestroyed message.
or
 2. the IPC connection to the renderer is closed because of the crash of the
renderer process.

1.1. ServiceWorkerHostMsg_ProviderDestroyed is sent from the renderer when
ServiceWorkerNetworkProvider in the renderer is deleted.
1.2. ServiceWorkerNetworkProvider is deleted when DocumentLoader is deleted in
FrameLoader.
1.3. DocumentLoader::detachFromFrame() must be called before DocumentLoader is
deleted. (see FrameLoader.cpp)
1.4. DocumentLoader::detachFromFrame() calcels the all pending requests. So
ResourceHostMsg_CancelRequest for the all pending requests are sent.
1.5. When the browser process recieves ResourceHostMsg_CancelRequest, URLRequest
is deleted.


2.1. ChannelProxy::Context::OnChannelClosed() is called when the IPC connection
to the renderer is closed.
2.2. ResourceMessageFilter::OnChannelClosing() is caleld before
ServiceWorkerDispatcherHost::OnFilterRemoved() is called in
ChannelProxy::Context::OnChannelClosed(). It is because ResourceMessageFilter is
added before ServiceWorkerDispatcherHost in
RenderProcessHostImpl::CreateMessageFilters().
2.3. ResourceMessageFilter::OnChannelClosing() deletes the all pending
URLRequest from the renderer.
2.4. ServiceWorkerDispatcherHost::OnFilterRemoved() deletes
ServiceWorkerProviderHost.

So, ServiceWorkerURLRequestJob::Kill() must be called before
ServiceWorkerProviderHost is deleted.


Is my understanding incorrect?

+      active_version->RemoveStreamingURLRequestJob(this);
+  }
   if (stream_) {
     stream_->RemoveReadObserver(this);
     stream_->Abort();
     stream_ = nullptr;
   }
   if (!waiting_stream_url_.is_empty()) {
     StreamRegistry* stream_registry =
         GetStreamContextForResourceContext(resource_context_)->registry();
     stream_registry->RemoveRegisterObserver(waiting_stream_url_);
     stream_registry->AbortPendingStream(waiting_stream_url_);
@@ skipping 431 matching lines @@
         net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED));
     return;
   }
 
   fetch_end_time_ = base::TimeTicks::Now();
   load_timing_info_.send_end = fetch_end_time_;
 
   // Set up a request for reading the stream.
   if (response.stream_url.is_valid()) {
     DCHECK(response.blob_uuid.empty());
+    DCHECK(provider_host_->active_version());
+    provider_host_->active_version()->AddStreamingURLRequestJob(this);

[kinuko, 2015/01/05 08:26:04]

Should we just always keep alive the version while we're serving from
ServiceWorker?

[horo, 2015/01/05 08:31:32]

We don't need to keep alive the version if ServiceWorkerURLRequestJob is reading
the data from a blob.

[kinuko, 2015/01/05 14:08:21]

Ok. I just thought it might be simpler and ok enough, but there's no reason to
keep versions alive longer than necessary. lgtm

     response_url_ = response.url;
     service_worker_response_type_ = response.response_type;
     CreateResponseHeader(
         response.status_code, response.status_text, response.headers);
     load_timing_info_.receive_headers_end = base::TimeTicks::Now();
     StreamContext* stream_context =
         GetStreamContextForResourceContext(resource_context_);
     stream_ =
         stream_context->registry()->GetStream(response.stream_url);
     if (!stream_.get()) {
@@ skipping 60 matching lines @@
 
 void ServiceWorkerURLRequestJob::DeliverErrorResponse() {
   // TODO(falken): Print an error to the console of the ServiceWorker and of
   // the requesting page.
   CreateResponseHeader(
       500, "Service Worker Response Error", ServiceWorkerHeaderMap());
   CommitResponseHeader();
 }
 
 }  // namespace content