diff options
| author | michaeln@chromium.org <michaeln@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-29 09:34:43 +0000 |
|---|---|---|
| committer | michaeln@chromium.org <michaeln@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-29 09:34:43 +0000 |
| commit | 72dcdec9774f490bdc3918b56d22ca121f03facd (patch) | |
| tree | 24a246c43a6a0de531a53384eb959ad4afedba00 | |
| parent | ed1b7924f642de3375a61c579957f777265dad31 (diff) | |
| download | chromium_src-72dcdec9774f490bdc3918b56d22ca121f03facd.zip chromium_src-72dcdec9774f490bdc3918b56d22ca121f03facd.tar.gz chromium_src-72dcdec9774f490bdc3918b56d22ca121f03facd.tar.bz2 | |
The serviceworker update algo has to wait until documents close prior to performing its final activation step. If the browser exits prior to that happening, the algo has to be completed in the next browsing session. This CL adds logic that will trigger the deferred activation when first navigating into a registered scope.
This CL also adds logic to defer navigations into a registered scope while ACTIVATING and not yet ACTIVATED.
BUG=371671
Review URL: https://codereview.chromium.org/413853002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286146 0039d316-1c4b-4281-b951-d872f2087c98
12 files changed, 334 insertions, 100 deletions
diff --git a/content/browser/service_worker/service_worker_controllee_request_handler.cc b/content/browser/service_worker/service_worker_controllee_request_handler.cc index fdb27c2..14212f2 100644 --- a/content/browser/service_worker/service_worker_controllee_request_handler.cc +++ b/content/browser/service_worker/service_worker_controllee_request_handler.cc @@ -112,9 +112,12 @@ void ServiceWorkerControlleeRequestHandler::PrepareForMainResource( const GURL& url) { DCHECK(job_.get()); DCHECK(context_); - // The corresponding provider_host may already have associate version in - // redirect case, unassociate it now. - provider_host_->UnsetVersion(NULL); + // The corresponding provider_host may already have associated a registration + // in redirect case, unassociate it now. + provider_host_->SetControllerVersion(NULL); + provider_host_->SetActiveVersion(NULL); + provider_host_->SetWaitingVersion(NULL); + provider_host_->SetInstallingVersion(NULL); GURL stripped_url = net::SimplifyUrlForRequest(url); provider_host_->SetDocumentUrl(stripped_url); @@ -129,19 +132,57 @@ ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource( ServiceWorkerStatusCode status, const scoped_refptr<ServiceWorkerRegistration>& registration) { DCHECK(job_.get()); - if (status != SERVICE_WORKER_OK || !registration->active_version()) { - // No registration, or no active version for the registration is available. + if (status != SERVICE_WORKER_OK) { job_->FallbackToNetwork(); - // TODO(michaeln): If there's a waiting version, activate it instead of - // using the network. return; } + DCHECK(registration); ServiceWorkerMetrics::CountControlledPageLoad(); - // TODO(michaeln): if 'activating' wait until it's activated before - // forwarding the request to the serviceworker. - DCHECK(registration); + // Initiate activation of a waiting version. + // Usually a register job initiates activation but that + // doesn't happen if the browser exits prior to activation + // having occurred. This check handles that case. + if (registration->waiting_version()) + registration->ActivateWaitingVersionWhenReady(); + + scoped_refptr<ServiceWorkerVersion> active_version = + registration->active_version(); + + // Wait until it's activated before firing fetch events. + if (active_version && + active_version->status() == ServiceWorkerVersion::ACTIVATING) { + registration->active_version()->RegisterStatusChangeCallback( + base::Bind(&self::OnVersionStatusChanged, + weak_factory_.GetWeakPtr(), + registration, + active_version)); + return; + } + + if (!active_version || + active_version->status() != ServiceWorkerVersion::ACTIVATED) { + job_->FallbackToNetwork(); + return; + } + + provider_host_->SetControllerVersion(registration->active_version()); + provider_host_->SetActiveVersion(registration->active_version()); + provider_host_->SetWaitingVersion(registration->waiting_version()); + provider_host_->SetInstallingVersion(registration->installing_version()); + + job_->ForwardToServiceWorker(); +} + +void ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged( + ServiceWorkerRegistration* registration, + ServiceWorkerVersion* version) { + if (version != registration->active_version() || + version->status() != ServiceWorkerVersion::ACTIVATED) { + job_->FallbackToNetwork(); + return; + } provider_host_->SetControllerVersion(registration->active_version()); provider_host_->SetActiveVersion(registration->active_version()); provider_host_->SetWaitingVersion(registration->waiting_version()); diff --git a/content/browser/service_worker/service_worker_controllee_request_handler.h b/content/browser/service_worker/service_worker_controllee_request_handler.h index b91cee8..1829774 100644 --- a/content/browser/service_worker/service_worker_controllee_request_handler.h +++ b/content/browser/service_worker/service_worker_controllee_request_handler.h @@ -5,6 +5,7 @@ #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CONTROLLEE_REQUEST_HANDLER_H_ #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CONTROLLEE_REQUEST_HANDLER_H_ +#include "base/gtest_prod_util.h" #include "content/browser/service_worker/service_worker_request_handler.h" namespace net { @@ -16,6 +17,7 @@ namespace content { class ServiceWorkerRegistration; class ServiceWorkerURLRequestJob; +class ServiceWorkerVersion; // A request handler derivative used to handle requests from // controlled documents. @@ -39,6 +41,8 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler GURL* original_url_via_service_worker) const OVERRIDE; private: + FRIEND_TEST_ALL_PREFIXES(ServiceWorkerControlleeRequestHandlerTest, + ActivateWaitingVersion); typedef ServiceWorkerControlleeRequestHandler self; // For main resource case. @@ -46,6 +50,9 @@ class CONTENT_EXPORT ServiceWorkerControlleeRequestHandler void DidLookupRegistrationForMainResource( ServiceWorkerStatusCode status, const scoped_refptr<ServiceWorkerRegistration>& registration); + void OnVersionStatusChanged( + ServiceWorkerRegistration* registration, + ServiceWorkerVersion* version); // For sub resource case. void PrepareForSubResource(); diff --git a/content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc b/content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc new file mode 100644 index 0000000..a2e6a05 --- /dev/null +++ b/content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc @@ -0,0 +1,124 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/files/scoped_temp_dir.h" +#include "base/logging.h" +#include "base/run_loop.h" +#include "content/browser/browser_thread_impl.h" +#include "content/browser/fileapi/mock_url_request_delegate.h" +#include "content/browser/service_worker/embedded_worker_test_helper.h" +#include "content/browser/service_worker/service_worker_context_core.h" +#include "content/browser/service_worker/service_worker_controllee_request_handler.h" +#include "content/browser/service_worker/service_worker_provider_host.h" +#include "content/browser/service_worker/service_worker_registration.h" +#include "content/browser/service_worker/service_worker_registration.h" +#include "content/browser/service_worker/service_worker_url_request_job.h" +#include "content/browser/service_worker/service_worker_utils.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "net/url_request/url_request_context.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace content { + +namespace { + +int kMockRenderProcessId = 1224; + +void EmptyCallback() {} + +} + +class ServiceWorkerControlleeRequestHandlerTest : public testing::Test { + public: + ServiceWorkerControlleeRequestHandlerTest() + : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {} + + virtual void SetUp() OVERRIDE { + helper_.reset(new EmbeddedWorkerTestHelper(kMockRenderProcessId)); + + // A new unstored registration/version. + scope_ = GURL("http://host/scope/*"); + script_url_ = GURL("http://host/script.js"); + registration_ = new ServiceWorkerRegistration( + scope_, script_url_, 1L, context()->AsWeakPtr()); + version_ = new ServiceWorkerVersion( + registration_, + 1L, context()->AsWeakPtr()); + + // An empty host. + scoped_ptr<ServiceWorkerProviderHost> host(new ServiceWorkerProviderHost( + kMockRenderProcessId, 1 /* provider_id */, + context()->AsWeakPtr(), NULL)); + provider_host_ = host->AsWeakPtr(); + context()->AddProviderHost(make_scoped_ptr(host.release())); + + context()->storage()->LazyInitialize(base::Bind(&EmptyCallback)); + base::RunLoop().RunUntilIdle(); + } + + virtual void TearDown() OVERRIDE { + version_ = NULL; + registration_ = NULL; + helper_.reset(); + } + + ServiceWorkerContextCore* context() const { return helper_->context(); } + + protected: + TestBrowserThreadBundle browser_thread_bundle_; + scoped_ptr<EmbeddedWorkerTestHelper> helper_; + scoped_refptr<ServiceWorkerRegistration> registration_; + scoped_refptr<ServiceWorkerVersion> version_; + base::WeakPtr<ServiceWorkerProviderHost> provider_host_; + net::URLRequestContext url_request_context_; + MockURLRequestDelegate url_request_delegate_; + GURL scope_; + GURL script_url_; +}; + +TEST_F(ServiceWorkerControlleeRequestHandlerTest, ActivateWaitingVersion) { + // Store a registration that is installed but not activated yet. + version_->SetStatus(ServiceWorkerVersion::INSTALLED); + registration_->SetWaitingVersion(version_); + context()->storage()->StoreRegistration( + registration_, version_, + base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)); + base::RunLoop().RunUntilIdle(); + + // Conduct a main resource load. + const GURL kDocUrl("http://host/scope/doc"); + scoped_ptr<net::URLRequest> request = url_request_context_.CreateRequest( + kDocUrl, + net::DEFAULT_PRIORITY, + &url_request_delegate_, + NULL); + scoped_ptr<ServiceWorkerControlleeRequestHandler> handler( + new ServiceWorkerControlleeRequestHandler( + context()->AsWeakPtr(), + provider_host_, + base::WeakPtr<webkit_blob::BlobStorageContext>(), + ResourceType::MAIN_FRAME)); + scoped_refptr<net::URLRequestJob> job = + handler->MaybeCreateJob(request.get(), NULL); + ServiceWorkerURLRequestJob* sw_job = + static_cast<ServiceWorkerURLRequestJob*>(job.get()); + + EXPECT_FALSE(sw_job->ShouldFallbackToNetwork()); + EXPECT_FALSE(sw_job->ShouldForwardToServiceWorker()); + EXPECT_FALSE(version_->HasControllee()); + + base::RunLoop().RunUntilIdle(); + + EXPECT_EQ(ServiceWorkerVersion::ACTIVATED, + version_->status()); + EXPECT_FALSE(sw_job->ShouldFallbackToNetwork()); + EXPECT_TRUE(sw_job->ShouldForwardToServiceWorker()); + EXPECT_TRUE(version_->HasControllee()); + + // Navigations should trigger an update too. + handler.reset(NULL); + EXPECT_TRUE(version_->update_timer_.IsRunning()); +} + +} // namespace content diff --git a/content/browser/service_worker/service_worker_register_job.cc b/content/browser/service_worker/service_worker_register_job.cc index dd96b60..9d1a83d 100644 --- a/content/browser/service_worker/service_worker_register_job.cc +++ b/content/browser/service_worker/service_worker_register_job.cc @@ -21,37 +21,6 @@ void RunSoon(const base::Closure& closure) { base::MessageLoop::current()->PostTask(FROM_HERE, closure); } -// Helper class for the [[Update]] algo. -class DeferredActivationHelper : public ServiceWorkerVersion::Listener { - public: - explicit DeferredActivationHelper(ServiceWorkerRegistration* registration) - : registration_(registration), - active_version_(registration->active_version()), - waiting_version_(registration->waiting_version()) { - active_version_->AddListener(this); - } - - virtual ~DeferredActivationHelper() {} - - private: - virtual void OnNoControllees(ServiceWorkerVersion* version) OVERRIDE { - DCHECK_EQ(active_version_, version); - scoped_ptr<DeferredActivationHelper> self_deletor(this); - active_version_->RemoveListener(this); - if (registration_->active_version() != active_version_ || - registration_->waiting_version() != waiting_version_) { - return; // Something has changed making activation n/a. - } - registration_->ActivateWaitingVersion( - base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)); - } - - scoped_refptr<ServiceWorkerRegistration> registration_; - scoped_refptr<ServiceWorkerVersion> active_version_; - scoped_refptr<ServiceWorkerVersion> waiting_version_; - DISALLOW_COPY_AND_ASSIGN(DeferredActivationHelper); -}; - } // namespace typedef ServiceWorkerRegisterJobBase::RegistrationJobType RegistrationJobType; @@ -185,9 +154,6 @@ void ServiceWorkerRegisterJob::SetPhase(Phase phase) { case STORE: DCHECK(phase_ == INSTALL) << phase_; break; - case ACTIVATE: - DCHECK(phase_ == STORE) << phase_; - break; case COMPLETE: DCHECK(phase_ != INITIAL && phase_ != COMPLETE) << phase_; break; @@ -392,20 +358,9 @@ void ServiceWorkerRegisterJob::OnStoreRegistrationComplete( // "14. Wait until no document is using registration as their // Service Worker registration." - if (registration()->active_version() && - registration()->active_version()->HasControllee()) { - scoped_ptr<DeferredActivationHelper> deferred_activation( - new DeferredActivationHelper(registration())); - // It will delete itself when done. - ignore_result(deferred_activation.release()); - Complete(SERVICE_WORKER_OK); - return; - } + registration()->ActivateWaitingVersionWhenReady(); - SetPhase(ACTIVATE); - registration()->ActivateWaitingVersion( - base::Bind(&ServiceWorkerRegisterJob::Complete, - weak_factory_.GetWeakPtr())); + Complete(SERVICE_WORKER_OK); } void ServiceWorkerRegisterJob::Complete(ServiceWorkerStatusCode status) { diff --git a/content/browser/service_worker/service_worker_register_job.h b/content/browser/service_worker/service_worker_register_job.h index 34a5721..548fa5a 100644 --- a/content/browser/service_worker/service_worker_register_job.h +++ b/content/browser/service_worker/service_worker_register_job.h @@ -96,7 +96,6 @@ class ServiceWorkerRegisterJob UPDATE, INSTALL, STORE, - ACTIVATE, COMPLETE, ABORT, }; diff --git a/content/browser/service_worker/service_worker_registration.cc b/content/browser/service_worker/service_worker_registration.cc index 1305595..9cd539d 100644 --- a/content/browser/service_worker/service_worker_registration.cc +++ b/content/browser/service_worker/service_worker_registration.cc @@ -30,6 +30,8 @@ ServiceWorkerRegistration::ServiceWorkerRegistration( : pattern_(pattern), script_url_(script_url), registration_id_(registration_id), + is_deleted_(false), + should_activate_when_ready_(false), context_(context) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); DCHECK(context_); @@ -40,6 +42,7 @@ ServiceWorkerRegistration::~ServiceWorkerRegistration() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); if (context_) context_->RemoveLiveRegistration(registration_id_); + ResetShouldActivateWhenReady(); } void ServiceWorkerRegistration::AddListener(Listener* listener) { @@ -63,12 +66,14 @@ ServiceWorkerRegistrationInfo ServiceWorkerRegistration::GetInfo() { void ServiceWorkerRegistration::SetActiveVersion( ServiceWorkerVersion* version) { + ResetShouldActivateWhenReady(); SetVersionInternal(version, &active_version_, ChangedVersionAttributesMask::ACTIVE_VERSION); } void ServiceWorkerRegistration::SetWaitingVersion( ServiceWorkerVersion* version) { + ResetShouldActivateWhenReady(); SetVersionInternal(version, &waiting_version_, ChangedVersionAttributesMask::WAITING_VERSION); } @@ -124,13 +129,37 @@ void ServiceWorkerRegistration::UnsetVersionInternal( } } -void ServiceWorkerRegistration::ActivateWaitingVersion( - const StatusCallback& completion_callback) { +void ServiceWorkerRegistration::ActivateWaitingVersionWhenReady() { + DCHECK(waiting_version()); + if (should_activate_when_ready_) + return; + if (active_version() && active_version()->HasControllee()) { + active_version()->AddListener(this); + should_activate_when_ready_ = true; + return; + } + ActivateWaitingVersion(); +} + +void ServiceWorkerRegistration::OnNoControllees(ServiceWorkerVersion* version) { + DCHECK_EQ(active_version(), version); + DCHECK(should_activate_when_ready_); + active_version_->RemoveListener(this); + should_activate_when_ready_ = false; + ActivateWaitingVersion(); +} + +void ServiceWorkerRegistration::ActivateWaitingVersion() { DCHECK(context_); DCHECK(waiting_version()); scoped_refptr<ServiceWorkerVersion> activating_version = waiting_version(); scoped_refptr<ServiceWorkerVersion> exiting_version = active_version(); + if (activating_version->is_doomed() || + activating_version->status() == ServiceWorkerVersion::REDUNDANT) { + return; // Activation is no longer relevant. + } + // "4. If exitingWorker is not null, if (exiting_version) { DCHECK(!exiting_version->HasControllee()); @@ -161,14 +190,13 @@ void ServiceWorkerRegistration::ActivateWaitingVersion( // "9. Queue a task to fire an event named activate..." activating_version->DispatchActivateEvent( base::Bind(&ServiceWorkerRegistration::OnActivateEventFinished, - this, activating_version, completion_callback)); + this, activating_version)); } void ServiceWorkerRegistration::OnActivateEventFinished( ServiceWorkerVersion* activating_version, - const StatusCallback& completion_callback, ServiceWorkerStatusCode status) { - if (activating_version != active_version()) + if (!context_ || activating_version != active_version()) return; // TODO(kinuko,falken): For some error cases (e.g. ServiceWorker is // unexpectedly terminated) we may want to retry sending the event again. @@ -178,12 +206,15 @@ void ServiceWorkerRegistration::OnActivateEventFinished( context_, activating_version); UnsetVersion(activating_version); activating_version->Doom(); - if (!active_version()) { + if (!waiting_version()) { + // Delete the records from the db. context_->storage()->DeleteRegistration( id(), script_url().GetOrigin(), - base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)); + base::Bind(&ServiceWorkerRegistration::OnDeleteFinished, this)); + // But not from memory if there is a version in the pipeline. + if (installing_version()) + is_deleted_ = false; } - completion_callback.Run(status); return; } @@ -195,7 +226,19 @@ void ServiceWorkerRegistration::OnActivateEventFinished( this, base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)); } - completion_callback.Run(SERVICE_WORKER_OK); +} + +void ServiceWorkerRegistration::ResetShouldActivateWhenReady() { + if (should_activate_when_ready_) { + active_version()->RemoveListener(this); + should_activate_when_ready_ = false; + } +} + +void ServiceWorkerRegistration::OnDeleteFinished( + ServiceWorkerStatusCode status) { + // Intentionally empty completion callback, used to prevent + // |this| from being deleted until the storage method completes. } } // namespace content diff --git a/content/browser/service_worker/service_worker_registration.h b/content/browser/service_worker/service_worker_registration.h index a8b0fa7..4109288 100644 --- a/content/browser/service_worker/service_worker_registration.h +++ b/content/browser/service_worker/service_worker_registration.h @@ -26,7 +26,8 @@ class ServiceWorkerVersion; // being associated with the same registration. The class roughly // corresponds to navigator.serviceWorker.registgration. class CONTENT_EXPORT ServiceWorkerRegistration - : NON_EXPORTED_BASE(public base::RefCounted<ServiceWorkerRegistration>) { + : NON_EXPORTED_BASE(public base::RefCounted<ServiceWorkerRegistration>), + public ServiceWorkerVersion::Listener { public: typedef base::Callback<void(ServiceWorkerStatusCode status)> StatusCallback; @@ -76,15 +77,19 @@ class CONTENT_EXPORT ServiceWorkerRegistration // listeners via OnVersionAttributesChanged. void UnsetVersion(ServiceWorkerVersion* version); - // This method corresponds to the [[Activate]] algorithm described in - // the service worker specification. It's only valid to call this method - // when the registration's active version has no controllees. - void ActivateWaitingVersion(const StatusCallback& completion_callback); + // Triggers the [[Activate]] algorithm when the currently active version + // has no controllees. If there are no controllees at the time the method + // is called, activation is initiated immediately. + void ActivateWaitingVersionWhenReady(); + + bool is_deleted() const { return is_deleted_; } + void set_is_deleted() { is_deleted_ = true; } private: - ~ServiceWorkerRegistration(); friend class base::RefCounted<ServiceWorkerRegistration>; + virtual ~ServiceWorkerRegistration(); + void SetVersionInternal( ServiceWorkerVersion* version, scoped_refptr<ServiceWorkerVersion>* data_member, @@ -92,14 +97,23 @@ class CONTENT_EXPORT ServiceWorkerRegistration void UnsetVersionInternal( ServiceWorkerVersion* version, ChangedVersionAttributesMask* mask); + + // ServiceWorkerVersion::Listener override. + virtual void OnNoControllees(ServiceWorkerVersion* version) OVERRIDE; + + // This method corresponds to the [[Activate]] algorithm. + void ActivateWaitingVersion(); void OnActivateEventFinished( ServiceWorkerVersion* activating_version, - const StatusCallback& completion_callback, ServiceWorkerStatusCode status); + void ResetShouldActivateWhenReady(); + void OnDeleteFinished(ServiceWorkerStatusCode status); const GURL pattern_; const GURL script_url_; const int64 registration_id_; + bool is_deleted_; + bool should_activate_when_ready_; scoped_refptr<ServiceWorkerVersion> active_version_; scoped_refptr<ServiceWorkerVersion> waiting_version_; scoped_refptr<ServiceWorkerVersion> installing_version_; diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc index ab73931..83e8c79 100644 --- a/content/browser/service_worker/service_worker_storage.cc +++ b/content/browser/service_worker/service_worker_storage.cc @@ -211,6 +211,15 @@ ServiceWorkerStorage::InitialData::InitialData() ServiceWorkerStorage::InitialData::~InitialData() { } +ServiceWorkerStorage:: +DidDeleteRegistrationParams::DidDeleteRegistrationParams() + : registration_id(kInvalidServiceWorkerRegistrationId) { +} + +ServiceWorkerStorage:: +DidDeleteRegistrationParams::~DidDeleteRegistrationParams() { +} + ServiceWorkerStorage::~ServiceWorkerStorage() { weak_factory_.InvalidateWeakPtrs(); database_task_runner_->DeleteSoon(FROM_HERE, database_.release()); @@ -466,6 +475,11 @@ void ServiceWorkerStorage::DeleteRegistration( if (!has_checked_for_stale_resources_) DeleteStaleResources(); + DidDeleteRegistrationParams params; + params.registration_id = registration_id; + params.origin = origin; + params.callback = callback; + database_task_runner_->PostTask( FROM_HERE, base::Bind(&DeleteRegistrationFromDB, @@ -473,12 +487,14 @@ void ServiceWorkerStorage::DeleteRegistration( base::MessageLoopProxy::current(), registration_id, origin, base::Bind(&ServiceWorkerStorage::DidDeleteRegistration, - weak_factory_.GetWeakPtr(), origin, callback))); + weak_factory_.GetWeakPtr(), params))); - // TODO(michaeln): Either its instance should also be - // removed from liveregistrations map or the live object - // should marked as deleted in some way and not 'findable' - // thereafter. + // The registration should no longer be findable. + pending_deletions_.insert(registration_id); + ServiceWorkerRegistration* registration = + context_->GetLiveRegistration(registration_id); + if (registration) + registration->set_is_deleted(); } scoped_ptr<ServiceWorkerResponseReader> @@ -702,7 +718,7 @@ void ServiceWorkerStorage::DidFindRegistrationForDocument( const ResourceList& resources, ServiceWorkerDatabase::Status status) { if (status == ServiceWorkerDatabase::STATUS_OK) { - callback.Run(SERVICE_WORKER_OK, GetOrCreateRegistration(data, resources)); + ReturnFoundRegistration(callback, data, resources); return; } @@ -728,7 +744,7 @@ void ServiceWorkerStorage::DidFindRegistrationForPattern( const ResourceList& resources, ServiceWorkerDatabase::Status status) { if (status == ServiceWorkerDatabase::STATUS_OK) { - callback.Run(SERVICE_WORKER_OK, GetOrCreateRegistration(data, resources)); + ReturnFoundRegistration(callback, data, resources); return; } @@ -752,8 +768,7 @@ void ServiceWorkerStorage::DidFindRegistrationForId( const ResourceList& resources, ServiceWorkerDatabase::Status status) { if (status == ServiceWorkerDatabase::STATUS_OK) { - callback.Run(SERVICE_WORKER_OK, - GetOrCreateRegistration(data, resources)); + ReturnFoundRegistration(callback, data, resources); return; } @@ -769,6 +784,20 @@ void ServiceWorkerStorage::DidFindRegistrationForId( scoped_refptr<ServiceWorkerRegistration>()); } +void ServiceWorkerStorage::ReturnFoundRegistration( + const FindRegistrationCallback& callback, + const ServiceWorkerDatabase::RegistrationData& data, + const ResourceList& resources) { + scoped_refptr<ServiceWorkerRegistration> registration = + GetOrCreateRegistration(data, resources); + if (registration->is_deleted()) { + // It's past the point of no return and no longer findable. + callback.Run(SERVICE_WORKER_ERROR_NOT_FOUND, NULL); + return; + } + callback.Run(SERVICE_WORKER_OK, registration); +} + void ServiceWorkerStorage::DidGetAllRegistrations( const GetAllRegistrationInfosCallback& callback, RegistrationList* registrations, @@ -863,20 +892,20 @@ void ServiceWorkerStorage::DidUpdateToActiveState( } void ServiceWorkerStorage::DidDeleteRegistration( - const GURL& origin, - const StatusCallback& callback, + const DidDeleteRegistrationParams& params, bool origin_is_deletable, int64 version_id, const std::vector<int64>& newly_purgeable_resources, ServiceWorkerDatabase::Status status) { + pending_deletions_.erase(params.registration_id); if (status != ServiceWorkerDatabase::STATUS_OK) { ScheduleDeleteAndStartOver(); - callback.Run(DatabaseStatusToStatusCode(status)); + params.callback.Run(DatabaseStatusToStatusCode(status)); return; } if (origin_is_deletable) - registered_origins_.erase(origin); - callback.Run(SERVICE_WORKER_OK); + registered_origins_.erase(params.origin); + params.callback.Run(SERVICE_WORKER_OK); if (!context_ || !context_->GetLiveVersion(version_id)) StartPurgingResources(newly_purgeable_resources); @@ -893,6 +922,10 @@ ServiceWorkerStorage::GetOrCreateRegistration( registration = new ServiceWorkerRegistration( data.scope, data.script, data.registration_id, context_); + if (pending_deletions_.find(data.registration_id) != + pending_deletions_.end()) { + registration->set_is_deleted(); + } scoped_refptr<ServiceWorkerVersion> version = context_->GetLiveVersion(data.version_id); if (!version) { @@ -900,10 +933,6 @@ ServiceWorkerStorage::GetOrCreateRegistration( version->SetStatus(data.is_active ? ServiceWorkerVersion::ACTIVATED : ServiceWorkerVersion::INSTALLED); version->script_cache_map()->SetResources(resources); - - // TODO(michaeln): need to activate a waiting version that wasn't - // actrivated in an earlier session, maybe test for this condition - // (waitingversion and no activeversion) when navigating to a page? } if (version->status() == ServiceWorkerVersion::ACTIVATED) diff --git a/content/browser/service_worker/service_worker_storage.h b/content/browser/service_worker/service_worker_storage.h index 03c5e0f..30db1e9 100644 --- a/content/browser/service_worker/service_worker_storage.h +++ b/content/browser/service_worker/service_worker_storage.h @@ -152,6 +152,7 @@ class CONTENT_EXPORT ServiceWorkerStorage private: friend class ServiceWorkerResourceStorageTest; + friend class ServiceWorkerControlleeRequestHandlerTest; FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest, DeleteRegistration_NoLiveVersion); FRIEND_TEST_ALL_PREFIXES(ServiceWorkerResourceStorageTest, @@ -173,6 +174,16 @@ class CONTENT_EXPORT ServiceWorkerStorage ~InitialData(); }; + // Because there are too many params for base::Bind to wrap a closure around. + struct DidDeleteRegistrationParams { + int64 registration_id; + GURL origin; + StatusCallback callback; + + DidDeleteRegistrationParams(); + ~DidDeleteRegistrationParams(); + }; + typedef std::vector<ServiceWorkerDatabase::RegistrationData> RegistrationList; typedef std::map<int64, scoped_refptr<ServiceWorkerRegistration> > RegistrationRefsById; @@ -241,12 +252,15 @@ class CONTENT_EXPORT ServiceWorkerStorage const StatusCallback& callback, ServiceWorkerDatabase::Status status); void DidDeleteRegistration( - const GURL& origin, - const StatusCallback& callback, + const DidDeleteRegistrationParams& params, bool origin_is_deletable, int64 version_id, const std::vector<int64>& newly_purgeable_resources, ServiceWorkerDatabase::Status status); + void ReturnFoundRegistration( + const FindRegistrationCallback& callback, + const ServiceWorkerDatabase::RegistrationData& data, + const ResourceList& resources); scoped_refptr<ServiceWorkerRegistration> GetOrCreateRegistration( const ServiceWorkerDatabase::RegistrationData& data, @@ -355,6 +369,7 @@ class CONTENT_EXPORT ServiceWorkerStorage std::deque<int64> purgeable_resource_ids_; bool is_purge_pending_; bool has_checked_for_stale_resources_; + std::set<int64> pending_deletions_; base::WeakPtrFactory<ServiceWorkerStorage> weak_factory_; diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index 4d73515..2820ec8 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc @@ -572,19 +572,22 @@ void ServiceWorkerVersion::OnGetClientDocuments(int request_id) { void ServiceWorkerVersion::OnActivateEventFinished( int request_id, blink::WebServiceWorkerEventResult result) { - DCHECK_EQ(ACTIVATING, status()) << status(); + DCHECK(ACTIVATING == status() || + REDUNDANT == status()) << status(); StatusCallback* callback = activate_callbacks_.Lookup(request_id); if (!callback) { NOTREACHED() << "Got unexpected message: " << request_id; return; } - ServiceWorkerStatusCode status = SERVICE_WORKER_OK; - if (result == blink::WebServiceWorkerEventResultRejected) - status = SERVICE_WORKER_ERROR_ACTIVATE_WORKER_FAILED; + ServiceWorkerStatusCode rv = SERVICE_WORKER_OK; + if (result == blink::WebServiceWorkerEventResultRejected || + status() != ACTIVATING) { + rv = SERVICE_WORKER_ERROR_ACTIVATE_WORKER_FAILED; + } scoped_refptr<ServiceWorkerVersion> protect(this); - callback->Run(status); + callback->Run(rv); activate_callbacks_.Remove(request_id); } diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index df37c87..1b8afee 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h @@ -227,12 +227,15 @@ class CONTENT_EXPORT ServiceWorkerVersion // the version is controlling a page, these changes will happen when the // version no longer controls any pages. void Doom(); + bool is_doomed() const { return is_doomed_; } private: + friend class base::RefCounted<ServiceWorkerVersion>; + FRIEND_TEST_ALL_PREFIXES(ServiceWorkerControlleeRequestHandlerTest, + ActivateWaitingVersion); typedef ServiceWorkerVersion self; typedef std::map<ServiceWorkerProviderHost*, int> ControlleeMap; typedef IDMap<ServiceWorkerProviderHost> ControlleeByIDMap; - friend class base::RefCounted<ServiceWorkerVersion>; virtual ~ServiceWorkerVersion(); diff --git a/content/content_tests.gypi b/content/content_tests.gypi index 82d49a1..2983a18 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -586,6 +586,7 @@ 'browser/service_worker/embedded_worker_test_helper.cc', 'browser/service_worker/embedded_worker_test_helper.h', 'browser/service_worker/service_worker_context_unittest.cc', + 'browser/service_worker/service_worker_controllee_request_handler_unittest.cc', 'browser/service_worker/service_worker_database_unittest.cc', 'browser/service_worker/service_worker_dispatcher_host_unittest.cc', 'browser/service_worker/service_worker_dispatcher_host_unittest.cc', |