Make RunApplication() return a MojoResult and TerminateApplication() take one. R=vardhan@google.com Review URL: https://codereview.chromium.org/2011593002 .

diff --git a/examples/spinning_cube/spinning_cube_app.cc b/examples/spinning_cube/spinning_cube_app.cc
index adcef63..eac07b2 100644
--- a/examples/spinning_cube/spinning_cube_app.cc
+++ b/examples/spinning_cube/spinning_cube_app.cc

@@ -95,6 +95,5 @@
 
 MojoResult MojoMain(MojoHandle application_request) {
   examples::SpinningCubeApp spinning_cube_app;
-  mojo::RunApplication(application_request, &spinning_cube_app);
-  return MOJO_RESULT_OK;
+  return mojo::RunApplication(application_request, &spinning_cube_app);
 }

diff --git a/mojo/public/cpp/application/lib/run_application.cc b/mojo/public/cpp/application/lib/run_application.cc
index 82d3089..03373ef 100644
--- a/mojo/public/cpp/application/lib/run_application.cc
+++ b/mojo/public/cpp/application/lib/run_application.cc

@@ -4,17 +4,66 @@
 
 #include "mojo/public/cpp/application/run_application.h"
 
+#include <assert.h>
+#include <pthread.h>
+
 #include "mojo/public/cpp/application/application_impl_base.h"
