Commit 294bdb38 authored by jyasskin@chromium.org's avatar jyasskin@chromium.org

Shut down the ServiceWorker system as the browser's shutting down.

This avoids a DCHECK in ProfileDestroyer::DestroyProfileWhenAppropriate.

I also added a Chrome-side browsertest for the Service Worker system, where we
can check other things that only show up with this embedder.

I tried to move the DCHECK into ~BrowserContext so that we wouldn't need to
expose the Terminate() function across the Content API, but:
* BrowserContext doesn't know whether it's a "testing" profile.
* There is no BrowserContext::GetOffTheRecordProfile with which to check that
  transitive processes have been shut down.
* (Minor) GetHostsForProfile() works by iterating over all hosts, and moving the
  DCHECK to ~BrowserContext would have doubled the number of iterations needed.

BUG=368570

Review URL: https://codereview.chromium.org/305723003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273741 0039d316-1c4b-4281-b951-d872f2087c98
parent 92ab63c6
...@@ -86,6 +86,8 @@ ...@@ -86,6 +86,8 @@
#include "content/public/browser/plugin_service.h" #include "content/public/browser/plugin_service.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/resource_dispatcher_host.h" #include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/browser/service_worker_context.h"
#include "content/public/browser/storage_partition.h"
#include "extensions/common/constants.h" #include "extensions/common/constants.h"
#include "extensions/common/extension_l10n_util.h" #include "extensions/common/extension_l10n_util.h"
#include "net/socket/client_socket_pool_manager.h" #include "net/socket/client_socket_pool_manager.h"
...@@ -325,6 +327,10 @@ unsigned int BrowserProcessImpl::AddRefModule() { ...@@ -325,6 +327,10 @@ unsigned int BrowserProcessImpl::AddRefModule() {
return module_ref_count_; return module_ref_count_;
} }
static void ShutdownServiceWorkerContext(content::StoragePartition* partition) {
partition->GetServiceWorkerContext()->Terminate();
}
unsigned int BrowserProcessImpl::ReleaseModule() { unsigned int BrowserProcessImpl::ReleaseModule() {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
DCHECK_NE(0u, module_ref_count_); DCHECK_NE(0u, module_ref_count_);
...@@ -332,6 +338,14 @@ unsigned int BrowserProcessImpl::ReleaseModule() { ...@@ -332,6 +338,14 @@ unsigned int BrowserProcessImpl::ReleaseModule() {
if (0 == module_ref_count_) { if (0 == module_ref_count_) {
release_last_reference_callstack_ = base::debug::StackTrace(); release_last_reference_callstack_ = base::debug::StackTrace();
// Stop service workers
ProfileManager* pm = profile_manager();
std::vector<Profile*> profiles(pm->GetLoadedProfiles());
for (size_t i = 0; i < profiles.size(); ++i) {
content::BrowserContext::ForEachStoragePartition(
profiles[i], base::Bind(ShutdownServiceWorkerContext));
}
#if defined(ENABLE_PRINTING) #if defined(ENABLE_PRINTING)
// Wait for the pending print jobs to finish. Don't do this later, since // Wait for the pending print jobs to finish. Don't do this later, since
// this might cause a nested message loop to run, and we don't want pending // this might cause a nested message loop to run, and we don't want pending
......
// 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.
// This file tests that Service Workers (a Content feature) work in the Chromium
// embedder.
#include "base/bind.h"
#include "base/files/scoped_temp_dir.h"
#include "base/numerics/safe_conversions.h"
#include "base/run_loop.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/service_worker_context.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
namespace {
class ChromeServiceWorkerTest : public InProcessBrowserTest {
protected:
ChromeServiceWorkerTest() {
EXPECT_TRUE(service_worker_dir_.CreateUniqueTempDir());
}
void WriteFile(const base::FilePath::StringType& filename,
base::StringPiece contents) {
EXPECT_EQ(base::checked_cast<int>(contents.size()),
base::WriteFile(service_worker_dir_.path().Append(filename),
contents.data(),
contents.size()));
}
base::ScopedTempDir service_worker_dir_;
};
static void ExpectResultAndRun(bool expected,
const base::Closure& continuation,
bool actual) {
EXPECT_EQ(expected, actual);
continuation.Run();
}
// http://crbug.com/368570
IN_PROC_BROWSER_TEST_F(ChromeServiceWorkerTest,
CanShutDownWithRegisteredServiceWorker) {
WriteFile(FILE_PATH_LITERAL("service_worker.js"), "");
embedded_test_server()->ServeFilesFromDirectory(service_worker_dir_.path());
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
content::ServiceWorkerContext* sw_context =
content::BrowserContext::GetDefaultStoragePartition(browser()->profile())
->GetServiceWorkerContext();
base::RunLoop run_loop;
sw_context->RegisterServiceWorker(
embedded_test_server()->GetURL("/*"),
embedded_test_server()->GetURL("/service_worker.js"),
base::Bind(&ExpectResultAndRun, true, run_loop.QuitClosure()));
run_loop.Run();
// Leave the Service Worker registered, and make sure that the browser can
// shut down without DCHECK'ing. It'd be nice to check here that the SW is
// actually occupying a process, but we don't yet have the public interface to
// do that.
}
} // namespace
...@@ -904,6 +904,7 @@ ...@@ -904,6 +904,7 @@
'browser/chrome_main_browsertest.cc', 'browser/chrome_main_browsertest.cc',
'browser/chrome_plugin_browsertest.cc', 'browser/chrome_plugin_browsertest.cc',
'browser/chrome_security_exploit_browsertest.cc', 'browser/chrome_security_exploit_browsertest.cc',
'browser/chrome_service_worker_browsertest.cc',
'browser/chrome_switches_browsertest.cc', 'browser/chrome_switches_browsertest.cc',
'browser/chromeos/accessibility/accessibility_manager_browsertest.cc', 'browser/chromeos/accessibility/accessibility_manager_browsertest.cc',
'browser/chromeos/accessibility/magnification_manager_browsertest.cc', 'browser/chromeos/accessibility/magnification_manager_browsertest.cc',
......
...@@ -117,6 +117,11 @@ void ServiceWorkerContextWrapper::UnregisterServiceWorker( ...@@ -117,6 +117,11 @@ void ServiceWorkerContextWrapper::UnregisterServiceWorker(
base::Bind(&FinishUnregistrationOnIO, continuation)); base::Bind(&FinishUnregistrationOnIO, continuation));
} }
void ServiceWorkerContextWrapper::Terminate() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
process_manager_->Shutdown();
}
void ServiceWorkerContextWrapper::AddObserver( void ServiceWorkerContextWrapper::AddObserver(
ServiceWorkerContextObserver* observer) { ServiceWorkerContextObserver* observer) {
observer_list_->AddObserver(observer); observer_list_->AddObserver(observer);
......
...@@ -62,6 +62,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper ...@@ -62,6 +62,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
virtual void UnregisterServiceWorker(const GURL& pattern, virtual void UnregisterServiceWorker(const GURL& pattern,
const ResultCallback& continuation) const ResultCallback& continuation)
OVERRIDE; OVERRIDE;
virtual void Terminate() OVERRIDE;
void AddObserver(ServiceWorkerContextObserver* observer); void AddObserver(ServiceWorkerContextObserver* observer);
void RemoveObserver(ServiceWorkerContextObserver* observer); void RemoveObserver(ServiceWorkerContextObserver* observer);
......
...@@ -41,12 +41,23 @@ void ServiceWorkerJobCoordinator::JobQueue::Pop( ...@@ -41,12 +41,23 @@ void ServiceWorkerJobCoordinator::JobQueue::Pop(
jobs_.front()->Start(); jobs_.front()->Start();
} }
void ServiceWorkerJobCoordinator::JobQueue::ClearForShutdown() {
STLDeleteElements(&jobs_);
}
ServiceWorkerJobCoordinator::ServiceWorkerJobCoordinator( ServiceWorkerJobCoordinator::ServiceWorkerJobCoordinator(
base::WeakPtr<ServiceWorkerContextCore> context) base::WeakPtr<ServiceWorkerContextCore> context)
: context_(context) { : context_(context) {
} }
ServiceWorkerJobCoordinator::~ServiceWorkerJobCoordinator() { ServiceWorkerJobCoordinator::~ServiceWorkerJobCoordinator() {
if (!context_) {
for (RegistrationJobMap::iterator it = jobs_.begin(); it != jobs_.end();
++it) {
it->second.ClearForShutdown();
}
jobs_.clear();
}
DCHECK(jobs_.empty()) << "Destroying ServiceWorkerJobCoordinator with " DCHECK(jobs_.empty()) << "Destroying ServiceWorkerJobCoordinator with "
<< jobs_.size() << " job queues"; << jobs_.size() << " job queues";
} }
......
...@@ -55,6 +55,10 @@ class CONTENT_EXPORT ServiceWorkerJobCoordinator { ...@@ -55,6 +55,10 @@ class CONTENT_EXPORT ServiceWorkerJobCoordinator {
bool empty() { return jobs_.empty(); } bool empty() { return jobs_.empty(); }
// Marks that the browser is shutting down, so jobs may be destroyed before
// finishing.
void ClearForShutdown();
private: private:
std::deque<ServiceWorkerRegisterJobBase*> jobs_; std::deque<ServiceWorkerRegisterJobBase*> jobs_;
}; };
......
...@@ -51,6 +51,14 @@ ServiceWorkerProcessManager::~ServiceWorkerProcessManager() { ...@@ -51,6 +51,14 @@ ServiceWorkerProcessManager::~ServiceWorkerProcessManager() {
void ServiceWorkerProcessManager::Shutdown() { void ServiceWorkerProcessManager::Shutdown() {
browser_context_ = NULL; browser_context_ = NULL;
for (std::map<int, ProcessInfo>::const_iterator it = instance_info_.begin();
it != instance_info_.end();
++it) {
RenderProcessHost* rph = RenderProcessHost::FromID(it->second.process_id);
DCHECK(rph);
static_cast<RenderProcessHostImpl*>(rph)->DecrementWorkerRefCount();
}
instance_info_.clear();
} }
void ServiceWorkerProcessManager::AllocateWorkerProcess( void ServiceWorkerProcessManager::AllocateWorkerProcess(
...@@ -147,6 +155,11 @@ void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) { ...@@ -147,6 +155,11 @@ void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) {
// RenderProcessHost. // RenderProcessHost.
return; return;
} }
if (browser_context_ == NULL) {
// Shutdown already released all instances.
DCHECK(instance_info_.empty());
return;
}
std::map<int, ProcessInfo>::iterator info = std::map<int, ProcessInfo>::iterator info =
instance_info_.find(embedded_worker_id); instance_info_.find(embedded_worker_id);
DCHECK(info != instance_info_.end()); DCHECK(info != instance_info_.end());
......
...@@ -38,7 +38,8 @@ ServiceWorkerRegisterJob::ServiceWorkerRegisterJob( ...@@ -38,7 +38,8 @@ ServiceWorkerRegisterJob::ServiceWorkerRegisterJob(
weak_factory_(this) {} weak_factory_(this) {}
ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() { ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() {
DCHECK(phase_ == INITIAL || phase_ == COMPLETE); DCHECK(!context_ || phase_ == INITIAL || phase_ == COMPLETE)
<< "Jobs should only be interrupted during shutdown.";
} }
void ServiceWorkerRegisterJob::AddCallback(const RegistrationCallback& callback, void ServiceWorkerRegisterJob::AddCallback(const RegistrationCallback& callback,
......
...@@ -49,6 +49,11 @@ class ServiceWorkerContext { ...@@ -49,6 +49,11 @@ class ServiceWorkerContext {
// TODO(jyasskin): Provide a way to SendMessage to a Scope. // TODO(jyasskin): Provide a way to SendMessage to a Scope.
// Synchronously releases all of the RenderProcessHosts that have Service
// Workers running inside them, and prevents any new Service Worker instances
// from starting up.
virtual void Terminate() = 0;
protected: protected:
ServiceWorkerContext() {} ServiceWorkerContext() {}
virtual ~ServiceWorkerContext() {} virtual ~ServiceWorkerContext() {}
......
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