From da409fb8d615c3e1aac6505827026843aee25324 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Sun, 21 Oct 2018 23:36:26 -0700 Subject: ui: Manage loaded resources with smart pointers. Test: Run recovery_unit_test on marlin. Test: `Run graphics test` on marlin. Change-Id: I8239c3d9fb288f80ee11f615402768ff8ef8ecd0 --- screen_ui.cpp | 207 +++++++++++++++++++++--------------------- screen_ui.h | 85 +++++++++-------- tests/unit/screen_ui_test.cpp | 30 +++--- wear_ui.cpp | 6 +- 4 files changed, 165 insertions(+), 163 deletions(-) diff --git a/screen_ui.cpp b/screen_ui.cpp index 2db27d6a7..ed71888d1 100644 --- a/screen_ui.cpp +++ b/screen_ui.cpp @@ -197,11 +197,16 @@ int TextMenu::DrawItems(int x, int y, int screen_width, bool long_press) const { return offset; } -GraphicMenu::GraphicMenu(GRSurface* graphic_headers, const std::vector& graphic_items, +GraphicMenu::GraphicMenu(const GRSurface* graphic_headers, + const std::vector& graphic_items, size_t initial_selection, const DrawInterface& draw_funcs) - : Menu(initial_selection, draw_funcs), - graphic_headers_(graphic_headers), - graphic_items_(graphic_items) {} + : Menu(initial_selection, draw_funcs) { + graphic_headers_ = graphic_headers->Clone(); + graphic_items_.reserve(graphic_items.size()); + for (const auto& item : graphic_items) { + graphic_items_.emplace_back(item->Clone()); + } +} int GraphicMenu::Select(int sel) { CHECK_LE(graphic_items_.size(), static_cast(std::numeric_limits::max())); @@ -221,7 +226,7 @@ int GraphicMenu::Select(int sel) { int GraphicMenu::DrawHeader(int x, int y) const { draw_funcs_.SetColor(UIElement::HEADER); - draw_funcs_.DrawTextIcon(x, y, graphic_headers_); + draw_funcs_.DrawTextIcon(x, y, graphic_headers_.get()); return graphic_headers_->height; } @@ -242,7 +247,7 @@ int GraphicMenu::DrawItems(int x, int y, int screen_width, bool long_press) cons // Bold white text for the selected item. draw_funcs_.SetColor(UIElement::MENU_SEL_FG); } - draw_funcs_.DrawTextIcon(x, y + offset, item); + draw_funcs_.DrawTextIcon(x, y + offset, item.get()); offset += item->height; draw_funcs_.SetColor(UIElement::MENU); @@ -251,8 +256,8 @@ int GraphicMenu::DrawItems(int x, int y, int screen_width, bool long_press) cons return offset; } -bool GraphicMenu::Validate(size_t max_width, size_t max_height, GRSurface* graphic_headers, - const std::vector& graphic_items) { +bool GraphicMenu::Validate(size_t max_width, size_t max_height, const GRSurface* graphic_headers, + const std::vector& graphic_items) { int offset = 0; if (!ValidateGraphicSurface(max_width, max_height, offset, graphic_headers)) { return false; @@ -307,7 +312,9 @@ ScreenRecoveryUI::ScreenRecoveryUI(bool scrollable_menu) animation_fps_( android::base::GetIntProperty("ro.recovery.ui.animation_fps", kDefaultAnimationFps)), density_(static_cast(android::base::GetIntProperty("ro.sf.lcd_density", 160)) / 160.f), - currentIcon(NONE), + current_icon_(NONE), + current_frame_(0), + intro_done_(false), progressBarType(EMPTY), progressScopeStart(0), progressScopeSize(0), @@ -322,10 +329,6 @@ ScreenRecoveryUI::ScreenRecoveryUI(bool scrollable_menu) show_text_ever(false), scrollable_menu_(scrollable_menu), file_viewer_text_(nullptr), - intro_frames(0), - loop_frames(0), - current_frame(0), - intro_done(false), stage(-1), max_stage(-1), locale_(""), @@ -340,23 +343,23 @@ ScreenRecoveryUI::~ScreenRecoveryUI() { gr_exit(); } -GRSurface* ScreenRecoveryUI::GetCurrentFrame() const { - if (currentIcon == INSTALLING_UPDATE || currentIcon == ERASING) { - return intro_done ? loopFrames[current_frame] : introFrames[current_frame]; +const GRSurface* ScreenRecoveryUI::GetCurrentFrame() const { + if (current_icon_ == INSTALLING_UPDATE || current_icon_ == ERASING) { + return intro_done_ ? loop_frames_[current_frame_].get() : intro_frames_[current_frame_].get(); } - return error_icon; + return error_icon_.get(); } -GRSurface* ScreenRecoveryUI::GetCurrentText() const { - switch (currentIcon) { +const GRSurface* ScreenRecoveryUI::GetCurrentText() const { + switch (current_icon_) { case ERASING: - return erasing_text; + return erasing_text_.get(); case ERROR: - return error_text; + return error_text_.get(); case INSTALLING_UPDATE: - return installing_text; + return installing_text_.get(); case NO_COMMAND: - return no_command_text; + return no_command_text_.get(); case NONE: abort(); } @@ -391,20 +394,21 @@ static constexpr int kLayouts[LAYOUT_MAX][DIMENSION_MAX] = { }; int ScreenRecoveryUI::GetAnimationBaseline() const { - return GetTextBaseline() - PixelsFromDp(kLayouts[layout_][ICON]) - gr_get_height(loopFrames[0]); + return GetTextBaseline() - PixelsFromDp(kLayouts[layout_][ICON]) - + gr_get_height(loop_frames_[0].get()); } int ScreenRecoveryUI::GetTextBaseline() const { return GetProgressBaseline() - PixelsFromDp(kLayouts[layout_][TEXT]) - - gr_get_height(installing_text); + gr_get_height(installing_text_.get()); } int ScreenRecoveryUI::GetProgressBaseline() const { - int elements_sum = gr_get_height(loopFrames[0]) + PixelsFromDp(kLayouts[layout_][ICON]) + - gr_get_height(installing_text) + PixelsFromDp(kLayouts[layout_][TEXT]) + - gr_get_height(progressBarFill); + int elements_sum = gr_get_height(loop_frames_[0].get()) + PixelsFromDp(kLayouts[layout_][ICON]) + + gr_get_height(installing_text_.get()) + PixelsFromDp(kLayouts[layout_][TEXT]) + + gr_get_height(progress_bar_fill_.get()); int bottom_gap = (ScreenHeight() - elements_sum) / 2; - return ScreenHeight() - bottom_gap - gr_get_height(progressBarFill); + return ScreenHeight() - bottom_gap - gr_get_height(progress_bar_fill_.get()); } // Clear the screen and draw the currently selected background icon (if any). @@ -413,20 +417,20 @@ void ScreenRecoveryUI::draw_background_locked() { pagesIdentical = false; gr_color(0, 0, 0, 255); gr_clear(); - if (currentIcon != NONE) { + if (current_icon_ != NONE) { if (max_stage != -1) { - int stage_height = gr_get_height(stageMarkerEmpty); - int stage_width = gr_get_width(stageMarkerEmpty); - int x = (ScreenWidth() - max_stage * gr_get_width(stageMarkerEmpty)) / 2; + int stage_height = gr_get_height(stage_marker_empty_.get()); + int stage_width = gr_get_width(stage_marker_empty_.get()); + int x = (ScreenWidth() - max_stage * gr_get_width(stage_marker_empty_.get())) / 2; int y = ScreenHeight() - stage_height - margin_height_; for (int i = 0; i < max_stage; ++i) { - GRSurface* stage_surface = (i < stage) ? stageMarkerFill : stageMarkerEmpty; - DrawSurface(stage_surface, 0, 0, stage_width, stage_height, x, y); + const auto& stage_surface = (i < stage) ? stage_marker_fill_ : stage_marker_empty_; + DrawSurface(stage_surface.get(), 0, 0, stage_width, stage_height, x, y); x += stage_width; } } - GRSurface* text_surface = GetCurrentText(); + const auto& text_surface = GetCurrentText(); int text_x = (ScreenWidth() - gr_get_width(text_surface)) / 2; int text_y = GetTextBaseline(); gr_color(255, 255, 255, 255); @@ -437,8 +441,8 @@ void ScreenRecoveryUI::draw_background_locked() { // Draws the animation and progress bar (if any) on the screen. Does not flip pages. Should only be // called with updateMutex locked. void ScreenRecoveryUI::draw_foreground_locked() { - if (currentIcon != NONE) { - GRSurface* frame = GetCurrentFrame(); + if (current_icon_ != NONE) { + const auto& frame = GetCurrentFrame(); int frame_width = gr_get_width(frame); int frame_height = gr_get_height(frame); int frame_x = (ScreenWidth() - frame_width) / 2; @@ -447,8 +451,8 @@ void ScreenRecoveryUI::draw_foreground_locked() { } if (progressBarType != EMPTY) { - int width = gr_get_width(progressBarEmpty); - int height = gr_get_height(progressBarEmpty); + int width = gr_get_width(progress_bar_empty_.get()); + int height = gr_get_height(progress_bar_empty_.get()); int progress_x = (ScreenWidth() - width) / 2; int progress_y = GetProgressBaseline(); @@ -464,19 +468,20 @@ void ScreenRecoveryUI::draw_foreground_locked() { if (rtl_locale_) { // Fill the progress bar from right to left. if (pos > 0) { - DrawSurface(progressBarFill, width - pos, 0, pos, height, progress_x + width - pos, - progress_y); + DrawSurface(progress_bar_fill_.get(), width - pos, 0, pos, height, + progress_x + width - pos, progress_y); } if (pos < width - 1) { - DrawSurface(progressBarEmpty, 0, 0, width - pos, height, progress_x, progress_y); + DrawSurface(progress_bar_empty_.get(), 0, 0, width - pos, height, progress_x, progress_y); } } else { // Fill the progress bar from left to right. if (pos > 0) { - DrawSurface(progressBarFill, 0, 0, pos, height, progress_x, progress_y); + DrawSurface(progress_bar_fill_.get(), 0, 0, pos, height, progress_x, progress_y); } if (pos < width - 1) { - DrawSurface(progressBarEmpty, pos, 0, width - pos, height, progress_x + pos, progress_y); + DrawSurface(progress_bar_empty_.get(), pos, 0, width - pos, height, progress_x + pos, + progress_y); } } } @@ -518,15 +523,14 @@ void ScreenRecoveryUI::SelectAndShowBackgroundText(const std::vector text_name = { "erasing_text", "error_text", "installing_text", "installing_security_text", "no_command_text" }; - std::unordered_map> surfaces; + std::unordered_map> surfaces; for (const auto& name : text_name) { - GRSurface* text_image = nullptr; - LoadLocalizedBitmap(name.c_str(), &text_image); + auto text_image = LoadLocalizedBitmap(name); if (!text_image) { Print("Failed to load %s\n", name.c_str()); return; } - surfaces.emplace(name, std::unique_ptr(text_image, &free)); + surfaces.emplace(name, std::move(text_image)); } std::lock_guard lg(updateMutex); @@ -753,16 +757,16 @@ void ScreenRecoveryUI::ProgressThreadLoop() { // update the installation animation, if active // skip this if we have a text overlay (too expensive to update) - if ((currentIcon == INSTALLING_UPDATE || currentIcon == ERASING) && !show_text) { - if (!intro_done) { - if (current_frame == intro_frames - 1) { - intro_done = true; - current_frame = 0; + if ((current_icon_ == INSTALLING_UPDATE || current_icon_ == ERASING) && !show_text) { + if (!intro_done_) { + if (current_frame_ == intro_frames_.size() - 1) { + intro_done_ = true; + current_frame_ = 0; } else { - ++current_frame; + ++current_frame_; } } else { - current_frame = (current_frame + 1) % loop_frames; + current_frame_ = (current_frame_ + 1) % loop_frames_.size(); } redraw = true; @@ -791,18 +795,23 @@ void ScreenRecoveryUI::ProgressThreadLoop() { } } -void ScreenRecoveryUI::LoadBitmap(const char* filename, GRSurface** surface) { - int result = res_create_display_surface(filename, surface); - if (result < 0) { - LOG(ERROR) << "couldn't load bitmap " << filename << " (error " << result << ")"; +std::unique_ptr ScreenRecoveryUI::LoadBitmap(const std::string& filename) { + GRSurface* surface; + if (auto result = res_create_display_surface(filename.c_str(), &surface); result < 0) { + LOG(ERROR) << "Failed to load bitmap " << filename << " (error " << result << ")"; + return nullptr; } + return std::unique_ptr(surface); } -void ScreenRecoveryUI::LoadLocalizedBitmap(const char* filename, GRSurface** surface) { - int result = res_create_localized_alpha_surface(filename, locale_.c_str(), surface); - if (result < 0) { - LOG(ERROR) << "couldn't load bitmap " << filename << " (error " << result << ")"; +std::unique_ptr ScreenRecoveryUI::LoadLocalizedBitmap(const std::string& filename) { + GRSurface* surface; + if (auto result = res_create_localized_alpha_surface(filename.c_str(), locale_.c_str(), &surface); + result < 0) { + LOG(ERROR) << "Failed to load bitmap " << filename << " (error " << result << ")"; + return nullptr; } + return std::unique_ptr(surface); } static char** Alloc2d(size_t rows, size_t cols) { @@ -817,9 +826,9 @@ static char** Alloc2d(size_t rows, size_t cols) { // Choose the right background string to display during update. void ScreenRecoveryUI::SetSystemUpdateText(bool security_update) { if (security_update) { - LoadLocalizedBitmap("installing_security_text", &installing_text); + installing_text_ = LoadLocalizedBitmap("installing_security_text"); } else { - LoadLocalizedBitmap("installing_text", &installing_text); + installing_text_ = LoadLocalizedBitmap("installing_text"); } Redraw(); } @@ -838,10 +847,6 @@ bool ScreenRecoveryUI::InitTextParams() { // TODO(xunchang) load localized text icons for the menu. (Init for screenRecoveryUI but // not wearRecoveryUI). bool ScreenRecoveryUI::LoadWipeDataMenuText() { - wipe_data_menu_header_text_ = nullptr; - factory_data_reset_text_ = nullptr; - try_again_text_ = nullptr; - return true; } @@ -869,21 +874,20 @@ bool ScreenRecoveryUI::Init(const std::string& locale) { // Set up the locale info. SetLocale(locale); - LoadBitmap("icon_error", &error_icon); + error_icon_ = LoadBitmap("icon_error"); - LoadBitmap("progress_empty", &progressBarEmpty); - LoadBitmap("progress_fill", &progressBarFill); + progress_bar_empty_ = LoadBitmap("progress_empty"); + progress_bar_fill_ = LoadBitmap("progress_fill"); + stage_marker_empty_ = LoadBitmap("stage_empty"); + stage_marker_fill_ = LoadBitmap("stage_fill"); - LoadBitmap("stage_empty", &stageMarkerEmpty); - LoadBitmap("stage_fill", &stageMarkerFill); + erasing_text_ = LoadLocalizedBitmap("erasing_text"); + no_command_text_ = LoadLocalizedBitmap("no_command_text"); + error_text_ = LoadLocalizedBitmap("error_text"); - // Background text for "installing_update" could be "installing update" - // or "installing security update". It will be set after UI init according - // to commands in BCB. - installing_text = nullptr; - LoadLocalizedBitmap("erasing_text", &erasing_text); - LoadLocalizedBitmap("no_command_text", &no_command_text); - LoadLocalizedBitmap("error_text", &error_text); + // Background text for "installing_update" could be "installing update" or + // "installing security update". It will be set after Init() according to the commands in BCB. + installing_text_.reset(); LoadWipeDataMenuText(); @@ -915,32 +919,34 @@ void ScreenRecoveryUI::LoadAnimation() { } } - intro_frames = intro_frame_names.size(); - loop_frames = loop_frame_names.size(); + size_t intro_frames = intro_frame_names.size(); + size_t loop_frames = loop_frame_names.size(); // It's okay to not have an intro. - if (intro_frames == 0) intro_done = true; + if (intro_frames == 0) intro_done_ = true; // But you must have an animation. if (loop_frames == 0) abort(); std::sort(intro_frame_names.begin(), intro_frame_names.end()); std::sort(loop_frame_names.begin(), loop_frame_names.end()); - introFrames = new GRSurface*[intro_frames]; - for (size_t i = 0; i < intro_frames; i++) { - LoadBitmap(intro_frame_names.at(i).c_str(), &introFrames[i]); + intro_frames_.clear(); + intro_frames_.reserve(intro_frames); + for (const auto& frame_name : intro_frame_names) { + intro_frames_.emplace_back(LoadBitmap(frame_name)); } - loopFrames = new GRSurface*[loop_frames]; - for (size_t i = 0; i < loop_frames; i++) { - LoadBitmap(loop_frame_names.at(i).c_str(), &loopFrames[i]); + loop_frames_.clear(); + loop_frames_.reserve(loop_frames); + for (const auto& frame_name : loop_frame_names) { + loop_frames_.emplace_back(LoadBitmap(frame_name)); } } void ScreenRecoveryUI::SetBackground(Icon icon) { std::lock_guard lg(updateMutex); - currentIcon = icon; + current_icon_ = icon; update_screen_locked(); } @@ -972,7 +978,7 @@ void ScreenRecoveryUI::SetProgress(float fraction) { if (fraction > 1.0) fraction = 1.0; if (progressBarType == DETERMINATE && fraction > progress) { // Skip updates that aren't visibly different. - int width = gr_get_width(progressBarEmpty); + int width = gr_get_width(progress_bar_empty_.get()); float scale = width * progressScopeSize; if ((int)(progress * scale) != (int)(fraction * scale)) { progress = fraction; @@ -1115,11 +1121,10 @@ void ScreenRecoveryUI::ShowFile(const std::string& filename) { text_row_ = old_text_row; } -std::unique_ptr ScreenRecoveryUI::CreateMenu(GRSurface* graphic_header, - const std::vector& graphic_items, - const std::vector& text_headers, - const std::vector& text_items, - size_t initial_selection) const { +std::unique_ptr ScreenRecoveryUI::CreateMenu( + const GRSurface* graphic_header, const std::vector& graphic_items, + const std::vector& text_headers, const std::vector& text_items, + size_t initial_selection) const { // horizontal unusable area: margin width + menu indent size_t max_width = ScreenWidth() - margin_width_ - kMenuIndent; // vertical unusable area: margin height + title lines + helper message + high light bar. @@ -1235,9 +1240,9 @@ size_t ScreenRecoveryUI::ShowMenu(const std::vector& headers, size_t ScreenRecoveryUI::ShowPromptWipeDataMenu(const std::vector& backup_headers, const std::vector& backup_items, const std::function& key_handler) { - auto wipe_data_menu = - CreateMenu(wipe_data_menu_header_text_, { try_again_text_, factory_data_reset_text_ }, - backup_headers, backup_items, 0); + auto wipe_data_menu = CreateMenu(wipe_data_menu_header_text_.get(), + { try_again_text_.get(), factory_data_reset_text_.get() }, + backup_headers, backup_items, 0); if (wipe_data_menu == nullptr) { return 0; } diff --git a/screen_ui.h b/screen_ui.h index 2d0d97d8d..ff245a2fb 100644 --- a/screen_ui.h +++ b/screen_ui.h @@ -162,32 +162,31 @@ class TextMenu : public Menu { int char_height_; }; -// This class uses GRSurfaces* as the menu header and items. +// This class uses GRSurface's as the menu header and items. class GraphicMenu : public Menu { public: // Constructs a Menu instance with the given |headers|, |items| and properties. Sets the initial - // selection to |initial_selection|. - GraphicMenu(GRSurface* graphic_headers, const std::vector& graphic_items, + // selection to |initial_selection|. |headers| and |items| will be made local copies. + GraphicMenu(const GRSurface* graphic_headers, const std::vector& graphic_items, size_t initial_selection, const DrawInterface& draw_funcs); int Select(int sel) override; int DrawHeader(int x, int y) const override; int DrawItems(int x, int y, int screen_width, bool long_press) const override; - // Checks if all the header and items are valid GRSurfaces; and that they can fit in the area + // Checks if all the header and items are valid GRSurface's; and that they can fit in the area // defined by |max_width| and |max_height|. - static bool Validate(size_t max_width, size_t max_height, GRSurface* graphic_headers, - const std::vector& graphic_items); + static bool Validate(size_t max_width, size_t max_height, const GRSurface* graphic_headers, + const std::vector& graphic_items); // Returns true if |surface| fits on the screen with a vertical offset |y|. static bool ValidateGraphicSurface(size_t max_width, size_t max_height, int y, const GRSurface* surface); private: - // Pointers to the menu headers and items in graphic icons. This class does not have the ownership - // of the these objects. - GRSurface* graphic_headers_; - std::vector graphic_items_; + // Menu headers and items in graphic icons. These are the copies owned by the class instance. + std::unique_ptr graphic_headers_; + std::vector> graphic_items_; }; // Implementation of RecoveryUI appropriate for devices with a screen @@ -243,6 +242,7 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { protected: static constexpr int kMenuIndent = 4; + // The margin that we don't want to use for showing texts (e.g. round screen, or screen with // rounded corners). const int margin_width_; @@ -261,8 +261,8 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { // Creates a GraphicMenu with |graphic_header| and |graphic_items|. If the GraphicMenu isn't // valid or it doesn't fit on the screen; falls back to create a TextMenu instead. If succeeds, // returns a unique pointer to the created menu; otherwise returns nullptr. - virtual std::unique_ptr CreateMenu(GRSurface* graphic_header, - const std::vector& graphic_items, + virtual std::unique_ptr CreateMenu(const GRSurface* graphic_header, + const std::vector& graphic_items, const std::vector& text_headers, const std::vector& text_items, size_t initial_selection) const; @@ -288,8 +288,8 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { virtual void update_screen_locked(); virtual void update_progress_locked(); - GRSurface* GetCurrentFrame() const; - GRSurface* GetCurrentText() const; + const GRSurface* GetCurrentFrame() const; + const GRSurface* GetCurrentText() const; void ProgressThreadLoop(); @@ -299,8 +299,8 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { void ClearText(); void LoadAnimation(); - void LoadBitmap(const char* filename, GRSurface** surface); - void LoadLocalizedBitmap(const char* filename, GRSurface** surface); + std::unique_ptr LoadBitmap(const std::string& filename); + std::unique_ptr LoadLocalizedBitmap(const std::string& filename); int PixelsFromDp(int dp) const; virtual int GetAnimationBaseline() const; @@ -324,30 +324,34 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { int DrawTextLines(int x, int y, const std::vector& lines) const override; int DrawWrappedTextLines(int x, int y, const std::vector& lines) const override; - Icon currentIcon; - // The layout to use. int layout_; - GRSurface* error_icon; - - GRSurface* erasing_text; - GRSurface* error_text; - GRSurface* installing_text; - GRSurface* no_command_text; - - // Graphs for the wipe data menu - GRSurface* wipe_data_menu_header_text_; - GRSurface* try_again_text_; - GRSurface* factory_data_reset_text_; - - GRSurface** introFrames; - GRSurface** loopFrames; - - GRSurface* progressBarEmpty; - GRSurface* progressBarFill; - GRSurface* stageMarkerEmpty; - GRSurface* stageMarkerFill; + // The images that contain localized texts. + std::unique_ptr erasing_text_; + std::unique_ptr error_text_; + std::unique_ptr installing_text_; + std::unique_ptr no_command_text_; + + // Localized text images for the wipe data menu. + std::unique_ptr wipe_data_menu_header_text_; + std::unique_ptr try_again_text_; + std::unique_ptr factory_data_reset_text_; + + // current_icon_ points to one of the frames in intro_frames_ or loop_frames_, indexed by + // current_frame_, or error_icon_. + Icon current_icon_; + std::unique_ptr error_icon_; + std::vector> intro_frames_; + std::vector> loop_frames_; + size_t current_frame_; + bool intro_done_; + + // progress_bar and stage_marker images. + std::unique_ptr progress_bar_empty_; + std::unique_ptr progress_bar_fill_; + std::unique_ptr stage_marker_empty_; + std::unique_ptr stage_marker_fill_; ProgressType progressBarType; @@ -377,13 +381,6 @@ class ScreenRecoveryUI : public RecoveryUI, public DrawInterface { std::thread progress_thread_; std::atomic progress_thread_stopped_{ false }; - // Number of intro frames and loop frames in the animation. - size_t intro_frames; - size_t loop_frames; - - size_t current_frame; - bool intro_done; - int stage, max_stage; int char_width_; diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp index 3246e6a6e..b780af430 100644 --- a/tests/unit/screen_ui_test.cpp +++ b/tests/unit/screen_ui_test.cpp @@ -231,12 +231,12 @@ TEST_F(ScreenUITest, WearMenuSelectItemsOverflow) { } TEST_F(ScreenUITest, GraphicMenuSelection) { - auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); - auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); - std::vector items = { - item.get(), - item.get(), - item.get(), + auto image = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto header = image->Clone(); + std::vector items = { + image.get(), + image.get(), + image.get(), }; GraphicMenu menu(header.get(), items, 0, draw_funcs_); @@ -258,12 +258,12 @@ TEST_F(ScreenUITest, GraphicMenuSelection) { } TEST_F(ScreenUITest, GraphicMenuValidate) { - auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); - auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); - std::vector items = { - item.get(), - item.get(), - item.get(), + auto image = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto header = image->Clone(); + std::vector items = { + image.get(), + image.get(), + image.get(), }; ASSERT_TRUE(GraphicMenu::Validate(200, 200, header.get(), items)); @@ -273,7 +273,7 @@ TEST_F(ScreenUITest, GraphicMenuValidate) { ASSERT_FALSE(GraphicMenu::Validate(299, 200, wide_surface.get(), items)); // Menu exceeds the vertical boundary. - items.push_back(item.get()); + items.emplace_back(image.get()); ASSERT_FALSE(GraphicMenu::Validate(200, 249, header.get(), items)); } @@ -539,8 +539,8 @@ TEST_F(ScreenRecoveryUITest, LoadAnimation) { ui_->LoadAnimation(); - ASSERT_EQ(2u, ui_->intro_frames); - ASSERT_EQ(3u, ui_->loop_frames); + ASSERT_EQ(2u, ui_->intro_frames_.size()); + ASSERT_EQ(3u, ui_->loop_frames_.size()); for (const auto& name : tempfiles) { ASSERT_EQ(0, unlink(name.c_str())); diff --git a/wear_ui.cpp b/wear_ui.cpp index 0611f94c9..6da84c924 100644 --- a/wear_ui.cpp +++ b/wear_ui.cpp @@ -51,8 +51,8 @@ void WearRecoveryUI::draw_background_locked() { gr_color(0, 0, 0, 255); gr_fill(0, 0, gr_fb_width(), gr_fb_height()); - if (currentIcon != NONE) { - GRSurface* frame = GetCurrentFrame(); + if (current_icon_ != NONE) { + const auto& frame = GetCurrentFrame(); int frame_width = gr_get_width(frame); int frame_height = gr_get_height(frame); int frame_x = (gr_fb_width() - frame_width) / 2; @@ -60,7 +60,7 @@ void WearRecoveryUI::draw_background_locked() { gr_blit(frame, 0, 0, frame_width, frame_height, frame_x, frame_y); // Draw recovery text on screen above progress bar. - GRSurface* text = GetCurrentText(); + const auto& text = GetCurrentText(); int text_x = (ScreenWidth() - gr_get_width(text)) / 2; int text_y = GetProgressBaseline() - gr_get_height(text) - 10; gr_color(255, 255, 255, 255); -- cgit v1.2.3 From 3cb3c524f63b7d52d464b2bb124889d8c5c398f9 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 2 Nov 2018 13:36:58 -0700 Subject: tests: Add a testcase for updater overrun while patching. For any patching command, the resulting data should always exactly fill up the given target range. Test: Run recovery_component_test on marlin. Change-Id: Ib3cc1fc5c11094e2eab3fe370753db51c7c4135c --- tests/component/updater_test.cpp | 165 ++++++++++++++++++++------------------- 1 file changed, 85 insertions(+), 80 deletions(-) diff --git a/tests/component/updater_test.cpp b/tests/component/updater_test.cpp index c611c2291..32fec3808 100644 --- a/tests/component/updater_test.cpp +++ b/tests/component/updater_test.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -134,9 +135,9 @@ static void RunBlockImageUpdate(bool is_verify, const PackageEntries& entries, CloseArchive(handle); } -static std::string get_sha1(const std::string& content) { +static std::string GetSha1(std::string_view content) { uint8_t digest[SHA_DIGEST_LENGTH]; - SHA1(reinterpret_cast(content.c_str()), content.size(), digest); + SHA1(reinterpret_cast(content.data()), content.size(), digest); return print_sha1(digest); } @@ -187,7 +188,7 @@ class UpdaterTest : public ::testing::Test { // Clear partition updated marker if any. std::string updated_marker{ temp_stash_base_.path }; - updated_marker += "/" + get_sha1(image_temp_file_.path) + ".UPDATED"; + updated_marker += "/" + GetSha1(image_temp_file_.path) + ".UPDATED"; ASSERT_TRUE(android::base::RemoveFileIfExists(updated_marker)); } @@ -223,14 +224,14 @@ TEST_F(UpdaterTest, patch_partition_check) { std::string source_content; ASSERT_TRUE(android::base::ReadFileToString(source_file, &source_content)); size_t source_size = source_content.size(); - std::string source_hash = get_sha1(source_content); + std::string source_hash = GetSha1(source_content); Partition source(source_file, source_size, source_hash); std::string target_file = from_testdata_base("recovery.img"); std::string target_content; ASSERT_TRUE(android::base::ReadFileToString(target_file, &target_content)); size_t target_size = target_content.size(); - std::string target_hash = get_sha1(target_content); + std::string target_hash = GetSha1(target_content); Partition target(target_file, target_size, target_hash); // One argument is not valid. @@ -619,88 +620,92 @@ TEST_F(UpdaterTest, block_image_update_parsing_error) { RunBlockImageUpdate(false, entries, image_file_, "", kArgsParsingFailure); } -TEST_F(UpdaterTest, block_image_update_patch_data) { - std::string src_content = std::string(4096, 'a') + std::string(4096, 'c'); - std::string tgt_content = std::string(4096, 'b') + std::string(4096, 'd'); - +// Generates the bsdiff of the given source and target images, and writes the result entries. +// target_blocks specifies the block count to be written into the `bsdiff` command, which may be +// different from the given target size in order to trigger overrun / underrun paths. +static void GetEntriesForBsdiff(std::string_view source, std::string_view target, + size_t target_blocks, PackageEntries* entries) { // Generate the patch data. TemporaryFile patch_file; - ASSERT_EQ(0, - bsdiff::bsdiff(reinterpret_cast(src_content.data()), src_content.size(), - reinterpret_cast(tgt_content.data()), tgt_content.size(), - patch_file.path, nullptr)); + ASSERT_EQ(0, bsdiff::bsdiff(reinterpret_cast(source.data()), source.size(), + reinterpret_cast(target.data()), target.size(), + patch_file.path, nullptr)); std::string patch_content; ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch_content)); // Create the transfer list that contains a bsdiff. - std::string src_hash = get_sha1(src_content); - std::string tgt_hash = get_sha1(tgt_content); + std::string src_hash = GetSha1(source); + std::string tgt_hash = GetSha1(target); + size_t source_blocks = source.size() / 4096; std::vector transfer_list{ // clang-format off "4", - "2", + std::to_string(target_blocks), "0", - "2", - "stash " + src_hash + " 2,0,2", - android::base::StringPrintf("bsdiff 0 %zu %s %s 2,0,2 2 - %s:2,0,2", patch_content.size(), - src_hash.c_str(), tgt_hash.c_str(), src_hash.c_str()), - "free " + src_hash, + "0", + // bsdiff patch_offset patch_length source_hash target_hash target_range source_block_count + // source_range + android::base::StringPrintf("bsdiff 0 %zu %s %s 2,0,%zu %zu 2,0,%zu", patch_content.size(), + src_hash.c_str(), tgt_hash.c_str(), target_blocks, source_blocks, + source_blocks), // clang-format on }; - PackageEntries entries{ + *entries = { { "new_data", "" }, { "patch_data", patch_content }, { "transfer_list", android::base::Join(transfer_list, '\n') }, }; +} - ASSERT_TRUE(android::base::WriteStringToFile(src_content, image_file_)); - +TEST_F(UpdaterTest, block_image_update_patch_data) { + // Both source and target images have 10 blocks. + std::string source = + std::string(4096, 'a') + std::string(4096, 'c') + std::string(4096 * 3, '\0'); + std::string target = + std::string(4096, 'b') + std::string(4096, 'd') + std::string(4096 * 3, '\0'); + ASSERT_TRUE(android::base::WriteStringToFile(source, image_file_)); + + PackageEntries entries; + GetEntriesForBsdiff(std::string_view(source).substr(0, 4096 * 2), + std::string_view(target).substr(0, 4096 * 2), 2, &entries); RunBlockImageUpdate(false, entries, image_file_, "t"); // The update_file should be patched correctly. - std::string updated_content; - ASSERT_TRUE(android::base::ReadFileToString(image_file_, &updated_content)); - ASSERT_EQ(tgt_content, updated_content); + std::string updated; + ASSERT_TRUE(android::base::ReadFileToString(image_file_, &updated)); + ASSERT_EQ(target, updated); } -TEST_F(UpdaterTest, block_image_update_patch_underrun) { - std::string src_content = std::string(4096, 'a') + std::string(4096, 'c'); - std::string tgt_content = std::string(4096, 'b') + std::string(4096, 'd'); +TEST_F(UpdaterTest, block_image_update_patch_overrun) { + // Both source and target images have 10 blocks. + std::string source = + std::string(4096, 'a') + std::string(4096, 'c') + std::string(4096 * 3, '\0'); + std::string target = + std::string(4096, 'b') + std::string(4096, 'd') + std::string(4096 * 3, '\0'); + ASSERT_TRUE(android::base::WriteStringToFile(source, image_file_)); - // Generate the patch data. We intentionally provide one-byte short target to trigger the underrun - // path. - TemporaryFile patch_file; - ASSERT_EQ(0, - bsdiff::bsdiff(reinterpret_cast(src_content.data()), src_content.size(), - reinterpret_cast(tgt_content.data()), - tgt_content.size() - 1, patch_file.path, nullptr)); - std::string patch_content; - ASSERT_TRUE(android::base::ReadFileToString(patch_file.path, &patch_content)); + // Provide one less block to trigger the overrun path. + PackageEntries entries; + GetEntriesForBsdiff(std::string_view(source).substr(0, 4096 * 2), + std::string_view(target).substr(0, 4096 * 2), 1, &entries); - // Create the transfer list that contains a bsdiff. - std::string src_hash = get_sha1(src_content); - std::string tgt_hash = get_sha1(tgt_content); - std::vector transfer_list{ - // clang-format off - "4", - "2", - "0", - "2", - "stash " + src_hash + " 2,0,2", - android::base::StringPrintf("bsdiff 0 %zu %s %s 2,0,2 2 - %s:2,0,2", patch_content.size(), - src_hash.c_str(), tgt_hash.c_str(), src_hash.c_str()), - "free " + src_hash, - // clang-format on - }; - - PackageEntries entries{ - { "new_data", "" }, - { "patch_data", patch_content }, - { "transfer_list", android::base::Join(transfer_list, '\n') }, - }; + // The update should fail due to overrun. + RunBlockImageUpdate(false, entries, image_file_, "", kPatchApplicationFailure); +} - ASSERT_TRUE(android::base::WriteStringToFile(src_content, image_file_)); +TEST_F(UpdaterTest, block_image_update_patch_underrun) { + // Both source and target images have 10 blocks. + std::string source = + std::string(4096, 'a') + std::string(4096, 'c') + std::string(4096 * 3, '\0'); + std::string target = + std::string(4096, 'b') + std::string(4096, 'd') + std::string(4096 * 3, '\0'); + ASSERT_TRUE(android::base::WriteStringToFile(source, image_file_)); + + // Provide one more block to trigger the overrun path. + PackageEntries entries; + GetEntriesForBsdiff(std::string_view(source).substr(0, 4096 * 2), + std::string_view(target).substr(0, 4096 * 2), 3, &entries); // The update should fail due to underrun. RunBlockImageUpdate(false, entries, image_file_, "", kPatchApplicationFailure); @@ -708,7 +713,7 @@ TEST_F(UpdaterTest, block_image_update_patch_underrun) { TEST_F(UpdaterTest, block_image_update_fail) { std::string src_content(4096 * 2, 'e'); - std::string src_hash = get_sha1(src_content); + std::string src_hash = GetSha1(src_content); // Stash and free some blocks, then fail the update intentionally. std::vector transfer_list{ // clang-format off @@ -734,7 +739,7 @@ TEST_F(UpdaterTest, block_image_update_fail) { RunBlockImageUpdate(false, entries, image_file_, ""); // Updater generates the stash name based on the input file name. - std::string name_digest = get_sha1(image_file_); + std::string name_digest = GetSha1(image_file_); std::string stash_base = std::string(temp_stash_base_.path) + "/" + name_digest; ASSERT_EQ(0, access(stash_base.c_str(), F_OK)); // Expect the stashed blocks to be freed. @@ -838,9 +843,9 @@ TEST_F(UpdaterTest, last_command_update) { std::string block1(4096, '1'); std::string block2(4096, '2'); std::string block3(4096, '3'); - std::string block1_hash = get_sha1(block1); - std::string block2_hash = get_sha1(block2); - std::string block3_hash = get_sha1(block3); + std::string block1_hash = GetSha1(block1); + std::string block2_hash = GetSha1(block2); + std::string block3_hash = GetSha1(block3); // Compose the transfer list to fail the first update. std::vector transfer_list_fail{ @@ -906,8 +911,8 @@ TEST_F(UpdaterTest, last_command_update) { TEST_F(UpdaterTest, last_command_update_unresumable) { std::string block1(4096, '1'); std::string block2(4096, '2'); - std::string block1_hash = get_sha1(block1); - std::string block2_hash = get_sha1(block2); + std::string block1_hash = GetSha1(block1); + std::string block2_hash = GetSha1(block2); // Construct an unresumable update with source blocks mismatch. std::vector transfer_list_unresumable{ @@ -943,9 +948,9 @@ TEST_F(UpdaterTest, last_command_verify) { std::string block1(4096, '1'); std::string block2(4096, '2'); std::string block3(4096, '3'); - std::string block1_hash = get_sha1(block1); - std::string block2_hash = get_sha1(block2); - std::string block3_hash = get_sha1(block3); + std::string block1_hash = GetSha1(block1); + std::string block2_hash = GetSha1(block2); + std::string block3_hash = GetSha1(block3); std::vector transfer_list_verify{ // clang-format off @@ -1014,7 +1019,7 @@ class ResumableUpdaterTest : public testing::TestWithParam { // Clear partition updated marker if any. std::string updated_marker{ temp_stash_base_.path }; - updated_marker += "/" + get_sha1(image_temp_file_.path) + ".UPDATED"; + updated_marker += "/" + GetSha1(image_temp_file_.path) + ".UPDATED"; ASSERT_TRUE(android::base::RemoveFileIfExists(updated_marker)); } @@ -1045,10 +1050,10 @@ static std::vector GenerateTransferList() { std::string i(4096, 'i'); std::string zero(4096, '\0'); - std::string a_hash = get_sha1(a); - std::string b_hash = get_sha1(b); - std::string c_hash = get_sha1(c); - std::string e_hash = get_sha1(e); + std::string a_hash = GetSha1(a); + std::string b_hash = GetSha1(b); + std::string c_hash = GetSha1(c); + std::string e_hash = GetSha1(e); auto loc = [](const std::string& range_text) { std::vector pieces = android::base::Split(range_text, "-"); @@ -1069,8 +1074,8 @@ static std::vector GenerateTransferList() { // patch 1: "b d c" -> "g" TemporaryFile patch_file_bdc_g; std::string bdc = b + d + c; - std::string bdc_hash = get_sha1(bdc); - std::string g_hash = get_sha1(g); + std::string bdc_hash = GetSha1(bdc); + std::string g_hash = GetSha1(g); CHECK_EQ(0, bsdiff::bsdiff(reinterpret_cast(bdc.data()), bdc.size(), reinterpret_cast(g.data()), g.size(), patch_file_bdc_g.path, nullptr)); @@ -1080,9 +1085,9 @@ static std::vector GenerateTransferList() { // patch 2: "a b c d" -> "d c b" TemporaryFile patch_file_abcd_dcb; std::string abcd = a + b + c + d; - std::string abcd_hash = get_sha1(abcd); + std::string abcd_hash = GetSha1(abcd); std::string dcb = d + c + b; - std::string dcb_hash = get_sha1(dcb); + std::string dcb_hash = GetSha1(dcb); CHECK_EQ(0, bsdiff::bsdiff(reinterpret_cast(abcd.data()), abcd.size(), reinterpret_cast(dcb.data()), dcb.size(), patch_file_abcd_dcb.path, nullptr)); -- cgit v1.2.3 From 6394a7371390587a90664201c612059d961b240a Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 30 Apr 2018 15:38:14 -0700 Subject: Add tools/ to the style-checking path. Test: Touch a Java file in tools/. `repo upload` gives warnings. Change-Id: I381b01038d8a0c0e90817e383ca5323908fdd592 --- PREUPLOAD.cfg | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg index 108429193..28aa06f45 100644 --- a/PREUPLOAD.cfg +++ b/PREUPLOAD.cfg @@ -7,5 +7,4 @@ clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp [Hook Scripts] checkstyle_hook = ${REPO_ROOT}/prebuilts/checkstyle/checkstyle.py --sha ${PREUPLOAD_COMMIT} - -fw updater_sample/ - + --file_whitelist tools/ updater_sample/ -- cgit v1.2.3 From 529bb742b791a4396b36472c8cc54d181bf88fdc Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 6 Nov 2018 11:24:53 -0800 Subject: image_generator: Fix the warnings on import order. [platform/bootable/recovery] tools/image_generator/ImageGenerator.java:36: Wrong order for java.util.StringTokenizer import. Use Ctrl+Shift+O (Eclipse) or Ctrl+Alt+O (Intellij) to sort imports. https://source.android.com/setup/code-style#order-import-statementsERRORS: [platform/bootable/recovery] tools/image_generator/ImageGenerator.java:43: Wrong order for org.apache.commons.cli.CommandLine import. Use Ctrl+Shift+O (Eclipse) or Ctrl+Alt+O (Intellij) to sort imports. https://source.android.com/setup/code-style#order-import-statementsERRORS: [platform/bootable/recovery] tools/image_generator/ImageGenerator.java:50: Extra separation in import group before 'org.w3c.dom.Document' Test: `mmma -j bootable/recovery` Test: `repo upload` no longer gives warnings. Change-Id: If24c6b7ca33b9223b3e326a48885c24c35b5fa68 --- tools/image_generator/ImageGenerator.java | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tools/image_generator/ImageGenerator.java b/tools/image_generator/ImageGenerator.java index 8ed696a45..8cdd4944a 100644 --- a/tools/image_generator/ImageGenerator.java +++ b/tools/image_generator/ImageGenerator.java @@ -16,6 +16,16 @@ package com.android.recovery.tools; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.GnuParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.OptionBuilder; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.w3c.dom.Document; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + import java.awt.Color; import java.awt.Font; import java.awt.FontFormatException; @@ -32,25 +42,14 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.TreeMap; import java.util.StringTokenizer; +import java.util.TreeMap; import javax.imageio.ImageIO; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.GnuParser; -import org.apache.commons.cli.HelpFormatter; -import org.apache.commons.cli.OptionBuilder; -import org.apache.commons.cli.Options; -import org.apache.commons.cli.ParseException; - -import org.w3c.dom.Document; -import org.w3c.dom.Node; -import org.w3c.dom.NodeList; - /** * Command line tool to generate the localized image for recovery mode. */ @@ -552,4 +551,3 @@ public class ImageGenerator { imageGenerator.generateImage(localizedStringMap, cmd.getOptionValue("output_file")); } } - -- cgit v1.2.3