[fuchsia] Add basic implementations of fidl::AsyncEventHandler

HLCPP bindings had a `set_error_handler` method on the `InterfacePtr` but that's no longer available for `fidl::Client`. Instead, it takes a `fidl::AsyncEventHandler` that has an `on_fidl_error` method as well as methods for other events available on the `Protocol`. The new API shape creates quite a bit of extra boilerplate for just adding a closure or logging on fidl error (when a service is disconnected). This CL adds two basic implementations, one that just logs an error, and another that takes a callback. Bug: 1351487 Change-Id: If6ba87f24751af8daafb54904652164eb47d9875 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4325064 Reviewed-by: Guocheng Wei <guochengwei@chromium.org> Reviewed-by: David Dorwin <ddorwin@chromium.org> Commit-Queue: Bryant Chandler <bryantchandler@chromium.org> Cr-Commit-Position: refs/heads/main@{#1118929}
chromium · Mar 17, 2023 · f412da9 · f412da9
1 parent 5207bba
commit f412da9
Show file tree

Hide file tree

Showing 7 changed files with 254 additions and 20 deletions.
diff --git a/base/BUILD.gn b/base/BUILD.gn
@@ -1404,6 +1404,7 @@ component("base") {
       "files/memory_mapped_file_posix.cc",
       "fuchsia/default_job.cc",
       "fuchsia/default_job.h",
+      "fuchsia/fidl_event_handler.h",
       "fuchsia/file_utils.cc",
       "fuchsia/file_utils.h",
       "fuchsia/filtered_service_directory.cc",
@@ -3877,6 +3878,7 @@ test("base_unittests") {
     sources += [
       "files/dir_reader_posix_unittest.cc",
       "files/file_descriptor_watcher_posix_unittest.cc",
+      "fuchsia/fidl_event_handler_unittest.cc",
       "fuchsia/file_utils_unittest.cc",
       "fuchsia/filtered_service_directory_unittest.cc",
       "fuchsia/fuchsia_logging_unittest.cc",

diff --git a/base/fuchsia/fidl_event_handler.h b/base/fuchsia/fidl_event_handler.h
@@ -0,0 +1,67 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_FUCHSIA_FIDL_EVENT_HANDLER_H_
+#define BASE_FUCHSIA_FIDL_EVENT_HANDLER_H_
+
+#include <lib/fidl/cpp/wire/client_base.h>
+#include <lib/fidl/cpp/wire/status.h>
+
+#include <string>
+
+#include "base/fuchsia/fuchsia_logging.h"
+#include "base/functional/callback.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
+
+namespace fidl {
+template <typename Protocol>
+class AsyncEventHandler;
+}
+
+namespace base {
+
+// An implementation of `fidl::AsyncEventhandler` that simply DLOGs an error
+// when `on_fidl_error` is called. The lifetime of an instance of this class
+// needs to match the lifetime of the `fidl::Client` that it is used with.
+template <typename Protocol>
+class LoggingFidlErrorEventHandler : public fidl::AsyncEventHandler<Protocol> {
+ public:
+  explicit LoggingFidlErrorEventHandler(
+      std::string protocol_name = fidl::DiscoverableProtocolName<Protocol>)
+      : protocol_name_(std::move(protocol_name)) {}
+
+  void on_fidl_error(fidl::UnbindInfo error) override {
+    DLOG(ERROR) << protocol_name_ << " was disconnected with "
+                << error.status_string() << ".";
+  }
+
+ private:
+  std::string protocol_name_;
+};
+
+// An implementation of `fidl::AsyncEventHandler` that invokes the
+// caller-supplied callback when `on_fidl_error` is called. The lifetime of an
+// instance of this class needs to match the lifetime of the `fidl::Client` that
+// it is used with.
+template <typename Protocol>
+class CallbackFidlErrorEventHandler : public fidl::AsyncEventHandler<Protocol> {
+ public:
+  using OnFidlErrorCallback = base::RepeatingCallback<void(fidl::UnbindInfo)>;
+
+  CallbackFidlErrorEventHandler() = delete;
+  explicit CallbackFidlErrorEventHandler(
+      OnFidlErrorCallback on_fidl_error_callback)
+      : on_fidl_error_callback_(std::move(on_fidl_error_callback)) {}
+
+  void on_fidl_error(fidl::UnbindInfo error) override {
+    on_fidl_error_callback_.Run(error);
+  }
+
+ private:
+  OnFidlErrorCallback on_fidl_error_callback_;
+};
+
+}  // namespace base
+
+#endif  // BASE_FUCHSIA_FIDL_EVENT_HANDLER_H_
diff --git a/base/fuchsia/fidl_event_handler_unittest.cc b/base/fuchsia/fidl_event_handler_unittest.cc
@@ -0,0 +1,159 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/fuchsia/fidl_event_handler.h"
+
+#include <fidl/base.testfidl/cpp/fidl.h>
+#include <fidl/fuchsia.logger/cpp/fidl.h>
+#include <lib/async/default.h>
+#include <lib/sys/cpp/component_context.h>
+
+#include "base/fuchsia/fuchsia_component_connect.h"
+#include "base/fuchsia/process_context.h"
+#include "base/fuchsia/scoped_service_binding.h"
+#include "base/fuchsia/test_component_context_for_process.h"
+#include "base/fuchsia/test_interface_natural_impl.h"
+#include "base/fuchsia/test_log_listener_safe.h"
+#include "base/task/thread_pool.h"
+#include "base/test/bind.h"
+#include "base/test/scoped_logging_settings.h"
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+constexpr char kBaseUnittestsExec[] = "base_unittests__exec";
+
+}  // namespace
+
+namespace base {
+
+class FidlEventHandlerTest : public testing::Test {
+ public:
+  FidlEventHandlerTest() {
+    test_context_.AddService(
+        fidl::DiscoverableProtocolName<fuchsia_logger::Log>);
+    ListenFilteredByCurrentProcessId(listener_);
+    // Initialize logging in the `scoped_logging_settings_`.
+    CHECK(logging::InitLogging({.logging_dest = logging::LOG_DEFAULT}));
+  }
+  FidlEventHandlerTest(const FidlEventHandlerTest&) = delete;
+  FidlEventHandlerTest& operator=(const FidlEventHandlerTest&) = delete;
+  ~FidlEventHandlerTest() override = default;
+
+ protected:
+  test::SingleThreadTaskEnvironment task_environment_{
+      test::SingleThreadTaskEnvironment::MainThreadType::IO};
+  SimpleTestLogListener listener_;
+
+  // Ensure that logging is directed to the system debug log.
+  logging::ScopedLoggingSettings scoped_logging_settings_;
+  TestComponentContextForProcess test_context_;
+  TestInterfaceNaturalImpl test_service_;
+};
+
+TEST_F(FidlEventHandlerTest, LoggingEventHandler) {
+  LoggingFidlErrorEventHandler<base_testfidl::TestInterface> event_handler;
+
+  event_handler.on_fidl_error(fidl::UnbindInfo::PeerClosed(ZX_ERR_PEER_CLOSED));
+
+  constexpr char kLogMessage[] =
+      "base.testfidl.TestInterface was disconnected with ZX_ERR_PEER_CLOSED";
+  absl::optional<fuchsia_logger::LogMessage> logged_message =
+      listener_.RunUntilMessageReceived(kLogMessage);
+
+  ASSERT_TRUE(logged_message.has_value());
+  EXPECT_EQ(logged_message->severity(),
+            static_cast<int32_t>(fuchsia_logger::LogLevelFilter::kError));
+  ASSERT_EQ(logged_message->tags().size(), 1u);
+  EXPECT_EQ(logged_message->tags()[0], kBaseUnittestsExec);
+}
+
+TEST_F(FidlEventHandlerTest, LoggingEventHandler_CustomProtocolName) {
+  LoggingFidlErrorEventHandler<base_testfidl::TestInterface> event_handler(
+      "test_protocol_name");
+
+  event_handler.on_fidl_error(fidl::UnbindInfo::PeerClosed(ZX_ERR_PEER_CLOSED));
+
+  constexpr char kLogMessage[] =
+      "test_protocol_name was disconnected with ZX_ERR_PEER_CLOSED";
+  absl::optional<fuchsia_logger::LogMessage> logged_message =
+      listener_.RunUntilMessageReceived(kLogMessage);
+
+  ASSERT_TRUE(logged_message.has_value());
+  EXPECT_EQ(logged_message->severity(),
+            static_cast<int32_t>(fuchsia_logger::LogLevelFilter::kError));
+  ASSERT_EQ(logged_message->tags().size(), 1u);
+  EXPECT_EQ(logged_message->tags()[0], kBaseUnittestsExec);
+}
+
+TEST_F(FidlEventHandlerTest, LoggingEventHandler_LogsOnServiceClosure) {
+  LoggingFidlErrorEventHandler<base_testfidl::TestInterface> event_handler;
+  auto client_end = fuchsia_component::ConnectAt<base_testfidl::TestInterface>(
+      test_context_.published_services_natural());
+  EXPECT_TRUE(client_end.is_ok());
+  fidl::Client client(std::move(*client_end), async_get_default_dispatcher(),
+                      &event_handler);
+
+  {
+    ScopedNaturalServiceBinding<base_testfidl::TestInterface> binding(
+        ComponentContextForProcess()->outgoing().get(), &test_service_,
+        async_get_default_dispatcher());
+
+    ASSERT_EQ(ZX_OK, VerifyTestInterface(client));
+  };
+
+  constexpr char kLogMessage[] =
+      "base.testfidl.TestInterface was disconnected with ZX_ERR_PEER_CLOSED";
+  absl::optional<fuchsia_logger::LogMessage> logged_message =
+      listener_.RunUntilMessageReceived(kLogMessage);
+
+  ASSERT_TRUE(logged_message.has_value());
+  EXPECT_EQ(logged_message->severity(),
+            static_cast<int32_t>(fuchsia_logger::LogLevelFilter::kError));
+  ASSERT_EQ(logged_message->tags().size(), 1u);
+  EXPECT_EQ(logged_message->tags()[0], kBaseUnittestsExec);
+}
+
+TEST_F(FidlEventHandlerTest, CallbackEventHandler) {
+  RunLoop loop;
+  CallbackFidlErrorEventHandler<base_testfidl::TestInterface> event_handler(
+      base::BindLambdaForTesting(
+          [quit_closure = loop.QuitClosure()](fidl::UnbindInfo error) {
+            ASSERT_TRUE(error.is_peer_closed());
+            quit_closure.Run();
+          }));
+
+  event_handler.on_fidl_error(fidl::UnbindInfo::PeerClosed(ZX_ERR_PEER_CLOSED));
+
+  loop.Run();
+}
+
+TEST_F(FidlEventHandlerTest, CallbackEventHandler_FiresOnServiceClosure) {
+  RunLoop loop;
+  CallbackFidlErrorEventHandler<base_testfidl::TestInterface> event_handler(
+      base::BindLambdaForTesting(
+          [quit_closure = loop.QuitClosure()](fidl::UnbindInfo error) {
+            ASSERT_TRUE(error.is_peer_closed());
+            quit_closure.Run();
+          }));
+
+  auto client_end = fuchsia_component::ConnectAt<base_testfidl::TestInterface>(
+      test_context_.published_services_natural());
+  EXPECT_TRUE(client_end.is_ok());
+  fidl::Client client(std::move(*client_end), async_get_default_dispatcher(),
+                      &event_handler);
+
+  {
+    ScopedNaturalServiceBinding<base_testfidl::TestInterface> binding(
+        ComponentContextForProcess()->outgoing().get(), &test_service_,
+        async_get_default_dispatcher());
+
+    ASSERT_EQ(ZX_OK, VerifyTestInterface(client));
+  };
+
+  loop.Run();
+}
+
+}  // namespace base
diff --git a/base/fuchsia/fuchsia_logging.h b/base/fuchsia/fuchsia_logging.h
@@ -14,6 +14,7 @@
 #include <string>
 
 #include "base/base_export.h"
+#include "base/check.h"
 #include "base/logging.h"
 #include "base/strings/string_piece_forward.h"
 #include "base/strings/stringprintf.h"

diff --git a/base/fuchsia/fuchsia_logging_unittest.cc b/base/fuchsia/fuchsia_logging_unittest.cc
@@ -14,7 +14,6 @@
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/fuchsia/test_log_listener_safe.h"
 #include "base/logging.h"
-#include "base/process/process.h"
 #include "base/test/scoped_logging_settings.h"
 #include "base/test/task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -29,23 +28,6 @@ class MockLogSource {
   MOCK_METHOD0(Log, const char*());
 };
 
-// Configures `listener` to listen for messages from the current process.
-void ListenFilteredByPid(SimpleTestLogListener& listener) {
-  // Connect the test LogListenerSafe to the Log.
-  auto log_client_end = fuchsia_component::Connect<fuchsia_logger::Log>();
-  EXPECT_TRUE(log_client_end.is_ok())
-      << FidlConnectionErrorMessage(log_client_end);
-  fidl::Client log_client(std::move(log_client_end.value()),
-                          async_get_default_dispatcher());
-  listener.ListenToLog(
-      log_client,
-      std::make_unique<fuchsia_logger::LogFilterOptions>(
-          fuchsia_logger::LogFilterOptions{
-              {.filter_by_pid = true,
-               .pid = Process::Current().Pid(),
-               .min_severity = fuchsia_logger::LogLevelFilter::kInfo}}));
-}
-
 }  // namespace
 
 // Verifies that calling the log macro goes to the Fuchsia system logs, by
