update_engine: Use DBus protobuf capabilities in dlcservice API methods
DBus have the capability to pass protobuf message directly in the method
without the need for serializing it manually. This CL uses those types
of arguments which is almost used everywhere in the platform2 code base.
BUG=chromium:978672
TEST=precq
Cq-Depend: chromium:1672684
Change-Id: I6e34ee76ede0c7b0b8cd1bba603a5836743d67fc
Reviewed-on: https://chromium-review.googlesource.com/1672021
Tested-by: Amin Hassani <ahassani@chromium.org>
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/client_library/client_dbus.cc b/client_library/client_dbus.cc
index 48a563b..18ae23b 100644
--- a/client_library/client_dbus.cc
+++ b/client_library/client_dbus.cc
@@ -92,13 +92,8 @@
dlc_parameters.add_dlc_module_infos();
dlc_module_info->set_dlc_id(dlc_id);
}
- string dlc_request;
- if (dlc_parameters.SerializeToString(&dlc_request)) {
- return proxy_->AttemptInstall(dlc_request, nullptr /* brillo::ErrorPtr* */);
- } else {
- LOG(ERROR) << "Fail to serialize a protobuf to a string.";
- return false;
- }
+ return proxy_->AttemptInstall(dlc_parameters,
+ nullptr /* brillo::ErrorPtr* */);
}
bool DBusUpdateEngineClient::GetStatus(int64_t* out_last_checked_time,
diff --git a/client_library/client_dbus.h b/client_library/client_dbus.h
index 1b127e3..c9631cf 100644
--- a/client_library/client_dbus.h
+++ b/client_library/client_dbus.h
@@ -23,6 +23,7 @@
#include <vector>
#include <base/macros.h>
+#include <dlcservice/proto_bindings/dlcservice.pb.h>
#include <update_engine/proto_bindings/update_engine.pb.h>
#include "update_engine/client_library/include/update_engine/client.h"
diff --git a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
index ef7bea7..a183147 100644
--- a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
+++ b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
@@ -35,7 +35,13 @@
<arg type="i" name="flags" direction="in" />
</method>
<method name="AttemptInstall">
- <arg type="s" name="dlc_request" direction="in" />
+ <arg type="ay" name="request" direction="in">
+ <tp:docstring>
+ The information about DLC modules that needs to be installed.
+ </tp:docstring>
+ <annotation name="org.chromium.DBus.Argument.ProtobufClass"
+ value="dlcservice::DlcModuleList"/>
+ </arg>
</method>
<method name="AttemptRollback">
<arg type="b" name="powerwash" direction="in" />
diff --git a/dbus_service.cc b/dbus_service.cc
index 105d581..4e37221 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -30,6 +30,7 @@
using brillo::ErrorPtr;
using chromeos_update_engine::UpdateEngineService;
+using dlcservice::DlcModuleList;
using std::string;
using std::vector;
using update_engine::StatusResult;
@@ -77,25 +78,17 @@
}
bool DBusUpdateEngineService::AttemptInstall(ErrorPtr* error,
- const string& dlc_request) {
- // Parse the raw parameters into protobuf.
- dlcservice::DlcModuleList dlc_parameters;
- if (!dlc_parameters.ParseFromString(dlc_request)) {
- *error = brillo::Error::Create(
- FROM_HERE, "update_engine", "INTERNAL", "parameters are invalid.");
- return false;
- }
- // Extract fields from the protobuf.
+ const DlcModuleList& request) {
vector<string> dlc_ids;
- for (const auto& dlc_module_info : dlc_parameters.dlc_module_infos()) {
+ for (const auto& dlc_module_info : request.dlc_module_infos()) {
if (dlc_module_info.dlc_id().empty()) {
*error = brillo::Error::Create(
- FROM_HERE, "update_engine", "INTERNAL", "parameters are invalid.");
+ FROM_HERE, "update_engine", "INTERNAL", "Empty DLC ID passed.");
return false;
}
dlc_ids.push_back(dlc_module_info.dlc_id());
}
- return common_->AttemptInstall(error, dlc_parameters.omaha_url(), dlc_ids);
+ return common_->AttemptInstall(error, request.omaha_url(), dlc_ids);
}
bool DBusUpdateEngineService::AttemptRollback(ErrorPtr* error,
diff --git a/dbus_service.h b/dbus_service.h
index 71a6d2b..2babf8c 100644
--- a/dbus_service.h
+++ b/dbus_service.h
@@ -24,6 +24,7 @@
#include <base/memory/ref_counted.h>
#include <brillo/errors/error.h>
+#include <dlcservice/proto_bindings/dlcservice.pb.h>
#include <update_engine/proto_bindings/update_engine.pb.h>
#include "update_engine/common_service.h"
@@ -51,7 +52,7 @@
int32_t in_flags_as_int) override;
bool AttemptInstall(brillo::ErrorPtr* error,
- const std::string& dlc_request) override;
+ const dlcservice::DlcModuleList& request) override;
bool AttemptRollback(brillo::ErrorPtr* error, bool in_powerwash) override;
diff --git a/dlcservice_chromeos.cc b/dlcservice_chromeos.cc
index e95f08f..b7dee36 100644
--- a/dlcservice_chromeos.cc
+++ b/dlcservice_chromeos.cc
@@ -16,11 +16,13 @@
#include "update_engine/dlcservice_chromeos.h"
-#include <dlcservice/dbus-proxies.h>
#include <dlcservice/proto_bindings/dlcservice.pb.h>
+// NOLINTNEXTLINE(build/include_alpha) "dbus-proxies.h" needs "dlcservice.pb.h"
+#include <dlcservice/dbus-proxies.h>
#include "update_engine/dbus_connection.h"
+using dlcservice::DlcModuleList;
using std::string;
using std::vector;
@@ -35,14 +37,10 @@
return false;
org::chromium::DlcServiceInterfaceProxy dlcservice_proxy(
DBusConnection::Get()->GetDBus());
- string dlc_module_list_str;
- if (!dlcservice_proxy.GetInstalled(&dlc_module_list_str, nullptr)) {
- LOG(ERROR) << "dlcservice does not return installed DLC module list.";
- return false;
- }
+
dlcservice::DlcModuleList dlc_module_list;
- if (!dlc_module_list.ParseFromString(dlc_module_list_str)) {
- LOG(ERROR) << "Errors parsing DlcModuleList protobuf.";
+ if (!dlcservice_proxy.GetInstalled(&dlc_module_list, nullptr)) {
+ LOG(ERROR) << "dlcservice does not return installed DLC module list.";
return false;
}
for (const auto& dlc_module_info : dlc_module_list.dlc_module_infos()) {