Remove QuitCurrent*() calls from //services.

This addresses flaky DCHECKs in ServiceManagerTests and LifecycleTests,
triggered by mixing of RunLoop::QuitClosure() with the calls to the
old RunLoop::QuitCurrent*Deprecated() APIs.

Change-Id: Ieb510aa1501f9d46ac606d66a3556744fd0f95da
Reviewed-on: https://chromium-review.googlesource.com/1170355Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582316}

mash/runner/main.cc

@@ -87,10 +87,12 @@ int main(int argc, char** argv) {
   base::test::ScopedTaskEnvironment scoped_task_environment;
   service_manager::Context service_manager_context(nullptr,
                                                    std::move(manifest_value));
+  base::RunLoop loop;
   scoped_task_environment.GetMainThreadTaskRunner()->PostTask(
       FROM_HERE,
       base::Bind(&service_manager::Context::RunCommandLineApplication,
-                 base::Unretained(&service_manager_context)));
-  base::RunLoop().Run();
+                 base::Unretained(&service_manager_context),
+                 loop.QuitClosure()));
+  loop.Run();
   return 0;
 }

services/service_manager/public/cpp/service_context.cc

@@ -113,9 +113,9 @@ void ServiceContext::DisconnectFromServiceManager() {
 void ServiceContext::QuitNow() {
   if (binding_.is_bound())
     binding_.Close();
-  if (!quit_closure_.is_null()) {
+  if (quit_closure_) {
     // CAUTION: May delete |this|.
-    base::ResetAndReturn(&quit_closure_).Run();
+    std::move(quit_closure_).Run();
   }
 }
 

services/service_manager/public/cpp/service_runner.cc

@@ -69,7 +69,7 @@ MojoResult ServiceRunner::Run(MojoHandle service_request_handle) {
 }
 
 void ServiceRunner::Quit() {
-  base::RunLoop::QuitCurrentWhenIdleDeprecated();
+  context_->QuitNow();
 }
 
 }  // namespace service_manager

services/service_manager/standalone/context.cc

@@ -60,9 +60,11 @@ class ServiceProcessLauncherFactoryImpl : public ServiceProcessLauncherFactory {
 };
 #endif  // !defined(OS_IOS)
 
-void OnInstanceQuit(const std::string& name, const Identity& identity) {
+void OnInstanceQuit(const std::string& name,
+                    base::RepeatingClosure on_quit,
+                    const Identity& identity) {
   if (name == identity.name())
-    base::RunLoop::QuitCurrentWhenIdleDeprecated();
+    on_quit.Run();
 }
 
 const char kService[] = "service";
@@ -92,14 +94,17 @@ Context::Context(
 
 Context::~Context() = default;
 
-void Context::RunCommandLineApplication() {
+void Context::RunCommandLineApplication(base::RepeatingClosure on_quit) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(kService))
-    Run(command_line->GetSwitchValueASCII(kService));
+    Run(command_line->GetSwitchValueASCII(kService), std::move(on_quit));
+  else
+    std::move(on_quit).Run();
 }
 
-void Context::Run(const std::string& name) {
-  service_manager_->SetInstanceQuitCallback(base::Bind(&OnInstanceQuit, name));
+void Context::Run(const std::string& name, base::RepeatingClosure on_quit) {
+  service_manager_->SetInstanceQuitCallback(
+      base::BindRepeating(&OnInstanceQuit, name, std::move(on_quit)));
 
   std::unique_ptr<ConnectParams> params(new ConnectParams);
   params->set_source(CreateServiceManagerIdentity());

services/service_manager/standalone/context.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -27,14 +28,15 @@ class Context {
           std::unique_ptr<base::Value> catalog_content);
   ~Context();
 
-  // Run the application specified on the command line.
-  void RunCommandLineApplication();
+  // Run the application specified on the command line, and run |on_quit| when
+  // the application instance quits.
+  void RunCommandLineApplication(base::RepeatingClosure on_quit);
 
   ServiceManager* service_manager() { return service_manager_.get(); }
 
  private:
   // Runs the app specified by |name|.
-  void Run(const std::string& name);
+  void Run(const std::string& name, base::RepeatingClosure on_quit);
 
   std::unique_ptr<ServiceManager> service_manager_;
   base::Time main_entry_time_;

services/service_manager/tests/connect/connect_test_package.cc

@@ -148,7 +148,7 @@ class ProvidedService : public Service,
 
   void OnConnectionError() {
     if (bindings_.empty())
-      base::RunLoop::QuitCurrentWhenIdleDeprecated();
+      context()->QuitNow();
   }
 
   const std::string title_;

services/service_manager/tests/lifecycle/app_client.cc

@@ -25,7 +25,7 @@ void AppClient::OnBindInterface(const BindSourceInfo& source_info,
 }
 
 bool AppClient::OnServiceManagerConnectionLost() {
-  base::RunLoop::QuitCurrentWhenIdleDeprecated();
+  context()->QuitNow();
   return true;
 }
 

services/service_manager/tests/lifecycle/package.cc

@@ -147,7 +147,7 @@ class Package : public service_manager::ForwardingService,
     contexts_.erase(it);
     id_to_context_.erase(id_it);
     if (contexts_.empty() && base::RunLoop::IsRunningOnCurrentThread())
-      base::RunLoop::QuitCurrentWhenIdleDeprecated();
+      context()->QuitNow();
   }
 
   service_manager::test::AppClient app_client_;

services/service_manager/tests/lifecycle/parent.cc

@@ -17,10 +17,6 @@
 
 namespace {
 
-void QuitLoop(base::RunLoop* loop) {
-  loop->Quit();
-}
-
 class Parent : public service_manager::Service,
                public service_manager::test::mojom::Parent {
  public:
@@ -50,12 +46,12 @@ class Parent : public service_manager::Service,
     context()->connector()->BindInterface("lifecycle_unittest_app", &lifecycle);
     {
       base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
-      lifecycle->Ping(base::Bind(&QuitLoop, &loop));
+      lifecycle->Ping(loop.QuitClosure());
       loop.Run();
     }
     std::move(callback).Run();
   }
-  void Quit() override { base::RunLoop::QuitCurrentWhenIdleDeprecated(); }
+  void Quit() override { context()->QuitNow(); }
 
   service_manager::BinderRegistry registry_;
   mojo::BindingSet<service_manager::test::mojom::Parent> parent_bindings_;

services/service_manager/tests/service_manager/embedder.cc

@@ -79,7 +79,7 @@ class Embedder : public service_manager::Service,
   }
 
   bool OnServiceManagerConnectionLost() override {
-    base::RunLoop::QuitCurrentWhenIdleDeprecated();
+    context()->QuitNow();
     return true;
   }