[fuchsia] Migrate base/fuchsia logger usages to natural FIDL bindings

Bug: 1351487 Change-Id: I808431f8b64837b5a7272d6d5ef799595aab8db0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4324165 Reviewed-by: David Dorwin <ddorwin@chromium.org> Commit-Queue: Bryant Chandler <bryantchandler@chromium.org> Cr-Commit-Position: refs/heads/main@{#1117803}
qweri0p · Mar 15, 2023 · d328b2d · d328b2d
1 parent 7f93742
commit d328b2d
Show file tree

Hide file tree

Showing 13 changed files with 181 additions and 128 deletions.
diff --git a/base/BUILD.gn b/base/BUILD.gn
@@ -1480,7 +1480,7 @@ component("base") {
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.intl:fuchsia.intl_hlcpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.io:fuchsia.io_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.io:fuchsia.io_hlcpp",
-      "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.logger:fuchsia.logger_hlcpp",
+      "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.logger:fuchsia.logger_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.mem:fuchsia.mem_hlcpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.process.lifecycle:fuchsia.process.lifecycle_cpp",
       "//third_party/fuchsia-sdk/sdk/pkg/async",
@@ -3903,7 +3903,7 @@ test("base_unittests") {
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.buildinfo:fuchsia.buildinfo_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.hwinfo:fuchsia.hwinfo_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.intl:fuchsia.intl_hlcpp",
-      "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.logger:fuchsia.logger_hlcpp",
+      "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.logger:fuchsia.logger_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.mem:fuchsia.mem_hlcpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.sys:fuchsia.sys_cpp",
       "//third_party/fuchsia-sdk/sdk/fidl/fuchsia.sys:fuchsia.sys_hlcpp",

diff --git a/base/fuchsia/fuchsia_component_connect.h b/base/fuchsia/fuchsia_component_connect.h
@@ -22,6 +22,16 @@
 // more details (Googlers only).
 namespace base::fuchsia_component {
 
+template <typename Protocol,
+          typename = std::enable_if_t<fidl::IsProtocolV<Protocol>>>
+BASE_EXPORT zx::result<> Connect(
+    fidl::ServerEnd<Protocol> server_end,
+    std::string name = fidl::DiscoverableProtocolName<Protocol>) {
+  return component::ConnectAt<Protocol>(
+      base::BorrowIncomingServiceDirectoryForProcess(), std::move(server_end),
+      name);
+}
+
 template <typename Protocol,
           typename = std::enable_if_t<fidl::IsProtocolV<Protocol>>>
 BASE_EXPORT zx::result<fidl::ClientEnd<Protocol>> Connect(

diff --git a/base/fuchsia/fuchsia_logging_unittest.cc b/base/fuchsia/fuchsia_logging_unittest.cc
@@ -5,12 +5,13 @@
 #include "base/fuchsia/scoped_fx_logger.h"
 
 #include <fidl/base.testfidl/cpp/fidl.h>
-#include <fuchsia/logger/cpp/fidl.h>
+#include <fidl/fuchsia.logger/cpp/fidl.h>
+#include <lib/async/default.h>
 #include <lib/fidl/cpp/binding.h>
 #include <lib/sys/cpp/component_context.h>
 
+#include "base/fuchsia/fuchsia_component_connect.h"
 #include "base/fuchsia/fuchsia_logging.h"
-#include "base/fuchsia/process_context.h"
 #include "base/fuchsia/test_log_listener_safe.h"
 #include "base/logging.h"
 #include "base/process/process.h"
@@ -28,17 +29,21 @@ class MockLogSource {
   MOCK_METHOD0(Log, const char*());
 };
 
-// Configures |listener| to listen for messages from the current process.
+// Configures `listener` to listen for messages from the current process.
 void ListenFilteredByPid(SimpleTestLogListener& listener) {
   // Connect the test LogListenerSafe to the Log.
-  std::unique_ptr<fuchsia::logger::LogFilterOptions> options =
-      std::make_unique<fuchsia::logger::LogFilterOptions>();
-  options->filter_by_pid = true;
-  options->pid = Process::Current().Pid();
-  options->min_severity = fuchsia::logger::LogLevelFilter::INFO;
-  fuchsia::logger::LogPtr log =
-      ComponentContextForProcess()->svc()->Connect<fuchsia::logger::Log>();
-  listener.ListenToLog(log.get(), std::move(options));
+  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
@@ -61,15 +66,15 @@ TEST(FuchsiaLoggingTest, SystemLogging) {
   // test listener.
   LOG(ERROR) << kLogMessage;
 
-  absl::optional<fuchsia::logger::LogMessage> logged_message =
+  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::ERROR));
-  ASSERT_EQ(logged_message->tags.size(), 1u);
+  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], "base_unittests__exec");
+  EXPECT_EQ(logged_message->tags()[0], "base_unittests__exec");
 }
 
 // Verifies that configuring a system logger with multiple tags works.
@@ -83,18 +88,23 @@ TEST(FuchsiaLoggingTest, SystemLoggingMultipleTags) {
   SimpleTestLogListener listener;
   ListenFilteredByPid(listener);
 
+  // Connect the test LogListenerSafe to the Log.
+  auto log_sink_client_end =
+      fuchsia_component::Connect<fuchsia_logger::LogSink>();
+  EXPECT_TRUE(log_sink_client_end.is_ok())
+      << FidlConnectionErrorMessage(log_sink_client_end);
+
   // Create a logger with multiple tags and emit a message to it.
   ScopedFxLogger logger = ScopedFxLogger::CreateFromLogSink(
-      ComponentContextForProcess()->svc()->Connect<fuchsia::logger::LogSink>(),
-      kTags);
+      std::move(log_sink_client_end.value()), kTags);
   logger.LogMessage("", 0, kLogMessage, FUCHSIA_LOG_ERROR);
 
-  absl::optional<fuchsia::logger::LogMessage> logged_message =
+  absl::optional<fuchsia_logger::LogMessage> logged_message =
       listener.RunUntilMessageReceived(kLogMessage);
 
   ASSERT_TRUE(logged_message.has_value());
-  auto tags = std::vector<StringPiece>(logged_message->tags.begin(),
-                                       logged_message->tags.end());
+  auto tags = std::vector<StringPiece>(logged_message->tags().begin(),
+                                       logged_message->tags().end());
   EXPECT_EQ(tags, kTags);
 }
 

diff --git a/base/fuchsia/scoped_fx_logger.cc b/base/fuchsia/scoped_fx_logger.cc
@@ -4,11 +4,13 @@
 
 #include "base/fuchsia/scoped_fx_logger.h"
 
+#include <lib/component/incoming/cpp/protocol.h>
 #include <lib/fdio/directory.h>
 #include <stdio.h>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/fuchsia/fuchsia_component_connect.h"
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/process/process.h"
 #include "base/strings/string_piece.h"
@@ -28,12 +30,9 @@ ScopedFxLogger ScopedFxLogger::CreateForProcess(
   // CHECK()ing or LOG()ing inside this function is safe, since it is only
   // called to initialize logging, not during individual logging operations.
 
-  fuchsia::logger::LogSinkHandle log_sink;
-  zx_status_t status =
-      fdio_service_connect("/svc/fuchsia.logger.LogSink",
-                           log_sink.NewRequest().TakeChannel().release());
-  if (status != ZX_OK) {
-    ZX_LOG(ERROR, status) << "connect(LogSink) failed";
+  auto log_sink_client_end = component::Connect<fuchsia_logger::LogSink>();
+  if (log_sink_client_end.is_error()) {
+    LOG(ERROR) << FidlConnectionErrorMessage(log_sink_client_end);
     return {};
   }
 
@@ -47,27 +46,31 @@ ScopedFxLogger ScopedFxLogger::CreateForProcess(
                                  .AsUTF8Unsafe();
   tags.insert(tags.begin(), program_name);
 
-  return CreateFromLogSink(std::move(log_sink), std::move(tags));
+  return CreateFromLogSink(std::move(log_sink_client_end.value()),
+                           std::move(tags));
 }
 
 // static
 ScopedFxLogger ScopedFxLogger::CreateFromLogSink(
-    fuchsia::logger::LogSinkHandle log_sink_handle,
+    fidl::ClientEnd<fuchsia_logger::LogSink> log_sink_client_end,
     std::vector<base::StringPiece> tags) {
   // CHECK()ing or LOG()ing inside this function is safe, since it is only
   // called to initialize logging, not during individual logging operations.
 
   // Attempts to create a kernel socket object should never fail.
   zx::socket local, remote;
-  zx_status_t status = zx::socket::create(ZX_SOCKET_DATAGRAM, &local, &remote);
-  ZX_CHECK(status == ZX_OK, status) << "zx_socket_create() failed";
+  zx_status_t socket_status =
+      zx::socket::create(ZX_SOCKET_DATAGRAM, &local, &remote);
+  ZX_CHECK(socket_status == ZX_OK, socket_status)
+      << "zx_socket_create() failed";
 
   // ConnectStructured() may fail if e.g. the LogSink has disconnected already.
-  fuchsia::logger::LogSinkSyncPtr log_sink;
-  log_sink.Bind(std::move(log_sink_handle));
-  status = log_sink->ConnectStructured(std::move(remote));
-  if (status != ZX_OK) {
-    ZX_LOG(ERROR, status) << "ConnectStructured() failed";
+  fidl::SyncClient log_sink(std::move(log_sink_client_end));
+  auto connect_structured_result =
+      log_sink->ConnectStructured(std::move(remote));
+  if (connect_structured_result.is_error()) {
+    ZX_LOG(ERROR, connect_structured_result.error_value().status())
+        << "ConnectStructured() failed";
     return {};
   }
 

diff --git a/base/fuchsia/scoped_fx_logger.h b/base/fuchsia/scoped_fx_logger.h
@@ -5,7 +5,7 @@
 #ifndef BASE_FUCHSIA_SCOPED_FX_LOGGER_H_
 #define BASE_FUCHSIA_SCOPED_FX_LOGGER_H_
 
-#include <fuchsia/logger/cpp/fidl.h>
+#include <fidl/fuchsia.logger/cpp/fidl.h>
 #include <lib/syslog/structured_backend/cpp/fuchsia_syslog.h>
 #include <lib/zx/socket.h>
 
@@ -32,13 +32,13 @@ class BASE_EXPORT ScopedFxLogger {
   // Returns an instance connected to the process' incoming LogSink service.
   // The returned instance has a single tag attributing the calling process in
   // some way (e.g. by Component or process name).
-  // Additional tags may optionally be specified via |tags|.
+  // Additional tags may optionally be specified via `tags`.
   static ScopedFxLogger CreateForProcess(
       std::vector<base::StringPiece> tags = {});
 
   // Returns an instance connected to the specified LogSink.
   static ScopedFxLogger CreateFromLogSink(
-      fuchsia::logger::LogSinkHandle,
+      fidl::ClientEnd<fuchsia_logger::LogSink> client_end,
       std::vector<base::StringPiece> tags = {});
 
   void LogMessage(base::StringPiece file,

diff --git a/base/fuchsia/test_log_listener_safe.cc b/base/fuchsia/test_log_listener_safe.cc
@@ -4,6 +4,10 @@
 
 #include "base/fuchsia/test_log_listener_safe.h"
 
+#include <lib/async/default.h>
+#include <lib/fidl/cpp/box.h>
+#include <lib/zx/clock.h>
+
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/functional/callback_helpers.h"
 #include "base/run_loop.h"
@@ -19,90 +23,100 @@ TestLogListenerSafe::TestLogListenerSafe() = default;
 TestLogListenerSafe::~TestLogListenerSafe() = default;
 
 void TestLogListenerSafe::set_on_log_message(
-    base::RepeatingCallback<void(const fuchsia::logger::LogMessage&)>
-        callback) {
+    base::RepeatingCallback<void(const fuchsia_logger::LogMessage&)> callback) {
   on_log_message_ = std::move(callback);
 }
 
-void TestLogListenerSafe::Log(fuchsia::logger::LogMessage message,
-                              LogCallback callback) {
+void TestLogListenerSafe::Log(
+    TestLogListenerSafe::LogRequest& request,
+    TestLogListenerSafe::LogCompleter::Sync& completer) {
   if (on_log_message_)
-    on_log_message_.Run(message);
-  callback();
+    on_log_message_.Run(request.log());
+  completer.Reply();
 }
 
 void TestLogListenerSafe::LogMany(
-    std::vector<fuchsia::logger::LogMessage> messages,
-    LogManyCallback callback) {
-  for (const auto& message : messages)
+    TestLogListenerSafe::LogManyRequest& request,
+    TestLogListenerSafe::LogManyCompleter::Sync& completer) {
+  for (const auto& message : request.log()) {
     on_log_message_.Run(message);
-  callback();
-}
-
-void TestLogListenerSafe::Done() {}
-
-void TestLogListenerSafe::NotImplemented_(const std::string& name) {
-  ADD_FAILURE() << "NotImplemented_: " << name;
+  }
+  completer.Reply();
 }
 
-SimpleTestLogListener::SimpleTestLogListener() : binding_(&listener_) {}
+void TestLogListenerSafe::Done(
+    TestLogListenerSafe::DoneCompleter::Sync& completer) {}
 
+SimpleTestLogListener::SimpleTestLogListener() = default;
 SimpleTestLogListener::~SimpleTestLogListener() = default;
 
 void SimpleTestLogListener::ListenToLog(
-    fuchsia::logger::Log* log,
-    std::unique_ptr<fuchsia::logger::LogFilterOptions> options) {
+    const fidl::Client<fuchsia_logger::Log>& log,
+    std::unique_ptr<fuchsia_logger::LogFilterOptions> options) {
+  auto listener_endpoints =
+      fidl::CreateEndpoints<fuchsia_logger::LogListenerSafe>();
+  ZX_CHECK(listener_endpoints.is_ok(), listener_endpoints.status_value())
+      << "Failed to create listener endpoints";
+  binding_.emplace(
+      async_get_default_dispatcher(), std::move(listener_endpoints->server),
+      &listener_, [](fidl::UnbindInfo info) {
+        ZX_LOG(ERROR, info.status()) << "LogListenerSafe disconnected";
+      });
+
   ignore_before_ = zx::clock::get_monotonic();
   listener_.set_on_log_message(base::BindRepeating(
       &SimpleTestLogListener::PushLoggedMessage, base::Unretained(this)));
-  log->ListenSafe(binding_.NewBinding(), std::move(options));
+  auto listen_safe_result =
+      log->ListenSafe({{.log_listener = std::move(listener_endpoints->client),
+                        .options = std::move(options)}});
+  if (listen_safe_result.is_error()) {
+    ZX_DLOG(ERROR, listen_safe_result.error_value().status())
+        << "ListenSafe() failed";
+  }
 }
 
-absl::optional<fuchsia::logger::LogMessage>
+absl::optional<fuchsia_logger::LogMessage>
 SimpleTestLogListener::RunUntilMessageReceived(
     base::StringPiece expected_string) {
   while (!logged_messages_.empty()) {
-    fuchsia::logger::LogMessage message = logged_messages_.front();
+    fuchsia_logger::LogMessage message = logged_messages_.front();
     logged_messages_.pop_front();
-    if (base::StringPiece(message.msg).find(expected_string) !=
+    if (base::StringPiece(message.msg()).find(expected_string) !=
         std::string::npos) {
       return message;
     }
   }
 
-  absl::optional<fuchsia::logger::LogMessage> logged_message;
+  absl::optional<fuchsia_logger::LogMessage> logged_message;
   base::RunLoop loop;
-  binding_.set_error_handler(
-      [quit_loop = loop.QuitClosure()](zx_status_t status) {
-        ZX_LOG(ERROR, status) << "LogListenerSafe disconnected";
-        quit_loop.Run();
-      });
   on_log_message_ = base::BindLambdaForTesting(
       [ignore_before = ignore_before_, &logged_message,
        expected_string = std::string(expected_string),
        quit_loop =
-           loop.QuitClosure()](const fuchsia::logger::LogMessage& message) {
-        if (zx::time(message.time) < ignore_before)
+           loop.QuitClosure()](const fuchsia_logger::LogMessage& message) {
+        if (zx::time(message.time()) < ignore_before) {
           return;
-        if (message.msg.find(expected_string) == std::string::npos)
+        }
+        if (message.msg().find(expected_string) == std::string::npos) {
           return;
+        }
         logged_message.emplace(message);
         quit_loop.Run();
       });
 
   loop.Run();
 
-  binding_.set_error_handler({});
   on_log_message_ = NullCallback();
 
   return logged_message;
 }
 
 void SimpleTestLogListener::PushLoggedMessage(
-    const fuchsia::logger::LogMessage& message) {
-  DVLOG(1) << "TestLogListener received: " << message.msg;
-  if (zx::time(message.time) < ignore_before_)
+    const fuchsia_logger::LogMessage& message) {
+  DVLOG(1) << "TestLogListener received: " << message.msg();
+  if (zx::time(message.time()) < ignore_before_) {
     return;
+  }
   if (on_log_message_) {
     DCHECK(logged_messages_.empty());
     on_log_message_.Run(message);