libchromeos: Add support for async D-Bus method handlers
Changed DBusObject implementation infrastructure in libchromeos
to add support for registering asynchronous D-Bus method handlers.
Also changed the simple synchronous method handler signature to
eliminate the confusion when a handler returning a value throws
an error.
Currently, there are four kinds of D-Bus method handlers:
- SimpleMethodHandler - has one of the following signatures:
RetVal Handler(Args... args)
void Handler(Args... args)
The first one takes only input parameters (by value or const
reference) and returns a single value as a function return.
The second handler can mix both input (value/const reference)
and output (pointer) values.
This handler is synchronous (response is sent as soon as the
handler returns) and does not provide any error returns
(it always succeeds).
- SimpleMethodHanderWithError
This is similar to SimpleMethodHandler but provides a way
to return an error. The handler signature is as follows:
bool Handler(ErrorPtr* error, Args... args)
The parameters can include both IN and OUT arguments.
If the handler succeeds, it must return true. On failures,
it returns false and provides error details in |error|.
- MethodHandler - a generic (possibly asynchronous) handler
that has the following signature:
void Handler(scoped_ptr<DBusMethodResponse> response,
Args... args)
The parameters include only IN arguments and the method
sends back any return values using the |response| object.
The handler owns the |response| so it can start an
asynchronous operation (that holds on to the response) and
exit immediately. The D-Bus method response is sent only
when the handler provides the reply using the response
object.
- RawMethodHandler
This is the low-level method handler that does not go through
any of the parameter/return value parsing. It is provided
with the raw method call D-Bus message and is expected to
provide the response manually. The handler signtaure is:
void Handler(dbus::MethodCall* method_call,
ResponseSender sender)
This type of handler is useful to implement D-Bus methods
with variable number of parameters or those which
can accept a number of different types of arguments.
BUG=chromium:428390
TEST=FEATURES=test emerge-link libchromeos peerd buffet attestation
CQ-DEPEND=CL:227281
Change-Id: I1dde6b279ada9d350a4d0e6743c56d3b12cc38cf
Reviewed-on: https://chromium-review.googlesource.com/226666
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/dbus_command_dispatcher_unittest.cc b/buffet/commands/dbus_command_dispatcher_unittest.cc
index 170d900..9de6889 100644
--- a/buffet/commands/dbus_command_dispatcher_unittest.cc
+++ b/buffet/commands/dbus_command_dispatcher_unittest.cc
@@ -108,7 +108,7 @@
}
void FinishCommand(DBusCommandProxy* proxy) {
- proxy->HandleDone(nullptr);
+ proxy->HandleDone();
}
void SetProgress(DBusCommandProxy* proxy, int progress) {
diff --git a/buffet/commands/dbus_command_proxy.cc b/buffet/commands/dbus_command_proxy.cc
index 0b97c06..07ead32 100644
--- a/buffet/commands/dbus_command_proxy.cc
+++ b/buffet/commands/dbus_command_proxy.cc
@@ -32,18 +32,18 @@
dbus_object_.AddOrGetInterface(dbus_constants::kCommandInterface);
// DBus methods.
- itf->AddMethodHandler(dbus_constants::kCommandSetProgress,
- base::Unretained(this),
- &DBusCommandProxy::HandleSetProgress);
- itf->AddMethodHandler(dbus_constants::kCommandAbort,
- base::Unretained(this),
- &DBusCommandProxy::HandleAbort);
- itf->AddMethodHandler(dbus_constants::kCommandCancel,
- base::Unretained(this),
- &DBusCommandProxy::HandleCancel);
- itf->AddMethodHandler(dbus_constants::kCommandDone,
- base::Unretained(this),
- &DBusCommandProxy::HandleDone);
+ itf->AddSimpleMethodHandlerWithError(dbus_constants::kCommandSetProgress,
+ base::Unretained(this),
+ &DBusCommandProxy::HandleSetProgress);
+ itf->AddSimpleMethodHandler(dbus_constants::kCommandAbort,
+ base::Unretained(this),
+ &DBusCommandProxy::HandleAbort);
+ itf->AddSimpleMethodHandler(dbus_constants::kCommandCancel,
+ base::Unretained(this),
+ &DBusCommandProxy::HandleCancel);
+ itf->AddSimpleMethodHandler(dbus_constants::kCommandDone,
+ base::Unretained(this),
+ &DBusCommandProxy::HandleDone);
// DBus properties.
itf->AddProperty(dbus_constants::kCommandName, &name_);
@@ -80,7 +80,7 @@
progress_.SetValue(progress);
}
-void DBusCommandProxy::HandleSetProgress(chromeos::ErrorPtr* error,
+bool DBusCommandProxy::HandleSetProgress(chromeos::ErrorPtr* error,
int32_t progress) {
LOG(INFO) << "Received call to Command<"
<< command_instance_->GetName() << ">::SetProgress("
@@ -89,24 +89,26 @@
// Validate |progress| parameter. Its value must be between 0 and 100.
IntPropType progress_type;
progress_type.AddMinMaxConstraint(0, 100);
- if (progress_type.ValidateValue(progress, error)) {
- command_instance_->SetProgress(progress);
- }
+ if (!progress_type.ValidateValue(progress, error))
+ return false;
+
+ command_instance_->SetProgress(progress);
+ return true;
}
-void DBusCommandProxy::HandleAbort(chromeos::ErrorPtr* error) {
+void DBusCommandProxy::HandleAbort() {
LOG(INFO) << "Received call to Command<"
<< command_instance_->GetName() << ">::Abort()";
command_instance_->Abort();
}
-void DBusCommandProxy::HandleCancel(chromeos::ErrorPtr* error) {
+void DBusCommandProxy::HandleCancel() {
LOG(INFO) << "Received call to Command<"
<< command_instance_->GetName() << ">::Cancel()";
command_instance_->Cancel();
}
-void DBusCommandProxy::HandleDone(chromeos::ErrorPtr* error) {
+void DBusCommandProxy::HandleDone() {
LOG(INFO) << "Received call to Command<"
<< command_instance_->GetName() << ">::Done()";
command_instance_->Done();
diff --git a/buffet/commands/dbus_command_proxy.h b/buffet/commands/dbus_command_proxy.h
index 553470e..50878fb 100644
--- a/buffet/commands/dbus_command_proxy.h
+++ b/buffet/commands/dbus_command_proxy.h
@@ -50,13 +50,13 @@
parameters_;
// Handles calls to org.chromium.Buffet.Command.SetProgress(progress).
- void HandleSetProgress(chromeos::ErrorPtr* error, int32_t progress);
+ bool HandleSetProgress(chromeos::ErrorPtr* error, int32_t progress);
// Handles calls to org.chromium.Buffet.Command.Abort().
- void HandleAbort(chromeos::ErrorPtr* error);
+ void HandleAbort();
// Handles calls to org.chromium.Buffet.Command.Cancel().
- void HandleCancel(chromeos::ErrorPtr* error);
+ void HandleCancel();
// Handles calls to org.chromium.Buffet.Command.Done().
- void HandleDone(chromeos::ErrorPtr* error);
+ void HandleDone();
dbus::ObjectPath object_path_;
CommandInstance* command_instance_;
diff --git a/buffet/commands/dbus_command_proxy_unittest.cc b/buffet/commands/dbus_command_proxy_unittest.cc
index f8fba8d..ae91b13 100644
--- a/buffet/commands/dbus_command_proxy_unittest.cc
+++ b/buffet/commands/dbus_command_proxy_unittest.cc
@@ -9,6 +9,7 @@
#include <dbus/mock_exported_object.h>
#include <dbus/property.h>
#include <chromeos/dbus/dbus_object.h>
+#include <chromeos/dbus/dbus_object_test_helpers.h>
#include <gtest/gtest.h>
#include "buffet/commands/command_dictionary.h"
@@ -131,8 +132,8 @@
dbus::MessageWriter writer(&method_call);
if (param_callback)
param_callback(&writer);
- return chromeos::dbus_utils::CallMethod(*GetProxyDBusObject(),
- &method_call);
+ return chromeos::dbus_utils::testing::CallMethod(*GetProxyDBusObject(),
+ &method_call);
}
static bool IsResponseError(const std::unique_ptr<dbus::Response>& response) {
@@ -157,8 +158,8 @@
dbus::MessageWriter writer(&method_call);
writer.AppendString(dbus_constants::kCommandInterface);
writer.AppendString(property_name);
- auto response = chromeos::dbus_utils::CallMethod(*GetProxyDBusObject(),
- &method_call);
+ auto response = chromeos::dbus_utils::testing::CallMethod(
+ *GetProxyDBusObject(), &method_call);
T value{};
VerifyResponse(response, [&value](dbus::MessageReader* reader) {
EXPECT_TRUE(chromeos::dbus_utils::PopValueFromReader(reader, &value));
diff --git a/buffet/manager.cc b/buffet/manager.cc
index bc9fdf8..7deb36f 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -25,6 +25,7 @@
#include "buffet/states/state_manager.h"
using chromeos::dbus_utils::AsyncEventSequencer;
+using chromeos::dbus_utils::DBusMethodResponse;
using chromeos::dbus_utils::ExportedObjectManager;
namespace buffet {
@@ -62,9 +63,9 @@
itf->AddMethodHandler(dbus_constants::kManagerAddCommand,
base::Unretained(this),
&Manager::HandleAddCommand);
- itf->AddMethodHandler(dbus_constants::kManagerTestMethod,
- base::Unretained(this),
- &Manager::HandleTestMethod);
+ itf->AddSimpleMethodHandler(dbus_constants::kManagerTestMethod,
+ base::Unretained(this),
+ &Manager::HandleTestMethod);
dbus_object_.RegisterAsync(cb);
command_manager_ =
std::make_shared<CommandManager>(dbus_object_.GetObjectManager());
@@ -78,78 +79,115 @@
device_info_->Load();
}
-void Manager::HandleStartDevice(chromeos::ErrorPtr* error) {
+void Manager::HandleStartDevice(scoped_ptr<DBusMethodResponse> response) {
LOG(INFO) << "Received call to Manager.StartDevice()";
- device_info_->StartDevice(error);
+ chromeos::ErrorPtr error;
+ device_info_->StartDevice(&error);
+ if (error)
+ response->ReplyWithError(error.get());
+ else
+ response->Return();
}
-std::string Manager::HandleCheckDeviceRegistered(chromeos::ErrorPtr* error) {
+void Manager::HandleCheckDeviceRegistered(
+ scoped_ptr<DBusMethodResponse> response) {
LOG(INFO) << "Received call to Manager.CheckDeviceRegistered()";
- std::string device_id;
- bool registered = device_info_->CheckRegistration(error);
+ chromeos::ErrorPtr error;
+ bool registered = device_info_->CheckRegistration(&error);
// If it fails due to any reason other than 'device not registered',
// treat it as a real error and report it to the caller.
if (!registered &&
- !(*error)->HasError(kErrorDomainGCD, "device_not_registered")) {
- return device_id;
+ !error->HasError(kErrorDomainGCD, "device_not_registered")) {
+ response->ReplyWithError(error.get());
+ return;
}
- error->reset();
+ std::string device_id;
+ if (registered) {
+ error.reset();
+ device_id = device_info_->GetDeviceId(&error);
+ if (error) {
+ response->ReplyWithError(error.get());
+ return;
+ }
+ }
- if (registered)
- device_id = device_info_->GetDeviceId(error);
-
- return device_id;
+ response->Return(device_id);
}
-std::string Manager::HandleGetDeviceInfo(chromeos::ErrorPtr* error) {
+void Manager::HandleGetDeviceInfo(
+ scoped_ptr<DBusMethodResponse> response) {
LOG(INFO) << "Received call to Manager.GetDeviceInfo()";
- std::string device_info_str;
- auto device_info = device_info_->GetDeviceInfo(error);
- if (!device_info)
- return device_info_str;
+ chromeos::ErrorPtr error;
+ auto device_info = device_info_->GetDeviceInfo(&error);
+ if (!device_info) {
+ response->ReplyWithError(error.get());
+ return;
+ }
+ std::string device_info_str;
base::JSONWriter::Write(device_info.get(), &device_info_str);
- return device_info_str;
+ response->Return(device_info_str);
}
-std::string Manager::HandleRegisterDevice(
- chromeos::ErrorPtr* error,
+void Manager::HandleRegisterDevice(
+ scoped_ptr<DBusMethodResponse> response,
const std::map<std::string, std::string>& params) {
LOG(INFO) << "Received call to Manager.RegisterDevice()";
- return device_info_->RegisterDevice(params, error);
+ chromeos::ErrorPtr error;
+ std::string device_id = device_info_->RegisterDevice(params, &error);
+ if (error)
+ response->ReplyWithError(error.get());
+ else
+ response->Return(device_id);
}
void Manager::HandleUpdateState(
- chromeos::ErrorPtr* error,
+ scoped_ptr<DBusMethodResponse> response,
const chromeos::VariantDictionary& property_set) {
+ chromeos::ErrorPtr error;
base::Time timestamp = base::Time::Now();
+ bool all_success = true;
for (const auto& pair : property_set) {
- state_manager_->SetPropertyValue(pair.first, pair.second, timestamp, error);
+ if (!state_manager_->SetPropertyValue(pair.first, pair.second,
+ timestamp, &error)) {
+ // Remember that an error occurred but keep going and update the rest of
+ // the properties if possible.
+ all_success = false;
+ }
}
+ if (!all_success)
+ response->ReplyWithError(error.get());
+ else
+ response->Return();
}
-void Manager::HandleAddCommand(
- chromeos::ErrorPtr* error, const std::string& json_command) {
+void Manager::HandleAddCommand(scoped_ptr<DBusMethodResponse> response,
+ const std::string& json_command) {
std::string error_message;
std::unique_ptr<base::Value> value(base::JSONReader::ReadAndReturnError(
json_command, base::JSON_PARSE_RFC, nullptr, &error_message));
if (!value) {
- chromeos::Error::AddTo(error, chromeos::errors::json::kDomain,
- chromeos::errors::json::kParseError, error_message);
+ response->ReplyWithError(chromeos::errors::json::kDomain,
+ chromeos::errors::json::kParseError,
+ error_message);
return;
}
+ chromeos::ErrorPtr error;
auto command_instance = buffet::CommandInstance::FromJson(
- value.get(), command_manager_->GetCommandDictionary(), error);
- if (command_instance)
- command_manager_->AddCommand(std::move(command_instance));
+ value.get(), command_manager_->GetCommandDictionary(), &error);
+ if (!command_instance) {
+ response->ReplyWithError(error.get());
+ return;
+ }
+ command_manager_->AddCommand(std::move(command_instance));
+ response->Return();
}
-std::string Manager::HandleTestMethod(chromeos::ErrorPtr* error,
- const std::string& message) {
+std::string Manager::HandleTestMethod(const std::string& message) {
LOG(INFO) << "Received call to test method: " << message;
return message;
}
diff --git a/buffet/manager.h b/buffet/manager.h
index 7ce33cc..b8ef91a 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -47,24 +47,28 @@
private:
// DBus methods:
// Handles calls to org.chromium.Buffet.Manager.StartDevice().
- void HandleStartDevice(chromeos::ErrorPtr* error);
+ void HandleStartDevice(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response);
// Handles calls to org.chromium.Buffet.Manager.CheckDeviceRegistered().
- std::string HandleCheckDeviceRegistered(chromeos::ErrorPtr* error);
+ void HandleCheckDeviceRegistered(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response);
// Handles calls to org.chromium.Buffet.Manager.GetDeviceInfo().
- std::string HandleGetDeviceInfo(chromeos::ErrorPtr* error);
+ void HandleGetDeviceInfo(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response);
// Handles calls to org.chromium.Buffet.Manager.RegisterDevice().
- std::string HandleRegisterDevice(
- chromeos::ErrorPtr* error,
+ void HandleRegisterDevice(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response,
const std::map<std::string, std::string>& params);
// Handles calls to org.chromium.Buffet.Manager.UpdateState().
- void HandleUpdateState(chromeos::ErrorPtr* error,
- const chromeos::VariantDictionary& property_set);
+ void HandleUpdateState(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response,
+ const chromeos::VariantDictionary& property_set);
// Handles calls to org.chromium.Buffet.Manager.AddCommand().
- void HandleAddCommand(chromeos::ErrorPtr* error,
- const std::string& json_command);
+ void HandleAddCommand(
+ scoped_ptr<chromeos::dbus_utils::DBusMethodResponse> response,
+ const std::string& json_command);
// Handles calls to org.chromium.Buffet.Manager.Test()
- std::string HandleTestMethod(chromeos::ErrorPtr* error,
- const std::string& message);
+ std::string HandleTestMethod(const std::string& message);
chromeos::dbus_utils::DBusObject dbus_object_;