@@ -56,7 +38,7 @@ TEST(FuchsiaLoggingTest, SystemLogging) {
   test::SingleThreadTaskEnvironment task_environment_{
       test::SingleThreadTaskEnvironment::MainThreadType::IO};
   SimpleTestLogListener listener;
-  ListenFilteredByPid(listener);
+  ListenFilteredByCurrentProcessId(listener);
 
   // Ensure that logging is directed to the system debug log.
   logging::ScopedLoggingSettings scoped_logging_settings;
@@ -86,7 +68,7 @@ TEST(FuchsiaLoggingTest, SystemLoggingMultipleTags) {
   test::SingleThreadTaskEnvironment task_environment_{
       test::SingleThreadTaskEnvironment::MainThreadType::IO};
   SimpleTestLogListener listener;
-  ListenFilteredByPid(listener);
+  ListenFilteredByCurrentProcessId(listener);
 
   // Connect the test LogListenerSafe to the Log.
   auto log_sink_client_end =

diff --git a/base/fuchsia/test_log_listener_safe.cc b/base/fuchsia/test_log_listener_safe.cc
@@ -8,8 +8,10 @@
 #include <lib/fidl/cpp/box.h>
 #include <lib/zx/clock.h>
 
+#include "base/fuchsia/fuchsia_component_connect.h"
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/functional/callback_helpers.h"
+#include "base/process/process.h"
 #include "base/run_loop.h"
 #include "base/strings/string_piece.h"
 #include "base/test/bind.h"
@@ -125,4 +127,20 @@ void SimpleTestLogListener::PushLoggedMessage(
   }
 }
 
+void ListenFilteredByCurrentProcessId(SimpleTestLogListener& listener) {
+  // Connect the test LogListenerSafe to the Log.
+  auto log_client_end = fuchsia_component::Connect<fuchsia_logger::Log>();
+  ASSERT_TRUE(log_client_end.is_ok())
+      << FidlConnectionErrorMessage(log_client_end);
+  fidl::Client log_client(std::move(log_client_end.value()),
+                          async_get_default_dispatcher());
+  listener.ListenToLog(
+      log_client,
+      std::make_unique<fuchsia_logger::LogFilterOptions>(
+          fuchsia_logger::LogFilterOptions{
+              {.filter_by_pid = true,
+               .pid = Process::Current().Pid(),
+               .min_severity = fuchsia_logger::LogLevelFilter::kInfo}}));
+}
+
 }  // namespace base
diff --git a/base/fuchsia/test_log_listener_safe.h b/base/fuchsia/test_log_listener_safe.h
@@ -7,8 +7,10 @@
 
 #include <fidl/fuchsia.logger/cpp/fidl.h>
 
+#include <lib/async/default.h>
 #include <lib/fidl/cpp/binding.h>
 #include <lib/zx/time.h>
+
 #include <memory>
 #include <string>
 #include <vector>
@@ -87,6 +89,9 @@ class SimpleTestLogListener {
   TestLogListenerSafe::OnLogMessageCallback on_log_message_;
 };
 
+// Configures `listener` to listen for messages from the current process.
+void ListenFilteredByCurrentProcessId(SimpleTestLogListener& listener);
+
 }  // namespace base
 
 #endif  // BASE_FUCHSIA_TEST_LOG_LISTENER_SAFE_H_