[ServiceWorker] Avoid using stale pointer of EmbeddedWorkerInstance
We're saving the raw pointer of an EmbeddedWorkerInstance in a
base::ScopedObserver member of ServiceWorkerRegisterJob, but, the
EmbeddedWorkerInstance is not guaranteed to outlive the
ServiceWorkerRegisterJob, it may get destroyed and lead to a situation
that ServiceWorkerRegisterJob holds a stale pointer, which could lead
to crash.
This CL lets ServiceWorkerRegisterJob listen OnDetached() from the
EmbeddedWorkerInstance so it can be aware that the EmbeddedWorkerInstanc
pointer is going to be stale and should be removed from the source list
of base::ScopedObserver.
This fix should be safe enough now, but in longer term we'll try to
remove EmbeddedWorkerInstance::Listener interface and let
EmbeddedWorkerInstance talk directly to its owner ServiceWorkerVersion,
then let ServiceWorkerRegisterJob observe the ServiceWorkerVersion it's
manipulating to get OnScriptLoaded() notification.
BUG=854063,855394
Change-Id: I9c5a46beda1aafff32a86b9055be2b53d50fda97
Reviewed-on: https://chromium-review.googlesource.com/1112904
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569906}diff --git a/content/browser/service_worker/service_worker_register_job.cc b/content/browser/service_worker/service_worker_register_job.cc
index 74645c0..1cb531d 100644
--- a/content/browser/service_worker/service_worker_register_job.cc
+++ b/content/browser/service_worker/service_worker_register_job.cc
@@ -623,6 +623,16 @@
new_version()->embedded_worker()->ResumeAfterDownload();
}
+void ServiceWorkerRegisterJob::OnDetached(EmbeddedWorkerStatus old_status) {
+ // The version's EmbeddedWorkerInstance may be getting destructed soon, so
+ // remove the observer to avoid a use-after-free. We expect to continue when
+ // the StartWorker() callback is called with failure.
+ // TODO(crbug.com/855852): Remove the EmbeddedWorkerInstance::Listener
+ // interface and have this class listen to ServiceWorkerVersion directly.
+ if (observer_.IsObserving(new_version()->embedded_worker()))
+ observer_.Remove(new_version()->embedded_worker());
+}
+
void ServiceWorkerRegisterJob::BumpLastUpdateCheckTimeIfNeeded() {
// Bump the last update check time only when the register/update job fetched
// the version having bypassed the network cache. We assume that the
diff --git a/content/browser/service_worker/service_worker_register_job.h b/content/browser/service_worker/service_worker_register_job.h
index 4ab705e..557c2027 100644
--- a/content/browser/service_worker/service_worker_register_job.h
+++ b/content/browser/service_worker/service_worker_register_job.h
@@ -140,6 +140,7 @@
// EmbeddedWorkerInstance::Listener implementation:
void OnScriptLoaded() override;
+ void OnDetached(EmbeddedWorkerStatus old_status) override;
void BumpLastUpdateCheckTimeIfNeeded();
