Skip to content

Commit

Permalink
[ServiceWorker] Refactor the codes for releasing the resources for st…
Browse files Browse the repository at this point in the history
…reaming in ServiceWorkerURLRequestJob.

As michaeln@ mentioned in https://codereview.chromium.org/832813002/, Kill() may not be called and provider_host_ may be deleted before Kill() is called.
So this change
 - adds streaming_version_ property to ServiceWorkerURLRequestJob.
 - adds ClearStream() method to releases the resources for streaming.
 - calls ClearStream() from both Kill() and the destructor.

BUG=445775

Review URL: https://codereview.chromium.org/844913002

Cr-Commit-Position: refs/heads/master@{#310944}
  • Loading branch information
horo-t authored and Commit bot committed Jan 10, 2015
1 parent c6eaa39 commit e952053
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
38 changes: 22 additions & 16 deletions content/browser/service_worker/service_worker_url_request_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,7 @@ void ServiceWorkerURLRequestJob::Start() {

void ServiceWorkerURLRequestJob::Kill() {
net::URLRequestJob::Kill();
if (stream_ || !waiting_stream_url_.is_empty()) {
if (ServiceWorkerVersion* active_version = provider_host_->active_version())
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_);
}
ClearStream();
fetch_dispatcher_.reset();
blob_request_.reset();
weak_factory_.InvalidateWeakPtrs();
Expand Down Expand Up @@ -341,6 +327,7 @@ void ServiceWorkerURLRequestJob::GetExtraResponseInfo(


ServiceWorkerURLRequestJob::~ServiceWorkerURLRequestJob() {
ClearStream();
}

void ServiceWorkerURLRequestJob::MaybeStartRequest() {
Expand Down Expand Up @@ -539,7 +526,8 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent(
if (response.stream_url.is_valid()) {
DCHECK(response.blob_uuid.empty());
DCHECK(provider_host_->active_version());
provider_host_->active_version()->AddStreamingURLRequestJob(this);
streaming_version_ = provider_host_->active_version();
streaming_version_->AddStreamingURLRequestJob(this);
response_url_ = response.url;
service_worker_response_type_ = response.response_type;
CreateResponseHeader(
Expand Down Expand Up @@ -619,4 +607,22 @@ void ServiceWorkerURLRequestJob::DeliverErrorResponse() {
CommitResponseHeader();
}

void ServiceWorkerURLRequestJob::ClearStream() {
if (streaming_version_) {
streaming_version_->RemoveStreamingURLRequestJob(this);
streaming_version_ = nullptr;
}
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_);
}
}

} // namespace content
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ResourceRequestBody;
class ServiceWorkerContextCore;
class ServiceWorkerFetchDispatcher;
class ServiceWorkerProviderHost;
class ServiceWorkerVersion;
class Stream;

class CONTENT_EXPORT ServiceWorkerURLRequestJob
Expand Down Expand Up @@ -159,6 +160,9 @@ class CONTENT_EXPORT ServiceWorkerURLRequestJob
// Creates and commits a response header indicating error.
void DeliverErrorResponse();

// Releases the resources for streaming.
void ClearStream();

base::WeakPtr<ServiceWorkerProviderHost> provider_host_;

// Timing info to show on the popup in Devtools' Network tab.
Expand Down Expand Up @@ -198,6 +202,7 @@ class CONTENT_EXPORT ServiceWorkerURLRequestJob
// using the userdata mechanism. So we have to keep it not to free the blobs.
scoped_refptr<ResourceRequestBody> body_;
scoped_ptr<storage::BlobDataHandle> request_body_blob_data_handle_;
scoped_refptr<ServiceWorkerVersion> streaming_version_;

base::WeakPtrFactory<ServiceWorkerURLRequestJob> weak_factory_;

Expand Down

0 comments on commit e952053

Please sign in to comment.