Mojo: Refactor the in-process-specific bits out of DynamicServiceLoader.

Put them into InProcessDynamicServiceRunner, which implements
DynamicServiceRunner.

(This basically makes DynamicServiceLoader only "load", and separates
the "running" part into a separate class.)

R=ben@chromium.org

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

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

6 files changed, 226 insertions, 64 deletions

diff --git a/mojo/shell/context.cc b/mojo/shell/context.cc
index 4b044fc..924c34d 100644
--- a/mojo/shell/context.cc
+++ b/mojo/shell/context.cc
@@ -6,6 +6,7 @@
 
 #include "mojo/gles2/gles2_support_impl.h"
 #include "mojo/shell/dynamic_service_loader.h"
+#include "mojo/shell/in_process_dynamic_service_runner.h"
 #include "mojo/shell/network_delegate.h"
 #include "mojo/system/embedder/embedder.h"
 
@@ -22,7 +23,11 @@ Context::Context()
               storage_.profile_path()) {
   embedder::Init();
   gles2::GLES2SupportImpl::Init();
-  dynamic_service_loader_.reset(new DynamicServiceLoader(this));
+
+  scoped_ptr<DynamicServiceRunnerFactory> runner_factory(
+      new InProcessDynamicServiceRunnerFactory());
+  dynamic_service_loader_.reset(
+      new DynamicServiceLoader(this, runner_factory.Pass()));
   service_manager_.set_default_loader(dynamic_service_loader_.get());
 }
 
diff --git a/mojo/shell/dynamic_service_loader.cc b/mojo/shell/dynamic_service_loader.cc
index 7d5e8a4..43766b7 100644
--- a/mojo/shell/dynamic_service_loader.cc
+++ b/mojo/shell/dynamic_service_loader.cc
@@ -4,19 +4,15 @@
 
 #include "mojo/shell/dynamic_service_loader.h"
 
-#include "base/callback_helpers.h"
 #include "base/command_line.h"
-#include "base/file_util.h"
-#include "base/scoped_native_library.h"
-#include "base/threading/simple_thread.h"
+#include "base/location.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/keep_alive.h"
 #include "mojo/shell/switches.h"
 
