commit 89a37127cfbde9502332601a64c7b9c39ff629a4
author Amin Hassani <ahassani@chromium.org> Fri Mar 23 12:59:24 2018 -0700
committer chrome-bot <chrome-bot@chromium.org> Mon Mar 26 02:07:32 2018 -0700
tree 4399603276b79354dbf2a6f3c24ac06b088f2426
parent eb8c00a44d5efab75ffcd3f5e2e876f51502ff88

update_engine: Reorder enum values in UpdateStatus

A new enum NEED_PERMISSION_TO_UPDATE was added in the middle of UpdateStatus
enum and this caused problems with Java interfaces in Android. This patch does
these things:

- Eliminates range based comparisons on UpdateStatus in the code.
- Moves the newly added NEED_PERMISSION_TO_UPDATE to the end of the enum.
- Assigns explicit values enum values in UpdateStatus.

Original CL: aosp/647793

BUG=b:62842358
TEST=unit tests

Change-Id: I1fd3ef0171e46250c68cf2ceada06ec815b725a6
Reviewed-on: https://chromium-review.googlesource.com/978676
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Tianjie Xu <xunchang@google.com>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

client_library/include/update_engine/update_status.h

diff --git a/client_library/include/update_engine/update_status.h b/client_library/include/update_engine/update_status.h
index 65c5874..5a3dccf 100644
--- a/client_library/include/update_engine/update_status.h
+++ b/client_library/include/update_engine/update_status.h
@@ -23,20 +23,24 @@
 
 namespace update_engine {
 
+// ATTENTION: When adding a new enum value here, always append at the end and
+// make sure to make proper adjustments in UpdateAttempter:ActionCompleted(). If
+// any enum memeber is deprecated, the assigned value of other members should
+// not change. See b/62842358.
 enum class UpdateStatus {
   IDLE = 0,
-  CHECKING_FOR_UPDATE,
-  UPDATE_AVAILABLE,
-  // Broadcast this state when an update aborts because user preferences does
-  // not allow update over cellular.
-  NEED_PERMISSION_TO_UPDATE,
-  DOWNLOADING,
-  VERIFYING,
-  FINALIZING,
-  UPDATED_NEED_REBOOT,
-  REPORTING_ERROR_EVENT,
-  ATTEMPTING_ROLLBACK,
-  DISABLED,
+  CHECKING_FOR_UPDATE = 1,
+  UPDATE_AVAILABLE = 2,
+  DOWNLOADING = 3,
+  VERIFYING = 4,
+  FINALIZING = 5,
+  UPDATED_NEED_REBOOT = 6,
+  REPORTING_ERROR_EVENT = 7,
+  ATTEMPTING_ROLLBACK = 8,
+  DISABLED = 9,
+  // Broadcast this state when an update aborts because user preferences do not
+  // allow updates, e.g. over cellular network.
+  NEED_PERMISSION_TO_UPDATE = 10,
 };
 
 // Enum of bit-wise flags for controlling how updates are attempted.

update_attempter.cc

diff --git a/update_attempter.cc b/update_attempter.cc
index 13bc098..ebf7fb0 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1076,9 +1076,23 @@
     // If the current state is at or past the download phase, count the failure
     // in case a switch to full update becomes necessary. Ignore network
     // transfer timeouts and failures.
-    if (status_ >= UpdateStatus::DOWNLOADING &&
-        code != ErrorCode::kDownloadTransferError) {
-      MarkDeltaUpdateFailure();
+    if (code != ErrorCode::kDownloadTransferError) {
+      switch (status_) {
+        case UpdateStatus::IDLE:
+        case UpdateStatus::CHECKING_FOR_UPDATE:
+        case UpdateStatus::UPDATE_AVAILABLE:
+        case UpdateStatus::NEED_PERMISSION_TO_UPDATE:
+          break;
+        case UpdateStatus::DOWNLOADING:
+        case UpdateStatus::VERIFYING:
+        case UpdateStatus::FINALIZING:
+        case UpdateStatus::UPDATED_NEED_REBOOT:
+        case UpdateStatus::REPORTING_ERROR_EVENT:
+        case UpdateStatus::ATTEMPTING_ROLLBACK:
+        case UpdateStatus::DISABLED:
+          MarkDeltaUpdateFailure();
+          break;
+      }
     }
     // On failure, schedule an error event to be sent to Omaha.
     CreatePendingErrorEvent(action, code);