From 46dc919ea544cbc839e35286f05318419cdc7312 Mon Sep 17 00:00:00 2001
From: Ambre Williams <ambre@google.com>
Date: Tue, 23 Apr 2019 17:20:27 +0000
Subject: [PATCH] [ledger] terminate CloudProvider after each validation test

Validation tests may create a new cloud provider process for each test.
We proxy the requests on the FIDL channel so we can know when the client
closes the channel and terminate the cloud provider instance launched
for this test.

TEST=run validation tests, fx shell ps, no cloud provider is running

Change-Id: I522901d2b913d06b8ab207b14ddd116f32394863
---
 .../tests/cloud_provider/launcher/BUILD.gn    |  2 ++
 .../launcher/validation_tests_launcher.cc     | 35 +++++++++++++++++--
 .../launcher/validation_tests_launcher.h      | 29 +++++++++++++--
 .../bin/validation/app.cc                     |  3 ++
 .../bin/validation/launch.cc                  |  5 ++-
 5 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/src/ledger/bin/tests/cloud_provider/launcher/BUILD.gn b/src/ledger/bin/tests/cloud_provider/launcher/BUILD.gn
index 5bdb88f3a18..9a7f3ae9c21 100644
--- a/src/ledger/bin/tests/cloud_provider/launcher/BUILD.gn
+++ b/src/ledger/bin/tests/cloud_provider/launcher/BUILD.gn
@@ -11,7 +11,9 @@ source_set("launcher") {
   ]
 
   public_deps = [
+    "//garnet/public/lib/callback",
     "//sdk/fidl/fuchsia.ledger.cloud",
+    "//sdk/fidl/fuchsia.sys",
     "//sdk/lib/fidl/cpp",
     "//sdk/lib/sys/cpp/testing:unit",
   ]
diff --git a/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.cc b/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.cc
index 979871a008f..2d65680c4c6 100644
--- a/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.cc
+++ b/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.cc
@@ -16,15 +16,44 @@ constexpr char kValidationTestsUrl[] =
     "#meta/cloud_provider_validation_tests.cmx";
 }  // namespace
 
