Skip to content

Commit

Permalink
Revert "[fuchsia] Add basic implementations of fidl::AsyncEventHandler"
Browse files Browse the repository at this point in the history
This reverts commit f412da9.

Reason for revert: FidlEventHandlerTest is failing on bots:

https://ci.chromium.org/ui/p/chromium/builders/ci/fuchsia-x64-rel/4408/overview
https://ci.chromium.org/ui/p/chromium/builders/ci/fuchsia-arm64-rel/4835/overview

Original change's description:
> [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}

Bug: 1351487
Change-Id: I04bf251545eb950475e75b629f4c9eae17b5fd29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4349565
Owners-Override: Tim Sergeant <tsergeant@chromium.org>
Commit-Queue: Tim Sergeant <tsergeant@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1119136}
  • Loading branch information
tgsergeant authored and Chromium LUCI CQ committed Mar 19, 2023
1 parent 9b90c5a commit 1a79da6
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 254 deletions.
2 changes: 0 additions & 2 deletions base/BUILD.gn
Expand Up @@ -1404,7 +1404,6 @@ 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",
Expand Down Expand Up @@ -3878,7 +3877,6 @@ 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",
Expand Down
67 changes: 0 additions & 67 deletions base/fuchsia/fidl_event_handler.h

This file was deleted.

159 changes: 0 additions & 159 deletions base/fuchsia/fidl_event_handler_unittest.cc

This file was deleted.

1 change: 0 additions & 1 deletion base/fuchsia/fuchsia_logging.h
Expand Up @@ -14,7 +14,6 @@
#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"
Expand Down
22 changes: 20 additions & 2 deletions base/fuchsia/fuchsia_logging_unittest.cc
Expand Up @@ -14,6 +14,7 @@
#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"
Expand All @@ -28,6 +29,23 @@ 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
Expand All @@ -38,7 +56,7 @@ TEST(FuchsiaLoggingTest, SystemLogging) {
test::SingleThreadTaskEnvironment task_environment_{
test::SingleThreadTaskEnvironment::MainThreadType::IO};
SimpleTestLogListener listener;
ListenFilteredByCurrentProcessId(listener);
ListenFilteredByPid(listener);

// Ensure that logging is directed to the system debug log.
logging::ScopedLoggingSettings scoped_logging_settings;
Expand Down Expand Up @@ -68,7 +86,7 @@ TEST(FuchsiaLoggingTest, SystemLoggingMultipleTags) {
test::SingleThreadTaskEnvironment task_environment_{
test::SingleThreadTaskEnvironment::MainThreadType::IO};
SimpleTestLogListener listener;
ListenFilteredByCurrentProcessId(listener);
ListenFilteredByPid(listener);

// Connect the test LogListenerSafe to the Log.
auto log_sink_client_end =
Expand Down
18 changes: 0 additions & 18 deletions base/fuchsia/test_log_listener_safe.cc
Expand Up @@ -8,10 +8,8 @@
#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"
Expand Down Expand Up @@ -127,20 +125,4 @@ 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
5 changes: 0 additions & 5 deletions base/fuchsia/test_log_listener_safe.h
Expand Up @@ -7,10 +7,8 @@

#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>
Expand Down Expand Up @@ -89,9 +87,6 @@ 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_

0 comments on commit 1a79da6

Please sign in to comment.