Use ApplicationRunner for consistency in CoreServices.

This way apps can make assumptions about orders of destruction of message loop, delegate, applicationimpl and it works the same across all apps.

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

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

mandoline/services/core_services/core_services_application_delegate.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/single_thread_task_runner.h"
+#include "base/threading/simple_thread.h"
 #include "components/clipboard/clipboard_application_delegate.h"
 #include "components/filesystem/file_system_app.h"
 #include "components/resource_provider/resource_provider_app.h"
@@ -14,6 +15,7 @@
 #include "mandoline/ui/browser/browser.h"
 #include "mojo/application/public/cpp/application_connection.h"
 #include "mojo/application/public/cpp/application_impl.h"
+#include "mojo/application/public/cpp/application_runner.h"
 #include "mojo/common/message_pump_mojo.h"
 #include "mojo/services/network/network_service_delegate.h"
 #include "mojo/services/tracing/tracing_app.h"
@@ -25,85 +27,51 @@
 
 namespace core_services {
 
-class ApplicationThread;
-
-// A base::Thread which hosts a mojo::ApplicationImpl on its own thread.
-//
-// Why not base::SimpleThread? The comments in SimpleThread discourage its use,
-// and we want most of what base::Thread provides. The hack where we call
-// SetThreadWasQuitProperly() in Run() is already used in the chrome codebase.
-// (By the time we building a base::Thread here, we already have a MessageLoop
-// on our thread, along with a lot of other bookkeeping objects, too. This is
-// why we don't call into ApplicationRunner; we already have an AtExitManager et
-// all at this point.)
-class ApplicationThread : public base::Thread {
+// A helper class for hosting a mojo::ApplicationImpl on its own thread.
+class ApplicationThread : public base::SimpleThread {
  public:
   ApplicationThread(
       const base::WeakPtr<CoreServicesApplicationDelegate>
           core_services_application,
-      const std::string& name,
+      const std::string& url,
       scoped_ptr<mojo::ApplicationDelegate> delegate,
       mojo::InterfaceRequest<mojo::Application> request)
-      : base::Thread(name),
+      : base::SimpleThread(url),
         core_services_application_(core_services_application),
         core_services_application_task_runner_(
             base::MessageLoop::current()->task_runner()),
+        url_(url),
         delegate_(delegate.Pass()),
         request_(request.Pass()) {
   }
 
   ~ApplicationThread() override {
-    Stop();
+    Join();
   }
 
-  // Overridden from base::Thread:
-  void Run(base::MessageLoop* message_loop) override {
-    {
-      application_impl_.reset(new mojo::ApplicationImpl(
-          delegate_.get(), request_.Pass()));
-      base::Thread::Run(message_loop);
-      application_impl_.reset();
+  // Overridden from base::SimpleThread:
+  void Run() override {
+    scoped_ptr<mojo::ApplicationRunner> runner(
+        new mojo::ApplicationRunner(delegate_.release()));
+    if (url_ == "mojo://network_service/") {
+      runner->set_message_loop_type(base::MessageLoop::TYPE_IO);
+    } else if (url_ == "mojo://view_manager/") {
+      runner->set_message_loop_type(base::MessageLoop::TYPE_UI);
     }
-    delegate_.reset();
+    runner->Run(request_.PassMessagePipe().release().value(), false);
 
     core_services_application_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&CoreServicesApplicationDelegate::ApplicationThreadDestroyed,
                    core_services_application_,
                    this));
-
-    // TODO(erg): This is a hack.
-    //
-    // Right now, most of our services do not receive
-    // Application::RequestQuit() calls. jam@ is currently working on shutting
-    // down everything cleanly. In the long run, we don't wan this here (we
-    // want this set in ShutdownCleanly()), but until we can rely on
-    // RequestQuit() getting delivered we have to manually do this here.
-    //
-    // Remove this ASAP.
-    Thread::SetThreadWasQuitProperly(true);
-  }
-
-  void RequestQuit() {
-    if (!IsRunning())
-      return;
-    task_runner()->PostTask(
-        FROM_HERE,
-        base::Bind(&ApplicationThread::ShutdownCleanly,
-                   base::Unretained(this)));
-
-  }
-
-  void ShutdownCleanly() {
-    application_impl_->QuitNow();
-    Thread::SetThreadWasQuitProperly(true);
   }
 
  private:
   base::WeakPtr<CoreServicesApplicationDelegate> core_services_application_;
   scoped_refptr<base::SingleThreadTaskRunner>
       core_services_application_task_runner_;
-  scoped_ptr<mojo::ApplicationImpl> application_impl_;
+  std::string url_;
   scoped_ptr<mojo::ApplicationDelegate> delegate_;
   mojo::InterfaceRequest<mojo::Application> request_;
 
@@ -135,11 +103,6 @@ bool CoreServicesApplicationDelegate::ConfigureIncomingConnection(
 }
 
 void CoreServicesApplicationDelegate::Quit() {
-  // Fire off RequestQuit() messages to all the threads before we try to join
-  // on them.
-  for (ApplicationThread* thread : application_threads_)
-    thread->RequestQuit();
-
   // This will delete all threads. This also performs a blocking join, waiting
   // for the threads to end.
   application_threads_.clear();
@@ -181,24 +144,10 @@ void CoreServicesApplicationDelegate::StartApplication(
   else
     NOTREACHED() << "This application package does not support " << url;
 
-  base::Thread::Options thread_options;
-
-  // In the case of mojo:network_service, we must use an IO message loop.
-  if (url == "mojo://network_service/") {
-    thread_options.message_loop_type = base::MessageLoop::TYPE_IO;
-  } else if (url == "mojo://view_manager/") {
-    thread_options.message_loop_type = base::MessageLoop::TYPE_UI;
-  } else {
-    // We must use a MessagePumpMojo to awake on mojo messages.
-    thread_options.message_pump_factory =
-        base::Bind(&mojo::common::MessagePumpMojo::Create);
-  }
-
   scoped_ptr<ApplicationThread> thread(
       new ApplicationThread(weak_factory_.GetWeakPtr(), url, delegate.Pass(),
-      request.Pass()));
-  thread->StartWithOptions(thread_options);
-
+                            request.Pass()));
+  thread->Start();
   application_threads_.push_back(thread.Pass());
 }
 

mandoline/services/core_services/core_services_application_delegate.h

@@ -8,7 +8,6 @@
 #include "base/macros.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/thread.h"
 #include "components/clipboard/public/interfaces/clipboard.mojom.h"
 #include "mojo/application/public/cpp/application_delegate.h"
 #include "mojo/application/public/cpp/interface_factory_impl.h"