+ValidationTestsLauncher::CloudProviderProxy::CloudProviderProxy(
+    fidl::InterfacePtr<fuchsia::ledger::cloud::CloudProvider> proxied,
+    fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider> request,
+    fuchsia::sys::ComponentControllerPtr controller)
+    : binding_(proxied.get(), std::move(request)),
+      proxied_(std::move(proxied)),
+      controller_(std::move(controller)) {
+  binding_.set_error_handler([this](zx_status_t status) {
+    if (on_empty_)
+      on_empty_();
+  });
+  proxied_.set_error_handler([this](zx_status_t status) {
+    if (on_empty_)
+      on_empty_();
+  });
+}
+
+ValidationTestsLauncher::CloudProviderProxy::~CloudProviderProxy(){};
+
+void ValidationTestsLauncher::CloudProviderProxy::set_on_empty(
+    fit::closure on_empty) {
+  on_empty_ = std::move(on_empty);
+}
+
 ValidationTestsLauncher::ValidationTestsLauncher(
     sys::ComponentContext* component_context,
-    fit::function<
-        void(fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>)>
+    fit::function<fuchsia::sys::ComponentControllerPtr(
+        fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>)>
         factory)
     : component_context_(component_context), factory_(std::move(factory)) {
   service_directory_provider_.AddService<fuchsia::ledger::cloud::CloudProvider>(
       [this](fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>
-                 request) { factory_(std::move(request)); });
+                 request) {
+        fidl::InterfacePtr<fuchsia::ledger::cloud::CloudProvider> proxied;
+        auto controller = factory_(proxied.NewRequest());
+        proxies_.emplace(std::move(proxied), std::move(request),
+                         std::move(controller));
+      });
 }
 
 void ValidationTestsLauncher::Run(const std::vector<std::string>& arguments,
diff --git a/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.h b/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.h
index 25a7cd472e8..1ec062d4d2f 100644
--- a/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.h
+++ b/src/ledger/bin/tests/cloud_provider/launcher/validation_tests_launcher.h
@@ -7,6 +7,7 @@
 
 #include <fuchsia/ledger/cloud/cpp/fidl.h>
 #include <fuchsia/sys/cpp/fidl.h>
+#include <lib/callback/auto_cleanable.h>
 #include <lib/sys/cpp/component_context.h>
 #include <lib/sys/cpp/testing/service_directory_provider.h>
 
@@ -21,10 +22,13 @@ class ValidationTestsLauncher {
   // The constructor.
   //
   // |factory| is called to produce instances of the cloud provider under test.
+  // It may return a component controller: when the cloud provider instance is
+  // not used anymore (ie. the other end of the interface request is closed),
+  // the component controller is closed, which terminates the cloud provider.
   ValidationTestsLauncher(
       sys::ComponentContext* component_context,
-      fit::function<
-          void(fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>)>
+      fit::function<fuchsia::sys::ComponentControllerPtr(
+          fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>)>
           factory);
 
   // Starts the tests.
@@ -36,13 +40,32 @@ class ValidationTestsLauncher {
            fit::function<void(int32_t)> callback);
 
  private:
+  // Proxies requests from |request| to |proxied|, and terminates the component
+  // controlled by |controller| when one of the ends closes the channel.
+  class CloudProviderProxy {
+   public:
+    CloudProviderProxy(
+        fidl::InterfacePtr<fuchsia::ledger::cloud::CloudProvider> proxied,
+        fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider> request,
+        fuchsia::sys::ComponentControllerPtr controller);
+    ~CloudProviderProxy();
+    void set_on_empty(fit::closure on_empty);
+
+   private:
+    fidl::Binding<fuchsia::ledger::cloud::CloudProvider> binding_;
+    fidl::InterfacePtr<fuchsia::ledger::cloud::CloudProvider> proxied_;
+    fuchsia::sys::ComponentControllerPtr controller_;
+    fit::closure on_empty_;
+  };
+
   sys::ComponentContext* const component_context_;
-  fit::function<void(
+  fit::function<fuchsia::sys::ComponentControllerPtr(
       fidl::InterfaceRequest<fuchsia::ledger::cloud::CloudProvider>)>
       factory_;
   sys::testing::ServiceDirectoryProvider service_directory_provider_;
   fuchsia::sys::ComponentControllerPtr validation_tests_controller_;
   fit::function<void(int32_t)> callback_;
+  callback::AutoCleanableSet<CloudProviderProxy> proxies_;
 };
 
 }  // namespace cloud_provider
diff --git a/src/ledger/cloud_provider_firestore/bin/validation/app.cc b/src/ledger/cloud_provider_firestore/bin/validation/app.cc
index c9e7ebf3da0..da3db95e4b1 100644
--- a/src/ledger/cloud_provider_firestore/bin/validation/app.cc
+++ b/src/ledger/cloud_provider_firestore/bin/validation/app.cc
@@ -61,6 +61,9 @@ int main(int argc, char** argv) {
         factory.MakeCloudProvider(
             cloud_provider_firestore::CloudProviderFactory::UserId::New(),
             std::move(request));
+        // Return null because we do not create individual instances of a
+        // component per request.
+        return nullptr;
       });
 
   int32_t return_code = -1;
diff --git a/src/ledger/cloud_provider_in_memory/bin/validation/launch.cc b/src/ledger/cloud_provider_in_memory/bin/validation/launch.cc
index 316371aad51..e6c2e0f6a86 100644
--- a/src/ledger/cloud_provider_in_memory/bin/validation/launch.cc
+++ b/src/ledger/cloud_provider_in_memory/bin/validation/launch.cc
@@ -34,9 +34,12 @@ int main(int argc, char** argv) {
         auto cloud_provider_services = sys::ServiceDirectory::CreateWithRequest(
             &launch_info.directory_request);
 
-        component_launcher->CreateComponent(std::move(launch_info), nullptr);
+        fuchsia::sys::ComponentControllerPtr controller;
+        component_launcher->CreateComponent(std::move(launch_info),
+                                            controller.NewRequest());
         cloud_provider_services->Connect(
             std::move(request), fuchsia::ledger::cloud::CloudProvider::Name_);
+        return controller;
       });
 
   int32_t return_code = -1;
-- 
GitLab