mojo: terminate apps if the shell goes away

Each application <> shell connection is represented by a ShellImpl instance
on the shell side. This CL makes the Application watch its ShellPtr for pipe
errors so it can Quit() itself if the shell goes away (shell loop destroyed).

mojo_shell_tests starts using a new method to terminate all shell connections
in this CL, and waits afterward until KeepAlive quits the loop signifying that
all apps are gone. This is done prior to destroying the shell MessageLoop so
that the test ensures apps have all died before the next test. In the future
it would be a test failure if this wasn't a no-op, but we have things in the
shell that don't quit themselves right now.

The shell itself won't wait around for apps to die (as of this CL), but
~MessageLoop will still send the message to Application sides that will quit.

** NOTE ** This does not require an app build target to explicitly add magic
*.cc files to their sources.  It requires selecting an appropriate
mojo_application_{chromium, standalone} library akin to mojo_environment_*.
We can possibly combine these two in the future.

BUG=394477

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287680

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287894 0039d316-1c4b-4281-b951-d872f2087c98

mojo/mojo.gyp

@@ -166,7 +166,7 @@
         'mojo_base.gyp:mojo_application_bindings',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_system_impl',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_external_service_bindings',
         'mojo_gles2_impl',
         'mojo_native_viewport_service',
@@ -367,7 +367,7 @@
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_run_all_unittests',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_service_manager',
       ],
       'includes': [ 'public/tools/bindings/mojom_bindings_generator.gypi' ],
@@ -457,7 +457,7 @@
             '../base/base.gyp:base',
             '../build/linux/system.gyp:dbus',
             '../dbus/dbus.gyp:dbus',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_system_impl',
             'mojo_external_service_bindings',

mojo/mojo_base.gyp

@@ -341,6 +341,19 @@
         '..',
       ],
     },
