From 3d0cd1d0360ab3e536be87798415fef60222aac4 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 13 Apr 2018 13:41:58 -0700 Subject: Expose PngHandler via resources.h. As a private header for testing purpose. PngHandler additionally loads a given filename if the one with '/res/images' prefix is not available. It also provides color_type/bit_depth that are parsed from the PNG file. This allows reusing the same code for the ResourcesTest (renamed from ResourceTest). Test: Run recovery_manual_test on marlin. Change-Id: I3f939d79a1cb1b83a899847dbe2d51bde15d16d8 --- minui/include/private/resources.h | 84 +++++++++++++++++++++ minui/resources.cpp | 71 ++++-------------- tests/manual/recovery_test.cpp | 150 ++++++++++++++------------------------ 3 files changed, 152 insertions(+), 153 deletions(-) create mode 100644 minui/include/private/resources.h diff --git a/minui/include/private/resources.h b/minui/include/private/resources.h new file mode 100644 index 000000000..2a83a1028 --- /dev/null +++ b/minui/include/private/resources.h @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include + +#include + +// This class handles the PNG file parsing. It also holds the ownership of the PNG pointer and the +// opened file pointer. Both will be destroyed / closed when this object goes out of scope. +class PngHandler { + public: + // Constructs an instance by loading the PNG file from '/res/images/.png', or ''. + PngHandler(const std::string& name); + + ~PngHandler(); + + png_uint_32 width() const { + return width_; + } + + png_uint_32 height() const { + return height_; + } + + png_byte channels() const { + return channels_; + } + + int bit_depth() const { + return bit_depth_; + } + + int color_type() const { + return color_type_; + } + + png_structp png_ptr() const { + return png_ptr_; + } + + png_infop info_ptr() const { + return info_ptr_; + } + + int error_code() const { + return error_code_; + }; + + operator bool() const { + return error_code_ == 0; + } + + private: + png_structp png_ptr_{ nullptr }; + png_infop info_ptr_{ nullptr }; + png_uint_32 width_; + png_uint_32 height_; + png_byte channels_; + int bit_depth_; + int color_type_; + + // The |error_code_| is set to a negative value if an error occurs when opening the png file. + int error_code_{ 0 }; + // After initialization, we'll keep the file pointer open before destruction of PngHandler. + std::unique_ptr png_fp_{ nullptr, fclose }; +}; diff --git a/minui/resources.cpp b/minui/resources.cpp index 52ab60b1b..9f67cf844 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "private/resources.h" + #include #include #include @@ -48,58 +50,13 @@ static GRSurface* malloc_surface(size_t data_size) { return surface; } -// This class handles the png file parsing. It also holds the ownership of the png pointer and the -// opened file pointer. Both will be destroyed/closed when this object goes out of scope. -class PngHandler { - public: - PngHandler(const std::string& name); - - ~PngHandler(); - - png_uint_32 width() const { - return width_; - } - - png_uint_32 height() const { - return height_; - } - - png_byte channels() const { - return channels_; - } - - png_structp png_ptr() const { - return png_ptr_; - } - - png_infop info_ptr() const { - return info_ptr_; - } - - int error_code() const { - return error_code_; - }; - - operator bool() const { - return error_code_ == 0; - } - - private: - png_structp png_ptr_{ nullptr }; - png_infop info_ptr_{ nullptr }; - png_uint_32 width_; - png_uint_32 height_; - png_byte channels_; - - // The |error_code_| is set to a negative value if an error occurs when opening the png file. - int error_code_; - // After initialization, we'll keep the file pointer open before destruction of PngHandler. - std::unique_ptr png_fp_; -}; - -PngHandler::PngHandler(const std::string& name) : error_code_(0), png_fp_(nullptr, fclose) { +PngHandler::PngHandler(const std::string& name) { std::string res_path = android::base::StringPrintf("/res/images/%s.png", name.c_str()); png_fp_.reset(fopen(res_path.c_str(), "rbe")); + // Try to read from |name| if the resource path does not work. + if (!png_fp_) { + png_fp_.reset(fopen(name.c_str(), "rbe")); + } if (!png_fp_) { error_code_ = -1; return; @@ -138,19 +95,17 @@ PngHandler::PngHandler(const std::string& name) : error_code_(0), png_fp_(nullpt png_set_sig_bytes(png_ptr_, sizeof(header)); png_read_info(png_ptr_, info_ptr_); - int color_type; - int bit_depth; - png_get_IHDR(png_ptr_, info_ptr_, &width_, &height_, &bit_depth, &color_type, nullptr, nullptr, + png_get_IHDR(png_ptr_, info_ptr_, &width_, &height_, &bit_depth_, &color_type_, nullptr, nullptr, nullptr); channels_ = png_get_channels(png_ptr_, info_ptr_); - if (bit_depth == 8 && channels_ == 3 && color_type == PNG_COLOR_TYPE_RGB) { + if (bit_depth_ == 8 && channels_ == 3 && color_type_ == PNG_COLOR_TYPE_RGB) { // 8-bit RGB images: great, nothing to do. - } else if (bit_depth <= 8 && channels_ == 1 && color_type == PNG_COLOR_TYPE_GRAY) { + } else if (bit_depth_ <= 8 && channels_ == 1 && color_type_ == PNG_COLOR_TYPE_GRAY) { // 1-, 2-, 4-, or 8-bit gray images: expand to 8-bit gray. png_set_expand_gray_1_2_4_to_8(png_ptr_); - } else if (bit_depth <= 8 && channels_ == 1 && color_type == PNG_COLOR_TYPE_PALETTE) { + } else if (bit_depth_ <= 8 && channels_ == 1 && color_type_ == PNG_COLOR_TYPE_PALETTE) { // paletted images: expand to 8-bit RGB. Note that we DON'T // currently expand the tRNS chunk (if any) to an alpha // channel, because minui doesn't support alpha channels in @@ -158,8 +113,8 @@ PngHandler::PngHandler(const std::string& name) : error_code_(0), png_fp_(nullpt png_set_palette_to_rgb(png_ptr_); channels_ = 3; } else { - fprintf(stderr, "minui doesn't support PNG depth %d channels %d color_type %d\n", bit_depth, - channels_, color_type); + fprintf(stderr, "minui doesn't support PNG depth %d channels %d color_type %d\n", bit_depth_, + channels_, color_type_); error_code_ = -7; } } diff --git a/tests/manual/recovery_test.cpp b/tests/manual/recovery_test.cpp index 224ed5b68..c992b07e2 100644 --- a/tests/manual/recovery_test.cpp +++ b/tests/manual/recovery_test.cpp @@ -15,10 +15,14 @@ */ #include +#include +#include +#include #include #include #include +#include #include #include @@ -26,13 +30,13 @@ #include #include #include -#include #include #include "minui/minui.h" +#include "private/resources.h" -static const std::string myFilename = "/data/misc/recovery/inject.txt"; -static const std::string myContent = "Hello World\nWelcome to my recovery\n"; +static const std::string kInjectTxtFilename = "/data/misc/recovery/inject.txt"; +static const std::string kInjectTxtContent = "Hello World\nWelcome to my recovery\n"; static const std::string kLocale = "zu"; // Failure is expected on systems that do not deliver either the @@ -43,9 +47,9 @@ static ssize_t __pmsg_fn(log_id_t logId, char prio, const char *filename, const char *buf, size_t len, void *arg) { EXPECT_EQ(LOG_ID_SYSTEM, logId); EXPECT_EQ(ANDROID_LOG_INFO, prio); - EXPECT_NE(std::string::npos, myFilename.find(filename)); - EXPECT_EQ(myContent, buf); - EXPECT_EQ(myContent.size(), len); + EXPECT_NE(std::string::npos, kInjectTxtFilename.find(filename)); + EXPECT_EQ(kInjectTxtContent, buf); + EXPECT_EQ(kInjectTxtContent.size(), len); EXPECT_EQ(nullptr, arg); return len; } @@ -58,13 +62,14 @@ TEST(recovery, refresh) { ssize_t ret = __android_log_pmsg_file_read( LOG_ID_SYSTEM, ANDROID_LOG_INFO, "recovery/", __pmsg_fn, nullptr); if (ret == -ENOENT) { - EXPECT_LT(0, __android_log_pmsg_file_write(LOG_ID_SYSTEM, ANDROID_LOG_INFO, - myFilename.c_str(), myContent.c_str(), myContent.size())); + EXPECT_LT(0, __android_log_pmsg_file_write( + LOG_ID_SYSTEM, ANDROID_LOG_INFO, kInjectTxtFilename.c_str(), + kInjectTxtContent.c_str(), kInjectTxtContent.size())); - fprintf(stderr, "injected test data, requires two intervening reboots " - "to check for replication\n"); + fprintf(stderr, + "injected test data, requires two intervening reboots to check for replication\n"); } - EXPECT_EQ(static_cast(myContent.size()), ret); + EXPECT_EQ(static_cast(kInjectTxtContent.size()), ret); } // recovery.persist - Requires recovery.inject, then a reboot, then @@ -75,30 +80,26 @@ TEST(recovery, persist) { ssize_t ret = __android_log_pmsg_file_read( LOG_ID_SYSTEM, ANDROID_LOG_INFO, "recovery/", __pmsg_fn, nullptr); if (ret == -ENOENT) { - EXPECT_LT(0, __android_log_pmsg_file_write(LOG_ID_SYSTEM, ANDROID_LOG_INFO, - myFilename.c_str(), myContent.c_str(), myContent.size())); + EXPECT_LT(0, __android_log_pmsg_file_write( + LOG_ID_SYSTEM, ANDROID_LOG_INFO, kInjectTxtFilename.c_str(), + kInjectTxtContent.c_str(), kInjectTxtContent.size())); - fprintf(stderr, "injected test data, requires intervening reboot " - "to check for storage\n"); + fprintf(stderr, "injected test data, requires intervening reboot to check for storage\n"); } std::string buf; - EXPECT_TRUE(android::base::ReadFileToString(myFilename, &buf)); - EXPECT_EQ(myContent, buf); - if (access(myFilename.c_str(), F_OK) == 0) { - fprintf(stderr, "Removing persistent test data, " - "check if reconstructed on reboot\n"); + EXPECT_TRUE(android::base::ReadFileToString(kInjectTxtFilename, &buf)); + EXPECT_EQ(kInjectTxtContent, buf); + if (access(kInjectTxtFilename.c_str(), F_OK) == 0) { + fprintf(stderr, "Removing persistent test data, check if reconstructed on reboot\n"); } - EXPECT_EQ(0, unlink(myFilename.c_str())); + EXPECT_EQ(0, unlink(kInjectTxtFilename.c_str())); } -std::vector image_dir { - "res-mdpi/images/", - "res-hdpi/images/", - "res-xhdpi/images/", - "res-xxhdpi/images/", - "res-xxxhdpi/images/" -}; +static const std::vector kResourceImagesDirs{ "res-mdpi/images/", "res-hdpi/images/", + "res-xhdpi/images/", + "res-xxhdpi/images/", + "res-xxxhdpi/images/" }; static int png_filter(const dirent* de) { if (de->d_type != DT_REG || !android::base::EndsWith(de->d_name, "_text.png")) { @@ -107,12 +108,12 @@ static int png_filter(const dirent* de) { return 1; } -// Find out all the PNG files to test, which stay under the same dir with the executable. +// Finds out all the PNG files to test, which stay under the same dir with the executable. static std::vector add_files() { std::string exec_dir = android::base::GetExecutableDirectory(); std::vector files; - for (const std::string& image : image_dir) { - std::string dir_path = exec_dir + "/" + image; + for (const std::string& images_dir : kResourceImagesDirs) { + std::string dir_path = exec_dir + "/" + images_dir; dirent** namelist; int n = scandir(dir_path.c_str(), &namelist, png_filter, alphasort); if (n == -1) { @@ -133,71 +134,29 @@ static std::vector add_files() { return files; } -class ResourceTest : public testing::TestWithParam { +class ResourcesTest : public testing::TestWithParam { public: static std::vector png_list; - // Parse a png file and test if it's qualified for the background text image - // under recovery. + protected: void SetUp() override { - std::string file_path = GetParam(); - fp = fopen(file_path.c_str(), "rbe"); - ASSERT_NE(nullptr, fp); - - unsigned char header[8]; - size_t bytesRead = fread(header, 1, sizeof(header), fp); - ASSERT_EQ(sizeof(header), bytesRead); - ASSERT_EQ(0, png_sig_cmp(header, 0, sizeof(header))); - - png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr); - ASSERT_NE(nullptr, png_ptr); - - info_ptr = png_create_info_struct(png_ptr); - ASSERT_NE(nullptr, info_ptr); - - png_init_io(png_ptr, fp); - png_set_sig_bytes(png_ptr, sizeof(header)); - png_read_info(png_ptr, info_ptr); - - int color_type, bit_depth; - png_get_IHDR(png_ptr, info_ptr, &width, &height, &bit_depth, &color_type, nullptr, nullptr, - nullptr); - ASSERT_EQ(PNG_COLOR_TYPE_GRAY, color_type) << "Recovery expects grayscale PNG file."; - ASSERT_LT(static_cast(5), width); - ASSERT_LT(static_cast(0), height); - if (bit_depth <= 8) { - // 1-, 2-, 4-, or 8-bit gray images: expand to 8-bit gray. - png_set_expand_gray_1_2_4_to_8(png_ptr); - } + png_ = std::make_unique(GetParam()); + ASSERT_TRUE(png_); - png_byte channels = png_get_channels(png_ptr, info_ptr); - ASSERT_EQ(1, channels) << "Recovery background text images expects 1-channel PNG file."; + ASSERT_EQ(PNG_COLOR_TYPE_GRAY, png_->color_type()) << "Recovery expects grayscale PNG file."; + ASSERT_LT(static_cast(5), png_->width()); + ASSERT_LT(static_cast(0), png_->height()); + ASSERT_EQ(1, png_->channels()) << "Recovery background text images expects 1-channel PNG file."; } - void TearDown() override { - if (png_ptr != nullptr && info_ptr != nullptr) { - png_destroy_read_struct(&png_ptr, &info_ptr, nullptr); - } - - if (fp != nullptr) { - fclose(fp); - } - } - - protected: - png_structp png_ptr; - png_infop info_ptr; - png_uint_32 width, height; - - FILE* fp; + std::unique_ptr png_{ nullptr }; }; -std::vector ResourceTest::png_list = add_files(); - -TEST_P(ResourceTest, ValidateLocale) { - std::vector row(width); - for (png_uint_32 y = 0; y < height; ++y) { - png_read_row(png_ptr, row.data(), nullptr); +// Parses a png file and tests if it's qualified for the background text image under recovery. +TEST_P(ResourcesTest, ValidateLocale) { + std::vector row(png_->width()); + for (png_uint_32 y = 0; y < png_->height(); ++y) { + png_read_row(png_->png_ptr(), row.data(), nullptr); int w = (row[1] << 8) | row[0]; int h = (row[3] << 8) | row[2]; int len = row[4]; @@ -206,19 +165,20 @@ TEST_P(ResourceTest, ValidateLocale) { EXPECT_LT(0, len) << "Locale string should be non-empty."; EXPECT_NE(0, row[5]) << "Locale string is missing."; - ASSERT_GT(height, y + 1 + h) << "Locale: " << kLocale << " is not found in the file."; + ASSERT_GT(png_->height(), y + 1 + h) << "Locale: " << kLocale << " is not found in the file."; char* loc = reinterpret_cast(&row[5]); if (matches_locale(loc, kLocale.c_str())) { EXPECT_TRUE(android::base::StartsWith(loc, kLocale)); break; - } else { - for (int i = 0; i < h; ++i, ++y) { - png_read_row(png_ptr, row.data(), nullptr); - } + } + for (int i = 0; i < h; ++i, ++y) { + png_read_row(png_->png_ptr(), row.data(), nullptr); } } } -INSTANTIATE_TEST_CASE_P(BackgroundTextValidation, ResourceTest, - ::testing::ValuesIn(ResourceTest::png_list.cbegin(), - ResourceTest::png_list.cend())); +std::vector ResourcesTest::png_list = add_files(); + +INSTANTIATE_TEST_CASE_P(BackgroundTextValidation, ResourcesTest, + ::testing::ValuesIn(ResourcesTest::png_list.cbegin(), + ResourcesTest::png_list.cend())); -- cgit v1.2.3