5 files changed, 64 insertions, 66 deletions

diff --git a/screen_ui.cpp b/screen_ui.cpp
index f1b38781a..0ee0ddcec 100644
--- a/screen_ui.cpp
+++ b/screen_ui.cpp
@@ -32,8 +32,10 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <chrono>
 #include <memory>
 #include <string>
+#include <thread>
 #include <unordered_map>
 #include <vector>
 
@@ -172,6 +174,11 @@ ScreenRecoveryUI::ScreenRecoveryUI(bool scrollable_menu)
       rtl_locale_(false),
       updateMutex(PTHREAD_MUTEX_INITIALIZER) {}
 
+ScreenRecoveryUI::~ScreenRecoveryUI() {
+  progress_thread_stopped_ = true;
+  progress_thread_.join();
+}
+
 GRSurface* ScreenRecoveryUI::GetCurrentFrame() const {
   if (currentIcon == INSTALLING_UPDATE || currentIcon == ERASING) {
     return intro_done ? loopFrames[current_frame] : introFrames[current_frame];
@@ -613,15 +620,9 @@ void ScreenRecoveryUI::update_progress_locked() {
   gr_flip();
 }
 
-// Keeps the progress bar updated, even when the process is otherwise busy.
-void* ScreenRecoveryUI::ProgressThreadStartRoutine(void* data) {
-  reinterpret_cast<ScreenRecoveryUI*>(data)->ProgressThreadLoop();
-  return nullptr;
-}
-
 void ScreenRecoveryUI::ProgressThreadLoop() {
   double interval = 1.0 / kAnimationFps;
-  while (true) {
+  while (!progress_thread_stopped_) {
     double start = now();
     pthread_mutex_lock(&updateMutex);
 
@@ -749,7 +750,8 @@ bool ScreenRecoveryUI::Init(const std::string& locale) {
 
   LoadAnimation();
 
-  pthread_create(&progress_thread_, nullptr, ProgressThreadStartRoutine, this);
+  // Keep the progress bar updated, even when the process is otherwise busy.
+  progress_thread_ = std::thread(&ScreenRecoveryUI::ProgressThreadLoop, this);
 
   return true;
 }
diff --git a/screen_ui.h b/screen_ui.h
index c90a2cd17..c3605161a 100644
--- a/screen_ui.h
+++ b/screen_ui.h
@@ -20,9 +20,11 @@
 #include <pthread.h>
 #include <stdio.h>
 
+#include <atomic>
 #include <functional>
 #include <memory>
 #include <string>
+#include <thread>
 #include <vector>
 
 #include "ui.h"
@@ -112,6 +114,7 @@ class ScreenRecoveryUI : public RecoveryUI {
 
   ScreenRecoveryUI();
   explicit ScreenRecoveryUI(bool scrollable_menu);
+  ~ScreenRecoveryUI() override;
 
   bool Init(const std::string& locale) override;
   std::string GetLocale() const override;
@@ -275,7 +278,8 @@ class ScreenRecoveryUI : public RecoveryUI {
   // An alternate text screen, swapped with 'text_' when we're viewing a log file.
   char** file_viewer_text_;
 
-  pthread_t progress_thread_;
+  std::thread progress_thread_;
+  std::atomic<bool> progress_thread_stopped_{ false };
 
   // Number of intro frames and loop frames in the animation.
   size_t intro_frames;
diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp
index 269222faa..25623074c 100644
--- a/tests/unit/screen_ui_test.cpp
+++ b/tests/unit/screen_ui_test.cpp
@@ -279,8 +279,6 @@ class ScreenRecoveryUITest : public ::testing::Test {
     testdata_dir_ = from_testdata_base("");
     Paths::Get().set_resource_dir(testdata_dir_);
     res_set_resource_dir(testdata_dir_);
-
-    ASSERT_TRUE(ui_->Init(kTestLocale));
   }
 
   std::unique_ptr<TestableScreenRecoveryUI> ui_;
@@ -288,6 +286,7 @@ class ScreenRecoveryUITest : public ::testing::Test {
 };
 
 TEST_F(ScreenRecoveryUITest, Init) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ASSERT_EQ(kTestLocale, ui_->GetLocale());
   ASSERT_FALSE(ui_->GetRtlLocale());
   ASSERT_FALSE(ui_->IsTextVisible());
@@ -295,6 +294,7 @@ TEST_F(ScreenRecoveryUITest, Init) {
 }
 
 TEST_F(ScreenRecoveryUITest, ShowText) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ASSERT_FALSE(ui_->IsTextVisible());
   ui_->ShowText(true);
   ASSERT_TRUE(ui_->IsTextVisible());
@@ -308,12 +308,15 @@ TEST_F(ScreenRecoveryUITest, ShowText) {
 TEST_F(ScreenRecoveryUITest, RtlLocale) {
   ASSERT_TRUE(ui_->Init(kTestRtlLocale));
   ASSERT_TRUE(ui_->GetRtlLocale());
+}
 
+TEST_F(ScreenRecoveryUITest, RtlLocaleWithSuffix) {
   ASSERT_TRUE(ui_->Init(kTestRtlLocaleWithSuffix));
   ASSERT_TRUE(ui_->GetRtlLocale());
 }
 
 TEST_F(ScreenRecoveryUITest, ShowMenu) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ui_->SetKeyBuffer({
       KeyCode::UP,
       KeyCode::DOWN,
@@ -339,6 +342,7 @@ TEST_F(ScreenRecoveryUITest, ShowMenu) {
 }
 
 TEST_F(ScreenRecoveryUITest, ShowMenu_NotMenuOnly) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ui_->SetKeyBuffer({
       KeyCode::MAGIC,
   });
@@ -349,6 +353,7 @@ TEST_F(ScreenRecoveryUITest, ShowMenu_NotMenuOnly) {
 }
 
 TEST_F(ScreenRecoveryUITest, ShowMenu_TimedOut) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ui_->SetKeyBuffer({
       KeyCode::TIMEOUT,
   });
@@ -356,6 +361,7 @@ TEST_F(ScreenRecoveryUITest, ShowMenu_TimedOut) {
 }
 
 TEST_F(ScreenRecoveryUITest, ShowMenu_TimedOut_TextWasEverVisible) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   ui_->ShowText(true);
   ui_->ShowText(false);
   ASSERT_TRUE(ui_->WasTextEverVisible());
@@ -371,6 +377,7 @@ TEST_F(ScreenRecoveryUITest, ShowMenu_TimedOut_TextWasEverVisible) {
 }
 
 TEST_F(ScreenRecoveryUITest, LoadAnimation) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   // Make a few copies of loop00000.png from testdata.
   std::string image_data;
   ASSERT_TRUE(android::base::ReadFileToString(testdata_dir_ + "/loop00000.png", &image_data));
@@ -398,6 +405,7 @@ TEST_F(ScreenRecoveryUITest, LoadAnimation) {
 }
 
 TEST_F(ScreenRecoveryUITest, LoadAnimation_MissingAnimation) {
+  ASSERT_TRUE(ui_->Init(kTestLocale));
   TemporaryDir resource_dir;
   Paths::Get().set_resource_dir(resource_dir.path);
   ASSERT_EXIT(ui_->RunLoadAnimation(), ::testing::KilledBySignal(SIGABRT), "");
diff --git a/ui.cpp b/ui.cpp
index 8983d7692..dbe77ae8c 100644
--- a/ui.cpp
+++ b/ui.cpp
@@ -20,29 +20,30 @@
 #include <fcntl.h>
 #include <linux/input.h>
 #include <pthread.h>
-#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <time.h>
 #include <unistd.h>
 
+#include <chrono>
 #include <functional>
 #include <string>
+#include <thread>
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
 #include <android-base/parseint.h>
 #include <android-base/strings.h>
-#include <minui/minui.h>
 
-#include "device.h"
+#include "minui/minui.h"
 #include "otautil/sysutil.h"
 #include "roots.h"
 
+using namespace std::chrono_literals;
+
 static constexpr int UI_WAIT_KEY_TIMEOUT_SEC = 120;
 static constexpr const char* BRIGHTNESS_FILE = "/sys/class/leds/lcd-backlight/brightness";
 static constexpr const char* MAX_BRIGHTNESS_FILE = "/sys/class/leds/lcd-backlight/max_brightness";
@@ -78,6 +79,12 @@ RecoveryUI::RecoveryUI()
   memset(key_pressed, 0, sizeof(key_pressed));
 }
 
+RecoveryUI::~RecoveryUI() {
+  ev_exit();
+  input_thread_stopped_ = true;
+  input_thread_.join();
+}
+
 void RecoveryUI::OnKeyDetected(int key_code) {
   if (key_code == KEY_POWER) {
     has_power_key = true;
@@ -90,16 +97,6 @@ void RecoveryUI::OnKeyDetected(int key_code) {
   }
 }
 
-// Reads input events, handles special hot keys, and adds to the key queue.
-static void* InputThreadLoop(void*) {
-  while (true) {
-    if (!ev_wait(-1)) {
-      ev_dispatch();
-    }
-  }
-  return nullptr;
-}
-
 bool RecoveryUI::InitScreensaver() {
   // Disabled.
   if (brightness_normal_ == 0 || brightness_dimmed_ > brightness_normal_) {
@@ -166,7 +163,15 @@ bool RecoveryUI::Init(const std::string& /* locale */) {
     LOG(INFO) << "Screensaver disabled";
   }
 
-  pthread_create(&input_thread_, nullptr, InputThreadLoop, nullptr);
+  // Create a separate thread that handles input events.
+  input_thread_ = std::thread([this]() {
+    while (!this->input_thread_stopped_) {
+      if (!ev_wait(500)) {
+        ev_dispatch();
+      }
+    }
+  });
+
   return true;
 }
 
@@ -323,22 +328,18 @@ int RecoveryUI::OnInputEvent(int fd, uint32_t epevents) {
   return 0;
 }
 
-// Process a key-up or -down event.  A key is "registered" when it is
-// pressed and then released, with no other keypresses or releases in
-// between.  Registered keys are passed to CheckKey() to see if it
-// should trigger a visibility toggle, an immediate reboot, or be
-// queued to be processed next time the foreground thread wants a key
-// (eg, for the menu).
+// Processes a key-up or -down event. A key is "registered" when it is pressed and then released,
+// with no other keypresses or releases in between. Registered keys are passed to CheckKey() to
+// see if it should trigger a visibility toggle, an immediate reboot, or be queued to be processed
+// next time the foreground thread wants a key (eg, for the menu).
 //
-// We also keep track of which keys are currently down so that
-// CheckKey can call IsKeyPressed to see what other keys are held when
-// a key is registered.
+// We also keep track of which keys are currently down so that CheckKey() can call IsKeyPressed()
+// to see what other keys are held when a key is registered.
 //
 // updown == 1 for key down events; 0 for key up events
 void RecoveryUI::ProcessKey(int key_code, int updown) {
   bool register_key = false;
   bool long_press = false;
-  bool reboot_enabled;
 
   pthread_mutex_lock(&key_queue_mutex);
   key_pressed[key_code] = updown;
@@ -346,13 +347,9 @@ void RecoveryUI::ProcessKey(int key_code, int updown) {
     ++key_down_count;
     key_last_down = key_code;
     key_long_press = false;
-    key_timer_t* info = new key_timer_t;
-    info->ui = this;
-    info->key_code = key_code;
-    info->count = key_down_count;
-    pthread_t thread;
-    pthread_create(&thread, nullptr, &RecoveryUI::time_key_helper, info);
-    pthread_detach(thread);
+
+    std::thread time_key_thread(&RecoveryUI::TimeKey, this, key_code, key_down_count);
+    time_key_thread.detach();
   } else {
     if (key_last_down == key_code) {
       long_press = key_long_press;
@@ -360,7 +357,7 @@ void RecoveryUI::ProcessKey(int key_code, int updown) {
     }
     key_last_down = -1;
   }
-  reboot_enabled = enable_reboot;
+  bool reboot_enabled = enable_reboot;
   pthread_mutex_unlock(&key_queue_mutex);
 
   if (register_key) {
@@ -388,15 +385,8 @@ void RecoveryUI::ProcessKey(int key_code, int updown) {
   }
 }
 
-void* RecoveryUI::time_key_helper(void* cookie) {
-  key_timer_t* info = static_cast<key_timer_t*>(cookie);
-  info->ui->time_key(info->key_code, info->count);
-  delete info;
-  return nullptr;
-}
-
-void RecoveryUI::time_key(int key_code, int count) {
-  usleep(750000);  // 750 ms == "long"
+void RecoveryUI::TimeKey(int key_code, int count) {
+  std::this_thread::sleep_for(750ms);  // 750 ms == "long"
   bool long_press = false;
   pthread_mutex_lock(&key_queue_mutex);
   if (key_last_down == key_code && key_down_count == count) {
diff --git a/ui.h b/ui.h
index a74b14f85..75390d83c 100644
--- a/ui.h
+++ b/ui.h
@@ -17,12 +17,13 @@
 #ifndef RECOVERY_UI_H
 #define RECOVERY_UI_H
 
-#include <linux/input.h>
+#include <linux/input.h>  // KEY_MAX
 #include <pthread.h>
-#include <time.h>
 
+#include <atomic>
 #include <functional>
 #include <string>
+#include <thread>
 #include <vector>
 
 // Abstract class for controlling the user interface during recovery.
@@ -51,7 +52,7 @@ class RecoveryUI {
 
   RecoveryUI();
 
-  virtual ~RecoveryUI() {}
+  virtual ~RecoveryUI();
 
   // Initializes the object; called before anything else. UI texts will be initialized according to
   // the given locale. Returns true on success.
@@ -172,12 +173,6 @@ class RecoveryUI {
     OFF
   };
 
-  struct key_timer_t {
-    RecoveryUI* ui;
-    int key_code;
-    int count;
-  };
-
   // The sensitivity when detecting a swipe.
   const int kTouchLowThreshold;
   const int kTouchHighThreshold;
@@ -186,12 +181,10 @@ class RecoveryUI {
   void OnTouchDetected(int dx, int dy);
   int OnInputEvent(int fd, uint32_t epevents);
   void ProcessKey(int key_code, int updown);
+  void TimeKey(int key_code, int count);
 
   bool IsUsbConnected();
 
-  static void* time_key_helper(void* cookie);
-  void time_key(int key_code, int count);
-
   bool InitScreensaver();
 
   // Key event input queue
@@ -223,7 +216,8 @@ class RecoveryUI {
   bool touch_swiping_;
   bool is_bootreason_recovery_ui_;
 
-  pthread_t input_thread_;
+  std::thread input_thread_;
+  std::atomic<bool> input_thread_stopped_{ false };
 
   ScreensaverState screensaver_state_;