Shutdown Service Workers before the profile is destroyed

chrome/browser/profiles/profile_destroyer.cc

 // Copyright (c) 2012 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 "chrome/browser/profiles/profile_destroyer.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/service_worker_context.h"
+#include "content/public/browser/storage_partition.h"
 
 namespace {
 
 #if defined(OS_ANDROID)
 // Set the render host waiting time to 5s on Android, that's the same
 // as an "Application Not Responding" timeout.
 const int64 kTimerDelaySeconds = 5;
 #else
 const int64 kTimerDelaySeconds = 1;
 #endif
 
 }  // namespace
 
 ProfileDestroyer::DestroyerSet* ProfileDestroyer::pending_destroyers_ = NULL;
 
+static void ShutdownServiceWorkerContext(content::StoragePartition* partition) {
+  partition->GetServiceWorkerContext()->Terminate();
+}
+
 // static
 void ProfileDestroyer::DestroyProfileWhenAppropriate(Profile* const profile) {
   TRACE_EVENT0("shutdown", "ProfileDestroyer::DestroyProfileWhenAppropriate");
 
   DCHECK(profile);
   profile->MaybeSendDestroyedNotification();
 
+  // Service Workers must shutdown before the profile is destroyed, since they
+  // keep render process hosts alive and code assumes that render process hosts
+  // die before the profile dies.
+  content::BrowserContext::ForEachStoragePartition(
+      profile, base::Bind(ShutdownServiceWorkerContext));

[falken, 2014/10/21 09:36:36]

I'm not sure all codepaths that destroy a Profile go through here. Ideally we
listen to PROFILE_DESTROYED_NOTIFICATION but that is only observable in chrome,
not in content. And we don't have Service Worker classes in chrome.

[michaeln, 2014/10/21 23:24:13]

Profile::MaybeSendDestroyedNotification() could call a method on its subclass to
propagate this event. If this is only use of SWC->Terminate(), we could probably
remove it from the public api and traverse storage partitions in the content
lib.

base::NotifyImminentDestruction() ?

[falken, 2014/10/22 01:27:12]

That sounds promising and more bulletproof than hooking in ProfileDestroyer.
Something like NotifyWillBeDestroyed() that BrowserContext overrides?

Alternatively, is it better to make a chrome class that listens to the
NOTIFICATION_PROFILE_DESTROYED and destroys SWC? It feels like the virtual
method is just making for SW a special way to listen for the notification.
I'm not sure what you mean by this one.

+
   HostSet hosts;
   // Testing profiles can simply be deleted directly. Some tests don't setup
   // RenderProcessHost correctly and don't necessary run on the UI thread
   // anyway, so we can't use the AllHostIterator.
   if (profile->AsTestingProfile() == NULL) {
     GetHostsForProfile(profile, &hosts);
     if (!profile->IsOffTheRecord() && profile->HasOffTheRecordProfile())
       GetHostsForProfile(profile->GetOffTheRecordProfile(), &hosts);
   }
   // Generally, !hosts.empty() means that there is a leak in a render process
   // host that MUST BE FIXED!!!
   //
   // However, off-the-record profiles are destroyed before their
   // RenderProcessHosts in order to erase private data quickly, and

[falken, 2014/10/21 09:36:35]

This comment is a bit sketchy, it seems to contradict what we see with
ExtensionService.

   // RenderProcessHostImpl::Release() avoids destroying RenderProcessHosts in
   // --single-process mode to avoid race conditions.
   DCHECK(hosts.empty() || profile->IsOffTheRecord() ||
       content::RenderProcessHost::run_renderer_in_process()) << \
       "Profile still has " << hosts.size() << " hosts";
   // Note that we still test for !profile->IsOffTheRecord here even though we
   // DCHECK'd above because we want to protect Release builds against this even
   // we need to identify if there are leaks when we run Debug builds.
   if (hosts.empty() || !profile->IsOffTheRecord()) {
     if (profile->IsOffTheRecord())
@@ skipping 114 matching lines @@
         content::RenderProcessHost::AllHostsIterator());
       !iter.IsAtEnd(); iter.Advance()) {
     content::RenderProcessHost* render_process_host = iter.GetCurrentValue();
     if (render_process_host &&
             render_process_host->GetBrowserContext() == profile) {
       hosts->insert(render_process_host);
     }
   }
   return !hosts->empty();
 }