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

content/browser/service_worker/service_worker_url_request_job.cc

Index: content/browser/service_worker/service_worker_url_request_job.cc
diff --git a/content/browser/service_worker/service_worker_url_request_job.cc b/content/browser/service_worker/service_worker_url_request_job.cc
index 44762fa1bce98e20d99e9da1c616c5d6f41af796..3a2dee5cb93e2c05868e019af17d51e54d031639 100644
--- a/content/browser/service_worker/service_worker_url_request_job.cc
+++ b/content/browser/service_worker/service_worker_url_request_job.cc
@@ -83,6 +83,10 @@ void ServiceWorkerURLRequestJob::Start() {
 
 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();
@@ -534,6 +538,8 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent(
   // 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(