Work in progress for end-to-end bindings

mojo/shell/app_container.cc

Index: mojo/shell/app_container.cc
diff --git a/mojo/shell/app_container.cc b/mojo/shell/app_container.cc
index 07346a6f70d201d59c5db77d54c57efc7ddc5e2d..0d0aec0ddd4636fcf6e24dbb10a8a6a3fb1426dd 100644
--- a/mojo/shell/app_container.cc
+++ b/mojo/shell/app_container.cc
@@ -23,38 +23,9 @@ typedef MojoResult (*MojoMainFunction)(mojo::Handle pipe);
 namespace mojo {
 namespace shell {
 
-void LaunchAppOnThread(
-    const base::FilePath& app_path,
-    Handle app_handle_raw) {
-  base::ScopedClosureRunner app_deleter(
-      base::Bind(base::IgnoreResult(&base::DeleteFile), app_path, false));
-  ScopedHandle app_handle(app_handle_raw);
-
-  base::ScopedNativeLibrary app_library(
-      base::LoadNativeLibrary(app_path, NULL));
-  if (!app_library.is_valid()) {
-    LOG(ERROR) << "Failed to load library: " << app_path.value().c_str();
-    return;
-  }
-
-  MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>(
-      app_library.GetFunctionPointer("MojoMain"));
-  if (!main_function) {
-    LOG(ERROR) << "Entrypoint MojoMain not found.";
-    return;
-  }
-
-  MojoResult result = main_function(app_handle.get());
-  if (result < MOJO_RESULT_OK) {
-    LOG(ERROR) << "MojoMain returned an error: " << result;
-    return;
-  }
-
-  LOG(INFO) << "MojoMain succeeded: " << result;
-}
-
 AppContainer::AppContainer(Context* context)
     : context_(context),
+      app_handle_raw_(mojo::kInvalidHandle),
       weak_factory_(this) {
 }
 
@@ -68,8 +39,7 @@ void AppContainer::Load(const GURL& app_url) {
 void AppContainer::DidCompleteLoad(const GURL& app_url,
                                    const base::FilePath& app_path) {
   Handle shell_handle;
-  Handle app_handle;
-  MojoResult result = CreateMessagePipe(&shell_handle, &app_handle);
+  MojoResult result = CreateMessagePipe(&shell_handle, &app_handle_raw_);
   if (result < MOJO_RESULT_OK) {
     // Failure..
   }
@@ -79,12 +49,10 @@ void AppContainer::DidCompleteLoad(const GURL& app_url,
 
   // Launch the app on its own thread.
   // TODO(beng): Create a unique thread name.
-  thread_.reset(new base::Thread("app_thread"));
+  app_path_ = app_path;
+  main_message_loop_proxy_ = base::MessageLoopProxy::current();
+  thread_.reset(new base::DelegateSimpleThread(this, "app_thread"));
   thread_->Start();
-  thread_->message_loop_proxy()->PostTaskAndReply(
-      FROM_HERE,
-      base::Bind(&LaunchAppOnThread, app_path, app_handle),
-      base::Bind(&AppContainer::AppCompleted, weak_factory_.GetWeakPtr()));
 
   // TODO(beng): This should be created on demand by the NativeViewportService
   //             when it is retrieved by the app.
@@ -92,11 +60,43 @@ void AppContainer::DidCompleteLoad(const GURL& app_url,
   //     new services::NativeViewportController(context_, shell_handle_));
 }
 
+void AppContainer::Run() {
+  base::ScopedClosureRunner app_deleter(
+      base::Bind(base::IgnoreResult(&base::DeleteFile), app_path_, false));
+  ScopedHandle app_handle(app_handle_raw_);
+
+  base::ScopedNativeLibrary app_library(
+      base::LoadNativeLibrary(app_path_, NULL));
+  if (!app_library.is_valid()) {
+    LOG(ERROR) << "Failed to load library: " << app_path_.value().c_str();
+    return;
+  }
+
+  MojoMainFunction main_function = reinterpret_cast<MojoMainFunction>(
+      app_library.GetFunctionPointer("MojoMain"));
+  if (!main_function) {
+    LOG(ERROR) << "Entrypoint MojoMain not found.";
+    return;
+  }
+
+  printf("Calling main_function\n");
+  MojoResult result = main_function(app_handle.get());
+  if (result < MOJO_RESULT_OK) {
+    LOG(ERROR) << "MojoMain returned an error: " << result;
+    return;
+  }
+  LOG(INFO) << "MojoMain succeeded: " << result;
+  main_message_loop_proxy_->PostTask(
+      FROM_HERE,
+      base::Bind(&AppContainer::AppCompleted, weak_factory_.GetWeakPtr()));

[darin (slow to review), 2013/11/15 00:01:37]

WeakPtr is not thread-safe. you need to call GetWeakPtr() on the same thread
where you wish to de-reference it. This means, you need to pass the WeakPtr as a
parameter to Run() somehow, perhaps as a member variable of |this|.

But, why do we even need a WeakPtr? It seems like we already expect that |this|
cannot become invalid until after Run() returns since we are freely
dereferencing |this| inside of Run(). It seems like we already know that the
main thread is not going away until Run() returns (i.e., until AppCompleted is
called).

Oh, I see... the TODO in mojo::shell::Run() seems to indicate that we just
haven't invented how shutdown sequencing should be done.

[DaveMoore, 2013/11/15 00:11:33]

Done.

+}
+
 void AppContainer::AppCompleted() {
   hello_world_service_.reset();
   // TODO(aa): This code gets replaced once we have a service manager.
   // native_viewport_controller_->Close();
 
+  thread_->Join();
   thread_.reset();
 }