7 files changed, 51 insertions, 39 deletions

diff --git a/install/adb_install.cpp b/install/adb_install.cpp
index 4dd1f1b09..2de1075d2 100644
--- a/install/adb_install.cpp
+++ b/install/adb_install.cpp
@@ -90,7 +90,7 @@ static bool WriteStatusToFd(MinadbdCommandStatus status, int fd) {
 
 // Installs the package from FUSE. Returns the installation result and whether it should continue
 // waiting for new commands.
-static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
+static auto AdbInstallPackageHandler(RecoveryUI* ui, InstallResult* result) {
   // How long (in seconds) we wait for the package path to be ready. It doesn't need to be too long
   // because the minadbd service has already issued an install command. FUSE_SIDELOAD_HOST_PATHNAME
   // will start to exist once the host connects and starts serving a package. Poll for its
@@ -110,7 +110,7 @@ static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
         break;
       }
     }
-    *result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0, ui);
+    *result = InstallPackage(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0, ui);
     break;
   }
 
@@ -120,7 +120,7 @@ static auto AdbInstallPackageHandler(RecoveryUI* ui, int* result) {
   return std::make_pair(*result == INSTALL_SUCCESS, should_continue);
 }
 
-static auto AdbRebootHandler(MinadbdCommand command, int* result,
+static auto AdbRebootHandler(MinadbdCommand command, InstallResult* result,
                              Device::BuiltinAction* reboot_action) {
   // Use Device::REBOOT_{FASTBOOT,RECOVERY,RESCUE}, instead of the ones with ENTER_. This allows
   // rebooting back into fastboot/recovery/rescue mode through bootloader, which may use a newly
@@ -331,7 +331,7 @@ static void CreateMinadbdServiceAndExecuteCommands(
   signal(SIGPIPE, SIG_DFL);
 }
 
-int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action) {
+InstallResult ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action) {
   // Save the usb state to restore after the sideload operation.
   std::string usb_state = android::base::GetProperty("sys.usb.state", "none");
   // Clean up state and stop adbd.
@@ -342,7 +342,7 @@ int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot
 
   RecoveryUI* ui = device->GetUI();
 
-  int install_result = INSTALL_ERROR;
+  InstallResult install_result = INSTALL_ERROR;
   std::map<MinadbdCommand, CommandFunction> command_map{
     { MinadbdCommand::kInstall, std::bind(&AdbInstallPackageHandler, ui, &install_result) },
     { MinadbdCommand::kRebootAndroid, std::bind(&AdbRebootHandler, MinadbdCommand::kRebootAndroid,
diff --git a/install/fuse_sdcard_install.cpp b/install/fuse_sdcard_install.cpp
index 1aa8768e7..a5caa6e75 100644
--- a/install/fuse_sdcard_install.cpp
+++ b/install/fuse_sdcard_install.cpp
@@ -133,7 +133,7 @@ static bool StartSdcardFuse(const std::string& path) {
   return run_fuse_sideload(std::move(file_data_reader)) == 0;
 }
 
-int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
+InstallResult ApplyFromSdcard(Device* device, RecoveryUI* ui) {
   if (ensure_path_mounted(SDCARD_ROOT) != 0) {
     LOG(ERROR) << "\n-- Couldn't mount " << SDCARD_ROOT << ".\n";
     return INSTALL_ERROR;
@@ -159,9 +159,8 @@ int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
     _exit(status ? EXIT_SUCCESS : EXIT_FAILURE);
   }
 
-  // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child
-  // process is ready.
-  int result = INSTALL_ERROR;
+  // FUSE_SIDELOAD_HOST_PATHNAME will start to exist once the fuse in child process is ready.
+  InstallResult result = INSTALL_ERROR;
   int status;
   bool waited = false;
   for (int i = 0; i < SDCARD_INSTALL_TIMEOUT; ++i) {
@@ -184,7 +183,7 @@ int ApplyFromSdcard(Device* device, RecoveryUI* ui) {
       }
     }
 
-    result = install_package(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0 /*retry_count*/, ui);
+    result = InstallPackage(FUSE_SIDELOAD_HOST_PATHNAME, false, false, 0 /* retry_count */, ui);
     break;
   }
 
diff --git a/install/include/install/adb_install.h b/install/include/install/adb_install.h
index 3a0a81747..880022361 100644
--- a/install/include/install/adb_install.h
+++ b/install/include/install/adb_install.h
@@ -16,9 +16,10 @@
 
 #pragma once
 
-#include <recovery_ui/device.h>
+#include "install/install.h"
+#include "recovery_ui/device.h"
 
-// Applies a package via `adb sideload` or `adb rescue`. Returns the install result (in `enum
-// InstallResult`). When a reboot has been requested, INSTALL_REBOOT will be the return value, with
-// the reboot target set in reboot_action.
-int ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action);
+// Applies a package via `adb sideload` or `adb rescue`. Returns the install result. When a reboot
+// has been requested, INSTALL_REBOOT will be the return value, with the reboot target set in
+// reboot_action.
+InstallResult ApplyFromAdb(Device* device, bool rescue_mode, Device::BuiltinAction* reboot_action);
diff --git a/install/include/install/fuse_sdcard_install.h b/install/include/install/fuse_sdcard_install.h
index d9214ca3b..e5bb01f09 100644
--- a/install/include/install/fuse_sdcard_install.h
+++ b/install/include/install/fuse_sdcard_install.h
@@ -16,7 +16,8 @@
 
 #pragma once
 
+#include "install/install.h"
 #include "recovery_ui/device.h"
 #include "recovery_ui/ui.h"
 
-int ApplyFromSdcard(Device* device, RecoveryUI* ui);
+InstallResult ApplyFromSdcard(Device* device, RecoveryUI* ui);
diff --git a/install/include/install/install.h b/install/include/install/install.h
index cc595f6b0..44a5cde91 100644
--- a/install/include/install/install.h
+++ b/install/include/install/install.h
@@ -47,8 +47,8 @@ enum class OtaType {
 // Installs the given update package. This function should also wipe the cache partition after a
 // successful installation if |should_wipe_cache| is true or an updater command asks to wipe the
 // cache.
-int install_package(const std::string& package, bool should_wipe_cache, bool needs_mount,
-                    int retry_count, RecoveryUI* ui);
+InstallResult InstallPackage(const std::string& package, bool should_wipe_cache, bool needs_mount,
+                             int retry_count, RecoveryUI* ui);
 
 // Verifies the package by ota keys. Returns true if the package is verified successfully,
 // otherwise returns false.
diff --git a/install/install.cpp b/install/install.cpp
index 3f4df13e1..5d514fa23 100644
--- a/install/install.cpp
+++ b/install/install.cpp
@@ -324,9 +324,9 @@ static void log_max_temperature(int* max_temperature, const std::atomic<bool>& l
 }
 
 // If the package contains an update binary, extract it and run it.
-static int try_update_binary(const std::string& package, ZipArchiveHandle zip, bool* wipe_cache,
-                             std::vector<std::string>* log_buffer, int retry_count,
-                             int* max_temperature, RecoveryUI* ui) {
+static InstallResult TryUpdateBinary(const std::string& package, ZipArchiveHandle zip,
+                                     bool* wipe_cache, std::vector<std::string>* log_buffer,
+                                     int retry_count, int* max_temperature, RecoveryUI* ui) {
   std::map<std::string, std::string> metadata;
   if (!ReadMetadataFromPackage(zip, &metadata)) {
     LOG(ERROR) << "Failed to parse metadata in the zip file";
@@ -569,9 +569,10 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) {
   return false;
 }
 
-static int really_install_package(const std::string& path, bool* wipe_cache, bool needs_mount,
-                                  std::vector<std::string>* log_buffer, int retry_count,
-                                  int* max_temperature, RecoveryUI* ui) {
+static InstallResult VerifyAndInstallPackage(const std::string& path, bool* wipe_cache,
+                                             bool needs_mount, std::vector<std::string>* log_buffer,
+                                             int retry_count, int* max_temperature,
+                                             RecoveryUI* ui) {
   ui->SetBackground(RecoveryUI::INSTALLING_UPDATE);
   ui->Print("Finding update package...\n");
   // Give verification half the progress bar...
@@ -622,16 +623,16 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo
     ui->Print("Retry attempt: %d\n", retry_count);
   }
   ui->SetEnableReboot(false);
-  int result =
-      try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui);
+  auto result =
+      TryUpdateBinary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature, ui);
   ui->SetEnableReboot(true);
   ui->Print("\n");
 
   return result;
 }
 
-int install_package(const std::string& path, bool should_wipe_cache, bool needs_mount,
-                    int retry_count, RecoveryUI* ui) {
+InstallResult InstallPackage(const std::string& path, bool should_wipe_cache, bool needs_mount,
+                             int retry_count, RecoveryUI* ui) {
   CHECK(!path.empty());
 
   auto start = std::chrono::system_clock::now();
@@ -639,15 +640,15 @@ int install_package(const std::string& path, bool should_wipe_cache, bool needs_
   int start_temperature = GetMaxValueFromThermalZone();
   int max_temperature = start_temperature;
 
-  int result;
+  InstallResult result;
   std::vector<std::string> log_buffer;
   if (setup_install_mounts() != 0) {
     LOG(ERROR) << "failed to set up expected mounts for install; aborting";
     result = INSTALL_ERROR;
   } else {
     bool updater_wipe_cache = false;
-    result = really_install_package(path, &updater_wipe_cache, needs_mount, &log_buffer,
-                                    retry_count, &max_temperature, ui);
+    result = VerifyAndInstallPackage(path, &updater_wipe_cache, needs_mount, &log_buffer,
+                                     retry_count, &max_temperature, ui);
     should_wipe_cache = should_wipe_cache || updater_wipe_cache;
   }
 
diff --git a/recovery.cpp b/recovery.cpp
index 36813904a..dbac3e01a 100644
--- a/recovery.cpp
+++ b/recovery.cpp
@@ -113,12 +113,12 @@ const char* reason = nullptr;
  * 3. main system reboots into recovery
  * 4. get_args() writes BCB with "boot-recovery" and "--update_package=..."
  *    -- after this, rebooting will attempt to reinstall the update --
- * 5. install_package() attempts to install the update
+ * 5. InstallPackage() attempts to install the update
  *    NOTE: the package install must itself be restartable from any point
  * 6. finish_recovery() erases BCB
  *    -- after this, rebooting will (try to) restart the main system --
  * 7. ** if install failed **
- *    7a. prompt_and_wait() shows an error icon and waits for the user
+ *    7a. PromptAndWait() shows an error icon and waits for the user
  *    7b. the user reboots (pulling the battery, etc) into the main system
  */
 
@@ -312,14 +312,18 @@ static void run_graphics_test() {
   ui->ShowText(true);
 }
 
-// Returns REBOOT, SHUTDOWN, or REBOOT_BOOTLOADER. Returning NO_ACTION means to take the default,
-// which is to reboot or shutdown depending on if the --shutdown_after flag was passed to recovery.
-static Device::BuiltinAction prompt_and_wait(Device* device, int status) {
+// Shows the recovery UI and waits for user input. Returns one of the device builtin actions, such
+// as REBOOT, SHUTDOWN, or REBOOT_BOOTLOADER. Returning NO_ACTION means to take the default, which
+// is to reboot or shutdown depending on if the --shutdown_after flag was passed to recovery.
+static Device::BuiltinAction PromptAndWait(Device* device, InstallResult status) {
   for (;;) {
     finish_recovery();
     switch (status) {
       case INSTALL_SUCCESS:
       case INSTALL_NONE:
+      case INSTALL_SKIPPED:
+      case INSTALL_RETRY:
+      case INSTALL_KEY_INTERRUPTED:
         ui->SetBackground(RecoveryUI::NO_COMMAND);
         break;
 
@@ -327,6 +331,12 @@ static Device::BuiltinAction prompt_and_wait(Device* device, int status) {
       case INSTALL_CORRUPT:
         ui->SetBackground(RecoveryUI::ERROR);
         break;
+
+      case INSTALL_REBOOT:
+        // All the reboots should have been handled prior to entering PromptAndWait() or immediately
+        // after installing a package.
+        LOG(FATAL) << "Invalid status code of INSTALL_REBOOT";
+        break;
     }
     ui->SetProgressType(RecoveryUI::EMPTY);
 
@@ -690,7 +700,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
 
   ui->Print("Supported API: %d\n", kRecoveryApiVersion);
 
-  int status = INSTALL_SUCCESS;
+  InstallResult status = INSTALL_SUCCESS;
   // next_action indicates the next target to reboot into upon finishing the install. It could be
   // overridden to a different reboot target per user request.
   Device::BuiltinAction next_action = shutdown_after ? Device::SHUTDOWN : Device::REBOOT;
@@ -720,7 +730,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
         set_retry_bootloader_message(retry_count + 1, args);
       }
 
-      status = install_package(update_package, should_wipe_cache, true, retry_count, ui);
+      status = InstallPackage(update_package, should_wipe_cache, true, retry_count, ui);
       if (status != INSTALL_SUCCESS) {
         ui->Print("Installation aborted.\n");
 
@@ -828,7 +838,7 @@ Device::BuiltinAction start_recovery(Device* device, const std::vector<std::stri
   //    for 5s followed by an automatic reboot.
   if (status != INSTALL_REBOOT) {
     if (status == INSTALL_NONE || ui->IsTextVisible()) {
-      Device::BuiltinAction temp = prompt_and_wait(device, status);
+      auto temp = PromptAndWait(device, status);
       if (temp != Device::NO_ACTION) {
         next_action = temp;
       }