From a40fe5165415ab509a42ebd4282f5843a691f1b7 Mon Sep 17 00:00:00 2001 From: Simon Fels Date: Fri, 25 Nov 2016 08:50:43 +0100 Subject: [PATCH] Use events instead of method invokes for window state updates and others --- android/service/local_socket_connection.cpp | 2 +- android/service/platform_api_stub.cpp | 71 ++++++++----------- android/service/platform_api_stub.h | 33 ++++++--- android/service/platform_service.cpp | 42 ++++++----- android/service/platform_service.h | 12 +--- src/anbox/bridge/platform_api_skeleton.cpp | 39 ++++------ src/anbox/bridge/platform_api_skeleton.h | 17 ++--- .../bridge/platform_message_processor.cpp | 18 +++-- src/anbox/cmds/run.cpp | 3 +- src/anbox/protobuf/anbox_bridge.proto | 19 ++++- src/anbox/protobuf/anbox_rpc.proto | 1 + src/anbox/rpc/channel.cpp | 20 ++++-- src/anbox/rpc/channel.h | 4 +- src/anbox/rpc/message_processor.cpp | 6 +- 14 files changed, 153 insertions(+), 134 deletions(-) diff --git a/android/service/local_socket_connection.cpp b/android/service/local_socket_connection.cpp index 9b8dbea5..c2ff0402 100644 --- a/android/service/local_socket_connection.cpp +++ b/android/service/local_socket_connection.cpp @@ -23,7 +23,7 @@ #include #include -#define LOG_TAG "Anbox" +#define LOG_TAG "Anboxd" #include namespace { diff --git a/android/service/platform_api_stub.cpp b/android/service/platform_api_stub.cpp index e18e0456..fd2d6059 100644 --- a/android/service/platform_api_stub.cpp +++ b/android/service/platform_api_stub.cpp @@ -21,59 +21,44 @@ #include "anbox_rpc.pb.h" #include "anbox_bridge.pb.h" -#define LOG_TAG "Anbox" -#include - namespace anbox { PlatformApiStub::PlatformApiStub(const std::shared_ptr &rpc_channel) : rpc_channel_(rpc_channel) { } void PlatformApiStub::boot_finished() { - auto c = std::make_shared>(); - - ALOGI("Boot finished"); - - { - std::lock_guard lock(mutex_); - boot_finished_wait_handle_.expect_result(); - } - - protobuf::rpc::Void message; - - rpc_channel_->call_method( - "boot_finished", - &message, c->response.get(), - google::protobuf::NewCallback(this, &PlatformApiStub::handle_boot_finished_response, c.get())); - - boot_finished_wait_handle_.wait_for_all(); - - ALOGI("Boot finished sent successfully!"); + protobuf::bridge::EventSequence seq; + auto event = seq.mutable_boot_finished(); + (void) event; + rpc_channel_->send_event(seq); } -void PlatformApiStub::handle_boot_finished_response(Request*) { - boot_finished_wait_handle_.result_received(); -} - -void PlatformApiStub::update_window_state(const anbox::protobuf::bridge::WindowStateUpdate &window_state) { - auto c = std::make_shared>(); - - ALOGI("Updating window state"); - - { - std::lock_guard lock(mutex_); - update_window_state_wait_handle_.expect_result(); +void PlatformApiStub::update_window_state(const WindowStateUpdate &state) { + protobuf::bridge::EventSequence seq; + auto event = seq.mutable_window_state_update(); + + auto convert_window = [](WindowStateUpdate::Window in, anbox::protobuf::bridge::WindowStateUpdateEvent_WindowState *out) { + out->set_display_id(in.display_id); + out->set_has_surface(in.has_surface); + out->set_package_name(in.package_name); + out->set_frame_left(in.frame.left); + out->set_frame_top(in.frame.top); + out->set_frame_right(in.frame.right); + out->set_frame_bottom(in.frame.bottom); + out->set_task_id(in.task_id); + out->set_stack_id(in.stack_id); + }; + + for (const auto &window : state.updated_windows) { + auto w = event->add_windows(); + convert_window(window, w); } - rpc_channel_->call_method( - "update_window_state", - &window_state, c->response.get(), - google::protobuf::NewCallback(this, &PlatformApiStub::handle_update_window_state_response, c.get())); - - update_window_state_wait_handle_.wait_for_all(); -} + for (const auto &window : state.removed_windows) { + auto w = event->add_removed_windows(); + convert_window(window, w); + } -void PlatformApiStub::handle_update_window_state_response(Request *request) { - update_window_state_wait_handle_.result_received(); + rpc_channel_->send_event(seq); } } // namespace anbox diff --git a/android/service/platform_api_stub.h b/android/service/platform_api_stub.h index e13a99ce..436facff 100644 --- a/android/service/platform_api_stub.h +++ b/android/service/platform_api_stub.h @@ -21,16 +21,14 @@ #include "anbox/common/wait_handle.h" #include +#include +#include namespace anbox { namespace protobuf { namespace rpc { class Void; -class WindowStateUpdate; } // namespace rpc -namespace bridge { -class WindowStateUpdate; -} // namespace bridge } // namespace protobuf namespace rpc { class Channel; @@ -40,7 +38,27 @@ public: PlatformApiStub(const std::shared_ptr &rpc_channel); void boot_finished(); - void update_window_state(const anbox::protobuf::bridge::WindowStateUpdate &window_state); + + struct WindowStateUpdate { + struct Window { + int display_id; + bool has_surface; + std::string package_name; + struct Frame { + int left; + int top; + int right; + int bottom; + }; + Frame frame; + int task_id; + int stack_id; + }; + std::vector updated_windows; + std::vector removed_windows; + }; + + void update_window_state(const WindowStateUpdate &state); private: template @@ -50,12 +68,7 @@ private: bool success; }; - void handle_boot_finished_response(Request *request); - void handle_update_window_state_response(Request *request); - mutable std::mutex mutex_; - common::WaitHandle boot_finished_wait_handle_; - common::WaitHandle update_window_state_wait_handle_; std::shared_ptr rpc_channel_; }; diff --git a/android/service/platform_service.cpp b/android/service/platform_service.cpp index 2c80f9b3..8ab0013e 100644 --- a/android/service/platform_service.cpp +++ b/android/service/platform_service.cpp @@ -16,7 +16,6 @@ */ #include "android/service/platform_service.h" -#include "android/service/platform_api_stub.h" #include "anbox/rpc/channel.h" #include "anbox_rpc.pb.h" @@ -39,8 +38,8 @@ status_t PlatformService::boot_finished() { return OK; } -void PlatformService::unpack_window_state(anbox::protobuf::bridge::WindowStateUpdate_WindowState *window, const Parcel &data) { - window->set_has_surface(data.readByte() != 0); +anbox::PlatformApiStub::WindowStateUpdate::Window PlatformService::unpack_window_state(const Parcel &data) { + bool has_surface = data.readByte() != 0; String8 package_name(data.readString16()); @@ -51,38 +50,45 @@ void PlatformService::unpack_window_state(anbox::protobuf::bridge::WindowStateUp auto task_id = data.readInt32(); auto stack_id = data.readInt32(); - window->set_package_name(package_name.string()); - window->set_frame_left(frame_left); - window->set_frame_top(frame_top); - window->set_frame_right(frame_right); - window->set_frame_bottom(frame_bottom); - window->set_task_id(task_id); - window->set_stack_id(stack_id); + ALOGI(" Window: package=%s frame={%d,%d,%d,%d} task=%d stack=%d", + package_name.string(), frame_left, frame_top, frame_right, frame_bottom, + task_id, stack_id); + + return anbox::PlatformApiStub::WindowStateUpdate::Window{ + -1, // Display id will be added by the caller + has_surface, + package_name.string(), + {frame_left, frame_top, frame_right, frame_bottom}, + task_id, + stack_id, + }; } status_t PlatformService::update_window_state(const Parcel &data) { - anbox::protobuf::bridge::WindowStateUpdate window_state; + anbox::PlatformApiStub::WindowStateUpdate state; + ALOGI("Udated windows:"); const auto num_displays = data.readInt32(); for (auto n = 0; n < num_displays; n++) { const auto display_id = data.readInt32(); const auto num_windows = data.readInt32(); + ALOGI(" Display: id=%d", display_id); for (auto m = 0; m < num_windows; m++) { - auto window = window_state.add_windows(); - window->set_display_id(display_id); - unpack_window_state(window, data); + auto window = unpack_window_state(data); + window.display_id = display_id; + state.updated_windows.push_back(window); } } + ALOGI("Removed windows:"); const auto num_removed_windows = data.readInt32(); for (auto n = 0; n < num_removed_windows; n++) { - auto window = window_state.add_removed_windows(); - window->set_display_id(0); - unpack_window_state(window, data); + auto window = unpack_window_state(data); + state.removed_windows.push_back(window); } - platform_api_stub_->update_window_state(window_state); + platform_api_stub_->update_window_state(state); return OK; } diff --git a/android/service/platform_service.h b/android/service/platform_service.h index 2578a587..8a4eadf5 100644 --- a/android/service/platform_service.h +++ b/android/service/platform_service.h @@ -19,20 +19,12 @@ #define ANBOX_ANDROID_PLATFORM_SERVICE_H_ #include "android/service/platform_service_interface.h" +#include "android/service/platform_api_stub.h" #include #include -namespace anbox { -class PlatformApiStub; -namespace protobuf { -namespace bridge { -class WindowStateUpdate_WindowState; -} // namespace bridge -} // namespace protobuf -} // namespace anbox - namespace android { class PlatformService : public BnPlatformService { public: @@ -44,7 +36,7 @@ public: status_t update_window_state(const Parcel &data) override; private: - void unpack_window_state(anbox::protobuf::bridge::WindowStateUpdate_WindowState *window, const Parcel &data); + anbox::PlatformApiStub::WindowStateUpdate::Window unpack_window_state(const Parcel &data); std::shared_ptr platform_api_stub_; }; } // namespace android diff --git a/src/anbox/bridge/platform_api_skeleton.cpp b/src/anbox/bridge/platform_api_skeleton.cpp index fafa4733..cb21e67a 100644 --- a/src/anbox/bridge/platform_api_skeleton.cpp +++ b/src/anbox/bridge/platform_api_skeleton.cpp @@ -33,28 +33,15 @@ PlatformApiSkeleton::PlatformApiSkeleton(const std::shared_ptrRun(); + if (boot_finished_handler_) + boot_finished_handler_(); } -void PlatformApiSkeleton::update_window_state(anbox::protobuf::bridge::WindowStateUpdate const *request, - anbox::protobuf::rpc::Void *response, - google::protobuf::Closure *done) { - (void) response; - - auto convert_window_state = [](const ::anbox::protobuf::bridge::WindowStateUpdate_WindowState &window) { - DEBUG("Window: display=%d has_surface=%d frame={%d,%d,%d,%d} package=%s task=%d stack=-1", - window.display_id(), window.has_surface(), - window.frame_left(), window.frame_top(), window.frame_right(), window.frame_bottom(), - window.package_name(), window.task_id()); +void PlatformApiSkeleton::handle_window_state_update_event(const anbox::protobuf::bridge::WindowStateUpdateEvent &event) { + auto convert_window_state = [](const ::anbox::protobuf::bridge::WindowStateUpdateEvent_WindowState &window) { return wm::WindowState( wm::Display::Id(window.display_id()), window.has_surface(), @@ -65,24 +52,22 @@ void PlatformApiSkeleton::update_window_state(anbox::protobuf::bridge::WindowSta }; wm::WindowState::List updated; - for (int n = 0; n < request->windows_size(); n++) { - const auto window = request->windows(n); + for (int n = 0; n < event.windows_size(); n++) { + const auto window = event.windows(n); updated.push_back(convert_window_state(window)); } wm::WindowState::List removed; - for (int n = 0; n < request->removed_windows_size(); n++) { - const auto window = request->removed_windows(n); + for (int n = 0; n < event.removed_windows_size(); n++) { + const auto window = event.removed_windows(n); removed.push_back(convert_window_state(window)); } window_manager_->apply_window_state_update(updated, removed); - - done->Run(); } -void PlatformApiSkeleton::on_boot_finished(const std::function &action) { - on_boot_finished_action_ = action; +void PlatformApiSkeleton::register_boot_finished_handler(const std::function &action) { + boot_finished_handler_ = action; } } // namespace bridge } // namespace anbox diff --git a/src/anbox/bridge/platform_api_skeleton.h b/src/anbox/bridge/platform_api_skeleton.h index e18f42e0..030e7ea5 100644 --- a/src/anbox/bridge/platform_api_skeleton.h +++ b/src/anbox/bridge/platform_api_skeleton.h @@ -32,8 +32,8 @@ namespace rpc { class Void; } // namespace rpc namespace bridge { -class Notification; -class WindowStateUpdate; +class BootFinishedEvent; +class WindowStateUpdateEvent; } // namespace bridge } // namespace protobuf namespace rpc { @@ -49,20 +49,15 @@ public: const std::shared_ptr &window_manager); virtual ~PlatformApiSkeleton(); - void boot_finished(anbox::protobuf::rpc::Void const *request, - anbox::protobuf::rpc::Void *response, - google::protobuf::Closure *done); + void handle_boot_finished_event(const anbox::protobuf::bridge::BootFinishedEvent &event); + void handle_window_state_update_event(const anbox::protobuf::bridge::WindowStateUpdateEvent &event); - void update_window_state(anbox::protobuf::bridge::WindowStateUpdate const *request, - anbox::protobuf::rpc::Void *response, - google::protobuf::Closure *done); - - void on_boot_finished(const std::function &action); + void register_boot_finished_handler(const std::function &action); private: std::shared_ptr pending_calls_; std::shared_ptr window_manager_; - std::function on_boot_finished_action_; + std::function boot_finished_handler_; }; } // namespace bridge } // namespace anbox diff --git a/src/anbox/bridge/platform_message_processor.cpp b/src/anbox/bridge/platform_message_processor.cpp index 59d6f28d..5b9892cc 100644 --- a/src/anbox/bridge/platform_message_processor.cpp +++ b/src/anbox/bridge/platform_message_processor.cpp @@ -18,6 +18,7 @@ #include "anbox/bridge/platform_message_processor.h" #include "anbox/bridge/platform_api_skeleton.h" #include "anbox/rpc/template_message_processor.h" +#include "anbox/logger.h" #include "anbox_bridge.pb.h" @@ -34,13 +35,20 @@ PlatformMessageProcessor::~PlatformMessageProcessor() { } void PlatformMessageProcessor::dispatch(rpc::Invocation const& invocation) { - if (invocation.method_name() == "boot_finished") - invoke(this, server_.get(), &PlatformApiSkeleton::boot_finished, invocation); - else if (invocation.method_name() == "update_window_state") - invoke(this, server_.get(), &PlatformApiSkeleton::update_window_state, invocation); } -void PlatformMessageProcessor::process_event_sequence(const std::string&) { +void PlatformMessageProcessor::process_event_sequence(const std::string &raw_events) { + anbox::protobuf::bridge::EventSequence seq; + if (!seq.ParseFromString(raw_events)) { + WARNING("Failed to parse events from raw string"); + return; + } + + if (seq.has_window_state_update()) + server_->handle_window_state_update_event(seq.window_state_update()); + + if (seq.has_boot_finished()) + server_->handle_boot_finished_event(seq.boot_finished()); } } // namespace anbox } // namespace network diff --git a/src/anbox/cmds/run.cpp b/src/anbox/cmds/run.cpp index 950fea89..d2841cae 100644 --- a/src/anbox/cmds/run.cpp +++ b/src/anbox/cmds/run.cpp @@ -130,7 +130,8 @@ anbox::cmds::Run::Run(const BusFactory& bus_factory) android_api_stub->set_rpc_channel(rpc_channel); auto server = std::make_shared(pending_calls, window_manager); - server->on_boot_finished([&]() { + server->register_boot_finished_handler([&]() { + DEBUG("Android successfully booted"); dispatcher->dispatch([&]() { // FIXME make this configurable or once we have a bridge let the host // act as a DNS proxy. diff --git a/src/anbox/protobuf/anbox_bridge.proto b/src/anbox/protobuf/anbox_bridge.proto index 0dce0dfe..0a3375ae 100644 --- a/src/anbox/protobuf/anbox_bridge.proto +++ b/src/anbox/protobuf/anbox_bridge.proto @@ -2,6 +2,11 @@ option optimize_for = LITE_RUNTIME; package anbox.protobuf.bridge; +message StructuredError { + optional uint32 domain = 1; + optional uint32 code = 2; +} + message Notification { required string package_name = 1; required string category = 2; @@ -27,7 +32,11 @@ message SetDnsServers { repeated Server servers = 2; } -message WindowStateUpdate { + +message BootFinishedEvent { +} + +message WindowStateUpdateEvent { message WindowState { required int32 display_id = 1; required bool has_surface = 2; @@ -42,3 +51,11 @@ message WindowStateUpdate { repeated WindowState windows = 1; repeated WindowState removed_windows = 2; } + +message EventSequence { + optional BootFinishedEvent boot_finished = 1; + optional WindowStateUpdateEvent window_state_update = 2; + + optional string error = 127; + optional StructuredError structured_error = 128; +} diff --git a/src/anbox/protobuf/anbox_rpc.proto b/src/anbox/protobuf/anbox_rpc.proto index dddbd41f..dc5f8a9a 100644 --- a/src/anbox/protobuf/anbox_rpc.proto +++ b/src/anbox/protobuf/anbox_rpc.proto @@ -12,6 +12,7 @@ message Invocation { message Result { optional uint32 id = 1; optional bytes response = 2; + repeated bytes events = 3; } message StructuredError { diff --git a/src/anbox/rpc/channel.cpp b/src/anbox/rpc/channel.cpp index a99da5aa..de524367 100644 --- a/src/anbox/rpc/channel.cpp +++ b/src/anbox/rpc/channel.cpp @@ -42,7 +42,17 @@ void Channel::call_method(std::string const& method_name, google::protobuf::Closure *complete) { auto const &invocation = invocation_for(method_name, parameters); pending_calls_->save_completion_details(invocation, response, complete); - send_message(invocation); + send_message(MessageType::invocation, invocation); +} + +void Channel::send_event(google::protobuf::MessageLite const& event) { + VariableLengthArray<2048> buffer{static_cast(event.ByteSize())}; + event.SerializeWithCachedSizesToArray(buffer.data()); + + anbox::protobuf::rpc::Result response; + response.add_events(buffer.data(), buffer.size()); + + send_message(MessageType::response, response); } protobuf::rpc::Invocation Channel::invocation_for(std::string const& method_name, @@ -62,17 +72,17 @@ protobuf::rpc::Invocation Channel::invocation_for(std::string const& method_name return invoke; } -void Channel::send_message(anbox::protobuf::rpc::Invocation const& invocation) { - const size_t size = invocation.ByteSize(); +void Channel::send_message(const std::uint8_t &type, google::protobuf::MessageLite const& message) { + const size_t size = message.ByteSize(); const unsigned char header_bytes[header_size] = { static_cast((size >> 8) & 0xff), static_cast((size >> 0) & 0xff), - MessageType::invocation, + type, }; std::vector send_buffer(sizeof(header_bytes) + size); std::copy(header_bytes, header_bytes + sizeof(header_bytes), send_buffer.begin()); - invocation.SerializeToArray(send_buffer.data() + sizeof(header_bytes), size); + message.SerializeToArray(send_buffer.data() + sizeof(header_bytes), size); try { std::lock_guard lock(write_mutex_); diff --git a/src/anbox/rpc/channel.h b/src/anbox/rpc/channel.h index 36a350bc..8b716595 100644 --- a/src/anbox/rpc/channel.h +++ b/src/anbox/rpc/channel.h @@ -52,11 +52,13 @@ public: google::protobuf::MessageLite *response, google::protobuf::Closure *complete); + void send_event(google::protobuf::MessageLite const& event); + private: protobuf::rpc::Invocation invocation_for( std::string const& method_name, google::protobuf::MessageLite const* request); - void send_message(anbox::protobuf::rpc::Invocation const& invocation); + void send_message(const std::uint8_t &type, google::protobuf::MessageLite const& message); int next_id(); void notify_disconnected(); diff --git a/src/anbox/rpc/message_processor.cpp b/src/anbox/rpc/message_processor.cpp index e19066b1..85d5d878 100644 --- a/src/anbox/rpc/message_processor.cpp +++ b/src/anbox/rpc/message_processor.cpp @@ -78,7 +78,11 @@ bool MessageProcessor::process_data(const std::vector &data) { buffer_.erase(buffer_.begin(), buffer_.begin() + message_size); - pending_calls_->complete_response(*result); + if (result->has_id()) + pending_calls_->complete_response(*result); + + for (int n = 0; n < result->events_size(); n++) + process_event_sequence(result->events(n)); } return true; -- GitLab