Get rid of KeepAlive. Quit shell when all urls run directly by Context are closed.

BUG=

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

Cr-Commit-Position: refs/heads/master@{#291723}

mojo/application_manager/application_loader.h

@@ -75,6 +75,7 @@ class MOJO_APPLICATION_MANAGER_EXPORT ApplicationLoader {
                     const GURL& url,
                     scoped_refptr<LoadCallbacks> callbacks) = 0;
 
+  // TODO(davemoore): Change name to OnApplicationError.
   virtual void OnServiceError(ApplicationManager* manager, const GURL& url) = 0;
 
  protected:

mojo/application_manager/application_manager.cc

@@ -36,6 +36,8 @@ class StubServiceProvider : public InterfaceImpl<ServiceProvider> {
 
 }  // namespace
 
+ApplicationManager::Delegate::~Delegate() {}
+
 class ApplicationManager::LoadCallbacksImpl
     : public ApplicationLoader::LoadCallbacks {
  public:
@@ -148,7 +150,9 @@ bool ApplicationManager::TestAPI::HasFactoryForURL(const GURL& url) const {
 }
 
 ApplicationManager::ApplicationManager()
-    : interceptor_(NULL), weak_ptr_factory_(this) {
+    : delegate_(NULL),
+      interceptor_(NULL),
+      weak_ptr_factory_(this) {
 }
 
 ApplicationManager::~ApplicationManager() {
@@ -285,6 +289,8 @@ void ApplicationManager::OnShellImplError(ShellImpl* shell_impl) {
   ApplicationLoader* loader = GetLoaderForURL(url);
   if (loader)
     loader->OnServiceError(this, url);
+  if (delegate_)
+    delegate_->OnApplicationError(url);
 }
 
 ScopedMessagePipeHandle ApplicationManager::ConnectToServiceByName(

mojo/application_manager/application_manager.h

@@ -20,6 +20,14 @@ namespace mojo {
 
 class MOJO_APPLICATION_MANAGER_EXPORT ApplicationManager {
  public:
+  class MOJO_APPLICATION_MANAGER_EXPORT Delegate {
+   public:
+    virtual ~Delegate();
+    // Send when the Applicaiton holding the handle on the other end of the
+    // Shell pipe goes away.
+    virtual void OnApplicationError(const GURL& url) = 0;
+  };
+
   // API for testing.
   class MOJO_APPLICATION_MANAGER_EXPORT TestAPI {
    public:
@@ -53,6 +61,8 @@ class MOJO_APPLICATION_MANAGER_EXPORT ApplicationManager {
   // Returns a shared instance, creating it if necessary.
   static ApplicationManager* GetInstance();
 
+  void SetDelegate(Delegate* delegate) { delegate_ = delegate; }
+
   // Loads a service if necessary and establishes a new client connection.
   void ConnectToApplication(const GURL& application_url,
                             const GURL& requestor_url,
@@ -70,6 +80,8 @@ class MOJO_APPLICATION_MANAGER_EXPORT ApplicationManager {
       const GURL& application_url,
       const std::string& interface_name);
 
+  void set_delegate(Delegate* delegate) { delegate_ = delegate; }
+
   // Sets the default Loader to be used if not overridden by SetLoaderForURL()
   // or SetLoaderForScheme().
   void set_default_loader(scoped_ptr<ApplicationLoader> loader) {
@@ -122,6 +134,7 @@ class MOJO_APPLICATION_MANAGER_EXPORT ApplicationManager {
   // Removes a ShellImpl when it encounters an error.
   void OnShellImplError(ShellImpl* shell_impl);
 
+  Delegate* delegate_;
   // Loader management.
   URLToLoaderMap url_to_loader_;
   SchemeToLoaderMap scheme_to_loader_;

mojo/mojo.gyp

@@ -179,14 +179,10 @@
         'shell/init.h',
         'shell/in_process_dynamic_service_runner.cc',
         'shell/in_process_dynamic_service_runner.h',
-        'shell/keep_alive.cc',
-        'shell/keep_alive.h',
         'shell/mojo_url_resolver.cc',
         'shell/mojo_url_resolver.h',
         'shell/out_of_process_dynamic_service_runner.cc',
         'shell/out_of_process_dynamic_service_runner.h',
-        'shell/run.cc',
-        'shell/run.h',
         'shell/switches.cc',
         'shell/switches.h',
         'shell/task_runners.cc',

mojo/public/cpp/application/application_impl.h

@@ -82,7 +82,10 @@ class ApplicationImpl : public InterfaceImpl<Application> {
 
   void BindShell(ScopedMessagePipeHandle shell_handle);
   void ClearConnections();
-  void OnShellError() { ClearConnections(); Terminate(); };
+  void OnShellError() {
+    ClearConnections();
+    Terminate();
+  };
 
   // Quits the main run loop for this application.
   static void Terminate();

mojo/shell/BUILD.gn

@@ -69,14 +69,10 @@ source_set("lib") {
     "init.h",
     "in_process_dynamic_service_runner.cc",
     "in_process_dynamic_service_runner.h",
-    "keep_alive.cc",
-    "keep_alive.h",
     "mojo_url_resolver.cc",
     "mojo_url_resolver.h",
     "out_of_process_dynamic_service_runner.cc",
     "out_of_process_dynamic_service_runner.h",
-    "run.cc",
-    "run.h",
     "switches.cc",
     "switches.h",
     "task_runners.cc",

mojo/shell/android/mojo_main.cc

@@ -18,7 +18,6 @@
 #include "mojo/application_manager/application_manager.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/init.h"
-#include "mojo/shell/run.h"
 #include "ui/gl/gl_surface_egl.h"
 
 using base::LazyInstance;
@@ -37,9 +36,13 @@ LazyInstance<scoped_ptr<base::android::JavaHandlerThread> > g_shell_thread =
     LAZY_INSTANCE_INITIALIZER;
 
 void RunShell(std::vector<GURL> app_urls) {
-  g_context.Get()->Init();
-  g_context.Get()->set_ui_loop(g_java_message_loop.Get().get());
-  shell::Run(g_context.Get().get(), app_urls);
+  shell::Context* context = g_context.Pointer()->get();
+  context->Init();
+  context->set_ui_loop(g_java_message_loop.Get().get());
+  for (std::vector<GURL>::const_iterator it = app_urls.begin();
+       it != app_urls.end(); ++it) {
+    context->Run(*it);
+  }
 }
 
 }  // namespace

mojo/shell/context.cc

@@ -96,6 +96,14 @@ void InitContentHandlers(DynamicApplicationLoader* loader,
   }
 }
 
+class EmptyServiceProvider : public InterfaceImpl<ServiceProvider> {
+ private:
+  virtual void ConnectToService(const mojo::String& service_name,
+                                ScopedMessagePipeHandle client_handle)
+      MOJO_OVERRIDE {
+  }
+};
+
 }  // namespace
 
 class Context::NativeViewportApplicationLoader
@@ -153,7 +161,12 @@ Context::Context() {
   DCHECK(!base::MessageLoop::current());
 }
 
+Context::~Context() {
+  DCHECK(!base::MessageLoop::current());
+}
+
 void Context::Init() {
+  application_manager_.set_delegate(this);
   setup.Get();
   task_runners_.reset(
       new TaskRunners(base::MessageLoop::current()->message_loop_proxy()));
@@ -229,8 +242,29 @@ void Context::Init() {
 #endif
 }
 
-Context::~Context() {
-  DCHECK(!base::MessageLoop::current());
+void Context::OnApplicationError(const GURL& gurl) {
+  if (app_urls_.find(gurl) != app_urls_.end()) {
+    app_urls_.erase(gurl);
+    if (app_urls_.empty() && base::MessageLoop::current()->is_running())
+      base::MessageLoop::current()->Quit();
+  }
+}
+
+void Context::Run(const GURL& url) {
+  EmptyServiceProvider* sp = new EmptyServiceProvider;
+  ServiceProviderPtr spp;
+  BindToProxy(sp, &spp);
+
+  app_urls_.insert(url);
+  application_manager_.ConnectToApplication(url, GURL(), spp.Pass());
+}
+
+ScopedMessagePipeHandle Context::ConnectToServiceByName(
+    const GURL& application_url,
+    const std::string& service_name) {
+  app_urls_.insert(application_url);
+  return application_manager_.ConnectToServiceByName(
+      application_url, service_name).Pass();
 }
 
 }  // namespace shell

mojo/shell/context.h

@@ -8,7 +8,6 @@
 #include <string>
 
 #include "mojo/application_manager/application_manager.h"
-#include "mojo/shell/keep_alive.h"
 #include "mojo/shell/mojo_url_resolver.h"
 #include "mojo/shell/task_runners.h"
 
@@ -25,16 +24,23 @@ namespace shell {
 class DynamicApplicationLoader;
 
 // The "global" context for the shell's main process.
-class Context {
+class Context : ApplicationManager::Delegate {
  public:
   Context();
-  ~Context();
+  virtual ~Context();
 
   void Init();
 
+  // ApplicationManager::Delegate override.
+  virtual void OnApplicationError(const GURL& gurl) OVERRIDE;
+
+  void Run(const GURL& url);
+  ScopedMessagePipeHandle ConnectToServiceByName(
+      const GURL& application_url,
+      const std::string& service_name);
+
   TaskRunners* task_runners() { return task_runners_.get(); }
   ApplicationManager* application_manager() { return &application_manager_; }
-  KeepAliveCounter* keep_alive_counter() { return &keep_alive_counter_; }
   MojoURLResolver* mojo_url_resolver() { return &mojo_url_resolver_; }
 
 #if defined(OS_ANDROID)
@@ -45,6 +51,7 @@ class Context {
  private:
   class NativeViewportApplicationLoader;
 
+  std::set<GURL> app_urls_;
   scoped_ptr<TaskRunners> task_runners_;
   ApplicationManager application_manager_;
   MojoURLResolver mojo_url_resolver_;
@@ -53,8 +60,6 @@ class Context {
   base::MessageLoop* ui_loop_;
 #endif  // defined(OS_ANDROID)
 
-  KeepAliveCounter keep_alive_counter_;
-
   DISALLOW_COPY_AND_ASSIGN(Context);
 };
 

mojo/shell/dbus_application_loader_linux.cc

@@ -21,7 +21,6 @@
 #include "mojo/embedder/platform_channel_pair.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/external_service.mojom.h"
-#include "mojo/shell/keep_alive.h"
 
 namespace mojo {
 namespace shell {
@@ -41,8 +40,7 @@ class DBusApplicationLoader::LoadContext {
         bus_(bus),
         service_dbus_proxy_(NULL),
         url_(url),
-        service_provider_handle_(service_provider_handle.Pass()),
-        keep_alive_(loader->context_) {
+        service_provider_handle_(service_provider_handle.Pass()) {
     base::PostTaskAndReplyWithResult(
         loader_->context_->task_runners()->io_runner(),
         FROM_HERE,
@@ -131,7 +129,6 @@ class DBusApplicationLoader::LoadContext {
   dbus::ObjectProxy* service_dbus_proxy_;  // Owned by bus_;
   const GURL url_;
   ScopedMessagePipeHandle service_provider_handle_;
-  KeepAlive keep_alive_;
   scoped_ptr<embedder::ChannelInit> channel_init_;
   ExternalServicePtr external_service_;
 

mojo/shell/dbus_application_loader_linux.h

@@ -11,7 +11,6 @@
 #include "base/memory/ref_counted.h"
 #include "mojo/application_manager/application_loader.h"
 #include "mojo/public/cpp/system/core.h"
-#include "mojo/shell/keep_alive.h"
 #include "url/gurl.h"
 
 namespace dbus {

mojo/shell/desktop/mojo_main.cc

@@ -10,10 +10,20 @@
 #include "mojo/shell/child_process.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/init.h"
-#include "mojo/shell/run.h"
 #include "mojo/shell/switches.h"
 #include "ui/gl/gl_surface.h"
 
+namespace {
+
+void RunApps(mojo::shell::Context* context, std::vector<GURL> app_urls) {
+  for (std::vector<GURL>::const_iterator it = app_urls.begin();
+       it != app_urls.end(); ++it) {
+    context->Run(*it);
+  }
+}
+
+}  // namespace
+
 int main(int argc, char** argv) {
   base::AtExitManager at_exit;
   base::CommandLine::Init(argc, argv);
@@ -49,7 +59,7 @@ int main(int argc, char** argv) {
         app_urls.push_back(GURL(*it));
 
       message_loop.PostTask(FROM_HERE,
-                            base::Bind(mojo::shell::Run,
+                            base::Bind(RunApps,
                                        &shell_context,
                                        app_urls));
       message_loop.Run();

mojo/shell/dynamic_application_loader.cc

@@ -14,24 +14,12 @@
 #include "mojo/common/data_pipe_utils.h"
 #include "mojo/services/public/interfaces/network/url_loader.mojom.h"
 #include "mojo/shell/context.h"
-#include "mojo/shell/keep_alive.h"
 #include "mojo/shell/switches.h"
 #include "net/base/filename_util.h"
 
 namespace mojo {
 namespace shell {
 
-namespace {
-
-void RunLibraryComplete(DynamicServiceRunner* runner,
-                        const base::FilePath& temp_file) {
-  delete runner;
-  if (!temp_file.empty())
-    base::DeleteFile(temp_file, false);
-}
-
-}  // namespace
-
 DynamicApplicationLoader::DynamicApplicationLoader(
     Context* context,
     scoped_ptr<DynamicServiceRunnerFactory> runner_factory)
@@ -143,28 +131,40 @@ void DynamicApplicationLoader::RunLibrary(
     scoped_refptr<LoadCallbacks> callbacks,
     bool delete_file_after,
     bool path_exists) {
-  // TODO(aa): We need to create a runner, even if we're not going to use it,
-  // because it getting destroyed is what causes shell to shut down. If we don't
-  // create this, in the case where shell never successfully creates even one
-  // app, then shell will never shut down, because no runners are ever
-  // destroyed.
-  scoped_ptr<DynamicServiceRunner> runner(runner_factory_->Create(context_));
+
+  ScopedMessagePipeHandle shell_handle = callbacks->RegisterApplication();
+  if (!shell_handle.is_valid())
+    return;
+
   if (!path_exists) {
     DVLOG(1) << "Library not started because library path '"
              << path.value() << "' does not exist.";
     return;
   }
 
-  ScopedMessagePipeHandle shell_handle = callbacks->RegisterApplication();
-  if (!shell_handle.is_valid())
-    return;
+  scoped_ptr<DynamicServiceRunner> runner =
+      runner_factory_->Create(context_).Pass();
+  runner->Start(path,
+                shell_handle.Pass(),
+                base::Bind(&DynamicApplicationLoader::OnRunLibraryComplete,
+                           weak_ptr_factory_.GetWeakPtr(),
+                           base::Unretained(runner.get()),
+                           delete_file_after ? path : base::FilePath()));
+  runners_.push_back(runner.release());
+}
 
-  DynamicServiceRunner* runner_raw = runner.release();
-  runner_raw->Start(path,
-                    shell_handle.Pass(),
-                    base::Bind(&RunLibraryComplete,
-                               base::Unretained(runner_raw),
-                               delete_file_after ? path : base::FilePath()));
+void DynamicApplicationLoader::OnRunLibraryComplete(
+    DynamicServiceRunner* runner,
+    const base::FilePath& temp_file) {
+  for (ScopedVector<DynamicServiceRunner>::iterator it = runners_.begin();
+       it != runners_.end(); ++it) {
+    if (*it == runner) {
+      runners_.erase(it);
+      if (!temp_file.empty())
+        base::DeleteFile(temp_file, false);
+      return;
+    }
+  }
 }
 
 void DynamicApplicationLoader::OnServiceError(ApplicationManager* manager,

mojo/shell/dynamic_application_loader.h

@@ -8,12 +8,12 @@
 #include <map>
 
 #include "base/macros.h"
+#include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "mojo/application_manager/application_loader.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/services/public/interfaces/network/network_service.mojom.h"
 #include "mojo/shell/dynamic_service_runner.h"
-#include "mojo/shell/keep_alive.h"
 #include "url/gurl.h"
 
 namespace mojo {
@@ -21,6 +21,7 @@ namespace shell {
 
 class Context;
 class DynamicServiceRunnerFactory;
+class DynamicServiceRunner;
 
 // An implementation of ApplicationLoader that retrieves a dynamic library
 // containing the implementation of the service and loads/runs it (via a
@@ -55,9 +56,12 @@ class DynamicApplicationLoader : public ApplicationLoader {
                   scoped_refptr<LoadCallbacks> callbacks,
                   bool delete_file_after,
                   bool response_path_exists);
+  void OnRunLibraryComplete(DynamicServiceRunner* runner,
+                            const base::FilePath& temp_file);
 
   Context* const context_;
   scoped_ptr<DynamicServiceRunnerFactory> runner_factory_;
+  ScopedVector<DynamicServiceRunner> runners_;
   NetworkServicePtr network_service_;
   URLLoaderPtr url_loader_;
   MimeTypeToURLMap mime_type_to_url_;

mojo/shell/dynamic_application_loader_unittest.cc

@@ -15,14 +15,21 @@ namespace shell {
 namespace {
 
 struct TestState {
-  TestState() : runner_was_started(false), runner_was_destroyed(false) {}
+  TestState()
+      : runner_was_created(false),
+        runner_was_started(false),
+        runner_was_destroyed(false) {}
+
+  bool runner_was_created;
   bool runner_was_started;
   bool runner_was_destroyed;
 };
 
 class TestDynamicServiceRunner : public DynamicServiceRunner {
  public:
-  explicit TestDynamicServiceRunner(TestState* state) : state_(state) {}
+  explicit TestDynamicServiceRunner(TestState* state) : state_(state) {
+    state_->runner_was_created = true;
+  }
   virtual ~TestDynamicServiceRunner() {
     state_->runner_was_destroyed = true;
     base::MessageLoop::current()->Quit();
@@ -79,9 +86,9 @@ TEST_F(DynamicApplicationLoaderTest, DoesNotExist) {
   scoped_refptr<ApplicationLoader::SimpleLoadCallbacks> callbacks(
       new ApplicationLoader::SimpleLoadCallbacks(pipe.handle0.Pass()));
   loader_->Load(context_.application_manager(), url, callbacks);
-  loop_.Run();
+  EXPECT_FALSE(state_.runner_was_created);
   EXPECT_FALSE(state_.runner_was_started);
-  EXPECT_TRUE(state_.runner_was_destroyed);
+  EXPECT_FALSE(state_.runner_was_destroyed);
 }
 
 }  // namespace shell

mojo/shell/in_process_dynamic_service_runner.cc

@@ -41,8 +41,7 @@ bool SetThunks(Thunks (*make_thunks)(),
 }
 
 InProcessDynamicServiceRunner::InProcessDynamicServiceRunner(
-    Context* context)
-    : keep_alive_(context) {
+    Context* context) {
 }
 
 InProcessDynamicServiceRunner::~InProcessDynamicServiceRunner() {

mojo/shell/in_process_dynamic_service_runner.h

@@ -11,7 +11,6 @@
 #include "base/scoped_native_library.h"
 #include "base/threading/simple_thread.h"
 #include "mojo/shell/dynamic_service_runner.h"
-#include "mojo/shell/keep_alive.h"
 
 namespace mojo {
 namespace shell {
@@ -34,7 +33,6 @@ class InProcessDynamicServiceRunner
   // |base::DelegateSimpleThread::Delegate| method:
   virtual void Run() OVERRIDE;
 
-  KeepAlive keep_alive_;
   base::FilePath app_path_;
   ScopedMessagePipeHandle service_handle_;
   base::Callback<bool(void)> app_completed_callback_runner_;

mojo/shell/keep_alive.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/shell/keep_alive.h"
-
-#include "base/bind.h"
-#include "mojo/shell/context.h"
-
-namespace mojo {
-namespace shell {
-
-KeepAlive::KeepAlive(Context* context) : context_(context) {
-  DCHECK(context_->task_runners()->shell_runner()->RunsTasksOnCurrentThread());
-  ++context_->keep_alive_counter()->count_;
-}
-
-KeepAlive::~KeepAlive() {
-  DCHECK(context_->task_runners()->shell_runner()->RunsTasksOnCurrentThread());
-  if (--context_->keep_alive_counter()->count_ == 0) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&KeepAlive::MaybeQuit, context_));
-  }
-}
-
-// static
-void KeepAlive::MaybeQuit(Context* context) {
-  if (context->keep_alive_counter()->count_ == 0)
-    base::MessageLoop::current()->Quit();
-}
-
-}  // namespace shell
-}  // namespace mojo

mojo/shell/keep_alive.h

-// 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.
-
-#ifndef MOJO_SHELL_KEEP_ALIVE_H_
-#define MOJO_SHELL_KEEP_ALIVE_H_
-
-#include "base/basictypes.h"
-
-namespace mojo {
-namespace shell {
-
-class Context;
-class KeepAlive;
-
-class KeepAliveCounter {
- public:
-  KeepAliveCounter() : count_(0) {
-  }
- private:
-  friend class KeepAlive;
-  int count_;
-};
-
-// Instantiate this class to extend the lifetime of the thread associated
-// with |context| (i.e., the shell's UI thread). Must only be used from
-// the shell's UI thread.
-class KeepAlive {
- public:
-  explicit KeepAlive(Context* context);
-  ~KeepAlive();
-
- private:
-  static void MaybeQuit(Context* context);
-
-  Context* context_;
-
-  DISALLOW_COPY_AND_ASSIGN(KeepAlive);
-};
-
-}  // namespace shell
-}  // namespace mojo
-
-#endif  // MOJO_SHELL_KEEP_ALIVE_H_

mojo/shell/out_of_process_dynamic_service_runner.cc

@@ -15,8 +15,7 @@ namespace shell {
 
 OutOfProcessDynamicServiceRunner::OutOfProcessDynamicServiceRunner(
     Context* context)
-    : context_(context),
-      keep_alive_(context) {
+    : context_(context) {
 }
 
 OutOfProcessDynamicServiceRunner::~OutOfProcessDynamicServiceRunner() {

mojo/shell/out_of_process_dynamic_service_runner.h

@@ -11,7 +11,6 @@
 #include "mojo/shell/app_child_process.mojom.h"
 #include "mojo/shell/app_child_process_host.h"
 #include "mojo/shell/dynamic_service_runner.h"
-#include "mojo/shell/keep_alive.h"
 
 namespace mojo {
 namespace shell {
@@ -36,7 +35,6 @@ class OutOfProcessDynamicServiceRunner
 
   Context* const context_;
 
-  KeepAlive keep_alive_;
   base::FilePath app_path_;
   ScopedMessagePipeHandle service_handle_;
   base::Closure app_completed_callback_;

mojo/shell/run.cc

@@ -7,7 +7,6 @@
 #include "base/logging.h"
 #include "mojo/application_manager/application_manager.h"
 #include "mojo/shell/context.h"
-#include "mojo/shell/keep_alive.h"
 
 namespace mojo {
 namespace shell {
@@ -22,8 +21,6 @@ class StubServiceProvider : public InterfaceImpl<ServiceProvider> {
 
 
 void Run(Context* context, const std::vector<GURL>& app_urls) {
-  KeepAlive keep_alive(context);
-
   if (app_urls.empty()) {
     LOG(ERROR) << "No app path specified";
     return;

mojo/shell/shell_test_base.cc

@@ -33,17 +33,6 @@ void ShellTestBase::SetUp() {
   test_server_->ServeFilesFromDirectory(service_dir);
 }
 
-ScopedMessagePipeHandle ShellTestBase::ConnectToServiceViaNetwork(
-    const GURL& application_url,
-    const std::string& service_name) {
-  shell_context_.mojo_url_resolver()->SetBaseURL(
-      test_server_->base_url());
-
-  return shell_context_.application_manager()
-      ->ConnectToServiceByName(application_url, service_name)
-      .Pass();
-}
-
 ScopedMessagePipeHandle ShellTestBase::ConnectToService(
     const GURL& application_url,
     const std::string& service_name) {
@@ -56,9 +45,18 @@ ScopedMessagePipeHandle ShellTestBase::ConnectToService(
   shell_context_.mojo_url_resolver()->SetBaseURL(
       net::FilePathToFileURL(service_dir));
 
-  return shell_context_.application_manager()
-      ->ConnectToServiceByName(application_url, service_name)
-      .Pass();
+  return shell_context_.ConnectToServiceByName(
+      application_url, service_name).Pass();
+}
+
+ScopedMessagePipeHandle ShellTestBase::ConnectToServiceViaNetwork(
+    const GURL& application_url,
+    const std::string& service_name) {
+  shell_context_.mojo_url_resolver()->SetBaseURL(
+      test_server_->base_url());
+
+  return shell_context_.ConnectToServiceByName(
+      application_url, service_name).Pass();
 }
 
 }  // namespace test

mojo/shell/shell_test_base.h

@@ -43,6 +43,19 @@ class ShellTestBase : public testing::Test {
       const GURL& application_url,
       const std::string& service_name);
 
+  template <typename Interface>
+  void ConnectToService(const GURL& application_url,
+                        InterfacePtr<Interface>* ptr) {
+    ptr->Bind(ConnectToService(application_url, Interface::Name_).Pass());
+  }
+
+  template <typename Interface>
+  void ConnectToServiceViaNetwork(const GURL& application_url,
+                        InterfacePtr<Interface>* ptr) {
+    ptr->Bind(
+        ConnectToServiceViaNetwork(application_url, Interface::Name_).Pass());
+  }
+
   base::MessageLoop* message_loop() { return &message_loop_; }
   Context* shell_context() { return &shell_context_; }
 

mojo/shell/shell_test_base_unittest.cc

@@ -44,8 +44,8 @@ class ShellTestBaseTest : public ShellTestBase {
   // Convenience helpers for use as callbacks in tests.
   template <typename T>
   base::Callback<void()> SetAndQuit(T* val, T result) {
-      return base::Bind(&ShellTestBaseTest::SetAndQuitImpl<T>,
-          base::Unretained(this), val, result);
+    return base::Bind(&ShellTestBaseTest::SetAndQuitImpl<T>,
+        base::Unretained(this), val, result);
   }
   template <typename T>
   base::Callback<void(T result)> SetAndQuit(T* val) {
@@ -57,9 +57,8 @@ class ShellTestBaseTest : public ShellTestBase {
   }
 
   void GetReport(std::vector<ServiceReport>* report) {
-    request_tracking_.Bind(
-        ConnectToService(GURL("mojo:mojo_test_request_tracker_app"),
-        TestTrackedRequestService::Name_).Pass());
+    ConnectToService(GURL("mojo:mojo_test_request_tracker_app"),
+                     &request_tracking_);
     request_tracking_->GetReport(base::Bind(&GetReportCallback,
         base::Unretained(message_loop()),
         base::Unretained(report)));
@@ -92,7 +91,7 @@ class QuitMessageLoopErrorHandler : public ErrorHandler {
 // Tests that we can connect to a single service within a single app.
 TEST_F(ShellTestBaseTest, ConnectBasic) {
   InterfacePtr<TestService> service;
-  service.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
 
   bool was_run = false;
   service->Ping(SetAndQuit<bool>(&was_run, true));
@@ -112,8 +111,7 @@ TEST_F(ShellTestBaseTest, ConnectBasic) {
 // terminates if no services are running.
 TEST_F(ShellTestBaseTest, ConnectInvalidService) {
   InterfacePtr<TestService> test_service;
-  test_service.Bind(ConnectToService(GURL("mojo:non_existent_service"),
-                                     TestService::Name_).Pass());
+  ConnectToService(GURL("mojo:non_existent_service"), &test_service);
 
   bool was_run = false;
   test_service->Ping(SetAndQuit<bool>(&was_run, true));
@@ -139,8 +137,7 @@ TEST_F(ShellTestBaseTest, ConnectInvalidService) {
 // subsequent tests can't init properly.
 TEST_F(ShellTestBaseTest, DISABLED_ConnectBasicNetwork) {
   InterfacePtr<TestService> service;
-  service.Bind(ConnectToServiceViaNetwork(
-      test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
 
   bool was_run = false;
   service->Ping(SetAndQuit<bool>(&was_run, true));
@@ -166,8 +163,7 @@ TEST_F(ShellTestBaseTest, DISABLED_ConnectBasicNetwork) {
 // subsequent tests can't init properly.
 TEST_F(ShellTestBaseTest, DISABLED_ConnectInvalidServiceNetwork) {
   InterfacePtr<TestService> test_service;
-  test_service.Bind(ConnectToServiceViaNetwork(
-      GURL("mojo:non_existent_service"), TestService::Name_).Pass());
+  ConnectToServiceViaNetwork(GURL("mojo:non_existent_service"), &test_service);
   QuitMessageLoopErrorHandler quitter;
   test_service.set_error_handler(&quitter);
   bool was_run = false;
@@ -187,8 +183,8 @@ TEST_F(ShellTestBaseTest, DISABLED_ConnectInvalidServiceNetwork) {
 TEST_F(ShellTestBaseTest, ConnectMultipleInstancesPerApp) {
   {
     TestServicePtr service1, service2;
-    service1.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
-    service2.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+    ConnectToService(test_app_url(), &service1);
+    ConnectToService(test_app_url(), &service2);
 
     bool was_run1 = false;
     bool was_run2 = false;
@@ -210,15 +206,14 @@ TEST_F(ShellTestBaseTest, ConnectDifferentServicesInSingleApp) {
   // Have a TestService GetPartyTime on a TestTimeService in the same app.
   int64 time_message;
   TestServicePtr service;
-  service.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
   service->ConnectToAppAndGetTime(test_app_url().spec(),
                                   SetAndQuit<int64>(&time_message));
   message_loop()->Run();
 
   // Verify by hitting the TimeService directly.
   TestTimeServicePtr time_service;
-  time_service.Bind(
-      ConnectToService(test_app_url(), TestTimeService::Name_).Pass());
+  ConnectToService(test_app_url(), &time_service);
   int64 party_time;
   time_service->GetPartyTime(SetAndQuit<int64>(&party_time));
   message_loop()->Run();
@@ -231,15 +226,14 @@ TEST_F(ShellTestBaseTest, ConnectDifferentServicesInSingleApp) {
 TEST_F(ShellTestBaseTest, ConnectDifferentServicesInDifferentApps) {
   int64 time_message;
   TestServicePtr service;
-  service.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
   service->ConnectToAppAndGetTime("mojo:mojo_test_request_tracker_app",
                                   SetAndQuit<int64>(&time_message));
   message_loop()->Run();
 
   // Verify by hitting the TimeService in the request tracker app directly.
   TestTimeServicePtr time_service;
-  time_service.Bind(ConnectToService(GURL("mojo:mojo_test_request_tracker_app"),
-                    TestTimeService::Name_).Pass());
+  ConnectToService(GURL("mojo:mojo_test_request_tracker_app"), &time_service);
   int64 party_time;
   time_service->GetPartyTime(SetAndQuit<int64>(&party_time));
   message_loop()->Run();
@@ -250,7 +244,7 @@ TEST_F(ShellTestBaseTest, ConnectDifferentServicesInDifferentApps) {
 // Tests that service A in App 1 can be a client of service B in App 2.
 TEST_F(ShellTestBaseTest, ConnectServiceAsClientOfSeparateApp) {
   TestServicePtr service;
-  service.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
   service->StartTrackingRequests(message_loop()->QuitWhenIdleClosure());
   service->Ping(mojo::Callback<void()>());
   message_loop()->Run();
@@ -278,7 +272,7 @@ TEST_F(ShellTestBaseTest, ConnectManyClientsAndServices) {
   // Make a request to the TestService and have it contact TimeService in the
   // tracking app. Do all this with tracking enabled, meaning both services
   // are connected as clients of the TrackedRequestService.
-  service.Bind(ConnectToService(test_app_url(), TestService::Name_).Pass());
+  ConnectToService(test_app_url(), &service);
   service->StartTrackingRequests(message_loop()->QuitWhenIdleClosure());
   message_loop()->Run();
   for (int i = 0; i < 5; i++)
@@ -289,8 +283,7 @@ TEST_F(ShellTestBaseTest, ConnectManyClientsAndServices) {
   message_loop()->Run();
 
   // Also make a few requests to the TimeService in the test_app.
-  time_service.Bind(
-      ConnectToService(test_app_url(), TestTimeService::Name_).Pass());
+  ConnectToService(test_app_url(), &time_service);
   time_service->StartTrackingRequests(message_loop()->QuitWhenIdleClosure());
   time_service->GetPartyTime(mojo::Callback<void(uint64_t)>());
   message_loop()->Run();