update_engine: Use operation instead of current_operation from update_engine.proto
The current mechanism for interchaning the current operation of
update_engine is quite old and very fragile to changes. Currently, each
client defines its own set of operations, writes their own string to
enum conversion logic, etc. We need to unify all these clients to use
just one set of well defined operations.
This CL uses the new enum Operation from the protobuf instead of
transferring a string to identify the current operation of the
update_engine.
BUG=chromium:977320
TEST=precq
Cq-Depend: chromium:1690424
Change-Id: I4d3a2a142c169cf6c972fe58d1d8d936d2349eed
Reviewed-on: https://chromium-review.googlesource.com/1690683
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: Xiaochu Liu <xiaochu@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Ben Chan <benchan@google.com>
diff --git a/client_library/client_dbus.cc b/client_library/client_dbus.cc
index 18ae23b..d046502 100644
--- a/client_library/client_dbus.cc
+++ b/client_library/client_dbus.cc
@@ -26,7 +26,6 @@
#include "update_engine/update_status_utils.h"
-using chromeos_update_engine::StringToUpdateStatus;
using dbus::Bus;
using org::chromium::UpdateEngineInterfaceProxy;
using std::string;
@@ -48,13 +47,13 @@
namespace {
// This converts the status from Protobuf |StatusResult| to The internal
// |UpdateEngineStatus| struct.
-bool ConvertToUpdateEngineStatus(const StatusResult& status,
+void ConvertToUpdateEngineStatus(const StatusResult& status,
UpdateEngineStatus* out_status) {
out_status->last_checked_time = status.last_checked_time();
out_status->progress = status.progress();
out_status->new_version = status.new_version();
out_status->new_size_bytes = status.new_size();
- return StringToUpdateStatus(status.current_operation(), &out_status->status);
+ out_status->status = static_cast<UpdateStatus>(status.current_operation());
}
} // namespace
@@ -110,7 +109,8 @@
*out_progress = status.progress();
*out_new_version = status.new_version();
*out_new_size = status.new_size();
- return StringToUpdateStatus(status.current_operation(), out_update_status);
+ *out_update_status = static_cast<UpdateStatus>(status.current_operation());
+ return true;
}
bool DBusUpdateEngineClient::GetStatus(UpdateEngineStatus* out_status) const {
@@ -119,7 +119,8 @@
return false;
}
- return ConvertToUpdateEngineStatus(status, out_status);
+ ConvertToUpdateEngineStatus(status, out_status);
+ return true;
}
bool DBusUpdateEngineClient::SetCohortHint(const string& cohort_hint) {
diff --git a/client_library/include/update_engine/update_status.h b/client_library/include/update_engine/update_status.h
index edf90b4..059181c 100644
--- a/client_library/include/update_engine/update_status.h
+++ b/client_library/include/update_engine/update_status.h
@@ -21,6 +21,11 @@
#include <brillo/enum_flags.h>
+// NOTE: Keep this file in sync with
+// platform2/system_api/dbus/update_engine/update_engine.proto especially:
+// - |UpdateStatus| <-> |Operation|
+// - |UpdateEngineStatus| <-> |StatusResult|
+
namespace update_engine {
// ATTENTION:
@@ -43,6 +48,13 @@
// Broadcast this state when an update aborts because user preferences do not
// allow updates, e.g. over cellular network.
NEED_PERMISSION_TO_UPDATE = 10,
+
+ // This value is exclusively used in Chrome. DO NOT define nor use it.
+ // TODO(crbug.com/977320): Remove this value from chrome by refactoring the
+ // Chrome code and evantually from here. This is not really an operation or
+ // state that the update engine stays on. This is the result of an internal
+ // failure and should be reflected differently.
+ // ERROR = -1,
};
// Enum of bit-wise flags for controlling how updates are attempted.
diff --git a/dbus_service.cc b/dbus_service.cc
index 4e37221..0cfe26b 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -33,8 +33,10 @@
using dlcservice::DlcModuleList;
using std::string;
using std::vector;
+using update_engine::Operation;
using update_engine::StatusResult;
using update_engine::UpdateEngineStatus;
+using update_engine::UpdateStatus;
namespace {
// Converts the internal |UpdateEngineStatus| to the protobuf |StatusResult|.
@@ -42,7 +44,7 @@
StatusResult* out_status) {
out_status->set_last_checked_time(ue_status.last_checked_time);
out_status->set_progress(ue_status.progress);
- out_status->set_current_operation(UpdateStatusToString(ue_status.status));
+ out_status->set_current_operation(static_cast<Operation>(ue_status.status));
out_status->set_new_version(ue_status.new_version);
out_status->set_new_size(ue_status.new_size_bytes);
}
@@ -240,7 +242,8 @@
// TODO(crbug.com/977320): Deprecate |StatusUpdate| signal.
SendStatusUpdateSignal(status.last_checked_time(),
status.progress(),
- status.current_operation(),
+ UpdateStatusToString(static_cast<UpdateStatus>(
+ status.current_operation())),
status.new_version(),
status.new_size());
diff --git a/update_status_utils.cc b/update_status_utils.cc
index cbc4f14..f3917d1 100644
--- a/update_status_utils.cc
+++ b/update_status_utils.cc
@@ -52,42 +52,4 @@
return nullptr;
}
-bool StringToUpdateStatus(const std::string& s, UpdateStatus* status) {
- if (s == update_engine::kUpdateStatusIdle) {
- *status = UpdateStatus::IDLE;
- return true;
- } else if (s == update_engine::kUpdateStatusCheckingForUpdate) {
- *status = UpdateStatus::CHECKING_FOR_UPDATE;
- return true;
- } else if (s == update_engine::kUpdateStatusUpdateAvailable) {
- *status = UpdateStatus::UPDATE_AVAILABLE;
- return true;
- } else if (s == update_engine::kUpdateStatusNeedPermissionToUpdate) {
- *status = UpdateStatus::NEED_PERMISSION_TO_UPDATE;
- return true;
- } else if (s == update_engine::kUpdateStatusDownloading) {
- *status = UpdateStatus::DOWNLOADING;
- return true;
- } else if (s == update_engine::kUpdateStatusVerifying) {
- *status = UpdateStatus::VERIFYING;
- return true;
- } else if (s == update_engine::kUpdateStatusFinalizing) {
- *status = UpdateStatus::FINALIZING;
- return true;
- } else if (s == update_engine::kUpdateStatusUpdatedNeedReboot) {
- *status = UpdateStatus::UPDATED_NEED_REBOOT;
- return true;
- } else if (s == update_engine::kUpdateStatusReportingErrorEvent) {
- *status = UpdateStatus::REPORTING_ERROR_EVENT;
- return true;
- } else if (s == update_engine::kUpdateStatusAttemptingRollback) {
- *status = UpdateStatus::ATTEMPTING_ROLLBACK;
- return true;
- } else if (s == update_engine::kUpdateStatusDisabled) {
- *status = UpdateStatus::DISABLED;
- return true;
- }
- return false;
-}
-
} // namespace chromeos_update_engine
diff --git a/update_status_utils.h b/update_status_utils.h
index 30ae53b..e3b8b43 100644
--- a/update_status_utils.h
+++ b/update_status_utils.h
@@ -25,9 +25,6 @@
const char* UpdateStatusToString(const update_engine::UpdateStatus& status);
-bool StringToUpdateStatus(const std::string& update_status_as_string,
- update_engine::UpdateStatus* status);
-
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_UPDATE_STATUS_UTILS_H_