[fuchsia] Add ScopedServicePublisher

Refactors ScopedServiceBinding's publishing functionality into a
separate class, allowing an embedder to manage its own fidl Binding or
BindingSet instance, while benefitting from the scoped service
registration lifetime.

Bug: b/154042778
Test: base_unittests
Change-Id: I959268b97d6c8d12487fb56db49ed8713f264169
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231552
Commit-Queue: Akira Baruah <akirabaruah@google.com>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776629}

base/BUILD.gn

@@ -1538,8 +1538,8 @@ jumbo_component("base") {
       "fuchsia/intl_profile_watcher.h",
       "fuchsia/process_context.cc",
       "fuchsia/process_context.h",
-      "fuchsia/scoped_service_binding.cc",
       "fuchsia/scoped_service_binding.h",
+      "fuchsia/scoped_service_publisher.h",
       "fuchsia/service_provider_impl.cc",
       "fuchsia/service_provider_impl.h",
       "fuchsia/startup_context.cc",
@@ -1597,6 +1597,7 @@ jumbo_component("base") {
       "//third_party/fuchsia-sdk/sdk/pkg/fdio",
       "//third_party/fuchsia-sdk/sdk/pkg/fidl_cpp",
       "//third_party/fuchsia-sdk/sdk/pkg/sys_cpp",
+      "//third_party/fuchsia-sdk/sdk/pkg/vfs_cpp",
       "//third_party/fuchsia-sdk/sdk/pkg/zx",
     ]
 
@@ -1608,7 +1609,6 @@ jumbo_component("base") {
       "//third_party/fuchsia-sdk/sdk/pkg/fidl",
       "//third_party/fuchsia-sdk/sdk/pkg/sys_inspect_cpp",
       "//third_party/fuchsia-sdk/sdk/pkg/syslog",
-      "//third_party/fuchsia-sdk/sdk/pkg/vfs_cpp",
       "//third_party/icu",
     ]
   }