+    {
+     'target_name': 'mojo_application_chromium',
+     'type': 'static_library',
+     'sources': [
+       'public/cpp/application/lib/application_impl_chromium.cc',
+      ],
+      'dependencies': [
+        'mojo_application_base',
+       ],
+      'export_dependent_settings': [
+        'mojo_application_base',
+       ],
+    },
     {
       # GN version: //mojo/bindings/js
       'target_name': 'mojo_js_bindings_lib',

mojo/mojo_examples.gypi

@@ -8,7 +8,7 @@
       'target_name': 'mojo_echo_client',
       'type': 'loadable_module',
       'dependencies': [
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
@@ -45,7 +45,7 @@
       'target_name': 'mojo_echo_service',
       'type': 'loadable_module',
       'dependencies': [
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
@@ -71,7 +71,7 @@
         # TODO(darin): we should not be linking against these libraries!
         '../ui/events/events.gyp:events',
         '../ui/gfx/gfx.gyp:gfx_geometry',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
@@ -104,7 +104,7 @@
         '../cc/cc.gyp:cc',
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/gfx/gfx.gyp:gfx_geometry',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_cc_support',
@@ -132,7 +132,7 @@
       'target_name': 'mojo_wget',
       'type': 'loadable_module',
       'dependencies': [
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
@@ -171,7 +171,7 @@
       'dependencies': [
         '../skia/skia.gyp:skia',
         '../ui/gfx/gfx.gyp:gfx',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_utility',
@@ -197,7 +197,7 @@
         '../ppapi/ppapi.gyp:ppapi_c',
         '../ppapi/ppapi_internal.gyp:ppapi_example_gles2_spinning_cube',
         '../ui/events/events.gyp:events_base',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_geometry_bindings',
@@ -293,7 +293,7 @@
         '../skia/skia.gyp:skia',
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/gfx/gfx.gyp:gfx_geometry',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_system_impl',
@@ -348,7 +348,7 @@
         '../skia/skia.gyp:skia',
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/gfx/gfx.gyp:gfx_geometry',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_system_impl',
@@ -383,7 +383,7 @@
             '../ui/compositor/compositor.gyp:compositor',
             '../ui/gfx/gfx.gyp:gfx',
             '../ui/gfx/gfx.gyp:gfx_geometry',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_aura_support',
@@ -402,7 +402,7 @@
           'type': 'loadable_module',
           'dependencies': [
             '../base/base.gyp:base',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_view_manager_bindings',
             '<(mojo_system_for_loadable_module)',
@@ -429,7 +429,7 @@
             '../ui/resources/ui_resources.gyp:ui_test_pak',
             '../ui/views/views.gyp:views',
             '../url/url.gyp:url_lib',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_aura_support',
@@ -464,7 +464,7 @@
             '../ui/gfx/gfx.gyp:gfx',
             '../ui/gfx/gfx.gyp:gfx_geometry',
             '../ui/gl/gl.gyp:gl',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_cpp_bindings',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_base.gyp:mojo_utility',
@@ -495,7 +495,7 @@
             '../ui/resources/ui_resources.gyp:ui_test_pak',
             '../ui/views/views.gyp:views',
             '../url/url.gyp:url_lib',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_aura_support',
@@ -561,7 +561,7 @@
             '../ui/resources/ui_resources.gyp:ui_resources',
             '../ui/resources/ui_resources.gyp:ui_test_pak',
             '../ui/views/views.gyp:views',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_cpp_bindings',
             'mojo_base.gyp:mojo_utility',
             'mojo_base.gyp:mojo_environment_chromium',
@@ -593,7 +593,7 @@
             '../ui/gfx/gfx.gyp:gfx_geometry',
             '../ui/gl/gl.gyp:gl',
             '../url/url.gyp:url_lib',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_cpp_bindings',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_base.gyp:mojo_utility',
@@ -617,7 +617,7 @@
             '../ui/gfx/gfx.gyp:gfx_geometry',
             '../ui/gl/gl.gyp:gl',
             '../url/url.gyp:url_lib',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_cpp_bindings',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_base.gyp:mojo_utility',
@@ -641,7 +641,7 @@
             '../skia/skia.gyp:skia',
             '../ui/gfx/gfx.gyp:gfx_geometry',
             '../ui/views/views.gyp:views',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_input_events_lib',
             'mojo_media_viewer_bindings',
@@ -661,7 +661,7 @@
           'type': 'loadable_module',
           'dependencies': [
             '../base/base.gyp:base',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_core_window_manager_lib',
             'mojo_view_manager_lib',
@@ -677,7 +677,7 @@
           'type': 'loadable_module',
           'dependencies': [
             '../base/base.gyp:base',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_view_manager_bindings',
             '<(mojo_system_for_loadable_module)',
@@ -692,7 +692,7 @@
           'type': 'loadable_module',
           'dependencies': [
             '../base/base.gyp:base',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_core_window_manager_bindings',
             'mojo_view_manager_lib',
@@ -712,7 +712,7 @@
           'type': 'loadable_module',
           'dependencies': [
             '../base/base.gyp:base',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_standalone',
             'mojo_base.gyp:mojo_cpp_bindings',
             'mojo_base.gyp:mojo_environment_standalone',
             'mojo_base.gyp:mojo_utility',

mojo/mojo_public.gypi

@@ -182,7 +182,7 @@
     },
     {
       # GN version: //mojo/public/cpp/application
-      'target_name': 'mojo_application',
+      'target_name': 'mojo_application_base',
       'type': 'static_library',
       'sources': [
         'public/cpp/application/application_connection.h',
@@ -206,6 +206,19 @@
         'mojo_application_bindings',
       ],
     },
+    {
+      'target_name': 'mojo_application_standalone',
+      'type': 'static_library',
+      'sources': [
+        'public/cpp/application/lib/application_impl_standalone.cc',
+      ],
+      'dependencies': [
+        'mojo_application_base',
+      ],
+      'export_dependent_settings': [
+        'mojo_application_base',
+      ],
+    },
   ],
   'conditions': [
     ['OS == "android"', {

mojo/mojo_public_tests.gypi

@@ -101,7 +101,7 @@
       'dependencies': [
         '../base/base.gyp:base',
         '../testing/gtest.gyp:gtest',
-        'mojo_application',
+        'mojo_application_standalone',
         'mojo_utility',
         'mojo_environment_standalone',
         'mojo_run_all_unittests',

mojo/mojo_services.gypi

@@ -32,7 +32,7 @@
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_utility',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_navigation_bindings',
         'mojo_network_bindings',
         'mojo_launcher_bindings',
@@ -303,7 +303,7 @@
         '../ui/gfx/gfx.gyp:gfx_geometry',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_geometry_bindings',
         'mojo_geometry_lib',
         'mojo_gles2_service',
@@ -392,7 +392,7 @@
         '../url/url.gyp:url_lib',
         'mojo_base.gyp:mojo_common_lib',
         'mojo_base.gyp:mojo_environment_chromium',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_network_bindings',
       ],
       'export_dependent_settings': [
@@ -434,7 +434,7 @@
         '../ui/gfx/gfx.gyp:gfx_geometry',
         'mojo_base.gyp:mojo_environment_chromium',
         'mojo_base.gyp:mojo_system_impl',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_cc_support',
         'mojo_geometry_bindings',
         'mojo_geometry_lib',
@@ -483,7 +483,7 @@
         '../url/url.gyp:url_lib',
         'mojo_base.gyp:mojo_cpp_bindings',
         'mojo_base.gyp:mojo_environment_chromium',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_launcher_bindings',
         'mojo_network_bindings',
         '<(mojo_system_for_loadable_module)',
@@ -521,7 +521,7 @@
         '../ui/events/events.gyp:events',
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/gfx/gfx.gyp:gfx_geometry',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_chromium',
         'mojo_base.gyp:mojo_application_bindings',
         'mojo_geometry_bindings',
         'mojo_geometry_lib',
@@ -629,7 +629,7 @@
         '../base/base.gyp:base',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_test_service_bindings',
         '<(mojo_system_for_loadable_module)',
       ],
@@ -653,7 +653,7 @@
         '../base/base.gyp:base',
         'mojo_base.gyp:mojo_environment_standalone',
         'mojo_base.gyp:mojo_utility',
-        'mojo_base.gyp:mojo_application',
+        'mojo_base.gyp:mojo_application_standalone',
         'mojo_test_service_bindings',
         '<(mojo_system_for_loadable_module)',
       ],
@@ -707,7 +707,7 @@
             '../webkit/common/gpu/webkit_gpu.gyp:webkit_gpu',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_cc_support',
             'mojo_geometry_bindings',
             'mojo_geometry_lib',
@@ -797,7 +797,7 @@
             '../ui/gl/gl.gyp:gl',
             'mojo_base.gyp:mojo_system_impl',
             'mojo_base.gyp:mojo_environment_chromium',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_geometry_bindings',
             'mojo_geometry_lib',
             'mojo_input_events_bindings',
@@ -832,7 +832,7 @@
             '../ui/wm/wm.gyp:wm',
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_aura_support',
             'mojo_core_window_manager_bindings',
             'mojo_view_manager_lib',
@@ -903,7 +903,7 @@
             'mojo_base.gyp:mojo_common_lib',
             'mojo_base.gyp:mojo_environment_chromium',
             'mojo_base.gyp:mojo_system_impl',
-            'mojo_base.gyp:mojo_application',
+            'mojo_base.gyp:mojo_application_chromium',
             'mojo_dbus_service',
             'mojo_echo_bindings',
           ],

mojo/public/BUILD.gn

@@ -5,7 +5,7 @@
 group("public") {
   deps = [
     "//mojo/public/c/system",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:standalone",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/environment:standalone",
     "//mojo/public/cpp/utility",

mojo/public/cpp/application/BUILD.gn

@@ -2,7 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-source_set("application") {
+source_set("base") {
   sources = [
     "application_impl.h",
     "connect.h",
@@ -21,12 +21,28 @@ source_set("application") {
   ]
 }
 
+source_set("chromium") {
+  sources = [
+    "lib/application_impl_chromium.cc"
+  ]
+
+  deps = [ ":base" ]
+}
+
+source_set("standalone") {
+  sources = [
+    "lib/application_impl_standalone.cc"
+  ]
+
+  deps = [ ":base" ]
+}
+
 source_set("main_chromium") {
   sources = [
     "lib/mojo_main_chromium.cc"
   ]
 
-  deps = [ ":application" ]
+  deps = [ ":chromium" ]
 }
 
 source_set("main_standalone") {
@@ -34,5 +50,5 @@ source_set("main_standalone") {
     "lib/mojo_main_standalone.cc"
   ]
 
-  deps = [ ":application" ]
+  deps = [ ":standalone" ]
 }

mojo/public/cpp/application/DEPS

@@ -4,11 +4,13 @@ include_rules = [
 ]
 
 specific_include_rules = {
-  "mojo_main_chromium.cc": [
+  r"(mojo_main_chromium.cc|"
+  r"application_impl_chromium.cc)": [
     "+base",
     "+mojo/public/cpp"
   ],
-  "mojo_main_standalone.cc": [
+  r"(mojo_main_standalone.cc|"
+  r"application_impl_standalone.cc)": [
     "+mojo/public/cpp"
   ],
 }

mojo/public/cpp/application/application_impl.h

@@ -59,7 +59,7 @@ class ApplicationDelegate;
 //   BarImpl(ApplicationContext* app_context, BarContext* service_context)
 //          : app_context_(app_context), servicecontext_(context) {}
 //
-// Create an ApplicationDele instance that collects any service implementations.
+// Create an ApplicationImpl instance that collects any service implementations.
 //
 // ApplicationImpl app(service_provider_handle);
 // app.AddService<FooImpl>();
@@ -89,10 +89,25 @@ class ApplicationImpl : public InterfaceImpl<Application> {
   }
 
  private:
+  class ShellPtrWatcher : public ErrorHandler {
+   public:
+    explicit ShellPtrWatcher(ApplicationImpl* impl);
+    virtual ~ShellPtrWatcher();
+    virtual void OnConnectionError() MOJO_OVERRIDE;
+   private:
+    ApplicationImpl* impl_;
+    MOJO_DISALLOW_COPY_AND_ASSIGN(ShellPtrWatcher);
+  };
+
   friend MojoResult (::MojoMain)(MojoHandle);
 
   void BindShell(ScopedMessagePipeHandle shell_handle);
   void BindShell(MojoHandle shell_handle);
+  void ClearConnections();
+  void OnShellError() { ClearConnections(); Terminate(); };
+
+  // Quits the main run loop for this application.
+  void Terminate();
 
   // Application implementation.
   virtual void AcceptConnection(const String& requestor_url,
@@ -103,6 +118,7 @@ class ApplicationImpl : public InterfaceImpl<Application> {
   ServiceRegistryList outgoing_service_registries_;
   ApplicationDelegate* delegate_;
   ShellPtr shell_;
+  ShellPtrWatcher shell_watch_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(ApplicationImpl);
 };

mojo/public/cpp/application/lib/application_impl.cc

@@ -10,28 +10,43 @@
 
 namespace mojo {
 
+ApplicationImpl::ShellPtrWatcher::ShellPtrWatcher(ApplicationImpl* impl)
+    : impl_(impl) {}
+
+ApplicationImpl::ShellPtrWatcher::~ShellPtrWatcher() {}
+
+void ApplicationImpl::ShellPtrWatcher::OnConnectionError() {
+  impl_->OnShellError();
+}
+
 ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate)
-    : delegate_(delegate) {}
+    : delegate_(delegate), shell_watch_(this) {}
 
 ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate,
                                  ScopedMessagePipeHandle shell_handle)
-    : delegate_(delegate) {
+    : delegate_(delegate), shell_watch_(this) {
   BindShell(shell_handle.Pass());
 }
 
 ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate,
                                  MojoHandle shell_handle)
-    : delegate_(delegate) {
+    : delegate_(delegate), shell_watch_(this) {
   BindShell(shell_handle);
 }
 
-ApplicationImpl::~ApplicationImpl() {
+void ApplicationImpl::ClearConnections() {
   for (ServiceRegistryList::iterator i(incoming_service_registries_.begin());
       i != incoming_service_registries_.end(); ++i)
     delete *i;
   for (ServiceRegistryList::iterator i(outgoing_service_registries_.begin());
       i != outgoing_service_registries_.end(); ++i)
     delete *i;
+  incoming_service_registries_.clear();
+  outgoing_service_registries_.clear();
+}
+
+ApplicationImpl::~ApplicationImpl() {
+  ClearConnections();
 }
 
 ApplicationConnection* ApplicationImpl::ConnectToApplication(
@@ -53,6 +68,7 @@ ApplicationConnection* ApplicationImpl::ConnectToApplication(
 void ApplicationImpl::BindShell(ScopedMessagePipeHandle shell_handle) {
   shell_.Bind(shell_handle.Pass());
   shell_.set_client(this);
+  shell_.set_error_handler(&shell_watch_);
   delegate_->Initialize(this);
 }
 

mojo/public/cpp/application/lib/application_impl_chromium.cc

+// 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.
+
+#include "base/message_loop/message_loop.h"
+#include "mojo/public/cpp/application/application_impl.h"
+
+namespace mojo {
+
+void ApplicationImpl::Terminate() {
+  if (base::MessageLoop::current()->is_running())
+    base::MessageLoop::current()->Quit();
+}
+
+}  // namespace mojo

mojo/public/cpp/application/lib/application_impl_standalone.cc

+// 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.
+
+#include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/utility/run_loop.h"
+
+namespace mojo {
+
+void ApplicationImpl::Terminate() {
+  mojo::RunLoop::current()->Quit();
+}
+
+}  // namespace mojo

mojo/service_manager/background_shell_service_loader.cc

@@ -35,8 +35,7 @@ BackgroundShellServiceLoader::BackgroundShellServiceLoader(
     scoped_ptr<ServiceLoader> real_loader,
     const std::string& thread_name,
     base::MessageLoop::Type message_loop_type)
-    : quit_on_shutdown_(false),
-      loader_(real_loader.Pass()),
+    : loader_(real_loader.Pass()),
       message_loop_type_(message_loop_type),
       thread_name_(thread_name),
       message_loop_created_(true, false),
@@ -44,11 +43,8 @@ BackgroundShellServiceLoader::BackgroundShellServiceLoader(
 }
 
 BackgroundShellServiceLoader::~BackgroundShellServiceLoader() {
-  if (thread_) {
-    if (quit_on_shutdown_)
-      task_runner_->PostTask(FROM_HERE, quit_closure_);
+  if (thread_)
     thread_->Join();
-  }
 }
 
 void BackgroundShellServiceLoader::LoadService(

mojo/service_manager/background_shell_service_loader.h

@@ -32,12 +32,6 @@ class MOJO_SERVICE_MANAGER_EXPORT BackgroundShellServiceLoader :
   virtual void OnServiceError(ServiceManager* manager,
                               const GURL& url) OVERRIDE;
 
-  // Post a task to quit the MessageLoop we are running for the app when we
-  // are destroyed. Most apps should quit themselves and not need this.
-  // TODO(tim): Remove this method once applications are smart enough
-  // to quit themselves. Bug 394477.
-  void set_quit_on_shutdown() { quit_on_shutdown_ = true; }
-
  private:
   class BackgroundLoader;
 

mojo/service_manager/background_shell_service_loader_unittest.cc

@@ -52,19 +52,4 @@ TEST(BackgroundShellServiceLoaderTest, Load) {
   loader.LoadService(NULL, GURL(), dummy.handle0.Pass());
 }
 
-// Test that an app that doesn't quit itself can still be handled.
-// TODO(tim): Remove this.
-TEST(BackgroundShellServiceLoaderTest, LoadMisbehavedService) {
-  scoped_ptr<DummyLoader> real_loader(new DummyLoader());
-  real_loader->DontSimulateAppQuit();
-  BackgroundShellServiceLoader loader(
-      real_loader.PassAs<ServiceLoader>(), "test",
-      base::MessageLoop::TYPE_DEFAULT);
-  // Because this app is mis-behaved (doesn't quit itself), we need to
-  // explicitly kill the thread.
-  loader.set_quit_on_shutdown();
-  MessagePipe dummy;
-  loader.LoadService(NULL, GURL(), dummy.handle0.Pass());
-}
-
 }  // namespace mojo

mojo/service_manager/service_manager_unittest.cc

@@ -618,14 +618,7 @@ TEST_F(ServiceManagerTest, ANoLoadB) {
 }
 
 TEST_F(ServiceManagerTest, NoServiceNoLoad) {
-  // Because we'll never successfully connect to anything here and apps are not
-  // capable of noticing zero incoming connections and quitting, we need to use
-  // a quittable loader.
-  scoped_ptr<BackgroundShellServiceLoader> loader(MakeLoader(std::string()));
-  loader->set_quit_on_shutdown();
-  service_manager_->SetLoaderForURL(loader.PassAs<ServiceLoader>(),
-                                    GURL(kTestAURLString));
-
+  AddLoaderForURL(GURL(kTestAURLString), std::string());
 
   // There is no TestC service implementation registered with ServiceManager,
   // so this cannot succeed (but also shouldn't crash).

mojo/services/native_viewport/BUILD.gn

@@ -12,7 +12,7 @@ component("native_viewport") {
     "//ui/events",
     "//ui/gfx",
     "//ui/gfx/geometry",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:chromium",
     "//mojo/common",
     "//mojo/environment:chromium",
     "//mojo/services/public/cpp/geometry",

mojo/services/network/BUILD.gn

@@ -7,7 +7,7 @@ source_set("network") {
     "//base",
     "//mojo/common",
     "//mojo/environment:chromium",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:chromium",
     "//mojo/services/public/interfaces/network",
     "//net",
     "//url",

mojo/services/public/cpp/view_manager/BUILD.gn

@@ -6,7 +6,7 @@ source_set("view_manager") {
   deps = [
     ":common",
     "//base",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:chromium",
     "//mojo/public/interfaces/application",
     "//mojo/services/public/cpp/geometry",
     "//mojo/services/public/interfaces/geometry",

mojo/services/view_manager/BUILD.gn

@@ -9,7 +9,7 @@ component("view_manager") {
     "//mojo/cc",
     "//mojo/common",
     "//mojo/environment:chromium",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:chromium",
     "//mojo/public/cpp/application:main_chromium",
     "//mojo/public/gles2",
     "//mojo/services/public/cpp/geometry",

mojo/shell/BUILD.gn

@@ -35,7 +35,7 @@ source_set("lib") {
     "//base:base_static",
     "//mojo/common",
     "//mojo/gles2",
-    "//mojo/public/cpp/application",
+    "//mojo/public/cpp/application:chromium",
     "//mojo/public/interfaces/application",
     "//mojo/service_manager",
     "//mojo/services/native_viewport",

mojo/shell/context.cc

@@ -122,8 +122,6 @@ void Context::Init() {
             scoped_ptr<ServiceLoader>(new NativeViewportServiceLoader()),
             "native_viewport",
              base::MessageLoop::TYPE_UI));
-    // TODO(tim): NativeViewportService doesn't quit itself yet.
-    loader->set_quit_on_shutdown();
     service_manager_.SetLoaderForURL(
         loader.PassAs<ServiceLoader>(),
         GURL("mojo:mojo_native_viewport_service"));
@@ -156,30 +154,14 @@ void Context::Init() {
             scoped_ptr<ServiceLoader>(new NetworkServiceLoader()),
             "network_service",
              base::MessageLoop::TYPE_IO));
-    // TODO(tim): NetworkService doesn't quit itself yet.
-    loader->set_quit_on_shutdown();
     service_manager_.SetLoaderForURL(loader.PassAs<ServiceLoader>(),
                                      GURL("mojo:mojo_network_service"));
   }
 #endif
 }
 
-void Context::Shutdown() {
-  // mojo_view_manager uses native_viewport. Destroy mojo_view_manager first so
-  // that there aren't shutdown ordering issues. Once native viewport service is
-  // moved into its own process this can likely be nuked.
-#if defined(USE_AURA)
-  service_manager_.SetLoaderForURL(
-      scoped_ptr<ServiceLoader>(),
-      GURL("mojo:mojo_view_manager"));
-#endif
-  service_manager_.set_default_loader(scoped_ptr<ServiceLoader>());
-  service_manager_.TerminateShellConnections();
-}
-
 Context::~Context() {
   DCHECK(!base::MessageLoop::current());
-  Shutdown();
 }
 
 }  // namespace shell

mojo/shell/context.h

@@ -31,9 +31,6 @@ class Context {
   ~Context();
 
   void Init();
-  // Tear down context in safe order prior to deleting. Shutdown() is also
-  // called by the destructor.
-  void Shutdown();
 
   TaskRunners* task_runners() { return task_runners_.get(); }
   ServiceManager* service_manager() { return &service_manager_; }

mojo/shell/shell_test_base_unittest.cc

@@ -135,7 +135,8 @@ TEST_F(ShellTestBaseTest, ConnectInvalidService) {
 
 // Tests that we can connect to a single service within a single app using
 // a network based loader instead of local files.
-// TODO(tim): Bug 394477. NetworkService doesn't currently terminate.
+// TODO(tim): Disabled because network service leaks NSS at exit, meaning
+// subsequent tests can't init properly.
 TEST_F(ShellTestBaseTest, DISABLED_ConnectBasicNetwork) {
   InterfacePtr<TestService> service;
   service.Bind(ConnectToServiceViaNetwork(
@@ -152,13 +153,17 @@ TEST_F(ShellTestBaseTest, DISABLED_ConnectBasicNetwork) {
   // magically exit when TestService is destroyed (unlike ConnectBasic).
   // Tearing down the shell context will kill connections. The shell loop will
   // exit as soon as no more apps are connected.
-  shell_context()->Shutdown();
-  message_loop()->Run();
+  // TODO(tim): crbug.com/392685.  Calling this explicitly shouldn't be
+  // necessary once the shell terminates if the primordial app exits, which
+  // we could enforce here by resetting |service|.
+  shell_context()->service_manager()->TerminateShellConnections();
+  message_loop()->Run();  // Waits for all connections to die.
 }
 
 // Tests that trying to connect to a service over network fails preoprly
 // if the service doesn't exist.
-// TODO(tim): Bug 394477. NetworkService doesn't currently terminate.
+// TODO(tim): Disabled because network service leaks NSS at exit, meaning
+// subsequent tests can't init properly.
 TEST_F(ShellTestBaseTest, DISABLED_ConnectInvalidServiceNetwork) {
   InterfacePtr<TestService> test_service;
   test_service.Bind(ConnectToServiceViaNetwork(
@@ -170,8 +175,11 @@ TEST_F(ShellTestBaseTest, DISABLED_ConnectInvalidServiceNetwork) {
   message_loop()->Run();
   EXPECT_TRUE(test_service.encountered_error());
 
-  shell_context()->Shutdown();
-  message_loop()->Run();
+  // TODO(tim): crbug.com/392685.  Calling this explicitly shouldn't be
+  // necessary once the shell terminates if the primordial app exits, which
+  // we could enforce here by resetting |service|.
+  shell_context()->service_manager()->TerminateShellConnections();
+  message_loop()->Run();  // Waits for all connections to die.
 }
 
 // Similar to ConnectBasic, but causes the app to instantiate multiple