Fix mojo::shell::Context shutdown.
Make shutdown explicit (i.e., if you call Init(), you have to call
Shutdown()).
R=jamesr@chromium.org
Review URL: https://codereview.chromium.org/939353002
diff --git a/shell/child_process_host_unittest.cc b/shell/child_process_host_unittest.cc
index 6aa77b4..c52c8c7 100644
--- a/shell/child_process_host_unittest.cc
+++ b/shell/child_process_host_unittest.cc
@@ -52,6 +52,8 @@
int exit_code = child_process_host.Join();
VLOG(2) << "Joined child: exit_code = " << exit_code;
EXPECT_EQ(0, exit_code);
+
+ context.Shutdown();
}
} // namespace
diff --git a/shell/context.cc b/shell/context.cc
index 0707cef..c3efa89 100644
--- a/shell/context.cc
+++ b/shell/context.cc
@@ -15,6 +15,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "base/path_service.h"
+#include "base/run_loop.h"
#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
@@ -55,8 +56,6 @@
DISALLOW_COPY_AND_ASSIGN(Setup);
};
-static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
-
void InitContentHandlers(NativeApplicationLoader* loader,
base::CommandLine* command_line) {
// Default content handlers.
@@ -194,7 +193,9 @@
DCHECK(!base::MessageLoop::current());
}
+// static
void Context::EnsureEmbedderIsInitialized() {
+ static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
setup.Get();
}
@@ -276,14 +277,21 @@
}
void Context::Shutdown() {
+ DCHECK_EQ(base::MessageLoop::current()->task_runner(),
+ task_runners_->shell_runner());
embedder::ShutdownIPCSupport();
+ // We'll quit when we get OnShutdownComplete().
+ base::MessageLoop::current()->Run();
}
void Context::OnApplicationError(const GURL& url) {
if (app_urls_.find(url) != app_urls_.end()) {
app_urls_.erase(url);
- if (app_urls_.empty() && base::MessageLoop::current()->is_running())
- Shutdown();
+ if (app_urls_.empty() && base::MessageLoop::current()->is_running()) {
+ DCHECK_EQ(base::MessageLoop::current()->task_runner(),
+ task_runners_->shell_runner());
+ base::MessageLoop::current()->Quit();
+ }
}
}
diff --git a/shell/context.h b/shell/context.h
index 4d2cf34..62c49c2 100644
--- a/shell/context.h
+++ b/shell/context.h
@@ -26,6 +26,8 @@
Context();
~Context() override;
+ static void EnsureEmbedderIsInitialized();
+
// Point to the directory containing installed services, such as the network
// service. By default this directory is used as the base URL for resolving
// unknown mojo: URLs. The network service will be loaded from this directory,
@@ -44,9 +46,14 @@
// nop for everything but relative file URLs or URLs without a scheme.
GURL ResolveCommandLineURL(const std::string& path);
- static void EnsureEmbedderIsInitialized();
+ // This must be called with a message loop set up for the current thread,
+ // which must remain alive until after Shutdown() is called. Returns true on
+ // success.
bool Init();
+ // If Init() was called and succeeded, this must be called before destruction.
+ void Shutdown();
+
void Run(const GURL& url);
ScopedMessagePipeHandle ConnectToServiceByName(
const GURL& application_url,
@@ -59,8 +66,6 @@
private:
class NativeViewportApplicationLoader;
- void Shutdown();
-
// ApplicationManager::Delegate overrides.
void OnApplicationError(const GURL& url) override;
GURL ResolveURL(const GURL& url) override;
diff --git a/shell/desktop/mojo_main.cc b/shell/desktop/mojo_main.cc
index 8989b9e..d8ac657 100644
--- a/shell/desktop/mojo_main.cc
+++ b/shell/desktop/mojo_main.cc
@@ -108,6 +108,9 @@
FROM_HERE,
base::Bind(&mojo::shell::RunCommandLineApps, &shell_context));
message_loop.Run();
+
+ // Must be called be |message_loop| is destroyed.
+ shell_context.Shutdown();
}
}
return 0;
diff --git a/shell/in_process_dynamic_service_runner_unittest.cc b/shell/in_process_dynamic_service_runner_unittest.cc
index a2ff900..d0c81ae 100644
--- a/shell/in_process_dynamic_service_runner_unittest.cc
+++ b/shell/in_process_dynamic_service_runner_unittest.cc
@@ -14,6 +14,7 @@
base::MessageLoop loop;
context.Init();
InProcessDynamicServiceRunner runner(&context);
+ context.Shutdown();
// Shouldn't crash or DCHECK on destruction.
}
diff --git a/shell/native_application_loader_unittest.cc b/shell/native_application_loader_unittest.cc
index 473412a..697f828 100644
--- a/shell/native_application_loader_unittest.cc
+++ b/shell/native_application_loader_unittest.cc
@@ -70,6 +70,7 @@
new TestDynamicServiceRunnerFactory(&state_));
loader_.reset(new NativeApplicationLoader(&context_, factory.Pass()));
}
+ void TearDown() override { context_.Shutdown(); }
protected:
Context context_;
diff --git a/shell/shell_test_base.cc b/shell/shell_test_base.cc
index 6e4f18c..20e6b35 100644
--- a/shell/shell_test_base.cc
+++ b/shell/shell_test_base.cc
@@ -24,10 +24,14 @@
}
void ShellTestBase::SetUp() {
- shell_context_.Init();
+ CHECK(shell_context_.Init());
SetUpTestApplications();
}
+void ShellTestBase::TearDown() {
+ shell_context_.Shutdown();
+}
+
ScopedMessagePipeHandle ShellTestBase::ConnectToService(
const GURL& application_url,
const std::string& service_name) {
diff --git a/shell/shell_test_base.h b/shell/shell_test_base.h
index 2679bb5..bfb320b 100644
--- a/shell/shell_test_base.h
+++ b/shell/shell_test_base.h
@@ -25,6 +25,7 @@
~ShellTestBase() override;
void SetUp() override;
+ void TearDown() override;
// |application_url| should typically be a mojo: URL (the origin will be set
// to an "appropriate" file: URL).
