Ensure ServiceWorkerContextWrapper is deleted on the UI thread

ServiceWorkerContextWrapper's observer methods are supposed to always be
dispatched on the UI thread. This CL ensures this is true with
additional DCHECKs and adds the DeleteOnUIThread custom deleter because
observers are also notified in the destructor.

Bug: 993029
Change-Id: I10c7baa6a908e61ea50a00486d0e3bae81e7e32c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841765Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703413}

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -343,12 +343,16 @@ BrowserThread::ID ServiceWorkerContext::GetCoreThreadId() {
 void ServiceWorkerContextWrapper::OnRegistrationCompleted(
     int64_t registration_id,
     const GURL& scope) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (auto& observer : observer_list_)
     observer.OnRegistrationCompleted(scope);
 }
 
 void ServiceWorkerContextWrapper::OnRegistrationStored(int64_t registration_id,
                                                        const GURL& scope) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (auto& observer : observer_list_)
     observer.OnRegistrationStored(registration_id, scope);
 }
@@ -356,12 +360,16 @@ void ServiceWorkerContextWrapper::OnRegistrationStored(int64_t registration_id,
 void ServiceWorkerContextWrapper::OnReportConsoleMessage(
     int64_t version_id,
     const ConsoleMessage& message) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (auto& observer : observer_list_)
     observer.OnReportConsoleMessage(version_id, message);
 }
 
 void ServiceWorkerContextWrapper::OnNoControllees(int64_t version_id,
                                                   const GURL& scope) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (auto& observer : observer_list_)
     observer.OnNoControllees(version_id, scope);
 }
@@ -407,6 +415,8 @@ void ServiceWorkerContextWrapper::OnRunningStateChanged(
 }
 
 void ServiceWorkerContextWrapper::OnDeleteAndStartOver() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (int version_id : running_service_workers_) {
     for (auto& observer : observer_list_) {
       observer.OnVersionRunningStatusChanged(this, version_id,
@@ -420,6 +430,8 @@ void ServiceWorkerContextWrapper::OnVersionStateChanged(
     int64_t version_id,
     const GURL& scope,
     ServiceWorkerVersion::Status status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   if (status == ServiceWorkerVersion::Status::ACTIVATED) {
     for (auto& observer : observer_list_)
       observer.OnVersionActivated(version_id, scope);
@@ -431,11 +443,15 @@ void ServiceWorkerContextWrapper::OnVersionStateChanged(
 
 void ServiceWorkerContextWrapper::AddObserver(
     ServiceWorkerContextObserver* observer) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   observer_list_.AddObserver(observer);
 }
 
 void ServiceWorkerContextWrapper::RemoveObserver(
     ServiceWorkerContextObserver* observer) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   observer_list_.RemoveObserver(observer);
 }
 
@@ -1451,6 +1467,8 @@ void ServiceWorkerContextWrapper::RemoveObserver(
 }
 
 ServiceWorkerContextWrapper::~ServiceWorkerContextWrapper() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   for (auto& observer : observer_list_)
     observer.OnDestruct(static_cast<ServiceWorkerContext*>(this));
 

content/browser/service_worker/service_worker_context_wrapper.h

@@ -20,6 +20,7 @@
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_core_observer.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/service_worker_running_info.h"
@@ -55,7 +56,8 @@ class URLLoaderFactoryGetter;
 class CONTENT_EXPORT ServiceWorkerContextWrapper
     : public ServiceWorkerContext,
       public ServiceWorkerContextCoreObserver,
-      public base::RefCountedThreadSafe<ServiceWorkerContextWrapper> {
+      public base::RefCountedThreadSafe<ServiceWorkerContextWrapper,
+                                        BrowserThread::DeleteOnUIThread> {
  public:
   using StatusCallback =
       base::OnceCallback<void(blink::ServiceWorkerStatusCode)>;
@@ -326,9 +328,9 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
 
  private:
   friend class BackgroundSyncManagerTest;
-  friend class base::RefCountedThreadSafe<ServiceWorkerContextWrapper>;
-  friend class EmbeddedWorkerTestHelper;
+  friend class base::DeleteHelper<ServiceWorkerContextWrapper>;
   friend class EmbeddedWorkerBrowserTest;
+  friend class EmbeddedWorkerTestHelper;
   friend class FakeServiceWorkerContextWrapper;
   friend class ServiceWorkerClientsApiBrowserTest;
   friend class ServiceWorkerInternalsUI;
@@ -336,6 +338,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   friend class ServiceWorkerProcessManager;
   friend class ServiceWorkerRequestHandler;
   friend class ServiceWorkerVersionBrowserTest;
+  friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
 
   ~ServiceWorkerContextWrapper() override;