Commit 08f163f9 authored by Patrick Monette's avatar Patrick Monette Committed by Commit Bot

Fix duplicate OnRunningStateChanged notification for the same version_id

When ServiceWorkerContextCore::DeleteAndStartOver() is used, the
existing service workers still exist for a short time but new instances
are created and they will reuse existing version_id because the count is
reset to kInvalidServiceWorkerVersionId. This leads to multiple running
notifications for indifferentiable service workers.

To fix this, the ServiceWorkerContextWrapper is notified when
DeleteAndStartOver() is called and prematurely sends a stopped
notification for all currently running service worker versions. Then, to
prevent future notifications from all the old versions, the
ServiceWorkerContextCore silently drops notifications if the core
associated with the version is no longer alive.

Bug: 993029
Change-Id: I27b192141265828139416abb37d8e0c762308679
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829865
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701448}
parent cc6261a5
...@@ -661,6 +661,10 @@ void ServiceWorkerContextCore::ScheduleDeleteAndStartOver() const { ...@@ -661,6 +661,10 @@ void ServiceWorkerContextCore::ScheduleDeleteAndStartOver() const {
void ServiceWorkerContextCore::DeleteAndStartOver(StatusCallback callback) { void ServiceWorkerContextCore::DeleteAndStartOver(StatusCallback callback) {
job_coordinator_->AbortAll(); job_coordinator_->AbortAll();
observer_list_->Notify(
FROM_HERE, &ServiceWorkerContextCoreObserver::OnDeleteAndStartOver);
storage_->DeleteAndStartOver(std::move(callback)); storage_->DeleteAndStartOver(std::move(callback));
} }
...@@ -743,6 +747,9 @@ void ServiceWorkerContextCore::OnStorageWiped() { ...@@ -743,6 +747,9 @@ void ServiceWorkerContextCore::OnStorageWiped() {
void ServiceWorkerContextCore::OnRunningStateChanged( void ServiceWorkerContextCore::OnRunningStateChanged(
ServiceWorkerVersion* version) { ServiceWorkerVersion* version) {
if (!version->context())
return;
observer_list_->Notify( observer_list_->Notify(
FROM_HERE, &ServiceWorkerContextCoreObserver::OnRunningStateChanged, FROM_HERE, &ServiceWorkerContextCoreObserver::OnRunningStateChanged,
version->version_id(), version->running_status()); version->version_id(), version->running_status());
......
...@@ -48,6 +48,11 @@ class ServiceWorkerContextCoreObserver { ...@@ -48,6 +48,11 @@ class ServiceWorkerContextCoreObserver {
virtual void OnNewLiveVersion(const ServiceWorkerVersionInfo& version_info) {} virtual void OnNewLiveVersion(const ServiceWorkerVersionInfo& version_info) {}
virtual void OnRunningStateChanged(int64_t version_id, virtual void OnRunningStateChanged(int64_t version_id,
EmbeddedWorkerStatus running_status) {} EmbeddedWorkerStatus running_status) {}
// Called when the context core is about to be deleted. After this is called,
// method calls on this observer will be for a new context core, possibly
// reusing version/registration IDs previously seen. So this method gives the
// observer a chance to discard any state it has.
virtual void OnDeleteAndStartOver() {}
virtual void OnVersionStateChanged(int64_t version_id, virtual void OnVersionStateChanged(int64_t version_id,
const GURL& scope, const GURL& scope,
ServiceWorkerVersion::Status status) {} ServiceWorkerVersion::Status status) {}
......
...@@ -369,6 +369,8 @@ void ServiceWorkerContextWrapper::OnNoControllees(int64_t version_id, ...@@ -369,6 +369,8 @@ void ServiceWorkerContextWrapper::OnNoControllees(int64_t version_id,
void ServiceWorkerContextWrapper::OnRunningStateChanged( void ServiceWorkerContextWrapper::OnRunningStateChanged(
int64_t version_id, int64_t version_id,
EmbeddedWorkerStatus running_status) { EmbeddedWorkerStatus running_status) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
// When |running_status| is RUNNING/STOPPING, the service // When |running_status| is RUNNING/STOPPING, the service
// worker is doing some actual tasks, so we consider the service worker is // worker is doing some actual tasks, so we consider the service worker is
// in a running status. // in a running status.
...@@ -383,7 +385,8 @@ void ServiceWorkerContextWrapper::OnRunningStateChanged( ...@@ -383,7 +385,8 @@ void ServiceWorkerContextWrapper::OnRunningStateChanged(
// process of the service worker is allocated/released, instead of using the // process of the service worker is allocated/released, instead of using the
// running status of the embedded worker. // running status of the embedded worker.
if (running_status == EmbeddedWorkerStatus::RUNNING) { if (running_status == EmbeddedWorkerStatus::RUNNING) {
running_service_workers_.emplace(version_id); bool inserted = running_service_workers_.emplace(version_id).second;
DCHECK(inserted);
for (auto& observer : observer_list_) { for (auto& observer : observer_list_) {
observer.OnVersionRunningStatusChanged(this, version_id, observer.OnVersionRunningStatusChanged(this, version_id,
true /* is_running */); true /* is_running */);
...@@ -394,12 +397,23 @@ void ServiceWorkerContextWrapper::OnRunningStateChanged( ...@@ -394,12 +397,23 @@ void ServiceWorkerContextWrapper::OnRunningStateChanged(
if (running_service_workers_.find(version_id) != if (running_service_workers_.find(version_id) !=
running_service_workers_.end() && running_service_workers_.end() &&
running_status == EmbeddedWorkerStatus::STOPPED) { running_status == EmbeddedWorkerStatus::STOPPED) {
running_service_workers_.erase(version_id); size_t removed = running_service_workers_.erase(version_id);
DCHECK_EQ(removed, 1u);
for (auto& observer : observer_list_) {
observer.OnVersionRunningStatusChanged(this, version_id,
false /* is_running */);
}
}
}
void ServiceWorkerContextWrapper::OnDeleteAndStartOver() {
for (int version_id : running_service_workers_) {
for (auto& observer : observer_list_) { for (auto& observer : observer_list_) {
observer.OnVersionRunningStatusChanged(this, version_id, observer.OnVersionRunningStatusChanged(this, version_id,
false /* is_running */); false /* is_running */);
} }
} }
running_service_workers_.clear();
} }
void ServiceWorkerContextWrapper::OnVersionStateChanged( void ServiceWorkerContextWrapper::OnVersionStateChanged(
......
...@@ -123,6 +123,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper ...@@ -123,6 +123,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
void OnNoControllees(int64_t version_id, const GURL& scope) override; void OnNoControllees(int64_t version_id, const GURL& scope) override;
void OnRunningStateChanged(int64_t version_id, void OnRunningStateChanged(int64_t version_id,
EmbeddedWorkerStatus running_status) override; EmbeddedWorkerStatus running_status) override;
void OnDeleteAndStartOver() override;
void OnVersionStateChanged(int64_t version_id, void OnVersionStateChanged(int64_t version_id,
const GURL& scope, const GURL& scope,
ServiceWorkerVersion::Status status) override; ServiceWorkerVersion::Status status) override;
......
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