@@ -3198,6 +3198,7 @@ test("base_unittests") {
       "fuchsia/filtered_service_directory_unittest.cc",
       "fuchsia/intl_profile_watcher_unittest.cc",
       "fuchsia/scoped_service_binding_unittest.cc",
+      "fuchsia/scoped_service_publisher_unittest.cc",
       "fuchsia/service_directory_test_base.cc",
       "fuchsia/service_directory_test_base.h",
       "fuchsia/service_provider_impl_unittest.cc",

base/fuchsia/scoped_service_binding.h

@@ -7,10 +7,12 @@
 
 #include <lib/fidl/cpp/binding.h>
 #include <lib/fidl/cpp/binding_set.h>
+#include <lib/fidl/cpp/interface_request.h>
 #include <lib/zx/channel.h>
 
 #include "base/base_export.h"
 #include "base/callback.h"
+#include "base/fuchsia/scoped_service_publisher.h"
 
 namespace sys {
 class OutgoingDirectory;
@@ -23,51 +25,21 @@ class PseudoDir;
 namespace base {
 namespace fuchsia {
 
-namespace internal {
-
-class BASE_EXPORT ScopedServiceBindingBase {
- public:
-  explicit ScopedServiceBindingBase(sys::OutgoingDirectory* outgoing_directory);
-  explicit ScopedServiceBindingBase(vfs::PseudoDir* pseudo_dir);
-
-  ~ScopedServiceBindingBase();
-
- protected:
-  // Same type as vfs::Service::Connector, so the value can be passed directly
-  // to vfs::Service.
-  using Connector =
-      fit::function<void(zx::channel channel, async_dispatcher_t* dispatcher)>;
-
-  void RegisterService(const char* service_name, Connector connector);
-  void UnregisterService(const char* service_name);
-
- private:
-  vfs::PseudoDir* const pseudo_dir_ = nullptr;
-};
-
-}  // namespace internal
-
 template <typename Interface>
-class ScopedServiceBinding : public internal::ScopedServiceBindingBase {
+class BASE_EXPORT ScopedServiceBinding {
  public:
   // Published a public service in the specified |outgoing_directory|.
   // |outgoing_directory| and |impl| must outlive the binding.
   ScopedServiceBinding(sys::OutgoingDirectory* outgoing_directory,
                        Interface* impl)
-      : ScopedServiceBindingBase(outgoing_directory), impl_(impl) {
-    RegisterService(Interface::Name_,
-                    fit::bind_member(this, &ScopedServiceBinding::BindClient));
-  }
+      : publisher_(outgoing_directory, bindings_.GetHandler(impl)) {}
 
   // Publishes a service in the specified |pseudo_dir|. |pseudo_dir| and |impl|
   // must outlive the binding.
   ScopedServiceBinding(vfs::PseudoDir* pseudo_dir, Interface* impl)
-      : ScopedServiceBindingBase(pseudo_dir), impl_(impl) {
-    RegisterService(Interface::Name_,
-                    fit::bind_member(this, &ScopedServiceBinding::BindClient));
-  }
+      : publisher_(pseudo_dir, bindings_.GetHandler(impl)) {}
 
-  ~ScopedServiceBinding() { UnregisterService(Interface::Name_); }
+  ~ScopedServiceBinding() = default;
 
   void SetOnLastClientCallback(base::OnceClosure on_last_client_callback) {
     on_last_client_callback_ = std::move(on_last_client_callback);
@@ -78,21 +50,13 @@ class ScopedServiceBinding : public internal::ScopedServiceBindingBase {
   bool has_clients() const { return bindings_.size() != 0; }
 
  private:
-  void BindClient(zx::channel channel, async_dispatcher_t* dispatcher) {
-    bindings_.AddBinding(impl_,
-                         fidl::InterfaceRequest<Interface>(std::move(channel)),
-                         dispatcher);
-  }
-
   void OnBindingSetEmpty() {
     bindings_.set_empty_set_handler(nullptr);
     std::move(on_last_client_callback_).Run();
   }
 
-  sys::OutgoingDirectory* const directory_ = nullptr;
-  vfs::PseudoDir* const pseudo_dir_ = nullptr;
-  Interface* const impl_;
   fidl::BindingSet<Interface> bindings_;
+  ScopedServicePublisher<Interface> publisher_;
   base::OnceClosure on_last_client_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedServiceBinding);
@@ -105,19 +69,18 @@ enum class ScopedServiceBindingPolicy { kPreferNew, kPreferExisting };
 template <typename Interface,
           ScopedServiceBindingPolicy Policy =
               ScopedServiceBindingPolicy::kPreferNew>
-class ScopedSingleClientServiceBinding
-    : public internal::ScopedServiceBindingBase {
+class BASE_EXPORT ScopedSingleClientServiceBinding {
  public:
   // |outgoing_directory| and |impl| must outlive the binding.
   ScopedSingleClientServiceBinding(sys::OutgoingDirectory* outgoing_directory,
                                    Interface* impl)
-      : ScopedServiceBindingBase(outgoing_directory), binding_(impl) {
-    RegisterService(
-        Interface::Name_,
-        fit::bind_member(this, &ScopedSingleClientServiceBinding::BindClient));
-  }
+      : binding_(impl),
+        publisher_(
+            outgoing_directory,
+            fit::bind_member(this,
+                             &ScopedSingleClientServiceBinding::BindClient)) {}
 
-  ~ScopedSingleClientServiceBinding() { UnregisterService(Interface::Name_); }
+  ~ScopedSingleClientServiceBinding() = default;
 
   typename Interface::EventSender_& events() { return binding_.events(); }
 
@@ -130,13 +93,12 @@ class ScopedSingleClientServiceBinding
   bool has_clients() const { return binding_.is_bound(); }
 
  private:
-  void BindClient(zx::channel channel, async_dispatcher_t* dispatcher) {
+  void BindClient(fidl::InterfaceRequest<Interface> request) {
     if (Policy == ScopedServiceBindingPolicy::kPreferExisting &&
         binding_.is_bound()) {
       return;
     }
-    binding_.Bind(fidl::InterfaceRequest<Interface>(std::move(channel)),
-                  dispatcher);
+    binding_.Bind(std::move(request));
   }
 
   void OnBindingEmpty(zx_status_t status) {
@@ -145,6 +107,7 @@ class ScopedSingleClientServiceBinding
   }
 
   fidl::Binding<Interface> binding_;
+  ScopedServicePublisher<Interface> publisher_;
   base::OnceClosure on_last_client_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedSingleClientServiceBinding);

base/fuchsia/scoped_service_publisher.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_FUCHSIA_SCOPED_SERVICE_PUBLISHER_H_
+#define BASE_FUCHSIA_SCOPED_SERVICE_PUBLISHER_H_
+
+#include <lib/async/dispatcher.h>
+#include <lib/fidl/cpp/interface_request.h>
+#include <lib/sys/cpp/outgoing_directory.h>
+#include <lib/vfs/cpp/pseudo_dir.h>
+#include <lib/vfs/cpp/service.h>
+#include <lib/zx/channel.h>
+
+#include "base/base_export.h"
+#include "base/macros.h"
+
+namespace base {
+namespace fuchsia {
+
+template <typename Interface>
+class BASE_EXPORT ScopedServicePublisher {
+ public:
+  // Publishes a public service in the specified |outgoing_directory|.
+  // |outgoing_directory| and |handler| must outlive the binding.
+  ScopedServicePublisher(sys::OutgoingDirectory* outgoing_directory,
+                         fidl::InterfaceRequestHandler<Interface> handler)
+      : ScopedServicePublisher(outgoing_directory->GetOrCreateDirectory("svc"),
+                               std::move(handler)) {}
+
+  // Publishes a service in the specified |pseudo_dir|. |pseudo_dir| and
+  // |handler| must outlive the binding.
+  ScopedServicePublisher(vfs::PseudoDir* pseudo_dir,
+                         fidl::InterfaceRequestHandler<Interface> handler)
+      : pseudo_dir_(pseudo_dir) {
+    pseudo_dir_->AddEntry(Interface::Name_,
+                          std::make_unique<vfs::Service>(std::move(handler)));
+  }
+
+  ~ScopedServicePublisher() { pseudo_dir_->RemoveEntry(Interface::Name_); }
+
+ private:
+  vfs::PseudoDir* const pseudo_dir_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedServicePublisher);
+};
+
+}  // namespace fuchsia
+}  // namespace base
+
+#endif  // BASE_FUCHSIA_SCOPED_SERVICE_PUBLISHER_H_

base/fuchsia/scoped_service_publisher_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/fuchsia/scoped_service_publisher.h"
+
+#include <lib/fidl/cpp/binding_set.h>
+
+#include "base/fuchsia/service_directory_test_base.h"
+#include "base/run_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace fuchsia {
+
+class ScopedServicePublisherTest : public ServiceDirectoryTestBase {};
+
+TEST_F(ScopedServicePublisherTest, ConstructorPublishesService) {
+  // Remove the default service binding.
+  service_binding_.reset();
+
+  // Create bindings and register using a publisher instance.
+  fidl::BindingSet<testfidl::TestInterface> bindings;
+  ScopedServicePublisher<testfidl::TestInterface> publisher(
+      outgoing_directory_.get(), bindings.GetHandler(&test_service_));
+  auto client = public_service_directory_->Connect<testfidl::TestInterface>();
+  VerifyTestInterface(&client, ZX_OK);
+}
+
+TEST_F(ScopedServicePublisherTest, DestructorRemovesService) {
+  // Remove the default service binding.
+  service_binding_.reset();
+
+  fidl::BindingSet<testfidl::TestInterface> bindings;
+  {
+    ScopedServicePublisher<testfidl::TestInterface> publisher(
+        outgoing_directory_.get(), bindings.GetHandler(&test_service_));
+  }
+  // Once the publisher leaves scope, the service shouldn't be available.
+  auto new_client =
+      public_service_directory_->Connect<testfidl::TestInterface>();
+  VerifyTestInterface(&new_client, ZX_ERR_PEER_CLOSED);
+}
+
+}  // namespace fuchsia
+}  // namespace base