From 9d05c8a35776444f6e967f8a4ac5863a31e54cf6 Mon Sep 17 00:00:00 2001 From: xunchang Date: Tue, 16 Apr 2019 12:07:42 -0700 Subject: Fall back to en-US if localized bitmap is missing for a locale We used to show the image for the last locale or an empty image, if the localized image is missing for a specific locale. As the default english one is more meaningful to users, we should just error out and fall back to use the default locale when the image loading fails. Bug: 128934634 Test: run graphic test, locale test Change-Id: Iafd3e8466aec63b4952d1959b2a3d37e358677d4 --- minui/resources.cpp | 14 ++++++++++---- recovery_main.cpp | 1 - recovery_ui/include/recovery_ui/ui.h | 2 ++ recovery_ui/screen_ui.cpp | 20 +++++++++++++++----- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/minui/resources.cpp b/minui/resources.cpp index 069a49529..53c932bff 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -414,12 +414,18 @@ int res_create_localized_alpha_surface(const char* name, __unused int len = row[4]; char* loc = reinterpret_cast(&row[5]); - if (y + 1 + h >= height || matches_locale(loc, locale)) { + // We need to include one additional line for the metadata of the localized image. + if (y + 1 + h > height) { + printf("Read exceeds the image boundary, y %u, h %d, height %u\n", y, h, height); + return -8; + } + + if (matches_locale(loc, locale)) { printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y); auto surface = GRSurface::Create(w, h, w, 1); if (!surface) { - return -8; + return -9; } for (int i = 0; i < h; ++i, ++y) { @@ -428,7 +434,7 @@ int res_create_localized_alpha_surface(const char* name, } *pSurface = surface.release(); - break; + return 0; } for (int i = 0; i < h; ++i, ++y) { @@ -436,7 +442,7 @@ int res_create_localized_alpha_surface(const char* name, } } - return 0; + return -10; } void res_free_surface(GRSurface* surface) { diff --git a/recovery_main.cpp b/recovery_main.cpp index 38e1db73b..37d9da0d7 100644 --- a/recovery_main.cpp +++ b/recovery_main.cpp @@ -373,7 +373,6 @@ int main(int argc, char** argv) { } if (locale.empty()) { - static constexpr const char* DEFAULT_LOCALE = "en-US"; locale = DEFAULT_LOCALE; } } diff --git a/recovery_ui/include/recovery_ui/ui.h b/recovery_ui/include/recovery_ui/ui.h index d55322cf0..797e2f0d5 100644 --- a/recovery_ui/include/recovery_ui/ui.h +++ b/recovery_ui/include/recovery_ui/ui.h @@ -27,6 +27,8 @@ #include #include +static constexpr const char* DEFAULT_LOCALE = "en-US"; + // Abstract class for controlling the user interface during recovery. class RecoveryUI { public: diff --git a/recovery_ui/screen_ui.cpp b/recovery_ui/screen_ui.cpp index 870db621c..823004521 100644 --- a/recovery_ui/screen_ui.cpp +++ b/recovery_ui/screen_ui.cpp @@ -817,12 +817,22 @@ std::unique_ptr ScreenRecoveryUI::LoadBitmap(const std::string& filen 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; + auto result = res_create_localized_alpha_surface(filename.c_str(), locale_.c_str(), &surface); + if (result == 0) { + return std::unique_ptr(surface); } - return std::unique_ptr(surface); + // TODO(xunchang) create a error code enum to refine the retry condition. + LOG(WARNING) << "Failed to load bitmap " << filename << " for locale " << locale_ << " (error " + << result << "). Falling back to use default locale."; + + result = res_create_localized_alpha_surface(filename.c_str(), DEFAULT_LOCALE, &surface); + if (result == 0) { + return std::unique_ptr(surface); + } + + LOG(ERROR) << "Failed to load bitmap " << filename << " for locale " << DEFAULT_LOCALE + << " (error " << result << ")"; + return nullptr; } static char** Alloc2d(size_t rows, size_t cols) { -- cgit v1.2.3 From 34723087fe79349607e3128707f0739e5e230a77 Mon Sep 17 00:00:00 2001 From: xunchang Date: Mon, 22 Apr 2019 15:32:17 -0700 Subject: matches_locale no longer accept empty locales in the png file The legacy png files have an empty line in the end. And the recovery used to match any missing locale, e.g. "he" with that line and gets an empty image. Since the empty image is barely useful, we should just error out and fall back to the default locale. This reversed the unit test check added in d17a6885253da909e376ba5ca5084f5281f3557c Bug: 128934634 Test: run locale test with "he" and legacy images, recovery reports error and doesn't crash even without default locale fall back Change-Id: Ibdb7dd0b42348de5e392c834cce67ff02be85c24 --- minui/resources.cpp | 4 ++++ tests/unit/locale_test.cpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/minui/resources.cpp b/minui/resources.cpp index 53c932bff..00d36d5fb 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -347,6 +347,10 @@ bool matches_locale(const std::string& prefix, const std::string& locale) { // match the locale string without the {script} section. // For instance, prefix == "en" matches locale == "en-US", prefix == "sr-Latn" matches locale // == "sr-Latn-BA", and prefix == "zh-CN" matches locale == "zh-Hans-CN". + if (prefix.empty()) { + return false; + } + if (android::base::StartsWith(locale, prefix)) { return true; } diff --git a/tests/unit/locale_test.cpp b/tests/unit/locale_test.cpp index cdaba0e8b..c69434c12 100644 --- a/tests/unit/locale_test.cpp +++ b/tests/unit/locale_test.cpp @@ -27,7 +27,7 @@ TEST(LocaleTest, Misc) { EXPECT_FALSE(matches_locale("en-GB", "en")); EXPECT_FALSE(matches_locale("en-GB", "en-US")); EXPECT_FALSE(matches_locale("en-US", "")); - // Empty locale prefix in the PNG file will match the input locale. - EXPECT_TRUE(matches_locale("", "en-US")); + // Empty locale prefix in the PNG file should not match the input locale. + EXPECT_FALSE(matches_locale("", "en-US")); EXPECT_TRUE(matches_locale("sr-Latn", "sr-Latn-BA")); } -- cgit v1.2.3