Commit e46ff16a authored by falken's avatar falken Committed by Commit bot

Service Worker: Detach the controller when falling back to network on failure

Otherwise, subresource loads would also go to the broken SW, defeating the
fallback mechanism. It also would skew start worker metrics if the SW fails
to startup on each requst.

The eviction mechanism already takes case of this in
ServiceWorkerRegistration::DeleteVersion, but we don't
currently evict everytime a fetch event fails to dispatch.

BUG=498661,448003

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

Cr-Commit-Position: refs/heads/master@{#333713}
parent 6c1bab73
...@@ -490,6 +490,9 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent( ...@@ -490,6 +490,9 @@ void ServiceWorkerURLRequestJob::DidDispatchFetchEvent(
if (status != SERVICE_WORKER_OK) { if (status != SERVICE_WORKER_OK) {
// TODO(falken): Add UMA and the report error to the version. // TODO(falken): Add UMA and the report error to the version.
if (is_main_resource_load_) { if (is_main_resource_load_) {
// Using the service worker failed, so fallback to network. Detach the
// controller so subresource requests also skip the worker.
provider_host_->NotifyControllerLost();
response_type_ = FALLBACK_TO_NETWORK; response_type_ = FALLBACK_TO_NETWORK;
NotifyRestartRequired(); NotifyRestartRequired();
} else { } else {
......
...@@ -168,9 +168,9 @@ class ServiceWorkerURLRequestJobTest : public testing::Test { ...@@ -168,9 +168,9 @@ class ServiceWorkerURLRequestJobTest : public testing::Test {
helper_->context()->AsWeakPtr(), helper_->context()->AsWeakPtr(),
nullptr)); nullptr));
provider_host->SetDocumentUrl(GURL("http://example.com/")); provider_host->SetDocumentUrl(GURL("http://example.com/"));
registration_->SetActiveVersion(version_);
provider_host->AssociateRegistration(registration_.get(), provider_host->AssociateRegistration(registration_.get(),
false /* notify_controllerchange */); false /* notify_controllerchange */);
registration_->SetActiveVersion(version_);
ChromeBlobStorageContext* chrome_blob_storage_context = ChromeBlobStorageContext* chrome_blob_storage_context =
ChromeBlobStorageContext::GetFor(browser_context_.get()); ChromeBlobStorageContext::GetFor(browser_context_.get());
...@@ -640,6 +640,10 @@ TEST_F(ServiceWorkerURLRequestJobTest, FailFetchDispatch) { ...@@ -640,6 +640,10 @@ TEST_F(ServiceWorkerURLRequestJobTest, FailFetchDispatch) {
EXPECT_EQ(200, request_->GetResponseCode()); EXPECT_EQ(200, request_->GetResponseCode());
EXPECT_EQ("PASS", url_request_delegate_.response_data()); EXPECT_EQ("PASS", url_request_delegate_.response_data());
EXPECT_FALSE(HasInflightRequests()); EXPECT_FALSE(HasInflightRequests());
ServiceWorkerProviderHost* host =
helper_->context()->GetProviderHost(kProcessID, kProviderID);
ASSERT_TRUE(host);
EXPECT_EQ(host->controlling_version(), nullptr);
} }
// TODO(horo): Remove this test when crbug.com/485900 is fixed. // TODO(horo): Remove this test when crbug.com/485900 is fixed.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment