Merge "screen_ui: Merge Menu::Start() into its ctor."

screen_ui.cpp

@@ -52,14 +52,23 @@ static double now() {
   return tv.tv_sec + tv.tv_usec / 1000000.0;
 }
 
-Menu::Menu(bool scrollable, size_t max_items, size_t max_length)
+Menu::Menu(bool scrollable, size_t max_items, size_t max_length, const char* const* headers,
+           const char* const* items, int initial_selection)
     : scrollable_(scrollable),
       max_display_items_(max_items),
       max_item_length_(max_length),
-      text_headers_(nullptr),
+      text_headers_(headers),
       menu_start_(0),
-      selection_(0) {
+      selection_(initial_selection) {
   CHECK_LE(max_items, static_cast<size_t>(std::numeric_limits<int>::max()));
+
+  // It's fine to have more entries than text_rows_ if scrollable menu is supported.
+  size_t max_items_count = scrollable_ ? std::numeric_limits<int>::max() : max_display_items_;
+  for (size_t i = 0; i < max_items_count && items[i] != nullptr; ++i) {
+    text_items_.emplace_back(items[i], strnlen(items[i], max_item_length_));
+  }
+
+  CHECK(!text_items_.empty());
 }
 
 const char* const* Menu::text_headers() const {
@@ -85,7 +94,7 @@ size_t Menu::ItemsCount() const {
 }
 
 bool Menu::ItemsOverflow(std::string* cur_selection_str) const {
-  if (!scrollable_ || static_cast<size_t>(ItemsCount()) <= max_display_items_) {
+  if (!scrollable_ || ItemsCount() <= max_display_items_) {
     return false;
   }
 
@@ -94,19 +103,6 @@ bool Menu::ItemsOverflow(std::string* cur_selection_str) const {
   return true;
 }
 
-void Menu::Start(const char* const* headers, const char* const* items, int initial_selection) {
-  text_headers_ = headers;
-
-  // It's fine to have more entries than text_rows_ if scrollable menu is supported.
-  size_t max_items_count = scrollable_ ? std::numeric_limits<int>::max() : max_display_items_;
-  for (size_t i = 0; i < max_items_count && items[i] != nullptr; ++i) {
-    text_items_.emplace_back(items[i], strnlen(items[i], max_item_length_));
-  }
-
-  CHECK(!text_items_.empty());
-  selection_ = initial_selection;
-}
-
 // TODO(xunchang) modify the function parameters to button up & down.
 int Menu::Select(int sel) {
   CHECK_LE(ItemsCount(), static_cast<size_t>(std::numeric_limits<int>::max()));
@@ -987,9 +983,8 @@ void ScreenRecoveryUI::StartMenu(const char* const* headers, const char* const*
                                  int initial_selection) {
   pthread_mutex_lock(&updateMutex);
   if (text_rows_ > 0 && text_cols_ > 1) {
-    menu_ = std::make_unique<Menu>(scrollable_menu_, text_rows_, text_cols_ - 1);
+    menu_ = std::make_unique<Menu>(scrollable_menu_, text_rows_, text_cols_ - 1, headers, items,
-    menu_->Start(headers, items, initial_selection);
+                                   initial_selection);
-
     update_screen_locked();
   }
   pthread_mutex_unlock(&updateMutex);

screen_ui.h

@@ -33,7 +33,10 @@ struct GRSurface;
 // This class maintains the menu selection and display of the screen ui.
 class Menu {
  public:
-  Menu(bool scrollable, size_t max_items, size_t max_length);
+  // Constructs a Menu instance with the given |headers|, |items| and properties. Sets the initial
+  // selection to |initial_selection|.
+  Menu(bool scrollable, size_t max_items, size_t max_length, const char* const* headers,
+       const char* const* items, int initial_selection);
 
   bool scrollable() const {
     return scrollable_;
@@ -45,8 +48,10 @@ class Menu {
 
   // Returns count of menu items.
   size_t ItemsCount() const;
+
   // Returns the index of the first menu item.
   size_t MenuStart() const;
+
   // Returns the index of the last menu item + 1.
   size_t MenuEnd() const;
 
@@ -68,10 +73,6 @@ class Menu {
   // |cur_selection_str| if the items exceed the screen limit.
   bool ItemsOverflow(std::string* cur_selection_str) const;
 
-  // Starts the menu with |headers| and |items| in text. Sets the default selection to
-  // |initial_selection|.
-  void Start(const char* const* headers, const char* const* items, int initial_selection);
-
   // Sets the current selection to |sel|. Handle the overflow cases depending on if the menu is
   // scrollable.
   int Select(int sel);

tests/unit/screen_ui_test.cpp

@@ -24,10 +24,8 @@ constexpr const char* HEADER[] = { "header", nullptr };
 constexpr const char* ITEMS[] = { "items1", "items2", "items3", "items4", "1234567890", nullptr };
 
 TEST(ScreenUITest, StartPhoneMenuSmoke) {
-  Menu menu(false, 10, 20);
+  Menu menu(false, 10, 20, HEADER, ITEMS, 0);
   ASSERT_FALSE(menu.scrollable());
-
-  menu.Start(HEADER, ITEMS, 0);
   ASSERT_EQ(HEADER[0], menu.text_headers()[0]);
   ASSERT_EQ(5u, menu.ItemsCount());
 
@@ -41,10 +39,8 @@ TEST(ScreenUITest, StartPhoneMenuSmoke) {
 }
 
 TEST(ScreenUITest, StartWearMenuSmoke) {
-  Menu menu(true, 10, 8);
+  Menu menu(true, 10, 8, HEADER, ITEMS, 1);
   ASSERT_TRUE(menu.scrollable());
-
-  menu.Start(HEADER, ITEMS, 1);
   ASSERT_EQ(HEADER[0], menu.text_headers()[0]);
   ASSERT_EQ(5u, menu.ItemsCount());
 
@@ -59,10 +55,8 @@ TEST(ScreenUITest, StartWearMenuSmoke) {
 }
 
 TEST(ScreenUITest, StartPhoneMenuItemsOverflow) {
-  Menu menu(false, 1, 20);
+  Menu menu(false, 1, 20, HEADER, ITEMS, 0);
   ASSERT_FALSE(menu.scrollable());
-
-  menu.Start(HEADER, ITEMS, 0);
   ASSERT_EQ(1u, menu.ItemsCount());
 
   std::string message;
@@ -76,10 +70,8 @@ TEST(ScreenUITest, StartPhoneMenuItemsOverflow) {
 }
 
 TEST(ScreenUITest, StartWearMenuItemsOverflow) {
-  Menu menu(true, 1, 20);
+  Menu menu(true, 1, 20, HEADER, ITEMS, 0);
   ASSERT_TRUE(menu.scrollable());
-
-  menu.Start(HEADER, ITEMS, 0);
   ASSERT_EQ(5u, menu.ItemsCount());
 
   std::string message;
@@ -95,10 +87,8 @@ TEST(ScreenUITest, StartWearMenuItemsOverflow) {
 }
 
 TEST(ScreenUITest, PhoneMenuSelectSmoke) {
-  Menu menu(false, 10, 20);
-
   int sel = 0;
-  menu.Start(HEADER, ITEMS, sel);
+  Menu menu(false, 10, 20, HEADER, ITEMS, sel);
   // Mimic down button 10 times (2 * items size)
   for (int i = 0; i < 10; i++) {
     sel = menu.Select(++sel);
@@ -126,10 +116,8 @@ TEST(ScreenUITest, PhoneMenuSelectSmoke) {
 }
 
 TEST(ScreenUITest, WearMenuSelectSmoke) {
-  Menu menu(true, 10, 20);
-
   int sel = 0;
-  menu.Start(HEADER, ITEMS, sel);
+  Menu menu(true, 10, 20, HEADER, ITEMS, sel);
   // Mimic pressing down button 10 times (2 * items size)
   for (int i = 0; i < 10; i++) {
     sel = menu.Select(++sel);
@@ -157,10 +145,8 @@ TEST(ScreenUITest, WearMenuSelectSmoke) {
 }
 
 TEST(ScreenUITest, WearMenuSelectItemsOverflow) {
-  Menu menu(true, 3, 20);
-
   int sel = 1;
-  menu.Start(HEADER, ITEMS, sel);
+  Menu menu(true, 3, 20, HEADER, ITEMS, sel);
   ASSERT_EQ(5u, menu.ItemsCount());
 
   // Scroll the menu to the end, and check the start & end of menu.

wear_ui.cpp

@@ -93,8 +93,7 @@ void WearRecoveryUI::StartMenu(const char* const* headers, const char* const* it
   pthread_mutex_lock(&updateMutex);
   if (text_rows_ > 0 && text_cols_ > 0) {
     menu_ = std::make_unique<Menu>(scrollable_menu_, text_rows_ - kMenuUnusableRows - 1,
-                                   text_cols_ - 1);
+                                   text_cols_ - 1, headers, items, initial_selection);
-    menu_->Start(headers, items, initial_selection);
 
     update_screen_locked();
   }