-typedef MojoResult (*MojoMainFunction)(MojoHandle pipe);
-
 namespace mojo {
 namespace shell {
+
 namespace {
 
 std::string MakeSharedLibraryName(const std::string& file_name) {
@@ -34,17 +30,16 @@ std::string MakeSharedLibraryName(const std::string& file_name) {
 
 }  // namespace
 
-class DynamicServiceLoader::LoadContext
-    : public mojo::shell::Loader::Delegate,
-      public base::DelegateSimpleThread::Delegate {
+class DynamicServiceLoader::LoadContext : public mojo::shell::Loader::Delegate {
  public:
   LoadContext(DynamicServiceLoader* loader,
               const GURL& url,
-              ScopedShellHandle service_handle)
-      : thread_(this, "app_thread"),
-        loader_(loader),
+              ScopedShellHandle service_handle,
+              scoped_ptr<DynamicServiceRunner> runner)
+      : loader_(loader),
         url_(url),
         service_handle_(service_handle.Pass()),
+        runner_(runner.Pass()),
         keep_alive_(loader->context_) {
     GURL url_to_load;
 
@@ -62,68 +57,48 @@ class DynamicServiceLoader::LoadContext
   }
 
   virtual ~LoadContext() {
-    thread_.Join();
   }
 
  private:
-  // From Loader::Delegate.
+  // |Loader::Delegate| method:
   virtual void DidCompleteLoad(const GURL& app_url,
                                const base::FilePath& app_path,
                                const std::string* mime_type) OVERRIDE {
-    app_path_ = app_path;
-    thread_.Start();
+    DVLOG(2) << "Completed load of " << app_url << " (" << url_ << ") to "
+             << app_path.value();
+
+    runner_->Start(
+        app_path,
+        service_handle_.Pass(),
+        base::Bind(&LoadContext::AppCompleted,
+                   scoped_refptr<base::TaskRunner>(
+                       loader_->context_->task_runners()->ui_runner()),
+                   base::Unretained(loader_), url_));
   }
 
-  // From base::DelegateSimpleThread::Delegate.
-  virtual void Run() OVERRIDE {
-    base::ScopedClosureRunner app_deleter(
-        base::Bind(base::IgnoreResult(&base::DeleteFile), app_path_, false));
-
-    do {
-      std::string load_error;
-      base::ScopedNativeLibrary app_library(
-          base::LoadNativeLibrary(app_path_, &load_error));
-      if (!app_library.is_valid()) {
-        LOG(ERROR) << "Failed to load library: " << app_path_.value() << " ("
-                   << url_.spec() << ")";
-        LOG(ERROR) << "error: " << load_error;
-        break;
-      }
-
-      MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>(
-          app_library.GetFunctionPointer("MojoMain"));
-      if (!main_function) {
-        LOG(ERROR) << "Entrypoint MojoMain not found.";
-        break;
-      }
-
-      // |MojoMain()| takes ownership of the service handle.
-      // TODO(darin): What if MojoMain does not close the service handle?
-      MojoResult result = main_function(service_handle_.release().value());
-      if (result < MOJO_RESULT_OK)
-        LOG(ERROR) << "MojoMain returned an error: " << result;
-    } while (false);
-
-    loader_->context_->task_runners()->ui_runner()->PostTask(
-        FROM_HERE,
-        base::Bind(&LoadContext::AppCompleted, base::Unretained(this)));
+  static void AppCompleted(scoped_refptr<base::TaskRunner> task_runner,
+                           DynamicServiceLoader* loader,
+                           const GURL& url) {
+    task_runner->PostTask(FROM_HERE,
+                          base::Bind(&DynamicServiceLoader::AppCompleted,
+                                     base::Unretained(loader), url));
   }
 
-  void AppCompleted() {
-    loader_->AppCompleted(url_);
-  }
-
-  base::DelegateSimpleThread thread_;
-  DynamicServiceLoader* loader_;
-  GURL url_;
-  base::FilePath app_path_;
+  DynamicServiceLoader* const loader_;
+  const GURL url_;
   scoped_ptr<mojo::shell::Loader::Job> request_;
   ScopedShellHandle service_handle_;
+  scoped_ptr<DynamicServiceRunner> runner_;
   KeepAlive keep_alive_;
+
+  DISALLOW_COPY_AND_ASSIGN(LoadContext);
 };
 
-DynamicServiceLoader::DynamicServiceLoader(Context* context)
-    : context_(context) {
+DynamicServiceLoader::DynamicServiceLoader(
+    Context* context,
+    scoped_ptr<DynamicServiceRunnerFactory> runner_factory)
+    : context_(context),
+      runner_factory_(runner_factory.Pass()) {
 }
 
 DynamicServiceLoader::~DynamicServiceLoader() {
@@ -134,12 +109,13 @@ void DynamicServiceLoader::LoadService(ServiceManager* manager,
                                        const GURL& url,
                                        ScopedShellHandle service_handle) {
   DCHECK(url_to_load_context_.find(url) == url_to_load_context_.end());
-  url_to_load_context_[url] = new LoadContext(this, url, service_handle.Pass());
+  url_to_load_context_[url] = new LoadContext(this, url, service_handle.Pass(),
+                                              runner_factory_->Create());
 }
 
 void DynamicServiceLoader::AppCompleted(const GURL& url) {
   LoadContextMap::iterator it = url_to_load_context_.find(url);
-  DCHECK(it != url_to_load_context_.end());
+  DCHECK(it != url_to_load_context_.end()) << url;
 
   LoadContext* doomed = it->second;
   url_to_load_context_.erase(it);
diff --git a/mojo/shell/dynamic_service_loader.h b/mojo/shell/dynamic_service_loader.h
index b6bd3c5..5331ae0 100644
--- a/mojo/shell/dynamic_service_loader.h
+++ b/mojo/shell/dynamic_service_loader.h
@@ -11,6 +11,7 @@
 #include "mojo/public/shell/shell.mojom.h"
 #include "mojo/public/system/core_cpp.h"
 #include "mojo/service_manager/service_loader.h"
+#include "mojo/shell/dynamic_service_runner.h"
 #include "mojo/shell/keep_alive.h"
 #include "url/gurl.h"
 
@@ -18,12 +19,14 @@ namespace mojo {
 namespace shell {
 
 class Context;
+class DynamicServiceRunnerFactory;
 
 // A subclass of ServiceManager::Loader that loads a dynamic library containing
 // the implementation of the service.
 class DynamicServiceLoader : public ServiceLoader {
  public:
-  explicit DynamicServiceLoader(Context* context);
+  DynamicServiceLoader(Context* context,
+                       scoped_ptr<DynamicServiceRunnerFactory> runner_factory);
   virtual ~DynamicServiceLoader();
 
   // Initiates the dynamic load. If the url is a mojo: scheme then the name
@@ -39,9 +42,11 @@ class DynamicServiceLoader : public ServiceLoader {
 
   void AppCompleted(const GURL& url);
 
+  Context* const context_;
+  scoped_ptr<DynamicServiceRunnerFactory> runner_factory_;
+
   typedef std::map<GURL, LoadContext*> LoadContextMap;
   LoadContextMap url_to_load_context_;
-  Context* context_;
 
   DISALLOW_COPY_AND_ASSIGN(DynamicServiceLoader);
 };
diff --git a/mojo/shell/dynamic_service_runner.h b/mojo/shell/dynamic_service_runner.h
new file mode 100644
index 0000000..d9742cc
--- /dev/null
+++ b/mojo/shell/dynamic_service_runner.h
@@ -0,0 +1,50 @@
+// 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_DYNAMIC_SERVICE_RUNNER_H_
+#define MOJO_SHELL_DYNAMIC_SERVICE_RUNNER_H_
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "mojo/public/shell/shell.mojom.h"
+
+namespace base {
+class FilePath;
+}
+
+namespace mojo {
+namespace shell {
+
+class DynamicServiceRunner {
+ public:
+  virtual ~DynamicServiceRunner() {}
+
+  // Takes ownership of the file at |app_path|.
+  virtual void Start(const base::FilePath& app_path,
+                     ScopedShellHandle service_handle,
+                     const base::Closure& app_completed_callback) = 0;
+};
+
+class DynamicServiceRunnerFactory {
+ public:
+  virtual ~DynamicServiceRunnerFactory() {}
+  virtual scoped_ptr<DynamicServiceRunner> Create() = 0;
+};
+
+// A generic factory.
+template <class DynamicServiceRunnerImpl>
+class DynamicServiceRunnerFactoryImpl : public DynamicServiceRunnerFactory {
+ public:
+  DynamicServiceRunnerFactoryImpl() {}
+  virtual ~DynamicServiceRunnerFactoryImpl() {}
+  virtual scoped_ptr<DynamicServiceRunner> Create() OVERRIDE {
+    return scoped_ptr<DynamicServiceRunner>(new DynamicServiceRunnerImpl());
+  }
+};
+
+}  // namespace shell
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_DYNAMIC_SERVICE_RUNNER_H_
diff --git a/mojo/shell/in_process_dynamic_service_runner.cc b/mojo/shell/in_process_dynamic_service_runner.cc
new file mode 100644
index 0000000..34a1142
--- /dev/null
+++ b/mojo/shell/in_process_dynamic_service_runner.cc
@@ -0,0 +1,78 @@
+// 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/in_process_dynamic_service_runner.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/scoped_native_library.h"
+
+namespace mojo {
+namespace shell {
+
+InProcessDynamicServiceRunner::InProcessDynamicServiceRunner()
+    : thread_(this, "app_thread") {
+}
+
+InProcessDynamicServiceRunner::~InProcessDynamicServiceRunner() {
+  if (thread_.HasBeenStarted()) {
+    DCHECK(!thread_.HasBeenJoined());
+    thread_.Join();
+  }
+}
+
+void InProcessDynamicServiceRunner::Start(
+    const base::FilePath& app_path,
+    ScopedShellHandle service_handle,
+    const base::Closure& app_completed_callback) {
+  app_path_ = app_path;
+
+  DCHECK(!service_handle_.is_valid());
+  service_handle_ = service_handle.Pass();
+
+  DCHECK(app_completed_callback_.is_null());
+  app_completed_callback_ = app_completed_callback;
+
+  DCHECK(!thread_.HasBeenStarted());
+  thread_.Start();
+}
+
+void InProcessDynamicServiceRunner::Run() {
+  DVLOG(2) << "Loading/running Mojo app from " << app_path_.value()
+           << " in process";
+
+  base::ScopedClosureRunner app_deleter(
+      base::Bind(base::IgnoreResult(&base::DeleteFile), app_path_, false));
+
+  do {
+    std::string load_error;
+    base::ScopedNativeLibrary app_library(
+        base::LoadNativeLibrary(app_path_, &load_error));
+    if (!app_library.is_valid()) {
+      LOG(ERROR) << "Failed to load library (error: " << load_error << ")";
+      break;
+    }
+
+    typedef MojoResult (*MojoMainFunction)(MojoHandle);
+    MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>(
+        app_library.GetFunctionPointer("MojoMain"));
+    if (!main_function) {
+      LOG(ERROR) << "Entrypoint MojoMain not found";
+      break;
+    }
+
+    // |MojoMain()| takes ownership of the service handle.
+    MojoResult result = main_function(service_handle_.release().value());
+    if (result < MOJO_RESULT_OK)
+      LOG(ERROR) << "MojoMain returned an error: " << result;
+  } while (false);
+
+  app_completed_callback_.Run();
+  app_completed_callback_.Reset();
+}
+
+}  // namespace shell
+}  // namespace mojo
diff --git a/mojo/shell/in_process_dynamic_service_runner.h b/mojo/shell/in_process_dynamic_service_runner.h
new file mode 100644
index 0000000..129df46
--- /dev/null
+++ b/mojo/shell/in_process_dynamic_service_runner.h
@@ -0,0 +1,48 @@
+// 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_IN_PROCESS_DYNAMIC_SERVICE_RUNNER_H_
+#define MOJO_SHELL_IN_PROCESS_DYNAMIC_SERVICE_RUNNER_H_
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/threading/simple_thread.h"
+#include "mojo/shell/dynamic_service_runner.h"
+
+namespace mojo {
+namespace shell {
+
+class InProcessDynamicServiceRunner
+    : public DynamicServiceRunner,
+      public base::DelegateSimpleThread::Delegate {
+ public:
+  InProcessDynamicServiceRunner();
+  virtual ~InProcessDynamicServiceRunner();
+
+  // |DynamicServiceRunner| method:
+  virtual void Start(const base::FilePath& app_path,
+                     ScopedShellHandle service_handle,
+                     const base::Closure& app_completed_callback) OVERRIDE;
+
+ private:
+  // |base::DelegateSimpleThread::Delegate| method:
+  virtual void Run() OVERRIDE;
+
+  base::FilePath app_path_;
+  ScopedShellHandle service_handle_;
+  base::Closure app_completed_callback_;
+
+  base::DelegateSimpleThread thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(InProcessDynamicServiceRunner);
+};
+
+typedef DynamicServiceRunnerFactoryImpl<InProcessDynamicServiceRunner>
+    InProcessDynamicServiceRunnerFactory;
+
+}  // namespace shell
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_IN_PROCESS_DYNAMIC_SERVICE_RUNNER_H_