summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatt Falkenhagen <falken@chromium.org>2015-05-15 10:03:18 +0900
committerMatt Falkenhagen <falken@chromium.org>2015-05-15 01:04:21 +0000
commitbee610e3fc2c416b295c4619c610cfb6f5b13d35 (patch)
treed992a7a7070707facdbcf0c78bd61997481d8d7d
parent1619d40a0ad1efb8887d6d8fd7b377d76c8a05db (diff)
downloadchromium_src-bee610e3fc2c416b295c4619c610cfb6f5b13d35.zip
chromium_src-bee610e3fc2c416b295c4619c610cfb6f5b13d35.tar.gz
chromium_src-bee610e3fc2c416b295c4619c610cfb6f5b13d35.tar.bz2
(Merge to M43) Evict Service Worker when reading it from disk cache fails.
If reading the SW resources from the disk cache fails, we should evict the worker or else we will keep getting the same failure. This patch unsets the worker from the live registration, and then deletes the registration if the SW was the stored version. This relands the original commit (64e3ffb1c), and fixes a problem where the test terminated before the eviction, which starts on the IO thread, then hops to the database task runner, finishes. Now the test ends by looking up the registration in storage, which hops to the database task runner, so it can only finish once eviction finishes. Original patch: https://codereview.chromium.org/1098083003/ BUG=488061 R=kinuko@chromium.org TBR=kinuko Review URL: https://codereview.chromium.org/1108253002 Cr-Commit-Position: refs/heads/master@{#327260} (cherry picked from commit 0e476caf67493d79b6dc40891e7ef8868871983e) Review URL: https://codereview.chromium.org/1141053002 Cr-Commit-Position: refs/branch-heads/2357@{#395} Cr-Branched-From: 59d4494849b405682265ed5d3f5164573b9a939b-refs/heads/master@{#323860}
-rw-r--r--content/browser/service_worker/service_worker_browsertest.cc188
-rw-r--r--content/browser/service_worker/service_worker_read_from_cache_job.cc20
-rw-r--r--content/browser/service_worker/service_worker_read_from_cache_job.h1
-rw-r--r--content/browser/service_worker/service_worker_registration.cc2
4 files changed, 204 insertions, 7 deletions
diff --git a/content/browser/service_worker/service_worker_browsertest.cc b/content/browser/service_worker/service_worker_browsertest.cc
index 3a23937..1f41351 100644
--- a/content/browser/service_worker/service_worker_browsertest.cc
+++ b/content/browser/service_worker/service_worker_browsertest.cc
@@ -131,6 +131,25 @@ ServiceWorkerVersion::FetchCallback CreateResponseReceiver(
result);
}
+void ReceiveFindRegistrationStatus(
+ BrowserThread::ID run_quit_thread,
+ const base::Closure& quit,
+ ServiceWorkerStatusCode* out_status,
+ ServiceWorkerStatusCode status,
+ const scoped_refptr<ServiceWorkerRegistration>& registration) {
+ *out_status = status;
+ if (!quit.is_null())
+ BrowserThread::PostTask(run_quit_thread, FROM_HERE, quit);
+}
+
+ServiceWorkerStorage::FindRegistrationCallback CreateFindRegistrationReceiver(
+ BrowserThread::ID run_quit_thread,
+ const base::Closure& quit,
+ ServiceWorkerStatusCode* status) {
+ return base::Bind(&ReceiveFindRegistrationStatus, run_quit_thread, quit,
+ status);
+}
+
void ReadResponseBody(std::string* body,
storage::BlobDataHandle* blob_data_handle) {
ASSERT_TRUE(blob_data_handle);
@@ -228,7 +247,7 @@ class LongLivedResourceInterceptor : public net::URLRequestInterceptor {
void CreateLongLivedResourceInterceptors(
const GURL& worker_url, const GURL& import_url) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
scoped_ptr<net::URLRequestInterceptor> interceptor;
interceptor.reset(new LongLivedResourceInterceptor(
@@ -515,6 +534,7 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
}
void SetUpRegistrationOnIOThread(const std::string& worker_url) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
const GURL pattern = embedded_test_server()->GetURL("/service_worker/");
registration_ = new ServiceWorkerRegistration(
pattern,
@@ -539,6 +559,32 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
version_->OnPingTimeout();
}
+ void AddControlleeOnIOThread() {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ scoped_ptr<ServiceWorkerProviderHost> host(new ServiceWorkerProviderHost(
+ 33 /* dummy render process id */,
+ MSG_ROUTING_NONE /* render_frame_id */, 1 /* dummy provider_id */,
+ SERVICE_WORKER_PROVIDER_FOR_WINDOW, wrapper()->context()->AsWeakPtr(),
+ NULL));
+ host->SetDocumentUrl(
+ embedded_test_server()->GetURL("/service_worker/host"));
+ host->AssociateRegistration(registration_.get(),
+ false /* notify_controllerchange */);
+ wrapper()->context()->AddProviderHost(host.Pass());
+ }
+
+ void AddWaitingWorkerOnIOThread(const std::string& worker_url) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ scoped_refptr<ServiceWorkerVersion> waiting_version(
+ new ServiceWorkerVersion(
+ registration_.get(), embedded_test_server()->GetURL(worker_url),
+ wrapper()->context()->storage()->NewVersionId(),
+ wrapper()->context()->AsWeakPtr()));
+ waiting_version->SetStatus(ServiceWorkerVersion::INSTALLED);
+ registration_->SetWaitingVersion(waiting_version.get());
+ registration_->ActivateWaitingVersionWhenReady();
+ }
+
void StartWorker(ServiceWorkerStatusCode expected_status) {
ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
@@ -563,6 +609,58 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
ASSERT_EQ(expected_status, status);
}
+ void StoreRegistration(int64 version_id,
+ ServiceWorkerStatusCode expected_status) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
+ base::RunLoop store_run_loop;
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&self::StoreOnIOThread, this, store_run_loop.QuitClosure(),
+ &status, version_id));
+ store_run_loop.Run();
+ ASSERT_EQ(expected_status, status);
+
+ RunOnIOThread(base::Bind(&self::NotifyDoneInstallingRegistrationOnIOThread,
+ this, status));
+ }
+
+ void FindRegistrationForId(int64 id,
+ const GURL& origin,
+ ServiceWorkerStatusCode expected_status) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
+ base::RunLoop run_loop;
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&self::FindRegistrationForIdOnIOThread, this,
+ run_loop.QuitClosure(), &status, id, origin));
+ run_loop.Run();
+ ASSERT_EQ(expected_status, status);
+ }
+
+ void FindRegistrationForIdOnIOThread(const base::Closure& done,
+ ServiceWorkerStatusCode* result,
+ int64 id,
+ const GURL& origin) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ wrapper()->context()->storage()->FindRegistrationForId(
+ id, origin,
+ CreateFindRegistrationReceiver(BrowserThread::UI, done, result));
+ }
+
+ void NotifyDoneInstallingRegistrationOnIOThread(
+ ServiceWorkerStatusCode status) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ wrapper()->context()->storage()->NotifyDoneInstallingRegistration(
+ registration_.get(), version_.get(), status);
+ }
+
+ void RemoveLiveRegistrationOnIOThread(int64 id) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ wrapper()->context()->RemoveLiveRegistration(id);
+ }
+
void StartOnIOThread(const base::Closure& done,
ServiceWorkerStatusCode* result) {
ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -577,10 +675,22 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
CreateReceiver(BrowserThread::UI, done, result));
}
+ void StoreOnIOThread(const base::Closure& done,
+ ServiceWorkerStatusCode* result,
+ int64 version_id) {
+ ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ ServiceWorkerVersion* version =
+ wrapper()->context()->GetLiveVersion(version_id);
+ wrapper()->context()->storage()->StoreRegistration(
+ registration_.get(), version,
+ CreateReceiver(BrowserThread::UI, done, result));
+ }
+
void ActivateOnIOThread(const base::Closure& done,
ServiceWorkerStatusCode* result) {
ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
version_->SetStatus(ServiceWorkerVersion::ACTIVATING);
+ registration_->SetActiveVersion(version_.get());
version_->DispatchActivateEvent(
CreateReceiver(BrowserThread::UI, done, result));
}
@@ -697,6 +807,76 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, StartNotFound) {
StartWorker(SERVICE_WORKER_ERROR_NETWORK);
}
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, ReadResourceFailure) {
+ // Create and store a registration.
+ RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,
+ "/service_worker/worker.js"));
+ version_->SetStatus(ServiceWorkerVersion::ACTIVATED);
+ StoreRegistration(version_->version_id(), SERVICE_WORKER_OK);
+
+ // Add a non-existent resource to the version.
+ std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+ records.push_back(
+ ServiceWorkerDatabase::ResourceRecord(30, version_->script_url(), 100));
+ version_->script_cache_map()->SetResources(records);
+
+ // Start the worker. We'll fail to read the resource.
+ StartWorker(SERVICE_WORKER_ERROR_START_WORKER_FAILED);
+ EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version_->status());
+
+ // The registration should be deleted from storage since the broken worker was
+ // the stored one.
+ RunOnIOThread(base::Bind(&self::RemoveLiveRegistrationOnIOThread, this,
+ registration_->id()));
+ FindRegistrationForId(registration_->id(),
+ registration_->pattern().GetOrigin(),
+ SERVICE_WORKER_ERROR_NOT_FOUND);
+}
+
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
+ ReadResourceFailure_WaitingWorker) {
+ // Create a registration and active version.
+ RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,
+ "/service_worker/worker.js"));
+ base::RunLoop activate_run_loop;
+ ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
+ BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+ base::Bind(&self::ActivateOnIOThread, this,
+ activate_run_loop.QuitClosure(), &status));
+ activate_run_loop.Run();
+ EXPECT_EQ(SERVICE_WORKER_OK, status);
+ ASSERT_TRUE(registration_->active_version());
+
+ // Give the version a controllee.
+ RunOnIOThread(base::Bind(&self::AddControlleeOnIOThread, this));
+
+ // Add a non-existent resource to the version.
+ std::vector<ServiceWorkerDatabase::ResourceRecord> records;
+ records.push_back(
+ ServiceWorkerDatabase::ResourceRecord(30, version_->script_url(), 100));
+ version_->script_cache_map()->SetResources(records);
+
+ // Make a waiting version and store it.
+ RunOnIOThread(base::Bind(&self::AddWaitingWorkerOnIOThread, this,
+ "/service_worker/worker.js"));
+ StoreRegistration(registration_->waiting_version()->version_id(),
+ SERVICE_WORKER_OK);
+
+ // Start the broken worker. We'll fail to read from disk and the worker should
+ // be doomed.
+ StopWorker(SERVICE_WORKER_OK); // in case it's already running
+ StartWorker(SERVICE_WORKER_ERROR_START_WORKER_FAILED);
+ EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version_->status());
+
+ // The registration should still be in storage since the waiting worker was
+ // the stored one.
+ RunOnIOThread(base::Bind(&self::RemoveLiveRegistrationOnIOThread, this,
+ registration_->id()));
+ FindRegistrationForId(registration_->id(),
+ registration_->pattern().GetOrigin(),
+ SERVICE_WORKER_OK);
+}
+
IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, Install) {
InstallTestHelper("/service_worker/worker.js", SERVICE_WORKER_OK);
}
@@ -1196,11 +1376,11 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserV8CacheTest, Restart) {
// Activate the worker.
ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
- base::RunLoop acrivate_run_loop;
+ base::RunLoop activate_run_loop;
BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
base::Bind(&self::ActivateOnIOThread, this,
- acrivate_run_loop.QuitClosure(), &status));
- acrivate_run_loop.Run();
+ activate_run_loop.QuitClosure(), &status));
+ activate_run_loop.Run();
ASSERT_EQ(SERVICE_WORKER_OK, status);
// Stop the worker.
StopWorker(SERVICE_WORKER_OK);
diff --git a/content/browser/service_worker/service_worker_read_from_cache_job.cc b/content/browser/service_worker/service_worker_read_from_cache_job.cc
index 135c589..3c8400b 100644
--- a/content/browser/service_worker/service_worker_read_from_cache_job.cc
+++ b/content/browser/service_worker/service_worker_read_from_cache_job.cc
@@ -12,6 +12,7 @@
#include "content/browser/service_worker/service_worker_context_core.h"
#include "content/browser/service_worker/service_worker_disk_cache.h"
#include "content/browser/service_worker/service_worker_metrics.h"
+#include "content/public/browser/browser_thread.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"
#include "net/http/http_request_headers.h"
@@ -157,7 +158,7 @@ void ServiceWorkerReadFromCacheJob::OnReadInfoComplete(int result) {
DCHECK_LT(result, 0);
ServiceWorkerMetrics::CountReadResponseResult(
ServiceWorkerMetrics::READ_HEADERS_ERROR);
- NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+ Done(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
return;
}
DCHECK_GE(result, 0);
@@ -198,14 +199,27 @@ void ServiceWorkerReadFromCacheJob::SetupRangeResponse(int resource_size) {
range_requested_, resource_size, true /* replace status line */);
}
+void ServiceWorkerReadFromCacheJob::Done(const net::URLRequestStatus& status) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ if (!status.is_success()) {
+ // TODO(falken): Retry before evicting.
+ if (context_) {
+ ServiceWorkerRegistration* registration =
+ context_->GetLiveRegistration(version_->registration_id());
+ registration->DeleteVersion(version_);
+ }
+ }
+ NotifyDone(status);
+}
+
void ServiceWorkerReadFromCacheJob::OnReadComplete(int result) {
ServiceWorkerMetrics::ReadResponseResult check_result;
if (result == 0) {
check_result = ServiceWorkerMetrics::READ_OK;
- NotifyDone(net::URLRequestStatus());
+ Done(net::URLRequestStatus());
} else if (result < 0) {
check_result = ServiceWorkerMetrics::READ_DATA_ERROR;
- NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
+ Done(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
} else {
check_result = ServiceWorkerMetrics::READ_OK;
SetStatus(net::URLRequestStatus()); // Clear the IO_PENDING status
diff --git a/content/browser/service_worker/service_worker_read_from_cache_job.h b/content/browser/service_worker/service_worker_read_from_cache_job.h
index 48c2aa5..82d9d5f 100644
--- a/content/browser/service_worker/service_worker_read_from_cache_job.h
+++ b/content/browser/service_worker/service_worker_read_from_cache_job.h
@@ -55,6 +55,7 @@ class CONTENT_EXPORT ServiceWorkerReadFromCacheJob
const net::HttpResponseInfo* http_info() const;
bool is_range_request() const { return range_requested_.IsValid(); }
void SetupRangeResponse(int response_data_size);
+ void Done(const net::URLRequestStatus& status);
base::WeakPtr<ServiceWorkerContextCore> context_;
scoped_refptr<ServiceWorkerVersion> version_;
diff --git a/content/browser/service_worker/service_worker_registration.cc b/content/browser/service_worker/service_worker_registration.cc
index a71daf1..2edf34a 100644
--- a/content/browser/service_worker/service_worker_registration.cc
+++ b/content/browser/service_worker/service_worker_registration.cc
@@ -242,6 +242,8 @@ void ServiceWorkerRegistration::ClearUserData(
}
void ServiceWorkerRegistration::OnNoControllees(ServiceWorkerVersion* version) {
+ if (!context_)
+ return;
DCHECK_EQ(active_version(), version);
if (is_uninstalling_)
Clear();