+#include "mojo/public/cpp/system/macros.h"
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "mojo/public/cpp/utility/run_loop.h"
 #include "mojo/public/interfaces/application/application.mojom.h"
 
 namespace mojo {
 
-void RunApplication(MojoHandle application_request_handle,
-                    ApplicationImplBase* application_impl) {
-  // TODO(vtl): Possibly we should have an assertion that we're not running, but
-  // that requires TLS.
+namespace {
+
+// We store a pointer to a |ResultHolder|, which just stores a |MojoResult|, in
+// TLS so that |TerminateApplication()| can provide the result that
+// |RunApplication()| will return. (The |ResultHolder| is just on
+// |RunApplication()|'s stack.)
+struct ResultHolder {
+#ifndef NDEBUG
+  bool is_set = false;
+#endif
+  MojoResult result = MOJO_RESULT_UNKNOWN;
+};
+
+pthread_key_t g_current_result_holder_key;
+
+// Ensures that we have a TLS slot to store the current result in.
+void InitializeCurrentResultHolderIfNecessary() {
+  static pthread_once_t current_result_holder_key_once = PTHREAD_ONCE_INIT;
+  int error = pthread_once(&current_result_holder_key_once, []() {
+    int error = pthread_key_create(&g_current_result_holder_key, nullptr);
+    MOJO_ALLOW_UNUSED_LOCAL(error);
+    assert(!error);
+  });
+  MOJO_ALLOW_UNUSED_LOCAL(error);
+  assert(!error);
+}
+
+ResultHolder* GetCurrentResultHolder() {
+  InitializeCurrentResultHolderIfNecessary();
+  return static_cast<ResultHolder*>(
+      pthread_getspecific(g_current_result_holder_key));
+}
+
+void SetCurrentResultHolder(ResultHolder* result_holder) {
+  InitializeCurrentResultHolderIfNecessary();
+
+  int error = pthread_setspecific(g_current_result_holder_key, result_holder);
+  MOJO_ALLOW_UNUSED_LOCAL(error);
+  assert(!error);
+}
+
+}  // namespace
+
+MojoResult RunApplication(MojoHandle application_request_handle,
+                          ApplicationImplBase* application_impl) {
+  assert(!GetCurrentResultHolder());
+
+  ResultHolder result_holder;
+  SetCurrentResultHolder(&result_holder);
 
   RunLoop loop;
   application_impl->Bind(InterfaceRequest<Application>(
@@ -23,10 +72,26 @@
 
   // TODO(vtl): Should we unbind stuff here? (Should there be "will start"/"did
   // stop" notifications to the |ApplicationImplBase|?)
+
+  SetCurrentResultHolder(nullptr);
+
+  // TODO(vtl): We'd like to enable the following assertion, but we do things
+  // like |RunLoop::current()->Quit()| in various places.
+  // assert(result_holder.is_set);
+
+  return result_holder.result;
 }
 
-void TerminateApplication() {
+void TerminateApplication(MojoResult result) {
   RunLoop::current()->Quit();
+
+  ResultHolder* result_holder = GetCurrentResultHolder();
+  assert(result_holder);
+  assert(!result_holder->is_set);
+  result_holder->result = result;
+#ifndef NDEBUG
+  result_holder->is_set = true;
+#endif
 }
 
 }  // namespace mojo

diff --git a/mojo/public/cpp/application/run_application.h b/mojo/public/cpp/application/run_application.h
index 98f0ef1..5bfbbd5 100644
--- a/mojo/public/cpp/application/run_application.h
+++ b/mojo/public/cpp/application/run_application.h

@@ -13,15 +13,15 @@
 class ApplicationImplBase;
 
 // Sets up a message (run) loop and runs the provided application
-// implementation.
+// implementation. The return value will be the one provided to
+// |TerminateApplication()|.
 //
 // Typical use (where |MyApplicationImpl| is an implementation of
 // |ApplicationImplBase|):
 //
 //   MojoResult MojoMain(MojoHandle application_request_handle) {
 //     MyApplicationImpl my_app;
-//     mojo::RunApplication(application_request_handle, &my_app);
-//     return MOJO_RESULT_OK;
+//     return mojo::RunApplication(application_request_handle, &my_app);
 //   }
 //
 // Note that this may actually be used on any thread (that is not already
@@ -30,23 +30,21 @@
 // |*application_impl| must remain alive until the message loop starts running
 // (thus it may own itself).
 //
-// TODO(vtl): Possibly this should return a |MojoResult| and
-// |TerminateApplication()| should take a |MojoResult| argument (which this
-// returns), but to communicate that value requires TLS.
 // TODO(vtl): Maybe this should be like |Environment|, with different possible
 // implementations (e.g., "standalone" vs "chromium"). However,
 // |ApplicationRunnerChromium| currently has additional goop specific to the
-// main thread. Probably we should have additional "MainRunApplication()" and
-// "MainTerminateApplication()" functions.
-void RunApplication(MojoHandle application_request_handle,
-                    ApplicationImplBase* application_impl);
+// main thread. Probably we should have additional "RunMainApplication()" and
+// "TerminateMainApplication()" functions.
+MojoResult RunApplication(MojoHandle application_request_handle,
+                          ApplicationImplBase* application_impl);
 
 // Terminates the currently-running application. This may only be called from
 // "inside" |RunApplication()| (i.e., it is on the stack, which means that the
 // message loop is running). This will cause |RunApplication()| to return "soon"
-// (assuming the message loop is not blocked processing some task). This may
-// cause queued work to *not* be executed.
-void TerminateApplication();
+// (assuming the message loop is not blocked processing some task), with return
+// value |result|. This may cause queued work to *not* be executed. This should
+// be executed at most once (per |RunApplication()|).
+void TerminateApplication(MojoResult result);
 
 }  // namespace mojo
 

diff --git a/mojo/public/cpp/bindings/tests/versioning_test_service.cc b/mojo/public/cpp/bindings/tests/versioning_test_service.cc
index 3f7d503..7c59787 100644
--- a/mojo/public/cpp/bindings/tests/versioning_test_service.cc
+++ b/mojo/public/cpp/bindings/tests/versioning_test_service.cc

@@ -121,6 +121,5 @@
 
 MojoResult MojoMain(MojoHandle application_request) {
   mojo::test::versioning::HumanResourceSystemServer hr_system_server;
-  mojo::RunApplication(application_request, &hr_system_server);
-  return MOJO_RESULT_OK;
+  return mojo::RunApplication(application_request, &hr_system_server);
 }

diff --git a/mojo/public/cpp/utility/lib/run_loop.cc b/mojo/public/cpp/utility/lib/run_loop.cc
index 1fde95c..2f3178d 100644
--- a/mojo/public/cpp/utility/lib/run_loop.cc
+++ b/mojo/public/cpp/utility/lib/run_loop.cc

@@ -24,7 +24,7 @@
 
 // Ensures that the "current run loop" functionality is available (i.e., that we
 // have a TLS slot).
-void EnsureCurrentRunLoopInitialized() {
+void InitializeCurrentRunLoopIfNecessary() {
   static pthread_once_t current_run_loop_key_once = PTHREAD_ONCE_INIT;
   int error = pthread_once(&current_run_loop_key_once, []() {
     int error = pthread_key_create(&g_current_run_loop_key, nullptr);
@@ -36,7 +36,7 @@
 }
 
 void SetCurrentRunLoop(RunLoop* run_loop) {
-  EnsureCurrentRunLoopInitialized();
+  InitializeCurrentRunLoopIfNecessary();
 
   int error = pthread_setspecific(g_current_run_loop_key, run_loop);
   MOJO_ALLOW_UNUSED_LOCAL(error);
@@ -71,7 +71,7 @@
 
 // static
 RunLoop* RunLoop::current() {
-  EnsureCurrentRunLoopInitialized();
+  InitializeCurrentRunLoopIfNecessary();
   return static_cast<RunLoop*>(pthread_getspecific(g_current_run_loop_key));
 }
 

diff --git a/mojo/services/log/cpp/log_client.h b/mojo/services/log/cpp/log_client.h
index 049ed16..4c10c5c 100644
--- a/mojo/services/log/cpp/log_client.h
+++ b/mojo/services/log/cpp/log_client.h

@@ -25,8 +25,7 @@
 //
 //  MojoResult MojoMain(MojoHandle app_request) {
 //    MyApp app;
-//    mojo::RunApplication(&app);
-//    return MOJO_RESULT_OK;
+//    return mojo::RunApplication(&app);
 //  }
 
 #ifndef MOJO_SERVICES_LOG_CPP_LOG_CLIENT_H_

diff --git a/services/test_service/test_request_tracker_application.cc b/services/test_service/test_request_tracker_application.cc
index 2c41f76..9a0f612 100644
--- a/services/test_service/test_request_tracker_application.cc
+++ b/services/test_service/test_request_tracker_application.cc

@@ -43,6 +43,5 @@
 
 MojoResult MojoMain(MojoHandle application_request) {
   mojo::test::TestRequestTrackerApplication app;
-  mojo::RunApplication(application_request, &app);
-  return MOJO_RESULT_OK;
+  return mojo::RunApplication(application_request, &app);
 }

diff --git a/services/test_service/test_service_application.cc b/services/test_service/test_service_application.cc
index c883cef..af116a3 100644
--- a/services/test_service/test_service_application.cc
+++ b/services/test_service/test_service_application.cc

@@ -44,7 +44,7 @@
   assert(ref_count_ > 0);
   ref_count_--;
   if (ref_count_ <= 0)
-    TerminateApplication();
+    TerminateApplication(MOJO_RESULT_OK);
 }
 
 }  // namespace test
@@ -52,6 +52,5 @@
 
 MojoResult MojoMain(MojoHandle application_request) {
   mojo::test::TestServiceApplication app;
-  mojo::RunApplication(application_request, &app);
-  return MOJO_RESULT_OK;
+  return mojo::RunApplication(application_request, &app);
 }