diff options
author | horo <horo@chromium.org> | 2015-01-05 14:21:00 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-01-05 22:22:07 +0000 |
commit | 4bffcd598dd89e0016208ce9312a1f477ff105d1 (patch) | |
tree | 836f13aebd7347c5efca5818cac89e3616628d52 | |
parent | 409e3e02bfc1d3d2b20b7a2e5eb725289cf90022 (diff) | |
download | chromium_src-4bffcd598dd89e0016208ce9312a1f477ff105d1.zip chromium_src-4bffcd598dd89e0016208ce9312a1f477ff105d1.tar.gz chromium_src-4bffcd598dd89e0016208ce9312a1f477ff105d1.tar.bz2 |
[ServiceWorker] Keep ServiceWorker alive while streaming the response for FetchEvent.
In current implementation the ServiceWorker is terminated 30 seconds after start loading the page even if the page is still loading.
(https://codereview.chromium.org/805083006/ changed the behavior of stopping the ServiceWorker.)
This cl changes ServiceWorkerVersion::HasInflightRequests() to check the existence of the active streaming ServiceWorkerURLRequestJobs.
BUG=445775
Review URL: https://codereview.chromium.org/832813002
Cr-Commit-Position: refs/heads/master@{#309970}
4 files changed, 56 insertions, 2 deletions
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 44762fa..3a2dee5 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()) + 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); response_url_ = response.url; service_worker_response_type_ = response.response_type; CreateResponseHeader( diff --git a/content/browser/service_worker/service_worker_url_request_job_unittest.cc b/content/browser/service_worker/service_worker_url_request_job_unittest.cc index 5987b5c..4199dd9 100644 --- a/content/browser/service_worker/service_worker_url_request_job_unittest.cc +++ b/content/browser/service_worker/service_worker_url_request_job_unittest.cc @@ -182,6 +182,10 @@ class ServiceWorkerURLRequestJobTest : public testing::Test { EXPECT_EQ(expected_response, url_request_delegate_.response_data()); } + bool HasInflightRequests() { + return version_->HasInflightRequests(); + } + TestBrowserThreadBundle thread_bundle_; scoped_ptr<TestBrowserContext> browser_context_; @@ -306,7 +310,6 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponse) { scoped_refptr<Stream> stream = new Stream(stream_context->registry(), nullptr, stream_url); SetUpWithHelper(new StreamResponder(kProcessID, stream_url)); - version_->SetStatus(ServiceWorkerVersion::ACTIVATED); request_ = url_request_context_.CreateRequest( GURL("http://example.com/foo.html"), @@ -324,13 +327,17 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponse) { } stream->Finalize(); + EXPECT_FALSE(HasInflightRequests()); base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(HasInflightRequests()); EXPECT_TRUE(request_->status().is_success()); EXPECT_EQ(200, request_->response_headers()->response_code()); EXPECT_EQ("OK", request_->response_headers()->GetStatusText()); EXPECT_EQ(expected_response, url_request_delegate_.response_data()); + request_.reset(); + EXPECT_FALSE(HasInflightRequests()); } TEST_F(ServiceWorkerURLRequestJobTest, StreamResponse_DelayedRegistration) { @@ -359,13 +366,17 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponse_DelayedRegistration) { } stream->Finalize(); + EXPECT_FALSE(HasInflightRequests()); base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(HasInflightRequests()); EXPECT_TRUE(request_->status().is_success()); EXPECT_EQ(200, request_->response_headers()->response_code()); EXPECT_EQ("OK", request_->response_headers()->GetStatusText()); EXPECT_EQ(expected_response, url_request_delegate_.response_data()); + request_.reset(); + EXPECT_FALSE(HasInflightRequests()); } @@ -393,13 +404,17 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponse_QuickFinalize) { nullptr); request_->set_method("GET"); request_->Start(); + EXPECT_FALSE(HasInflightRequests()); base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(HasInflightRequests()); EXPECT_TRUE(request_->status().is_success()); EXPECT_EQ(200, request_->response_headers()->response_code()); EXPECT_EQ("OK", request_->response_headers()->GetStatusText()); EXPECT_EQ(expected_response, url_request_delegate_.response_data()); + request_.reset(); + EXPECT_FALSE(HasInflightRequests()); } @@ -458,7 +473,9 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponseAndCancel) { nullptr); request_->set_method("GET"); request_->Start(); + EXPECT_FALSE(HasInflightRequests()); base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(HasInflightRequests()); std::string expected_response; expected_response.reserve((sizeof(kTestData) - 1) * 1024); @@ -468,6 +485,7 @@ TEST_F(ServiceWorkerURLRequestJobTest, StreamResponseAndCancel) { } ASSERT_TRUE(stream_context->registry()->GetStream(stream_url).get()); request_->Cancel(); + EXPECT_FALSE(HasInflightRequests()); ASSERT_FALSE(stream_context->registry()->GetStream(stream_url).get()); for (int i = 0; i < 512; ++i) { expected_response += kTestData; @@ -495,8 +513,11 @@ TEST_F(ServiceWorkerURLRequestJobTest, nullptr); request_->set_method("GET"); request_->Start(); + EXPECT_FALSE(HasInflightRequests()); base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(HasInflightRequests()); request_->Cancel(); + EXPECT_FALSE(HasInflightRequests()); scoped_refptr<Stream> stream = new Stream(stream_context->registry(), nullptr, stream_url); diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index 1d70efe..476340b 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc @@ -627,6 +627,18 @@ void ServiceWorkerVersion::RemoveControllee( ScheduleStopWorker(); } +void ServiceWorkerVersion::AddStreamingURLRequestJob( + const ServiceWorkerURLRequestJob* request_job) { + DCHECK(streaming_url_request_jobs_.find(request_job) == + streaming_url_request_jobs_.end()); + streaming_url_request_jobs_.insert(request_job); +} + +void ServiceWorkerVersion::RemoveStreamingURLRequestJob( + const ServiceWorkerURLRequestJob* request_job) { + streaming_url_request_jobs_.erase(request_job); +} + void ServiceWorkerVersion::AddListener(Listener* listener) { listeners_.AddObserver(listener); } @@ -706,6 +718,8 @@ void ServiceWorkerVersion::OnStopped( SERVICE_WORKER_ERROR_FAILED, false); + streaming_url_request_jobs_.clear(); + FOR_EACH_OBSERVER(Listener, listeners_, OnWorkerStopped(this)); // There should be no more communication from/to a stopped worker. Deleting @@ -1131,7 +1145,8 @@ bool ServiceWorkerVersion::HasInflightRequests() const { !push_callbacks_.IsEmpty() || !geofencing_callbacks_.IsEmpty() || !get_client_info_callbacks_.IsEmpty() || - !cross_origin_connect_callbacks_.IsEmpty(); + !cross_origin_connect_callbacks_.IsEmpty() || + !streaming_url_request_jobs_.empty(); } void ServiceWorkerVersion::DoomInternal() { diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index 6e02c2d..2b9da6a 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h @@ -6,6 +6,7 @@ #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_ #include <map> +#include <set> #include <string> #include <vector> @@ -40,6 +41,7 @@ struct PlatformNotificationData; class ServiceWorkerContextCore; class ServiceWorkerProviderHost; class ServiceWorkerRegistration; +class ServiceWorkerURLRequestJob; class ServiceWorkerVersionInfo; // This class corresponds to a specific version of a ServiceWorker @@ -266,6 +268,13 @@ class CONTENT_EXPORT ServiceWorkerVersion // Returns if it has controllee. bool HasControllee() const { return !controllee_map_.empty(); } + // Adds and removes |request_job| as a dependent job not to stop the + // ServiceWorker while |request_job| is reading the stream of the fetch event + // response from the ServiceWorker. + void AddStreamingURLRequestJob(const ServiceWorkerURLRequestJob* request_job); + void RemoveStreamingURLRequestJob( + const ServiceWorkerURLRequestJob* request_job); + // Adds and removes Listeners. void AddListener(Listener* listener); void RemoveListener(Listener* listener); @@ -287,6 +296,7 @@ class CONTENT_EXPORT ServiceWorkerVersion class GetClientDocumentsCallback; friend class base::RefCounted<ServiceWorkerVersion>; + friend class ServiceWorkerURLRequestJobTest; FRIEND_TEST_ALL_PREFIXES(ServiceWorkerControlleeRequestHandlerTest, ActivateWaitingVersion); FRIEND_TEST_ALL_PREFIXES(ServiceWorkerVersionTest, ScheduleStopWorker); @@ -384,6 +394,8 @@ class CONTENT_EXPORT ServiceWorkerVersion IDMap<CrossOriginConnectCallback, IDMapOwnPointer> cross_origin_connect_callbacks_; + std::set<const ServiceWorkerURLRequestJob*> streaming_url_request_jobs_; + ControlleeMap controllee_map_; ControlleeByIDMap controllee_by_id_; base::WeakPtr<ServiceWorkerContextCore> context_; |