diff options
65 files changed, 1218 insertions, 1153 deletions
diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index 33db6db60c..56470d6764 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -4,6 +4,7 @@ cc_defaults { cflags: [ "-Wall", "-Werror", + "-Wextra", ], srcs: [ "CacheItem.cpp", @@ -25,6 +26,17 @@ cc_defaults { ], clang: true, + + tidy: true, + tidy_checks: [ + "-*", + "clang-analyzer-security*", + "cert-*", + "-cert-err58-cpp", + ], + tidy_flags: [ + "-warnings-as-errors=clang-analyzer-security*,cert-*" + ], } // diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index af7455a519..a72be59f74 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -711,6 +711,9 @@ binder::Status InstalldNativeService::fixupAppData(const std::unique_ptr<std::st // Ignore all other GID transitions, since they're kinda shady LOG(WARNING) << "Ignoring " << p->fts_path << " with unexpected GID " << actual << " instead of " << expected; + if (!(flags & FLAG_FORCE)) { + fts_set(fts, p, FTS_SKIP); + } } } } @@ -1863,7 +1866,7 @@ binder::Status InstalldNativeService::markBootComplete(const std::string& instru char boot_marker_path[PKG_PATH_MAX]; sprintf(boot_marker_path, "%s/%s/%s/.booting", - android_data_dir.path, + android_data_dir.c_str(), DALVIK_CACHE, instruction_set); diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 6f7ab6b52f..6a7d84580d 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -41,6 +41,7 @@ #include <system/thread_defs.h> #include "dexopt.h" +#include "globals.h" #include "installd_deps.h" #include "otapreopt_utils.h" #include "utils.h" @@ -156,7 +157,7 @@ static int split_count(const char *str) int count = 0; char buf[kPropertyValueMax]; - strncpy(buf, str, sizeof(buf)); + strlcpy(buf, str, sizeof(buf)); char *pBuf = buf; while(strtok_r(pBuf, " ", &ctx) != NULL) { @@ -333,7 +334,8 @@ static void run_dex2oat(int zip_fd, int oat_fd, int input_vdex_fd, int output_vd bool have_dex2oat_compiler_filter_flag = false; if (skip_compilation) { - strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=extract"); + strlcpy(dex2oat_compiler_filter_arg, "--compiler-filter=extract", + sizeof(dex2oat_compiler_filter_arg)); have_dex2oat_compiler_filter_flag = true; have_dex2oat_relocation_skip_flag = true; } else if (compiler_filter != nullptr) { @@ -955,14 +957,6 @@ static std::string create_vdex_filename(const std::string& oat_path) { return replace_file_extension(oat_path, ".vdex"); } -static bool add_extension_to_file_name(char* file_name, const char* extension) { - if (strlen(file_name) + strlen(extension) + 1 > PKG_PATH_MAX) { - return false; - } - strcat(file_name, extension); - return true; -} - static int open_output_file(const char* file_name, bool recreate, int permissions) { int flags = O_RDWR | O_CREAT; if (recreate) { @@ -1198,21 +1192,16 @@ unique_fd maybe_open_dexopt_swap_file(const char* out_oat_path) { if (!ShouldUseSwapFileForDexopt()) { return invalid_unique_fd(); } - // Make sure there really is enough space. - char swap_file_name[PKG_PATH_MAX]; - strcpy(swap_file_name, out_oat_path); - if (!add_extension_to_file_name(swap_file_name, ".swap")) { - return invalid_unique_fd(); - } + auto swap_file_name = std::string(out_oat_path) + ".swap"; unique_fd swap_fd(open_output_file( - swap_file_name, /*recreate*/true, /*permissions*/0600)); + swap_file_name.c_str(), /*recreate*/true, /*permissions*/0600)); if (swap_fd.get() < 0) { // Could not create swap file. Optimistically go on and hope that we can compile // without it. - ALOGE("installd could not create '%s' for swap during dexopt\n", swap_file_name); + ALOGE("installd could not create '%s' for swap during dexopt\n", swap_file_name.c_str()); } else { // Immediately unlink. We don't really want to hit flash. - if (unlink(swap_file_name) < 0) { + if (unlink(swap_file_name.c_str()) < 0) { PLOG(ERROR) << "Couldn't unlink swap file " << swap_file_name; } } @@ -2040,5 +2029,98 @@ bool delete_odex(const char* apk_path, const char* instruction_set, const char* return return_value_oat && return_value_art && return_value_vdex; } +static bool is_absolute_path(const std::string& path) { + if (path.find('/') != 0 || path.find("..") != std::string::npos) { + LOG(ERROR) << "Invalid absolute path " << path; + return false; + } else { + return true; + } +} + +static bool is_valid_instruction_set(const std::string& instruction_set) { + // TODO: add explicit whitelisting of instruction sets + if (instruction_set.find('/') != std::string::npos) { + LOG(ERROR) << "Invalid instruction set " << instruction_set; + return false; + } else { + return true; + } +} + +bool calculate_oat_file_path_default(char path[PKG_PATH_MAX], const char *oat_dir, + const char *apk_path, const char *instruction_set) { + std::string oat_dir_ = oat_dir; + std::string apk_path_ = apk_path; + std::string instruction_set_ = instruction_set; + + if (!is_absolute_path(oat_dir_)) return false; + if (!is_absolute_path(apk_path_)) return false; + if (!is_valid_instruction_set(instruction_set_)) return false; + + std::string::size_type end = apk_path_.rfind('.'); + std::string::size_type start = apk_path_.rfind('/', end); + if (end == std::string::npos || start == std::string::npos) { + LOG(ERROR) << "Invalid apk_path " << apk_path_; + return false; + } + + std::string res_ = oat_dir_ + '/' + instruction_set + '/' + + apk_path_.substr(start + 1, end - start - 1) + ".odex"; + const char* res = res_.c_str(); + if (strlen(res) >= PKG_PATH_MAX) { + LOG(ERROR) << "Result too large"; + return false; + } else { + strlcpy(path, res, PKG_PATH_MAX); + return true; + } +} + +bool calculate_odex_file_path_default(char path[PKG_PATH_MAX], const char *apk_path, + const char *instruction_set) { + std::string apk_path_ = apk_path; + std::string instruction_set_ = instruction_set; + + if (!is_absolute_path(apk_path_)) return false; + if (!is_valid_instruction_set(instruction_set_)) return false; + + std::string::size_type end = apk_path_.rfind('.'); + std::string::size_type start = apk_path_.rfind('/', end); + if (end == std::string::npos || start == std::string::npos) { + LOG(ERROR) << "Invalid apk_path " << apk_path_; + return false; + } + + std::string oat_dir = apk_path_.substr(0, start + 1) + "oat"; + return calculate_oat_file_path_default(path, oat_dir.c_str(), apk_path, instruction_set); +} + +bool create_cache_path_default(char path[PKG_PATH_MAX], const char *src, + const char *instruction_set) { + std::string src_ = src; + std::string instruction_set_ = instruction_set; + + if (!is_absolute_path(src_)) return false; + if (!is_valid_instruction_set(instruction_set_)) return false; + + for (auto it = src_.begin() + 1; it < src_.end(); ++it) { + if (*it == '/') { + *it = '@'; + } + } + + std::string res_ = android_data_dir + DALVIK_CACHE + '/' + instruction_set_ + src_ + + DALVIK_CACHE_POSTFIX; + const char* res = res_.c_str(); + if (strlen(res) >= PKG_PATH_MAX) { + LOG(ERROR) << "Result too large"; + return false; + } else { + strlcpy(path, res, PKG_PATH_MAX); + return true; + } +} + } // namespace installd } // namespace android diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h index 23446da43c..1f41e67e26 100644 --- a/cmds/installd/dexopt.h +++ b/cmds/installd/dexopt.h @@ -17,6 +17,8 @@ #ifndef DEXOPT_H_ #define DEXOPT_H_ +#include "installd_constants.h" + #include <sys/types.h> #include <cutils/multiuser.h> @@ -66,6 +68,15 @@ int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *ins const char* volume_uuid, const char* class_loader_context, const char* se_info, bool downgrade); +bool calculate_oat_file_path_default(char path[PKG_PATH_MAX], const char *oat_dir, + const char *apk_path, const char *instruction_set); + +bool calculate_odex_file_path_default(char path[PKG_PATH_MAX], const char *apk_path, + const char *instruction_set); + +bool create_cache_path_default(char path[PKG_PATH_MAX], const char *src, + const char *instruction_set); + } // namespace installd } // namespace android diff --git a/cmds/installd/globals.cpp b/cmds/installd/globals.cpp index edcdb6a1e2..b3a6dafa9a 100644 --- a/cmds/installd/globals.cpp +++ b/cmds/installd/globals.cpp @@ -16,15 +16,15 @@ #define LOG_TAG "installd" -#include <stdlib.h> -#include <string.h> - -#include <log/log.h> // TODO: Move everything to base::logging. - #include <globals.h> #include <installd_constants.h> #include <utils.h> +#include <android-base/logging.h> + +#include <stdlib.h> +#include <string.h> + namespace android { namespace installd { @@ -44,106 +44,78 @@ static constexpr const char* PROFILES_SUBDIR = "misc/profiles"; // sub-directory static constexpr const char* PRIVATE_APP_SUBDIR = "app-private/"; // sub-directory under // ANDROID_DATA -/* Directory records that are used in execution of commands. */ -dir_rec_t android_app_dir; -dir_rec_t android_app_ephemeral_dir; -dir_rec_t android_app_lib_dir; -dir_rec_t android_app_private_dir; -dir_rec_t android_asec_dir; -dir_rec_t android_data_dir; -dir_rec_t android_media_dir; -dir_rec_t android_mnt_expand_dir; -dir_rec_t android_profiles_dir; - -dir_rec_array_t android_system_dirs; - -/** - * Initialize all the global variables that are used elsewhere. Returns 0 upon - * success and -1 on error. - */ -void free_globals() { - size_t i; - - for (i = 0; i < android_system_dirs.count; i++) { - if (android_system_dirs.dirs[i].path != NULL) { - free(android_system_dirs.dirs[i].path); - } +std::string android_app_dir; +std::string android_app_ephemeral_dir; +std::string android_app_lib_dir; +std::string android_app_private_dir; +std::string android_asec_dir; +std::string android_data_dir; +std::string android_media_dir; +std::string android_mnt_expand_dir; +std::string android_profiles_dir; +std::string android_root_dir; + +std::vector<std::string> android_system_dirs; + +bool init_globals_from_data_and_root() { + const char* data_path = getenv("ANDROID_DATA"); + if (data_path == nullptr) { + LOG(ERROR) << "Could not find ANDROID_DATA"; + return false; + } + const char* root_path = getenv("ANDROID_ROOT"); + if (root_path == nullptr) { + LOG(ERROR) << "Could not find ANDROID_ROOT"; + return false; } + return init_globals_from_data_and_root(data_path, root_path); +} - free(android_system_dirs.dirs); +static std::string ensure_trailing_slash(const std::string& path) { + if (path.rfind('/') != path.size() - 1) { + return path + '/'; + } else { + return path; + } } bool init_globals_from_data_and_root(const char* data, const char* root) { // Get the android data directory. - if (get_path_from_string(&android_data_dir, data) < 0) { - return false; - } + android_data_dir = ensure_trailing_slash(data); + + // Get the android root directory. + android_root_dir = ensure_trailing_slash(root); // Get the android app directory. - if (copy_and_append(&android_app_dir, &android_data_dir, APP_SUBDIR) < 0) { - return false; - } + android_app_dir = android_data_dir + APP_SUBDIR; // Get the android protected app directory. - if (copy_and_append(&android_app_private_dir, &android_data_dir, PRIVATE_APP_SUBDIR) < 0) { - return false; - } + android_app_private_dir = android_data_dir + PRIVATE_APP_SUBDIR; // Get the android ephemeral app directory. - if (copy_and_append(&android_app_ephemeral_dir, &android_data_dir, EPHEMERAL_APP_SUBDIR) < 0) { - return false; - } + android_app_ephemeral_dir = android_data_dir + EPHEMERAL_APP_SUBDIR; // Get the android app native library directory. - if (copy_and_append(&android_app_lib_dir, &android_data_dir, APP_LIB_SUBDIR) < 0) { - return false; - } + android_app_lib_dir = android_data_dir + APP_LIB_SUBDIR; // Get the sd-card ASEC mount point. - if (get_path_from_env(&android_asec_dir, ASEC_MOUNTPOINT_ENV_NAME) < 0) { - return false; - } + android_asec_dir = ensure_trailing_slash(getenv(ASEC_MOUNTPOINT_ENV_NAME)); // Get the android media directory. - if (copy_and_append(&android_media_dir, &android_data_dir, MEDIA_SUBDIR) < 0) { - return false; - } + android_media_dir = android_data_dir + MEDIA_SUBDIR; // Get the android external app directory. - if (get_path_from_string(&android_mnt_expand_dir, "/mnt/expand/") < 0) { - return false; - } + android_mnt_expand_dir = "/mnt/expand/"; // Get the android profiles directory. - if (copy_and_append(&android_profiles_dir, &android_data_dir, PROFILES_SUBDIR) < 0) { - return false; - } + android_profiles_dir = android_data_dir + PROFILES_SUBDIR; // Take note of the system and vendor directories. - android_system_dirs.count = 4; - - android_system_dirs.dirs = (dir_rec_t*) calloc(android_system_dirs.count, sizeof(dir_rec_t)); - if (android_system_dirs.dirs == NULL) { - ALOGE("Couldn't allocate array for dirs; aborting\n"); - return false; - } - - dir_rec_t android_root_dir; - if (get_path_from_string(&android_root_dir, root) < 0) { - return false; - } - - android_system_dirs.dirs[0].path = build_string2(android_root_dir.path, APP_SUBDIR); - android_system_dirs.dirs[0].len = strlen(android_system_dirs.dirs[0].path); - - android_system_dirs.dirs[1].path = build_string2(android_root_dir.path, PRIV_APP_SUBDIR); - android_system_dirs.dirs[1].len = strlen(android_system_dirs.dirs[1].path); - - android_system_dirs.dirs[2].path = strdup("/vendor/app/"); - android_system_dirs.dirs[2].len = strlen(android_system_dirs.dirs[2].path); - - android_system_dirs.dirs[3].path = strdup("/oem/app/"); - android_system_dirs.dirs[3].len = strlen(android_system_dirs.dirs[3].path); + android_system_dirs.clear(); + android_system_dirs.push_back(android_root_dir + APP_SUBDIR); + android_system_dirs.push_back(android_root_dir + PRIV_APP_SUBDIR); + android_system_dirs.push_back("/vendor/app/"); + android_system_dirs.push_back("/oem/app/"); return true; } diff --git a/cmds/installd/globals.h b/cmds/installd/globals.h index c90beec49c..633e33bb7e 100644 --- a/cmds/installd/globals.h +++ b/cmds/installd/globals.h @@ -19,40 +19,29 @@ #define GLOBALS_H_ #include <inttypes.h> +#include <string> +#include <vector> namespace android { namespace installd { -/* constants */ - // Name of the environment variable that contains the asec mountpoint. static constexpr const char* ASEC_MOUNTPOINT_ENV_NAME = "ASEC_MOUNTPOINT"; -/* data structures */ - -struct dir_rec_t { - char* path; - size_t len; -}; - -struct dir_rec_array_t { - size_t count; - dir_rec_t* dirs; -}; - -extern dir_rec_t android_app_dir; -extern dir_rec_t android_app_ephemeral_dir; -extern dir_rec_t android_app_lib_dir; -extern dir_rec_t android_app_private_dir; -extern dir_rec_t android_asec_dir; -extern dir_rec_t android_data_dir; -extern dir_rec_t android_media_dir; -extern dir_rec_t android_mnt_expand_dir; -extern dir_rec_t android_profiles_dir; +extern std::string android_app_dir; +extern std::string android_app_ephemeral_dir; +extern std::string android_app_lib_dir; +extern std::string android_app_private_dir; +extern std::string android_asec_dir; +extern std::string android_data_dir; +extern std::string android_media_dir; +extern std::string android_mnt_expand_dir; +extern std::string android_profiles_dir; +extern std::string android_root_dir; -extern dir_rec_array_t android_system_dirs; +extern std::vector<std::string> android_system_dirs; -void free_globals(); +bool init_globals_from_data_and_root(); bool init_globals_from_data_and_root(const char* data, const char* root); } // namespace installd diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp index 35936a23af..95ed2fff35 100644 --- a/cmds/installd/installd.cpp +++ b/cmds/installd/installd.cpp @@ -30,6 +30,7 @@ #include <private/android_filesystem_config.h> #include "InstalldNativeService.h" +#include "dexopt.h" #include "globals.h" #include "installd_constants.h" #include "installd_deps.h" // Need to fill in requirements of commands. @@ -50,133 +51,22 @@ int get_property(const char *key, char *value, const char *default_value) { return property_get(key, value, default_value); } -// Compute the output path of -bool calculate_oat_file_path(char path[PKG_PATH_MAX], - const char *oat_dir, - const char *apk_path, - const char *instruction_set) { - const char *file_name_start; - const char *file_name_end; - - file_name_start = strrchr(apk_path, '/'); - if (file_name_start == NULL) { - SLOGE("apk_path '%s' has no '/'s in it\n", apk_path); - return false; - } - file_name_end = strrchr(apk_path, '.'); - if (file_name_end < file_name_start) { - SLOGE("apk_path '%s' has no extension\n", apk_path); - return false; - } - - // Calculate file_name - int file_name_len = file_name_end - file_name_start - 1; - char file_name[file_name_len + 1]; - memcpy(file_name, file_name_start + 1, file_name_len); - file_name[file_name_len] = '\0'; - - // <apk_parent_dir>/oat/<isa>/<file_name>.odex - snprintf(path, PKG_PATH_MAX, "%s/%s/%s.odex", oat_dir, instruction_set, file_name); - return true; +bool calculate_oat_file_path(char path[PKG_PATH_MAX], const char *oat_dir, const char *apk_path, + const char *instruction_set) { + return calculate_oat_file_path_default(path, oat_dir, apk_path, instruction_set); } -/* - * Computes the odex file for the given apk_path and instruction_set. - * /system/framework/whatever.jar -> /system/framework/oat/<isa>/whatever.odex - * - * Returns false if it failed to determine the odex file path. - */ -bool calculate_odex_file_path(char path[PKG_PATH_MAX], - const char *apk_path, - const char *instruction_set) { - if (strlen(apk_path) + strlen("oat/") + strlen(instruction_set) - + strlen("/") + strlen("odex") + 1 > PKG_PATH_MAX) { - SLOGE("apk_path '%s' may be too long to form odex file path.\n", apk_path); - return false; - } - - strcpy(path, apk_path); - char *end = strrchr(path, '/'); - if (end == NULL) { - SLOGE("apk_path '%s' has no '/'s in it?!\n", apk_path); - return false; - } - const char *apk_end = apk_path + (end - path); // strrchr(apk_path, '/'); - - strcpy(end + 1, "oat/"); // path = /system/framework/oat/\0 - strcat(path, instruction_set); // path = /system/framework/oat/<isa>\0 - strcat(path, apk_end); // path = /system/framework/oat/<isa>/whatever.jar\0 - end = strrchr(path, '.'); - if (end == NULL) { - SLOGE("apk_path '%s' has no extension.\n", apk_path); - return false; - } - strcpy(end + 1, "odex"); - return true; +bool calculate_odex_file_path(char path[PKG_PATH_MAX], const char *apk_path, + const char *instruction_set) { + return calculate_odex_file_path_default(path, apk_path, instruction_set); } -bool create_cache_path(char path[PKG_PATH_MAX], - const char *src, - const char *instruction_set) { - /* demand that we are an absolute path */ - if ((src == nullptr) || (src[0] != '/') || strstr(src,"..")) { - return false; - } - - size_t srclen = strlen(src); - - if (srclen > PKG_PATH_MAX) { // XXX: PKG_NAME_MAX? - return false; - } - - size_t dstlen = - android_data_dir.len + - strlen(DALVIK_CACHE) + - 1 + - strlen(instruction_set) + - srclen + - strlen(DALVIK_CACHE_POSTFIX) + 2; - - if (dstlen > PKG_PATH_MAX) { - return false; - } - - sprintf(path,"%s%s/%s/%s", - android_data_dir.path, - DALVIK_CACHE, - instruction_set, - src + 1 /* skip the leading / */); - - char* tmp = - path + - android_data_dir.len + - strlen(DALVIK_CACHE) + - 1 + - strlen(instruction_set) + 1; - - for(; *tmp; tmp++) { - if (*tmp == '/') { - *tmp = '@'; - } - } - - strcat(path, DALVIK_CACHE_POSTFIX); - return true; +bool create_cache_path(char path[PKG_PATH_MAX], const char *src, const char *instruction_set) { + return create_cache_path_default(path, src, instruction_set); } static bool initialize_globals() { - const char* data_path = getenv("ANDROID_DATA"); - if (data_path == nullptr) { - SLOGE("Could not find ANDROID_DATA"); - return false; - } - const char* root_path = getenv("ANDROID_ROOT"); - if (root_path == nullptr) { - SLOGE("Could not find ANDROID_ROOT"); - return false; - } - - return init_globals_from_data_and_root(data_path, root_path); + return init_globals_from_data_and_root(); } static int initialize_directories() { @@ -184,7 +74,7 @@ static int initialize_directories() { // Read current filesystem layout version to handle upgrade paths char version_path[PATH_MAX]; - snprintf(version_path, PATH_MAX, "%s.layout_version", android_data_dir.path); + snprintf(version_path, PATH_MAX, "%s.layout_version", android_data_dir.c_str()); int oldVersion; if (fs_read_atomic_int(version_path, &oldVersion) == -1) { @@ -206,7 +96,7 @@ static int initialize_directories() { SLOGD("Upgrading to /data/misc/user directories"); char misc_dir[PATH_MAX]; - snprintf(misc_dir, PATH_MAX, "%smisc", android_data_dir.path); + snprintf(misc_dir, PATH_MAX, "%smisc", android_data_dir.c_str()); char keychain_added_dir[PATH_MAX]; snprintf(keychain_added_dir, PATH_MAX, "%s/keychain/cacerts-added", misc_dir); @@ -227,7 +117,7 @@ static int initialize_directories() { if ((name[1] == '.') && (name[2] == 0)) continue; } - uint32_t user_id = atoi(name); + uint32_t user_id = std::stoi(name); // /data/misc/user/<user_id> if (ensure_config_user_dirs(user_id) == -1) { @@ -281,7 +171,7 @@ fail: return res; } -static int log_callback(int type, const char *fmt, ...) { +static int log_callback(int type, const char *fmt, ...) { // NOLINT va_list ap; int priority; diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp index 09e1a00d10..a58ba4117b 100644 --- a/cmds/installd/otapreopt.cpp +++ b/cmds/installd/otapreopt.cpp @@ -146,12 +146,12 @@ class OTAPreoptService { return 0; } // Copy in the default value. - strncpy(value, default_value, kPropertyValueMax - 1); + strlcpy(value, default_value, kPropertyValueMax - 1); value[kPropertyValueMax - 1] = 0; return strlen(default_value);// TODO: Need to truncate? } size_t size = std::min(kPropertyValueMax - 1, prop_value->length()); - strncpy(value, prop_value->data(), size); + strlcpy(value, prop_value->data(), size); value[size] = 0; return static_cast<int>(size); } diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp index 34818f65aa..ca812bdeeb 100644 --- a/cmds/installd/tests/installd_service_test.cpp +++ b/cmds/installd/tests/installd_service_test.cpp @@ -25,6 +25,7 @@ #include <gtest/gtest.h> #include "InstalldNativeService.h" +#include "dexopt.h" #include "globals.h" #include "utils.h" @@ -41,25 +42,18 @@ int get_property(const char *key, char *value, const char *default_value) { return property_get(key, value, default_value); } -bool calculate_oat_file_path(char path[PKG_PATH_MAX] ATTRIBUTE_UNUSED, - const char *oat_dir ATTRIBUTE_UNUSED, - const char *apk_path ATTRIBUTE_UNUSED, - const char *instruction_set ATTRIBUTE_UNUSED) { - return false; +bool calculate_oat_file_path(char path[PKG_PATH_MAX], const char *oat_dir, const char *apk_path, + const char *instruction_set) { + return calculate_oat_file_path_default(path, oat_dir, apk_path, instruction_set); } -bool calculate_odex_file_path(char path[PKG_PATH_MAX] ATTRIBUTE_UNUSED, - const char *apk_path ATTRIBUTE_UNUSED, - const char *instruction_set ATTRIBUTE_UNUSED) { - return false; +bool calculate_odex_file_path(char path[PKG_PATH_MAX], const char *apk_path, + const char *instruction_set) { + return calculate_odex_file_path_default(path, apk_path, instruction_set); } -bool create_cache_path(char path[PKG_PATH_MAX], - const char *src, - const char *instruction_set) { - // Not really a valid path but it's good enough for testing. - sprintf(path,"/data/dalvik-cache/%s/%s", instruction_set, src); - return true; +bool create_cache_path(char path[PKG_PATH_MAX], const char *src, const char *instruction_set) { + return create_cache_path_default(path, src, instruction_set); } static void mkdir(const char* path, uid_t owner, gid_t group, mode_t mode) { @@ -102,6 +96,8 @@ protected: testUuid = std::make_unique<std::string>(); *testUuid = std::string(kTestUuid); system("mkdir -p /data/local/tmp/user/0"); + + init_globals_from_data_and_root(); } virtual void TearDown() { @@ -153,12 +149,28 @@ TEST_F(ServiceTest, FixupAppData_Moved) { EXPECT_EQ(10000, stat_gid("com.example/bar/file")); } -TEST_F(ServiceTest, RmDexNoDalvikCache) { - LOG(INFO) << "RmDexNoDalvikCache"; +TEST_F(ServiceTest, CalculateOat) { + char buf[PKG_PATH_MAX]; + + EXPECT_TRUE(calculate_oat_file_path(buf, "/path/to/oat", "/path/to/file.apk", "isa")); + EXPECT_EQ("/path/to/oat/isa/file.odex", std::string(buf)); + + EXPECT_FALSE(calculate_oat_file_path(buf, "/path/to/oat", "/path/to/file", "isa")); + EXPECT_FALSE(calculate_oat_file_path(buf, "/path/to/oat", "file", "isa")); +} + +TEST_F(ServiceTest, CalculateOdex) { + char buf[PKG_PATH_MAX]; + + EXPECT_TRUE(calculate_odex_file_path(buf, "/path/to/file.apk", "isa")); + EXPECT_EQ("/path/to/oat/isa/file.odex", std::string(buf)); +} + +TEST_F(ServiceTest, CalculateCache) { + char buf[PKG_PATH_MAX]; - // Try to remove a non existing dalvik cache dex. The call should be - // successful because there's nothing to remove. - EXPECT_TRUE(service->rmdex("com.example", "arm").isOk()); + EXPECT_TRUE(create_cache_path(buf, "/path/to/file.apk", "isa")); + EXPECT_EQ("/data/dalvik-cache/isa/path@to@file.apk@classes.dex", std::string(buf)); } } // namespace installd diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp index 46ed85fd59..09dd25ae16 100644 --- a/cmds/installd/tests/installd_utils_test.cpp +++ b/cmds/installd/tests/installd_utils_test.cpp @@ -17,6 +17,7 @@ #include <stdlib.h> #include <string.h> +#include <android-base/logging.h> #include <gtest/gtest.h> #include "InstalldNativeService.h" @@ -27,6 +28,7 @@ #define LOG_TAG "utils_test" #define TEST_DATA_DIR "/data/" +#define TEST_ROOT_DIR "/system/" #define TEST_APP_DIR "/data/app/" #define TEST_APP_PRIVATE_DIR "/data/app-private/" #define TEST_APP_EPHEMERAL_DIR "/data/app-ephemeral/" @@ -44,39 +46,13 @@ namespace installd { class UtilsTest : public testing::Test { protected: virtual void SetUp() { - android_app_dir.path = (char*) TEST_APP_DIR; - android_app_dir.len = strlen(TEST_APP_DIR); + setenv("ANDROID_LOG_TAGS", "*:v", 1); + android::base::InitLogging(nullptr); - android_app_private_dir.path = (char*) TEST_APP_PRIVATE_DIR; - android_app_private_dir.len = strlen(TEST_APP_PRIVATE_DIR); - - android_app_ephemeral_dir.path = (char*) TEST_APP_EPHEMERAL_DIR; - android_app_ephemeral_dir.len = strlen(TEST_APP_EPHEMERAL_DIR); - - android_data_dir.path = (char*) TEST_DATA_DIR; - android_data_dir.len = strlen(TEST_DATA_DIR); - - android_asec_dir.path = (char*) TEST_ASEC_DIR; - android_asec_dir.len = strlen(TEST_ASEC_DIR); - - android_mnt_expand_dir.path = (char*) TEST_EXPAND_DIR; - android_mnt_expand_dir.len = strlen(TEST_EXPAND_DIR); - - android_system_dirs.count = 2; - - android_system_dirs.dirs = (dir_rec_t*) calloc(android_system_dirs.count, sizeof(dir_rec_t)); - android_system_dirs.dirs[0].path = (char*) TEST_SYSTEM_DIR1; - android_system_dirs.dirs[0].len = strlen(TEST_SYSTEM_DIR1); - - android_system_dirs.dirs[1].path = (char*) TEST_SYSTEM_DIR2; - android_system_dirs.dirs[1].len = strlen(TEST_SYSTEM_DIR2); - - android_profiles_dir.path = (char*) TEST_PROFILE_DIR; - android_profiles_dir.len = strlen(TEST_PROFILE_DIR); + init_globals_from_data_and_root(TEST_DATA_DIR, TEST_ROOT_DIR); } virtual void TearDown() { - free(android_system_dirs.dirs); } std::string create_too_long_path(const std::string& seed) { @@ -276,184 +252,6 @@ TEST_F(UtilsTest, CheckSystemApp_Subdir) { << badapp2 << " should be rejected not a system path"; } -TEST_F(UtilsTest, GetPathFromString_NullPathFail) { - dir_rec_t test1; - EXPECT_EQ(-1, get_path_from_string(&test1, (const char *) NULL)) - << "Should not allow NULL as a path."; -} - -TEST_F(UtilsTest, GetPathFromString_EmptyPathFail) { - dir_rec_t test1; - EXPECT_EQ(-1, get_path_from_string(&test1, "")) - << "Should not allow empty paths."; -} - -TEST_F(UtilsTest, GetPathFromString_RelativePathFail) { - dir_rec_t test1; - EXPECT_EQ(-1, get_path_from_string(&test1, "mnt/asec")) - << "Should not allow relative paths."; -} - -TEST_F(UtilsTest, GetPathFromString_NonCanonical) { - dir_rec_t test1; - - EXPECT_EQ(0, get_path_from_string(&test1, "/mnt/asec")) - << "Should be able to canonicalize directory /mnt/asec"; - EXPECT_STREQ("/mnt/asec/", test1.path) - << "/mnt/asec should be canonicalized to /mnt/asec/"; - EXPECT_EQ(10, (ssize_t) test1.len) - << "path len should be equal to the length of /mnt/asec/ (10)"; - free(test1.path); -} - -TEST_F(UtilsTest, GetPathFromString_CanonicalPath) { - dir_rec_t test3; - EXPECT_EQ(0, get_path_from_string(&test3, "/data/app/")) - << "Should be able to canonicalize directory /data/app/"; - EXPECT_STREQ("/data/app/", test3.path) - << "/data/app/ should be canonicalized to /data/app/"; - EXPECT_EQ(10, (ssize_t) test3.len) - << "path len should be equal to the length of /data/app/ (10)"; - free(test3.path); -} - -TEST_F(UtilsTest, CreatePkgPath_LongPkgNameSuccess) { - char path[PKG_PATH_MAX]; - - // Create long packagename of "aaaaa..." - size_t pkgnameSize = PKG_NAME_MAX; - char pkgname[pkgnameSize + 1]; - memset(pkgname, 'a', pkgnameSize); - pkgname[1] = '.'; - pkgname[pkgnameSize] = '\0'; - - EXPECT_EQ(0, create_pkg_path(path, pkgname, "", 0)) - << "Should successfully be able to create package name."; - - std::string prefix = std::string(TEST_DATA_DIR) + PRIMARY_USER_PREFIX; - size_t offset = prefix.length(); - - EXPECT_STREQ(pkgname, path + offset) - << "Package path should be a really long string of a's"; -} - -TEST_F(UtilsTest, CreatePkgPath_LongPostfixFail) { - char path[PKG_PATH_MAX]; - - // Create long packagename of "aaaaa..." - size_t postfixSize = PKG_PATH_MAX; - char postfix[postfixSize + 1]; - memset(postfix, 'a', postfixSize); - postfix[postfixSize] = '\0'; - - EXPECT_EQ(-1, create_pkg_path(path, "com.example.package", postfix, 0)) - << "Should return error because postfix is too long."; -} - -TEST_F(UtilsTest, CreatePkgPath_PrimaryUser) { - char path[PKG_PATH_MAX]; - - EXPECT_EQ(0, create_pkg_path(path, "com.example.package", "", 0)) - << "Should return error because postfix is too long."; - - std::string p = std::string(TEST_DATA_DIR) - + PRIMARY_USER_PREFIX - + "com.example.package"; - EXPECT_STREQ(p.c_str(), path) - << "Package path should be in /data/data/"; -} - -TEST_F(UtilsTest, CreatePkgPath_SecondaryUser) { - char path[PKG_PATH_MAX]; - - EXPECT_EQ(0, create_pkg_path(path, "com.example.package", "", 1)) - << "Should successfully create package path."; - - std::string p = std::string(TEST_DATA_DIR) - + SECONDARY_USER_PREFIX - + "1/com.example.package"; - EXPECT_STREQ(p.c_str(), path) - << "Package path should be in /data/user/"; -} - -TEST_F(UtilsTest, CreateMovePath_Primary) { - char path[PKG_PATH_MAX]; - - EXPECT_EQ(0, create_move_path(path, "com.android.test", "shared_prefs", 0)) - << "Should be able to create move path for primary user"; - - EXPECT_STREQ("/data/data/com.android.test/shared_prefs", path) - << "Primary user package directory should be created correctly"; -} - - -TEST_F(UtilsTest, CreateMovePath_Fail_AppTooLong) { - char path[PKG_PATH_MAX]; - std::string really_long_app_name = create_too_long_path("com.example"); - EXPECT_EQ(-1, create_move_path(path, really_long_app_name.c_str(), "shared_prefs", 0)) - << "Should fail to create move path for primary user"; -} - -TEST_F(UtilsTest, CreateMovePath_Fail_LeafTooLong) { - char path[PKG_PATH_MAX]; - std::string really_long_leaf_name = create_too_long_path("leaf_"); - EXPECT_EQ(-1, create_move_path(path, "com.android.test", really_long_leaf_name.c_str(), 0)) - << "Should fail to create move path for primary user"; -} - -TEST_F(UtilsTest, CopyAndAppend_Normal) { - //int copy_and_append(dir_rec_t* dst, dir_rec_t* src, char* suffix) - dir_rec_t dst; - dir_rec_t src; - - src.path = (char*) "/data/"; - src.len = strlen(src.path); - - EXPECT_EQ(0, copy_and_append(&dst, &src, "app/")) - << "Should return error because postfix is too long."; - - EXPECT_STREQ("/data/app/", dst.path) - << "Appended path should be correct"; - - EXPECT_EQ(10, (ssize_t) dst.len) - << "Appended path should be length of '/data/app/' (10)"; -} - -TEST_F(UtilsTest, AppendAndIncrement_Normal) { - size_t dst_size = 10; - char dst[dst_size]; - char *dstp = dst; - const char* src = "FOO"; - - EXPECT_EQ(0, append_and_increment(&dstp, src, &dst_size)) - << "String should append successfully"; - - EXPECT_STREQ("FOO", dst) - << "String should append correctly"; - - EXPECT_EQ(0, append_and_increment(&dstp, src, &dst_size)) - << "String should append successfully again"; - - EXPECT_STREQ("FOOFOO", dst) - << "String should append correctly again"; -} - -TEST_F(UtilsTest, AppendAndIncrement_TooBig) { - size_t dst_size = 5; - char dst[dst_size]; - char *dstp = dst; - const char* src = "FOO"; - - EXPECT_EQ(0, append_and_increment(&dstp, src, &dst_size)) - << "String should append successfully"; - - EXPECT_STREQ("FOO", dst) - << "String should append correctly"; - - EXPECT_EQ(-1, append_and_increment(&dstp, src, &dst_size)) - << "String should fail because it's too large to fit"; -} - TEST_F(UtilsTest, CreateDataPath) { EXPECT_EQ("/data", create_data_path(nullptr)); EXPECT_EQ("/mnt/expand/57f8f4bc-abf4-655f-bf67-946fc0f9f25b", diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index dd32ac6425..c25b2ce633 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -129,24 +129,6 @@ std::string create_data_user_de_package_path(const char* volume_uuid, create_data_user_de_path(volume_uuid, user).c_str(), package_name); } -int create_pkg_path(char path[PKG_PATH_MAX], const char *pkgname, - const char *postfix, userid_t userid) { - if (!is_valid_package_name(pkgname)) { - path[0] = '\0'; - return -1; - } - - std::string _tmp(create_data_user_ce_package_path(nullptr, userid, pkgname) + postfix); - const char* tmp = _tmp.c_str(); - if (strlen(tmp) >= PKG_PATH_MAX) { - path[0] = '\0'; - return -1; - } else { - strcpy(path, tmp); - return 0; - } -} - std::string create_data_path(const char* volume_uuid) { if (volume_uuid == nullptr) { return "/data"; @@ -213,7 +195,7 @@ std::string create_data_misc_legacy_path(userid_t userid) { } std::string create_primary_cur_profile_dir_path(userid_t userid) { - return StringPrintf("%s/cur/%u", android_profiles_dir.path, userid); + return StringPrintf("%s/cur/%u", android_profiles_dir.c_str(), userid); } std::string create_primary_current_profile_package_dir_path(userid_t user, @@ -224,12 +206,12 @@ std::string create_primary_current_profile_package_dir_path(userid_t user, } std::string create_primary_ref_profile_dir_path() { - return StringPrintf("%s/ref", android_profiles_dir.path); + return StringPrintf("%s/ref", android_profiles_dir.c_str()); } std::string create_primary_reference_profile_package_dir_path(const std::string& package_name) { check_package_name(package_name.c_str()); - return StringPrintf("%s/ref/%s", android_profiles_dir.path, package_name.c_str()); + return StringPrintf("%s/ref/%s", android_profiles_dir.c_str(), package_name.c_str()); } std::string create_data_dalvik_cache_path() { @@ -378,20 +360,6 @@ int calculate_tree_size(const std::string& path, int64_t* size, return 0; } -int create_move_path(char path[PKG_PATH_MAX], - const char* pkgname, - const char* leaf, - userid_t userid ATTRIBUTE_UNUSED) -{ - if ((android_data_dir.len + strlen(PRIMARY_USER_PREFIX) + strlen(pkgname) + strlen(leaf) + 1) - >= PKG_PATH_MAX) { - return -1; - } - - sprintf(path, "%s%s%s/%s", android_data_dir.path, PRIMARY_USER_PREFIX, pkgname, leaf); - return 0; -} - /** * Checks whether the package name is valid. Returns -1 on error and * 0 on success. @@ -761,22 +729,33 @@ std::string read_path_inode(const std::string& parent, const char* name, const c * The path is allowed to have at most one subdirectory and no indirections * to top level directories (i.e. have ".."). */ -static int validate_path(const dir_rec_t* dir, const char* path, int maxSubdirs) { - size_t dir_len = dir->len; - const char* subdir = strchr(path + dir_len, '/'); - - // Only allow the path to have at most one subdirectory. - if (subdir != NULL) { - ++subdir; - if ((--maxSubdirs == 0) && strchr(subdir, '/') != NULL) { - ALOGE("invalid apk path '%s' (subdir?)\n", path); - return -1; - } +static int validate_path(const std::string& dir, const std::string& path, int maxSubdirs) { + // Argument sanity checking + if (dir.find('/') != 0 || dir.rfind('/') != dir.size() - 1 + || dir.find("..") != std::string::npos) { + LOG(ERROR) << "Invalid directory " << dir; + return -1; + } + if (path.find("..") != std::string::npos) { + LOG(ERROR) << "Invalid path " << path; + return -1; } - // Directories can't have a period directly after the directory markers to prevent "..". - if ((path[dir_len] == '.') || ((subdir != NULL) && (*subdir == '.'))) { - ALOGE("invalid apk path '%s' (trickery)\n", path); + if (path.compare(0, dir.size(), dir) != 0) { + // Common case, path isn't under directory + return -1; + } + + // Count number of subdirectories + auto pos = path.find('/', dir.size()); + int count = 0; + while (pos != std::string::npos) { + pos = path.find('/', pos + 1); + count++; + } + + if (count > maxSubdirs) { + LOG(ERROR) << "Invalid path depth " << path << " when tested against " << dir; return -1; } @@ -788,15 +767,12 @@ static int validate_path(const dir_rec_t* dir, const char* path, int maxSubdirs) * if it is a system app or -1 if it is not. */ int validate_system_app_path(const char* path) { - size_t i; - - for (i = 0; i < android_system_dirs.count; i++) { - const size_t dir_len = android_system_dirs.dirs[i].len; - if (!strncmp(path, android_system_dirs.dirs[i].path, dir_len)) { - return validate_path(android_system_dirs.dirs + i, path, 1); + std::string path_ = path; + for (const auto& dir : android_system_dirs) { + if (validate_path(dir, path, 1) == 0) { + return 0; } } - return -1; } @@ -834,116 +810,26 @@ bool validate_secondary_dex_path(const std::string& pkgname, const std::string& } /** - * Get the contents of a environment variable that contains a path. Caller - * owns the string that is inserted into the directory record. Returns - * 0 on success and -1 on error. - */ -int get_path_from_env(dir_rec_t* rec, const char* var) { - const char* path = getenv(var); - int ret = get_path_from_string(rec, path); - if (ret < 0) { - ALOGW("Problem finding value for environment variable %s\n", var); - } - return ret; -} - -/** - * Puts the string into the record as a directory. Appends '/' to the end - * of all paths. Caller owns the string that is inserted into the directory - * record. A null value will result in an error. - * - * Returns 0 on success and -1 on error. - */ -int get_path_from_string(dir_rec_t* rec, const char* path) { - if (path == NULL) { - return -1; - } else { - const size_t path_len = strlen(path); - if (path_len <= 0) { - return -1; - } - - // Make sure path is absolute. - if (path[0] != '/') { - return -1; - } - - if (path[path_len - 1] == '/') { - // Path ends with a forward slash. Make our own copy. - - rec->path = strdup(path); - if (rec->path == NULL) { - return -1; - } - - rec->len = path_len; - } else { - // Path does not end with a slash. Generate a new string. - char *dst; - - // Add space for slash and terminating null. - size_t dst_size = path_len + 2; - - rec->path = (char*) malloc(dst_size); - if (rec->path == NULL) { - return -1; - } - - dst = rec->path; - - if (append_and_increment(&dst, path, &dst_size) < 0 - || append_and_increment(&dst, "/", &dst_size)) { - ALOGE("Error canonicalizing path"); - return -1; - } - - rec->len = dst - rec->path; - } - } - return 0; -} - -int copy_and_append(dir_rec_t* dst, const dir_rec_t* src, const char* suffix) { - dst->len = src->len + strlen(suffix); - const size_t dstSize = dst->len + 1; - dst->path = (char*) malloc(dstSize); - - if (dst->path == NULL - || snprintf(dst->path, dstSize, "%s%s", src->path, suffix) - != (ssize_t) dst->len) { - ALOGE("Could not allocate memory to hold appended path; aborting\n"); - return -1; - } - - return 0; -} - -/** * Check whether path points to a valid path for an APK file. The path must * begin with a whitelisted prefix path and must be no deeper than |maxSubdirs| within * that path. Returns -1 when an invalid path is encountered and 0 when a valid path * is encountered. */ static int validate_apk_path_internal(const char *path, int maxSubdirs) { - const dir_rec_t* dir = NULL; - if (!strncmp(path, android_app_dir.path, android_app_dir.len)) { - dir = &android_app_dir; - } else if (!strncmp(path, android_app_private_dir.path, android_app_private_dir.len)) { - dir = &android_app_private_dir; - } else if (!strncmp(path, android_app_ephemeral_dir.path, android_app_ephemeral_dir.len)) { - dir = &android_app_ephemeral_dir; - } else if (!strncmp(path, android_asec_dir.path, android_asec_dir.len)) { - dir = &android_asec_dir; - } else if (!strncmp(path, android_mnt_expand_dir.path, android_mnt_expand_dir.len)) { - dir = &android_mnt_expand_dir; - if (maxSubdirs < 2) { - maxSubdirs = 2; - } + std::string path_ = path; + if (validate_path(android_app_dir, path_, maxSubdirs) == 0) { + return 0; + } else if (validate_path(android_app_private_dir, path_, maxSubdirs) == 0) { + return 0; + } else if (validate_path(android_app_ephemeral_dir, path_, maxSubdirs) == 0) { + return 0; + } else if (validate_path(android_asec_dir, path_, maxSubdirs) == 0) { + return 0; + } else if (validate_path(android_mnt_expand_dir, path_, std::max(maxSubdirs, 2)) == 0) { + return 0; } else { return -1; } - - return validate_path(dir, path, maxSubdirs); } int validate_apk_path(const char* path) { @@ -954,48 +840,6 @@ int validate_apk_path_subdirs(const char* path) { return validate_apk_path_internal(path, 3 /* maxSubdirs */); } -int append_and_increment(char** dst, const char* src, size_t* dst_size) { - ssize_t ret = strlcpy(*dst, src, *dst_size); - if (ret < 0 || (size_t) ret >= *dst_size) { - return -1; - } - *dst += ret; - *dst_size -= ret; - return 0; -} - -char *build_string2(const char *s1, const char *s2) { - if (s1 == NULL || s2 == NULL) return NULL; - - int len_s1 = strlen(s1); - int len_s2 = strlen(s2); - int len = len_s1 + len_s2 + 1; - char *result = (char *) malloc(len); - if (result == NULL) return NULL; - - strcpy(result, s1); - strcpy(result + len_s1, s2); - - return result; -} - -char *build_string3(const char *s1, const char *s2, const char *s3) { - if (s1 == NULL || s2 == NULL || s3 == NULL) return NULL; - - int len_s1 = strlen(s1); - int len_s2 = strlen(s2); - int len_s3 = strlen(s3); - int len = len_s1 + len_s2 + len_s3 + 1; - char *result = (char *) malloc(len); - if (result == NULL) return NULL; - - strcpy(result, s1); - strcpy(result + len_s1, s2); - strcpy(result + len_s1 + len_s2, s3); - - return result; -} - int ensure_config_user_dirs(userid_t userid) { // writable by system, readable by any app within the same user const int uid = multiuser_get_uid(userid, AID_SYSTEM); @@ -1068,7 +912,7 @@ int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t ta } else { // Mismatched GID/mode is recoverable; fall through to update LOG(DEBUG) << "Mismatched cache GID/mode at " << path << ": found " << st.st_gid - << " but expected " << gid; + << "/" << actual_mode << " but expected " << gid << "/" << target_mode; } // Directory is owned correctly, but GID or mode mismatch means it's diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h index e938042a3b..5af575607f 100644 --- a/cmds/installd/utils.h +++ b/cmds/installd/utils.h @@ -41,18 +41,11 @@ namespace android { namespace installd { -struct dir_rec_t; - constexpr const char* kXattrInodeCache = "user.inode_cache"; constexpr const char* kXattrInodeCodeCache = "user.inode_code_cache"; constexpr const char* kXattrCacheGroup = "user.cache_group"; constexpr const char* kXattrCacheTombstone = "user.cache_tombstone"; -int create_pkg_path(char path[PKG_PATH_MAX], - const char *pkgname, - const char *postfix, - userid_t userid); - std::string create_data_path(const char* volume_uuid); std::string create_data_app_path(const char* volume_uuid); @@ -96,11 +89,6 @@ int calculate_tree_size(const std::string& path, int64_t* size, int create_user_config_path(char path[PKG_PATH_MAX], userid_t userid); -int create_move_path(char path[PKG_PATH_MAX], - const char* pkgname, - const char* leaf, - userid_t userid); - bool is_valid_filename(const std::string& name); bool is_valid_package_name(const std::string& packageName); @@ -127,20 +115,9 @@ int validate_system_app_path(const char* path); bool validate_secondary_dex_path(const std::string& pkgname, const std::string& dex_path, const char* volume_uuid, int uid, int storage_flag, bool validate_package_path = true); -int get_path_from_env(dir_rec_t* rec, const char* var); - -int get_path_from_string(dir_rec_t* rec, const char* path); - -int copy_and_append(dir_rec_t* dst, const dir_rec_t* src, const char* suffix); - int validate_apk_path(const char *path); int validate_apk_path_subdirs(const char *path); -int append_and_increment(char** dst, const char* src, size_t* dst_size); - -char *build_string2(const char *s1, const char *s2); -char *build_string3(const char *s1, const char *s2, const char *s3); - int ensure_config_user_dirs(userid_t userid); int wait_child(pid_t pid); diff --git a/cmds/lshal/Android.bp b/cmds/lshal/Android.bp index 5a87505113..6cbe7e24fa 100644 --- a/cmds/lshal/Android.bp +++ b/cmds/lshal/Android.bp @@ -20,6 +20,7 @@ cc_library_shared { "libutils", "libhidlbase", "libhidltransport", + "libhidl-gen-hash", "libhidl-gen-utils", "libvintf", ], diff --git a/cmds/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp index 8b59fb867d..73996925b1 100644 --- a/cmds/lshal/ListCommand.cpp +++ b/cmds/lshal/ListCommand.cpp @@ -27,6 +27,7 @@ #include <android-base/parseint.h> #include <android/hidl/manager/1.0/IServiceManager.h> +#include <hidl-hash/Hash.h> #include <hidl-util/FQName.h> #include <private/android_filesystem_config.h> #include <sys/stat.h> @@ -39,6 +40,9 @@ #include "utils.h" using ::android::hardware::hidl_string; +using ::android::hardware::hidl_vec; +using ::android::hidl::base::V1_0::DebugInfo; +using ::android::hidl::base::V1_0::IBase; using ::android::hidl::manager::V1_0::IServiceManager; namespace android { @@ -85,7 +89,7 @@ void ListCommand::removeDeadProcesses(Pids *pids) { }), pids->end()); } -bool scanBinderContext(pid_t pid, +static bool scanBinderContext(pid_t pid, const std::string &contextName, std::function<void(const std::string&)> eachLine) { std::ifstream ifs("/d/binder/proc/" + std::to_string(pid)); @@ -171,6 +175,16 @@ bool ListCommand::getPidInfo( }); } +const PidInfo* ListCommand::getPidInfoCached(pid_t serverPid) { + auto pair = mCachedPidInfos.insert({serverPid, PidInfo{}}); + if (pair.second /* did insertion take place? */) { + if (!getPidInfo(serverPid, &pair.first->second)) { + return nullptr; + } + } + return &pair.first->second; +} + // Must process hwbinder services first, then passthrough services. void ListCommand::forEachTable(const std::function<void(Table &)> &f) { f(mServicesTable); @@ -445,10 +459,7 @@ Status ListCommand::fetchAllLibraries(const sp<IServiceManager> &manager) { entries.emplace(interfaceName, TableEntry{ .interfaceName = interfaceName, .transport = "passthrough", - .serverPid = NO_PID, - .serverObjectAddress = NO_PTR, .clientPids = info.clientPids, - .arch = ARCH_UNKNOWN }).first->second.arch |= fromBaseArchitecture(info.arch); } for (auto &&pair : entries) { @@ -479,7 +490,6 @@ Status ListCommand::fetchPassthrough(const sp<IServiceManager> &manager) { std::string{info.instanceName.c_str()}, .transport = "passthrough", .serverPid = info.clientPids.size() == 1 ? info.clientPids[0] : NO_PID, - .serverObjectAddress = NO_PTR, .clientPids = info.clientPids, .arch = fromBaseArchitecture(info.arch) }); @@ -494,10 +504,6 @@ Status ListCommand::fetchPassthrough(const sp<IServiceManager> &manager) { } Status ListCommand::fetchBinderized(const sp<IServiceManager> &manager) { - using namespace ::std; - using namespace ::android::hardware; - using namespace ::android::hidl::manager::V1_0; - using namespace ::android::hidl::base::V1_0; const std::string mode = "hwbinder"; hidl_vec<hidl_string> fqInstanceNames; @@ -512,80 +518,117 @@ Status ListCommand::fetchBinderized(const sp<IServiceManager> &manager) { } Status status = OK; - // server pid, .ptr value of binder object, child pids - std::map<std::string, DebugInfo> allDebugInfos; - std::map<pid_t, PidInfo> allPids; + std::map<std::string, TableEntry> allTableEntries; for (const auto &fqInstanceName : fqInstanceNames) { - const auto pair = splitFirst(fqInstanceName, '/'); - const auto &serviceName = pair.first; - const auto &instanceName = pair.second; - auto getRet = timeoutIPC(manager, &IServiceManager::get, serviceName, instanceName); - if (!getRet.isOk()) { - err() << "Warning: Skipping \"" << fqInstanceName << "\": " - << "cannot be fetched from service manager:" - << getRet.description() << std::endl; - status |= DUMP_BINDERIZED_ERROR; - continue; - } - sp<IBase> service = getRet; - if (service == nullptr) { - err() << "Warning: Skipping \"" << fqInstanceName << "\": " - << "cannot be fetched from service manager (null)" - << std::endl; - status |= DUMP_BINDERIZED_ERROR; - continue; - } - auto debugRet = timeoutIPC(service, &IBase::getDebugInfo, [&] (const auto &debugInfo) { - allDebugInfos[fqInstanceName] = debugInfo; - if (debugInfo.pid >= 0) { - allPids[static_cast<pid_t>(debugInfo.pid)] = PidInfo(); - } + // create entry and default assign all fields. + TableEntry& entry = allTableEntries[fqInstanceName]; + entry.interfaceName = fqInstanceName; + entry.transport = mode; + + status |= fetchBinderizedEntry(manager, &entry); + } + + for (auto& pair : allTableEntries) { + putEntry(HWSERVICEMANAGER_LIST, std::move(pair.second)); + } + return status; +} + +Status ListCommand::fetchBinderizedEntry(const sp<IServiceManager> &manager, + TableEntry *entry) { + Status status = OK; + const auto handleError = [&](Status additionalError, const std::string& msg) { + err() << "Warning: Skipping \"" << entry->interfaceName << "\": " << msg << std::endl; + status |= DUMP_BINDERIZED_ERROR | additionalError; + }; + + const auto pair = splitFirst(entry->interfaceName, '/'); + const auto &serviceName = pair.first; + const auto &instanceName = pair.second; + auto getRet = timeoutIPC(manager, &IServiceManager::get, serviceName, instanceName); + if (!getRet.isOk()) { + handleError(TRANSACTION_ERROR, + "cannot be fetched from service manager:" + getRet.description()); + return status; + } + sp<IBase> service = getRet; + if (service == nullptr) { + handleError(NO_INTERFACE, "cannot be fetched from service manager (null)"); + return status; + } + + // getDebugInfo + do { + DebugInfo debugInfo; + auto debugRet = timeoutIPC(service, &IBase::getDebugInfo, [&] (const auto &received) { + debugInfo = received; }); if (!debugRet.isOk()) { - err() << "Warning: Skipping \"" << fqInstanceName << "\": " - << "debugging information cannot be retrieved:" - << debugRet.description() << std::endl; - status |= DUMP_BINDERIZED_ERROR; + handleError(TRANSACTION_ERROR, + "debugging information cannot be retrieved: " + debugRet.description()); + break; // skip getPidInfo } - } - for (auto &pair : allPids) { - pid_t serverPid = pair.first; - if (!getPidInfo(serverPid, &allPids[serverPid])) { - err() << "Warning: no information for PID " << serverPid - << ", are you root?" << std::endl; - status |= DUMP_BINDERIZED_ERROR; + entry->serverPid = debugInfo.pid; + entry->serverObjectAddress = debugInfo.ptr; + entry->arch = fromBaseArchitecture(debugInfo.arch); + + if (debugInfo.pid != NO_PID) { + const PidInfo* pidInfo = getPidInfoCached(debugInfo.pid); + if (pidInfo == nullptr) { + handleError(IO_ERROR, + "no information for PID " + std::to_string(debugInfo.pid) + + ", are you root?"); + break; + } + if (debugInfo.ptr != NO_PTR) { + auto it = pidInfo->refPids.find(debugInfo.ptr); + if (it != pidInfo->refPids.end()) { + entry->clientPids = it->second; + } + } + entry->threadUsage = pidInfo->threadUsage; + entry->threadCount = pidInfo->threadCount; } - } - for (const auto &fqInstanceName : fqInstanceNames) { - auto it = allDebugInfos.find(fqInstanceName); - if (it == allDebugInfos.end()) { - putEntry(HWSERVICEMANAGER_LIST, { - .interfaceName = fqInstanceName, - .transport = mode, - .serverPid = NO_PID, - .serverObjectAddress = NO_PTR, - .clientPids = {}, - .threadUsage = 0, - .threadCount = 0, - .arch = ARCH_UNKNOWN - }); - continue; + } while (0); + + // hash + do { + ssize_t hashIndex = -1; + auto ifaceChainRet = timeoutIPC(service, &IBase::interfaceChain, [&] (const auto& c) { + for (size_t i = 0; i < c.size(); ++i) { + if (serviceName == c[i]) { + hashIndex = static_cast<ssize_t>(i); + break; + } + } + }); + if (!ifaceChainRet.isOk()) { + handleError(TRANSACTION_ERROR, + "interfaceChain fails: " + ifaceChainRet.description()); + break; // skip getHashChain } - const DebugInfo &info = it->second; - bool writePidInfo = info.pid != NO_PID && info.ptr != NO_PTR; - - putEntry(HWSERVICEMANAGER_LIST, { - .interfaceName = fqInstanceName, - .transport = mode, - .serverPid = info.pid, - .serverObjectAddress = info.ptr, - .clientPids = writePidInfo ? allPids[info.pid].refPids[info.ptr] : Pids{}, - .threadUsage = writePidInfo ? allPids[info.pid].threadUsage : 0, - .threadCount = writePidInfo ? allPids[info.pid].threadCount : 0, - .arch = fromBaseArchitecture(info.arch), + if (hashIndex < 0) { + handleError(BAD_IMPL, "Interface name does not exist in interfaceChain."); + break; // skip getHashChain + } + auto hashRet = timeoutIPC(service, &IBase::getHashChain, [&] (const auto& hashChain) { + if (static_cast<size_t>(hashIndex) >= hashChain.size()) { + handleError(BAD_IMPL, + "interfaceChain indicates position " + std::to_string(hashIndex) + + " but getHashChain returns " + std::to_string(hashChain.size()) + + " hashes"); + return; + } + + auto&& hashArray = hashChain[hashIndex]; + std::vector<uint8_t> hashVec{hashArray.data(), hashArray.data() + hashArray.size()}; + entry->hash = Hash::hexString(hashVec); }); - } + if (!hashRet.isOk()) { + handleError(TRANSACTION_ERROR, "getHashChain failed: " + hashRet.description()); + } + } while (0); return status; } @@ -623,6 +666,10 @@ void ListCommand::registerAllOptions() { thiz->mSelectedColumns.push_back(TableColumnType::INTERFACE_NAME); return OK; }, "print the instance name column"}); + mOptions.push_back({'l', "released", no_argument, v++, [](ListCommand* thiz, const char*) { + thiz->mSelectedColumns.push_back(TableColumnType::RELEASED); + return OK; + }, "print the 'is released?' column\n(Y=released, empty=unreleased or unknown)"}); mOptions.push_back({'t', "transport", no_argument, v++, [](ListCommand* thiz, const char*) { thiz->mSelectedColumns.push_back(TableColumnType::TRANSPORT); return OK; @@ -631,6 +678,10 @@ void ListCommand::registerAllOptions() { thiz->mSelectedColumns.push_back(TableColumnType::ARCH); return OK; }, "print the bitness column"}); + mOptions.push_back({'s', "hash", no_argument, v++, [](ListCommand* thiz, const char*) { + thiz->mSelectedColumns.push_back(TableColumnType::HASH); + return OK; + }, "print hash of the interface"}); mOptions.push_back({'p', "pid", no_argument, v++, [](ListCommand* thiz, const char*) { thiz->mSelectedColumns.push_back(TableColumnType::SERVER_PID); return OK; @@ -773,7 +824,8 @@ Status ListCommand::parseArgs(const Arg &arg) { } if (mSelectedColumns.empty()) { - mSelectedColumns = {TableColumnType::INTERFACE_NAME, TableColumnType::THREADS, + mSelectedColumns = {TableColumnType::RELEASED, + TableColumnType::INTERFACE_NAME, TableColumnType::THREADS, TableColumnType::SERVER_PID, TableColumnType::CLIENT_PIDS}; } @@ -841,7 +893,7 @@ void ListCommand::usage() const { err() << "list:" << std::endl << " lshal" << std::endl << " lshal list" << std::endl - << " List all hals with default ordering and columns (`lshal list -iepc`)" << std::endl + << " List all hals with default ordering and columns (`lshal list -riepc`)" << std::endl << " lshal list [-h|--help]" << std::endl << " -h, --help: Print help message for list (`lshal help list`)" << std::endl << " lshal [list] [OPTIONS...]" << std::endl; diff --git a/cmds/lshal/ListCommand.h b/cmds/lshal/ListCommand.h index 5bc834c320..7e252fc167 100644 --- a/cmds/lshal/ListCommand.h +++ b/cmds/lshal/ListCommand.h @@ -78,14 +78,21 @@ public: protected: Status parseArgs(const Arg &arg); Status fetch(); - void postprocess(); + virtual void postprocess(); Status dump(); void putEntry(TableEntrySource source, TableEntry &&entry); Status fetchPassthrough(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager); Status fetchBinderized(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager); Status fetchAllLibraries(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager); + Status fetchBinderizedEntry(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager, + TableEntry *entry); + + // Get relevant information for a PID by parsing files under /d/binder. + // It is a virtual member function so that it can be mocked. virtual bool getPidInfo(pid_t serverPid, PidInfo *info) const; + // Retrieve from mCachedPidInfos and call getPidInfo if necessary. + const PidInfo* getPidInfoCached(pid_t serverPid); void dumpTable(const NullableOStream<std::ostream>& out) const; void dumpVintf(const NullableOStream<std::ostream>& out) const; @@ -129,6 +136,9 @@ protected: // If an entry exist and not empty, it contains the cached content of /proc/{pid}/cmdline. std::map<pid_t, std::string> mCmdlines; + // Cache for getPidInfo. + std::map<pid_t, PidInfo> mCachedPidInfos; + RegisteredOptions mOptions; // All selected columns std::vector<TableColumnType> mSelectedColumns; diff --git a/cmds/lshal/TableEntry.cpp b/cmds/lshal/TableEntry.cpp index cbcf979f90..e8792a4307 100644 --- a/cmds/lshal/TableEntry.cpp +++ b/cmds/lshal/TableEntry.cpp @@ -16,6 +16,8 @@ #define LOG_TAG "lshal" #include <android-base/logging.h> +#include <hidl-hash/Hash.h> + #include "TableEntry.h" #include "TextTable.h" @@ -53,8 +55,10 @@ static std::string getTitle(TableColumnType type) { case TableColumnType::CLIENT_CMDS: return "Clients CMD"; case TableColumnType::ARCH: return "Arch"; case TableColumnType::THREADS: return "Thread Use"; + case TableColumnType::RELEASED: return "R"; + case TableColumnType::HASH: return "Hash"; default: - LOG(FATAL) << "Should not reach here."; + LOG(FATAL) << __func__ << "Should not reach here. " << static_cast<int>(type); return ""; } } @@ -79,12 +83,25 @@ std::string TableEntry::getField(TableColumnType type) const { return getArchString(arch); case TableColumnType::THREADS: return getThreadUsage(); + case TableColumnType::RELEASED: + return isReleased(); + case TableColumnType::HASH: + return hash; default: - LOG(FATAL) << "Should not reach here."; + LOG(FATAL) << __func__ << "Should not reach here. " << static_cast<int>(type); return ""; } } +std::string TableEntry::isReleased() const { + static const std::string unreleased = Hash::hexString(Hash::kEmptyHash); + + if (hash.empty() || hash == unreleased) { + return " "; // unknown or unreleased + } + return "Y"; // released +} + TextTable Table::createTextTable(bool neat, const std::function<std::string(const std::string&)>& emitDebugInfo) const { diff --git a/cmds/lshal/TableEntry.h b/cmds/lshal/TableEntry.h index 7a3b22ea67..69206cc51b 100644 --- a/cmds/lshal/TableEntry.h +++ b/cmds/lshal/TableEntry.h @@ -55,19 +55,28 @@ enum class TableColumnType : unsigned int { CLIENT_CMDS, ARCH, THREADS, + RELEASED, + HASH, +}; + +enum { + NO_PID = -1, + NO_PTR = 0 }; struct TableEntry { - std::string interfaceName; - std::string transport; - int32_t serverPid; - uint32_t threadUsage; - uint32_t threadCount; - std::string serverCmdline; - uint64_t serverObjectAddress; - Pids clientPids; - std::vector<std::string> clientCmdlines; - Architecture arch; + std::string interfaceName{}; + std::string transport{}; + int32_t serverPid{NO_PID}; + uint32_t threadUsage{0}; + uint32_t threadCount{0}; + std::string serverCmdline{}; + uint64_t serverObjectAddress{NO_PTR}; + Pids clientPids{}; + std::vector<std::string> clientCmdlines{}; + Architecture arch{ARCH_UNKNOWN}; + // empty: unknown, all zeros: unreleased, otherwise: released + std::string hash{}; static bool sortByInterfaceName(const TableEntry &a, const TableEntry &b) { return a.interfaceName < b.interfaceName; @@ -84,6 +93,8 @@ struct TableEntry { return std::to_string(threadUsage) + "/" + std::to_string(threadCount); } + std::string isReleased() const; + std::string getField(TableColumnType type) const; bool operator==(const TableEntry& other) const; @@ -129,11 +140,6 @@ private: std::vector<const Table*> mTables; }; -enum { - NO_PID = -1, - NO_PTR = 0 -}; - } // namespace lshal } // namespace android diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp index 06b68199cd..9220fc09fb 100644 --- a/cmds/lshal/test.cpp +++ b/cmds/lshal/test.cpp @@ -39,6 +39,7 @@ using ::android::hidl::base::V1_0::DebugInfo; using ::android::hidl::base::V1_0::IBase; using ::android::hidl::manager::V1_0::IServiceManager; using ::android::hidl::manager::V1_0::IServiceNotification; +using ::android::hardware::hidl_array; using ::android::hardware::hidl_death_recipient; using ::android::hardware::hidl_handle; using ::android::hardware::hidl_string; @@ -46,6 +47,8 @@ using ::android::hardware::hidl_vec; using InstanceDebugInfo = IServiceManager::InstanceDebugInfo; +using hidl_hash = hidl_array<uint8_t, 32>; + namespace android { namespace hardware { namespace tests { @@ -185,6 +188,9 @@ public: Status parseArgs(const Arg& arg) { return ListCommand::parseArgs(arg); } Status main(const Arg& arg) { return ListCommand::main(arg); } + void forEachTable(const std::function<void(Table &)> &f) { + return ListCommand::forEachTable(f); + } void forEachTable(const std::function<void(const Table &)> &f) const { return ListCommand::forEachTable(f); } @@ -192,7 +198,12 @@ public: void dumpVintf(const NullableOStream<std::ostream>& out) { return ListCommand::dumpVintf(out); } + void internalPostprocess() { ListCommand::postprocess(); } + const PidInfo* getPidInfoCached(pid_t serverPid) { + return ListCommand::getPidInfoCached(serverPid); + } + MOCK_METHOD0(postprocess, void()); MOCK_CONST_METHOD2(getPidInfo, bool(pid_t, PidInfo*)); MOCK_CONST_METHOD1(parseCmdline, std::string(pid_t)); }; @@ -210,16 +221,6 @@ public: std::stringstream output; }; -TEST_F(ListParseArgsTest, Default) { - // default args - EXPECT_EQ(0u, mockList->parseArgs(createArg({}))); - mockList->forEachTable([](const Table& table) { - EXPECT_EQ(SelectedColumns({TableColumnType::INTERFACE_NAME, TableColumnType::THREADS, - TableColumnType::SERVER_PID, TableColumnType::CLIENT_PIDS}), - table.getSelectedColumns()); - }); -} - TEST_F(ListParseArgsTest, Args) { EXPECT_EQ(0u, mockList->parseArgs(createArg({"lshal", "-p", "-i", "-a", "-c"}))); mockList->forEachTable([](const Table& table) { @@ -232,9 +233,14 @@ TEST_F(ListParseArgsTest, Args) { TEST_F(ListParseArgsTest, Cmds) { EXPECT_EQ(0u, mockList->parseArgs(createArg({"lshal", "-m"}))); mockList->forEachTable([](const Table& table) { - EXPECT_EQ(SelectedColumns({TableColumnType::INTERFACE_NAME, TableColumnType::THREADS, - TableColumnType::SERVER_CMD, TableColumnType::CLIENT_CMDS}), - table.getSelectedColumns()); + EXPECT_THAT(table.getSelectedColumns(), Not(Contains(TableColumnType::SERVER_PID))) + << "should not print server PID with -m"; + EXPECT_THAT(table.getSelectedColumns(), Not(Contains(TableColumnType::CLIENT_PIDS))) + << "should not print client PIDs with -m"; + EXPECT_THAT(table.getSelectedColumns(), Contains(TableColumnType::SERVER_CMD)) + << "should print server cmd with -m"; + EXPECT_THAT(table.getSelectedColumns(), Contains(TableColumnType::CLIENT_CMDS)) + << "should print client cmds with -m"; }); } @@ -274,17 +280,34 @@ static std::string getCmdlineFromId(pid_t serverId) { if (serverId == NO_PID) return ""; return "command_line_" + std::to_string(serverId); } +static bool getIsReleasedFromId(pid_t p) { return p % 2 == 0; } +static hidl_hash getHashFromId(pid_t serverId) { + hidl_hash hash; + bool isReleased = getIsReleasedFromId(serverId); + for (size_t i = 0; i < hash.size(); ++i) { + hash[i] = isReleased ? static_cast<uint8_t>(serverId) : 0u; + } + return hash; +} // Fake service returned by mocked IServiceManager::get. class TestService : public IBase { public: - TestService(DebugInfo&& info) : mInfo(std::move(info)) {} + TestService(pid_t id) : mId(id) {} hardware::Return<void> getDebugInfo(getDebugInfo_cb cb) override { - cb(mInfo); + cb({ mId /* pid */, getPtr(mId), DebugInfo::Architecture::IS_64BIT }); + return hardware::Void(); + } + hardware::Return<void> interfaceChain(interfaceChain_cb cb) override { + cb({getInterfaceName(mId), IBase::descriptor}); + return hardware::Void(); + } + hardware::Return<void> getHashChain(getHashChain_cb cb) override { + cb({getHashFromId(mId), getHashFromId(0xff)}); return hardware::Void(); } private: - DebugInfo mInfo; + pid_t mId; }; class ListTest : public ::testing::Test { @@ -303,6 +326,13 @@ public: return true; })); ON_CALL(*mockList, parseCmdline(_)).WillByDefault(Invoke(&getCmdlineFromId)); + ON_CALL(*mockList, postprocess()).WillByDefault(Invoke([&]() { + mockList->internalPostprocess(); + size_t i = 0; + mockList->forEachTable([&](Table& table) { + table.setDescription("[fake description " + std::to_string(i++) + "]"); + }); + })); } void initMockServiceManager() { @@ -318,7 +348,7 @@ public: ON_CALL(*serviceManager, get(_, _)).WillByDefault(Invoke( [&](const hidl_string&, const hidl_string& instance) { int id = getIdFromInstanceName(instance); - return sp<IBase>(new TestService({ id /* pid */, getPtr(id), A::IS_64BIT })); + return sp<IBase>(new TestService(id)); })); ON_CALL(*serviceManager, debugDump(_)).WillByDefault(Invoke( @@ -348,6 +378,13 @@ public: sp<MockServiceManager> passthruManager; }; +TEST_F(ListTest, GetPidInfoCached) { + EXPECT_CALL(*mockList, getPidInfo(5, _)).Times(1); + + EXPECT_NE(nullptr, mockList->getPidInfoCached(5)); + EXPECT_NE(nullptr, mockList->getPidInfoCached(5)); +} + TEST_F(ListTest, Fetch) { EXPECT_EQ(0u, mockList->fetch()); std::array<std::string, 6> transports{{"hwbinder", "hwbinder", "passthrough", @@ -456,24 +493,68 @@ TEST_F(ListTest, DumpVintf) { << vintf::gHalManifestConverter.lastError(); } +// test default columns +TEST_F(ListTest, DumpDefault) { + const std::string expected = + "[fake description 0]\n" + "R Interface Thread Use Server Clients\n" + " a.h.foo1@1.0::IFoo/1 11/21 1 2 4\n" + "Y a.h.foo2@2.0::IFoo/2 12/22 2 3 5\n" + "\n" + "[fake description 1]\n" + "R Interface Thread Use Server Clients\n" + " a.h.foo3@3.0::IFoo/3 N/A N/A 4 6\n" + " a.h.foo4@4.0::IFoo/4 N/A N/A 5 7\n" + "\n" + "[fake description 2]\n" + "R Interface Thread Use Server Clients\n" + " a.h.foo5@5.0::IFoo/5 N/A N/A 6 8\n" + " a.h.foo6@6.0::IFoo/6 N/A N/A 7 9\n" + "\n"; + + optind = 1; // mimic Lshal::parseArg() + EXPECT_EQ(0u, mockList->main(createArg({"lshal"}))); + EXPECT_EQ(expected, out.str()); + EXPECT_EQ("", err.str()); +} + +TEST_F(ListTest, DumpHash) { + const std::string expected = + "[fake description 0]\n" + "Interface R Hash\n" + "a.h.foo1@1.0::IFoo/1 0000000000000000000000000000000000000000000000000000000000000000\n" + "a.h.foo2@2.0::IFoo/2 Y 0202020202020202020202020202020202020202020202020202020202020202\n" + "\n" + "[fake description 1]\n" + "Interface R Hash\n" + "a.h.foo3@3.0::IFoo/3 \n" + "a.h.foo4@4.0::IFoo/4 \n" + "\n" + "[fake description 2]\n" + "Interface R Hash\n" + "a.h.foo5@5.0::IFoo/5 \n" + "a.h.foo6@6.0::IFoo/6 \n" + "\n"; + + optind = 1; // mimic Lshal::parseArg() + EXPECT_EQ(0u, mockList->main(createArg({"lshal", "-ils"}))); + EXPECT_EQ(expected, out.str()); + EXPECT_EQ("", err.str()); +} + TEST_F(ListTest, Dump) { const std::string expected = - "All binderized services (registered services through hwservicemanager)\n" + "[fake description 0]\n" "Interface Transport Arch Thread Use Server PTR Clients\n" "a.h.foo1@1.0::IFoo/1 hwbinder 64 11/21 1 0000000000002711 2 4\n" "a.h.foo2@2.0::IFoo/2 hwbinder 64 12/22 2 0000000000002712 3 5\n" "\n" - "All interfaces that getService() has ever return as a passthrough interface;\n" - "PIDs / processes shown below might be inaccurate because the process\n" - "might have relinquished the interface or might have died.\n" - "The Server / Server CMD column can be ignored.\n" - "The Clients / Clients CMD column shows all process that have ever dlopen'ed \n" - "the library and successfully fetched the passthrough implementation.\n" + "[fake description 1]\n" "Interface Transport Arch Thread Use Server PTR Clients\n" "a.h.foo3@3.0::IFoo/3 passthrough 32 N/A N/A N/A 4 6\n" "a.h.foo4@4.0::IFoo/4 passthrough 32 N/A N/A N/A 5 7\n" "\n" - "All available passthrough implementations (all -impl.so files)\n" + "[fake description 2]\n" "Interface Transport Arch Thread Use Server PTR Clients\n" "a.h.foo5@5.0::IFoo/5 passthrough 32 N/A N/A N/A 6 8\n" "a.h.foo6@6.0::IFoo/6 passthrough 32 N/A N/A N/A 7 9\n" @@ -487,22 +568,17 @@ TEST_F(ListTest, Dump) { TEST_F(ListTest, DumpCmdline) { const std::string expected = - "All binderized services (registered services through hwservicemanager)\n" + "[fake description 0]\n" "Interface Transport Arch Thread Use Server CMD PTR Clients CMD\n" "a.h.foo1@1.0::IFoo/1 hwbinder 64 11/21 command_line_1 0000000000002711 command_line_2;command_line_4\n" "a.h.foo2@2.0::IFoo/2 hwbinder 64 12/22 command_line_2 0000000000002712 command_line_3;command_line_5\n" "\n" - "All interfaces that getService() has ever return as a passthrough interface;\n" - "PIDs / processes shown below might be inaccurate because the process\n" - "might have relinquished the interface or might have died.\n" - "The Server / Server CMD column can be ignored.\n" - "The Clients / Clients CMD column shows all process that have ever dlopen'ed \n" - "the library and successfully fetched the passthrough implementation.\n" + "[fake description 1]\n" "Interface Transport Arch Thread Use Server CMD PTR Clients CMD\n" "a.h.foo3@3.0::IFoo/3 passthrough 32 N/A N/A command_line_4;command_line_6\n" "a.h.foo4@4.0::IFoo/4 passthrough 32 N/A N/A command_line_5;command_line_7\n" "\n" - "All available passthrough implementations (all -impl.so files)\n" + "[fake description 2]\n" "Interface Transport Arch Thread Use Server CMD PTR Clients CMD\n" "a.h.foo5@5.0::IFoo/5 passthrough 32 N/A N/A command_line_6;command_line_8\n" "a.h.foo6@6.0::IFoo/6 passthrough 32 N/A N/A command_line_7;command_line_9\n" @@ -524,7 +600,7 @@ TEST_F(ListTest, DumpNeat) { "a.h.foo6@6.0::IFoo/6 N/A N/A 7 9\n"; optind = 1; // mimic Lshal::parseArg() - EXPECT_EQ(0u, mockList->main(createArg({"lshal", "--neat"}))); + EXPECT_EQ(0u, mockList->main(createArg({"lshal", "-iepc", "--neat"}))); EXPECT_EQ(expected, out.str()); EXPECT_EQ("", err.str()); } diff --git a/cmds/lshal/utils.h b/cmds/lshal/utils.h index 7eca14e0f7..c09e8b1666 100644 --- a/cmds/lshal/utils.h +++ b/cmds/lshal/utils.h @@ -44,6 +44,8 @@ enum : unsigned int { NO_INTERFACE = 1 << 7, // Transaction error from hwbinder transactions TRANSACTION_ERROR = 1 << 8, + // No transaction error, but return value is unexpected. + BAD_IMPL = 1 << 9, }; using Status = unsigned int; diff --git a/libs/binder/aidl/android/content/pm/IPackageManagerNative.aidl b/libs/binder/aidl/android/content/pm/IPackageManagerNative.aidl index 6b7254cbb3..3264666a21 100644 --- a/libs/binder/aidl/android/content/pm/IPackageManagerNative.aidl +++ b/libs/binder/aidl/android/content/pm/IPackageManagerNative.aidl @@ -38,4 +38,20 @@ interface IPackageManagerNative { * strings. */ @utf8InCpp String[] getNamesForUids(in int[] uids); + + /** + * Returns the name of the installer (a package) which installed the named + * package. Preloaded packages return the string "preload". Sideloaded packages + * return an empty string. Unknown or unknowable are returned as empty strings. + */ + + @utf8InCpp String getInstallerForPackage(in String packageName); + + /** + * Returns the version code of the named package. + * Unknown or unknowable versions are returned as 0. + */ + + int getVersionCodeForPackage(in String packageName); + } diff --git a/libs/gui/LayerDebugInfo.cpp b/libs/gui/LayerDebugInfo.cpp index 57ddde075a..d3dc16d30e 100644 --- a/libs/gui/LayerDebugInfo.cpp +++ b/libs/gui/LayerDebugInfo.cpp @@ -43,7 +43,10 @@ status_t LayerDebugInfo::writeToParcel(Parcel* parcel) const { RETURN_ON_ERROR(parcel->writeInt32(mHeight)); RETURN_ON_ERROR(parcel->write(mCrop)); RETURN_ON_ERROR(parcel->write(mFinalCrop)); - RETURN_ON_ERROR(parcel->writeFloat(mAlpha)); + RETURN_ON_ERROR(parcel->writeFloat(mColor.r)); + RETURN_ON_ERROR(parcel->writeFloat(mColor.g)); + RETURN_ON_ERROR(parcel->writeFloat(mColor.b)); + RETURN_ON_ERROR(parcel->writeFloat(mColor.a)); RETURN_ON_ERROR(parcel->writeUint32(mFlags)); RETURN_ON_ERROR(parcel->writeInt32(mPixelFormat)); RETURN_ON_ERROR(parcel->writeUint32(static_cast<uint32_t>(mDataSpace))); @@ -79,7 +82,14 @@ status_t LayerDebugInfo::readFromParcel(const Parcel* parcel) { RETURN_ON_ERROR(parcel->readInt32(&mHeight)); RETURN_ON_ERROR(parcel->read(mCrop)); RETURN_ON_ERROR(parcel->read(mFinalCrop)); - RETURN_ON_ERROR(parcel->readFloat(&mAlpha)); + mColor.r = parcel->readFloat(); + RETURN_ON_ERROR(parcel->errorCheck()); + mColor.g = parcel->readFloat(); + RETURN_ON_ERROR(parcel->errorCheck()); + mColor.b = parcel->readFloat(); + RETURN_ON_ERROR(parcel->errorCheck()); + mColor.a = parcel->readFloat(); + RETURN_ON_ERROR(parcel->errorCheck()); RETURN_ON_ERROR(parcel->readUint32(&mFlags)); RETURN_ON_ERROR(parcel->readInt32(&mPixelFormat)); // \todo [2017-07-25 kraita]: Static casting mDataSpace pointer to an uint32 does work. Better ways? @@ -116,8 +126,10 @@ std::string to_string(const LayerDebugInfo& info) { result.appendFormat("isOpaque=%1d, invalidate=%1d, ", info.mIsOpaque, info.mContentDirty); result.appendFormat("dataspace=%s, ", dataspaceDetails(info.mDataSpace).c_str()); result.appendFormat("pixelformat=%s, ", decodePixelFormat(info.mPixelFormat).c_str()); - result.appendFormat("alpha=%.3f, flags=0x%08x, ", - static_cast<double>(info.mAlpha), info.mFlags); + result.appendFormat("color=(%.3f,%.3f,%.3f,%.3f), flags=0x%08x, ", + static_cast<double>(info.mColor.r), static_cast<double>(info.mColor.g), + static_cast<double>(info.mColor.b), static_cast<double>(info.mColor.a), + info.mFlags); result.appendFormat("tr=[%.2f, %.2f][%.2f, %.2f]", static_cast<double>(info.mMatrix[0][0]), static_cast<double>(info.mMatrix[0][1]), static_cast<double>(info.mMatrix[1][0]), static_cast<double>(info.mMatrix[1][1])); diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 573f6856d6..fcee73f445 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -46,7 +46,9 @@ status_t layer_state_t::write(Parcel& output) const output.writeStrongBinder(IInterface::asBinder(barrierGbp)); output.writeStrongBinder(relativeLayerHandle); output.writeStrongBinder(parentHandleForChild); - output.writeStrongBinder(childHandle); + output.writeFloat(color.r); + output.writeFloat(color.g); + output.writeFloat(color.b); output.write(transparentRegion); return NO_ERROR; } @@ -80,7 +82,9 @@ status_t layer_state_t::read(const Parcel& input) interface_cast<IGraphicBufferProducer>(input.readStrongBinder()); relativeLayerHandle = input.readStrongBinder(); parentHandleForChild = input.readStrongBinder(); - childHandle = input.readStrongBinder(); + color.r = input.readFloat(); + color.g = input.readFloat(); + color.b = input.readFloat(); input.read(transparentRegion); return NO_ERROR; } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index b0ae7e0bdf..c5a4389ae8 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -158,6 +158,8 @@ public: const Region& transparentRegion); status_t setAlpha(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, float alpha); + status_t setColor(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, + const half3& color); status_t setMatrix(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, float dsdx, float dtdx, float dtdy, float dsdy); status_t setOrientation(int orientation); @@ -176,9 +178,8 @@ public: status_t reparentChildren(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, const sp<IBinder>& newParentHandle); - status_t reparentChild(const sp<SurfaceComposerClient>& client, - const sp<IBinder>& id, const sp<IBinder>& newParentHandle, - const sp<IBinder>& childHandle); + status_t reparent(const sp<SurfaceComposerClient>& client, + const sp<IBinder>& id, const sp<IBinder>& newParentHandle); status_t detachChildren(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id); status_t setOverrideScalingMode(const sp<SurfaceComposerClient>& client, @@ -403,6 +404,17 @@ status_t Composer::setAlpha(const sp<SurfaceComposerClient>& client, return NO_ERROR; } +status_t Composer::setColor(const sp<SurfaceComposerClient>& client, + const sp<IBinder>& id, const half3& color) { + Mutex::Autolock _l(mLock); + layer_state_t* s = getLayerStateLocked(client, id); + if (!s) + return BAD_INDEX; + s->what |= layer_state_t::eColorChanged; + s->color = color; + return NO_ERROR; +} + status_t Composer::setLayerStack(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, uint32_t layerStack) { Mutex::Autolock _l(mLock); @@ -496,18 +508,16 @@ status_t Composer::reparentChildren( return NO_ERROR; } -status_t Composer::reparentChild(const sp<SurfaceComposerClient>& client, +status_t Composer::reparent(const sp<SurfaceComposerClient>& client, const sp<IBinder>& id, - const sp<IBinder>& newParentHandle, - const sp<IBinder>& childHandle) { + const sp<IBinder>& newParentHandle) { Mutex::Autolock lock(mLock); layer_state_t* s = getLayerStateLocked(client, id); if (!s) { return BAD_INDEX; } - s->what |= layer_state_t::eReparentChild; + s->what |= layer_state_t::eReparent; s->parentHandleForChild = newParentHandle; - s->childHandle = childHandle; return NO_ERROR; } @@ -825,6 +835,10 @@ status_t SurfaceComposerClient::setAlpha(const sp<IBinder>& id, float alpha) { return getComposer().setAlpha(this, id, alpha); } +status_t SurfaceComposerClient::setColor(const sp<IBinder>& id, const half3& color) { + return getComposer().setColor(this, id, color); +} + status_t SurfaceComposerClient::setLayerStack(const sp<IBinder>& id, uint32_t layerStack) { return getComposer().setLayerStack(this, id, layerStack); } @@ -849,9 +863,9 @@ status_t SurfaceComposerClient::reparentChildren(const sp<IBinder>& id, return getComposer().reparentChildren(this, id, newParentHandle); } -status_t SurfaceComposerClient::reparentChild(const sp<IBinder>& id, - const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle) { - return getComposer().reparentChild(this, id, newParentHandle, childHandle); +status_t SurfaceComposerClient::reparent(const sp<IBinder>& id, + const sp<IBinder>& newParentHandle) { + return getComposer().reparent(this, id, newParentHandle); } status_t SurfaceComposerClient::detachChildren(const sp<IBinder>& id) { diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp index b9c5ef9580..9e1d7b6647 100644 --- a/libs/gui/SurfaceControl.cpp +++ b/libs/gui/SurfaceControl.cpp @@ -155,6 +155,11 @@ status_t SurfaceControl::setAlpha(float alpha) { if (err < 0) return err; return mClient->setAlpha(mHandle, alpha); } +status_t SurfaceControl::setColor(const half3& color) { + status_t err = validate(); + if (err < 0) return err; + return mClient->setColor(mHandle, color); +} status_t SurfaceControl::setMatrix(float dsdx, float dtdx, float dtdy, float dsdy) { status_t err = validate(); if (err < 0) return err; @@ -191,11 +196,10 @@ status_t SurfaceControl::reparentChildren(const sp<IBinder>& newParentHandle) { return mClient->reparentChildren(mHandle, newParentHandle); } -status_t SurfaceControl::reparentChild(const sp<IBinder>& newParentHandle, - const sp<IBinder>& childHandle) { +status_t SurfaceControl::reparent(const sp<IBinder>& newParentHandle) { status_t err = validate(); if (err < 0) return err; - return mClient->reparentChild(mHandle, newParentHandle, childHandle); + return mClient->reparent(mHandle, newParentHandle); } status_t SurfaceControl::detachChildren() { diff --git a/libs/gui/include/gui/ISurfaceComposerClient.h b/libs/gui/include/gui/ISurfaceComposerClient.h index 2c613ea8c5..d5bbef25f8 100644 --- a/libs/gui/include/gui/ISurfaceComposerClient.h +++ b/libs/gui/include/gui/ISurfaceComposerClient.h @@ -41,7 +41,7 @@ public: eCursorWindow = 0x00002000, eFXSurfaceNormal = 0x00000000, - eFXSurfaceDim = 0x00020000, + eFXSurfaceColor = 0x00020000, eFXSurfaceMask = 0x000F0000, }; diff --git a/libs/gui/include/gui/LayerDebugInfo.h b/libs/gui/include/gui/LayerDebugInfo.h index 8453e043ef..92bd8c5b28 100644 --- a/libs/gui/include/gui/LayerDebugInfo.h +++ b/libs/gui/include/gui/LayerDebugInfo.h @@ -22,6 +22,7 @@ #include <ui/Region.h> #include <string> +#include <math/vec4.h> namespace android { @@ -52,7 +53,7 @@ public: int32_t mHeight = -1; Rect mCrop = Rect::INVALID_RECT; Rect mFinalCrop = Rect::INVALID_RECT; - float mAlpha = 0.f; + half4 mColor = half4(1.0_hf, 1.0_hf, 1.0_hf, 0.0_hf); uint32_t mFlags = 0; PixelFormat mPixelFormat = PIXEL_FORMAT_NONE; android_dataspace mDataSpace = HAL_DATASPACE_UNKNOWN; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 6e2cb83544..17181430c7 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -32,6 +32,7 @@ #include <gui/CpuConsumer.h> #include <gui/SurfaceControl.h> +#include <math/vec3.h> namespace android { @@ -149,6 +150,7 @@ public: status_t setRelativeLayer(const sp<IBinder>& id, const sp<IBinder>& relativeTo, int32_t layer); status_t setAlpha(const sp<IBinder>& id, float alpha=1.0f); + status_t setColor(const sp<IBinder>& id, const half3& color); status_t setMatrix(const sp<IBinder>& id, float dsdx, float dtdx, float dtdy, float dsdy); status_t setPosition(const sp<IBinder>& id, float x, float y); status_t setSize(const sp<IBinder>& id, uint32_t w, uint32_t h); @@ -161,8 +163,7 @@ public: const sp<Surface>& handle, uint64_t frameNumber); status_t reparentChildren(const sp<IBinder>& id, const sp<IBinder>& newParentHandle); - status_t reparentChild(const sp<IBinder>& id, const sp<IBinder>& newParentHandle, - const sp<IBinder>& childHandle); + status_t reparent(const sp<IBinder>& id, const sp<IBinder>& newParentHandle); status_t detachChildren(const sp<IBinder>& id); status_t setOverrideScalingMode(const sp<IBinder>& id, int32_t overrideScalingMode); diff --git a/libs/gui/include/gui/SurfaceControl.h b/libs/gui/include/gui/SurfaceControl.h index d8b67ef96a..e98e26a391 100644 --- a/libs/gui/include/gui/SurfaceControl.h +++ b/libs/gui/include/gui/SurfaceControl.h @@ -29,6 +29,7 @@ #include <ui/Region.h> #include <gui/ISurfaceComposerClient.h> +#include <math/vec3.h> namespace android { @@ -90,6 +91,7 @@ public: status_t setFlags(uint32_t flags, uint32_t mask); status_t setTransparentRegionHint(const Region& transparent); status_t setAlpha(float alpha=1.0f); + status_t setColor(const half3& color); // Experimentarily it appears that the matrix transforms the // on-screen rectangle and it's contents before the position is @@ -124,11 +126,10 @@ public: // Reparents all children of this layer to the new parent handle. status_t reparentChildren(const sp<IBinder>& newParentHandle); - // Reparents a specified child from this layer to the new parent handle. - // The child, parent, and new parent must all have the same client. + // Reparents the current layer to the new parent handle. The new parent must not be null. // This can be used instead of reparentChildren if the caller wants to - // only re-parent specific children. - status_t reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle); + // only re-parent a specific child. + status_t reparent(const sp<IBinder>& newParentHandle); // Detaches all child surfaces (and their children recursively) // from their SurfaceControl. diff --git a/libs/gui/include/private/gui/LayerState.h b/libs/gui/include/private/gui/LayerState.h index 4f73e04e22..bd42634730 100644 --- a/libs/gui/include/private/gui/LayerState.h +++ b/libs/gui/include/private/gui/LayerState.h @@ -25,6 +25,7 @@ #include <ui/Region.h> #include <ui/Rect.h> #include <gui/IGraphicBufferProducer.h> +#include <math/vec3.h> namespace android { @@ -60,7 +61,8 @@ struct layer_state_t { eReparentChildren = 0x00002000, eDetachChildren = 0x00004000, eRelativeLayerChanged = 0x00008000, - eReparentChild = 0x00010000 + eReparent = 0x00010000, + eColorChanged = 0x00020000 }; layer_state_t() @@ -109,7 +111,8 @@ struct layer_state_t { sp<IBinder> relativeLayerHandle; sp<IBinder> parentHandleForChild; - sp<IBinder> childHandle; + + half3 color; // non POD must be last. see write/read Region transparentRegion; diff --git a/libs/hwc2on1adapter/HWC2On1Adapter.cpp b/libs/hwc2on1adapter/HWC2On1Adapter.cpp index 8c6ef691a2..e1b9a8a6f1 100644 --- a/libs/hwc2on1adapter/HWC2On1Adapter.cpp +++ b/libs/hwc2on1adapter/HWC2On1Adapter.cpp @@ -2005,10 +2005,21 @@ Error HWC2On1Adapter::Layer::setTransform(Transform transform) { return Error::None; } +static bool compareRects(const hwc_rect_t& rect1, const hwc_rect_t& rect2) { + return rect1.left == rect2.left && + rect1.right == rect2.right && + rect1.top == rect2.top && + rect1.bottom == rect2.bottom; +} + Error HWC2On1Adapter::Layer::setVisibleRegion(hwc_region_t visible) { - mVisibleRegion.resize(visible.numRects); - std::copy_n(visible.rects, visible.numRects, mVisibleRegion.begin()); - mDisplay.markGeometryChanged(); + if ((getNumVisibleRegions() != visible.numRects) || + !std::equal(mVisibleRegion.begin(), mVisibleRegion.end(), visible.rects, + compareRects)) { + mVisibleRegion.resize(visible.numRects); + std::copy_n(visible.rects, visible.numRects, mVisibleRegion.begin()); + mDisplay.markGeometryChanged(); + } return Error::None; } diff --git a/libs/ui/GraphicBufferMapper.cpp b/libs/ui/GraphicBufferMapper.cpp index d52c508003..163432821f 100644 --- a/libs/ui/GraphicBufferMapper.cpp +++ b/libs/ui/GraphicBufferMapper.cpp @@ -99,7 +99,7 @@ status_t GraphicBufferMapper::unlock(buffer_handle_t handle) { int32_t fenceFd = -1; status_t error = unlockAsync(handle, &fenceFd); - if (error == NO_ERROR) { + if (error == NO_ERROR && fenceFd >= 0) { sync_wait(fenceFd, -1); close(fenceFd); } diff --git a/libs/vr/libvrflinger/display_manager_service.cpp b/libs/vr/libvrflinger/display_manager_service.cpp index 40396b90c5..ef8cca38dd 100644 --- a/libs/vr/libvrflinger/display_manager_service.cpp +++ b/libs/vr/libvrflinger/display_manager_service.cpp @@ -65,6 +65,7 @@ void DisplayManagerService::OnChannelClose( } pdx::Status<void> DisplayManagerService::HandleMessage(pdx::Message& message) { + ATRACE_NAME("DisplayManagerService::HandleMessage"); auto channel = std::static_pointer_cast<DisplayManager>(message.GetChannel()); switch (message.GetOp()) { diff --git a/libs/vr/libvrflinger/display_service.cpp b/libs/vr/libvrflinger/display_service.cpp index 10abc5efa3..ac68a5e3a4 100644 --- a/libs/vr/libvrflinger/display_service.cpp +++ b/libs/vr/libvrflinger/display_service.cpp @@ -124,6 +124,8 @@ void DisplayService::OnChannelClose(pdx::Message& message, // surface-specific messages to the per-instance handlers. Status<void> DisplayService::HandleMessage(pdx::Message& message) { ALOGD_IF(TRACE, "DisplayService::HandleMessage: opcode=%d", message.GetOp()); + ATRACE_NAME("DisplayService::HandleMessage"); + switch (message.GetOp()) { case DisplayProtocol::GetMetrics::Opcode: DispatchRemoteMethod<DisplayProtocol::GetMetrics>( diff --git a/libs/vr/libvrflinger/hardware_composer.cpp b/libs/vr/libvrflinger/hardware_composer.cpp index 9c4278cbea..1039fc5f10 100644 --- a/libs/vr/libvrflinger/hardware_composer.cpp +++ b/libs/vr/libvrflinger/hardware_composer.cpp @@ -5,12 +5,14 @@ #include <fcntl.h> #include <log/log.h> #include <poll.h> +#include <stdint.h> #include <sync/sync.h> #include <sys/eventfd.h> #include <sys/prctl.h> #include <sys/resource.h> #include <sys/system_properties.h> #include <sys/timerfd.h> +#include <sys/types.h> #include <time.h> #include <unistd.h> #include <utils/Trace.h> @@ -30,7 +32,9 @@ using android::hardware::Return; using android::hardware::Void; +using android::pdx::ErrorStatus; using android::pdx::LocalHandle; +using android::pdx::Status; using android::pdx::rpc::EmptyVariant; using android::pdx::rpc::IfAnyOf; @@ -82,6 +86,29 @@ bool SetThreadPolicy(const std::string& scheduler_class, return true; } +// Utility to generate scoped tracers with arguments. +// TODO(eieio): Move/merge this into utils/Trace.h? +class TraceArgs { + public: + template <typename... Args> + TraceArgs(const char* format, Args&&... args) { + std::array<char, 1024> buffer; + snprintf(buffer.data(), buffer.size(), format, std::forward<Args>(args)...); + atrace_begin(ATRACE_TAG, buffer.data()); + } + + ~TraceArgs() { atrace_end(ATRACE_TAG); } + + private: + TraceArgs(const TraceArgs&) = delete; + void operator=(const TraceArgs&) = delete; +}; + +// Macro to define a scoped tracer with arguments. Uses PASTE(x, y) macro +// defined in utils/Trace.h. +#define TRACE_FORMAT(format, ...) \ + TraceArgs PASTE(__tracer, __LINE__) { format, ##__VA_ARGS__ } + } // anonymous namespace HardwareComposer::HardwareComposer() @@ -411,14 +438,12 @@ void HardwareComposer::PostLayers() { retire_fence_fds_.erase(retire_fence_fds_.begin()); } - const bool is_frame_pending = IsFramePendingInDriver(); const bool is_fence_pending = static_cast<int32_t>(retire_fence_fds_.size()) > post_thread_config_.allowed_pending_fence_count; - if (is_fence_pending || is_frame_pending) { + if (is_fence_pending) { ATRACE_INT("frame_skip_count", ++frame_skip_count_); - ALOGW_IF(is_frame_pending, "Warning: frame already queued, dropping frame"); ALOGW_IF(is_fence_pending, "Warning: dropping a frame to catch up with HWC (pending = %zd)", retire_fence_fds_.size()); @@ -587,18 +612,28 @@ int HardwareComposer::PostThreadPollInterruptible( } } +Status<int64_t> HardwareComposer::GetVSyncTime() { + auto status = composer_callback_->GetVsyncTime(HWC_DISPLAY_PRIMARY); + ALOGE_IF(!status, + "HardwareComposer::GetVSyncTime: Failed to get vsync timestamp: %s", + status.GetErrorMessage().c_str()); + return status; +} + // Waits for the next vsync and returns the timestamp of the vsync event. If // vsync already passed since the last call, returns the latest vsync timestamp // instead of blocking. -int HardwareComposer::WaitForVSync(int64_t* timestamp) { - int error = PostThreadPollInterruptible(composer_callback_->GetVsyncEventFd(), - POLLIN, /*timeout_ms*/ 1000); - if (error == kPostThreadInterrupted || error < 0) { +Status<int64_t> HardwareComposer::WaitForVSync() { + const int64_t predicted_vsync_time = + last_vsync_timestamp_ + + display_metrics_.vsync_period_ns * vsync_prediction_interval_; + const int error = SleepUntil(predicted_vsync_time); + if (error < 0) { + ALOGE("HardwareComposer::WaifForVSync:: Failed to sleep: %s", + strerror(-error)); return error; - } else { - *timestamp = composer_callback_->GetVsyncTime(); - return 0; } + return {predicted_vsync_time}; } int HardwareComposer::SleepUntil(int64_t wakeup_timestamp) { @@ -704,26 +739,41 @@ void HardwareComposer::PostThread() { thread_policy_setup = SetThreadPolicy("graphics:high", "/system/performance"); } + + // Initialize the last vsync timestamp with the current time. The + // predictor below uses this time + the vsync interval in absolute time + // units for the initial delay. Once the driver starts reporting vsync the + // predictor will sync up with the real vsync. + last_vsync_timestamp_ = GetSystemClockNs(); } int64_t vsync_timestamp = 0; { - std::array<char, 128> buf; - snprintf(buf.data(), buf.size(), "wait_vsync|vsync=%d|", - vsync_count_ + 1); - ATRACE_NAME(buf.data()); + TRACE_FORMAT("wait_vsync|vsync=%u;last_timestamp=%" PRId64 + ";prediction_interval=%d|", + vsync_count_ + 1, last_vsync_timestamp_, + vsync_prediction_interval_); - const int error = WaitForVSync(&vsync_timestamp); + auto status = WaitForVSync(); ALOGE_IF( - error < 0, + !status, "HardwareComposer::PostThread: Failed to wait for vsync event: %s", - strerror(-error)); - // Don't bother processing this frame if a pause was requested - if (error == kPostThreadInterrupted) + status.GetErrorMessage().c_str()); + + // If there was an error either sleeping was interrupted due to pausing or + // there was an error getting the latest timestamp. + if (!status) continue; + + // Predicted vsync timestamp for this interval. This is stable because we + // use absolute time for the wakeup timer. + vsync_timestamp = status.get(); } - ++vsync_count_; + // Advance the vsync counter only if the system is keeping up with hardware + // vsync to give clients an indication of the delays. + if (vsync_prediction_interval_ == 1) + ++vsync_count_; const bool layer_config_changed = UpdateLayerConfig(); @@ -773,6 +823,38 @@ void HardwareComposer::PostThread() { } } + { + auto status = GetVSyncTime(); + if (!status) { + ALOGE("HardwareComposer::PostThread: Failed to get VSYNC time: %s", + status.GetErrorMessage().c_str()); + } + + // If we failed to read vsync there might be a problem with the driver. + // Since there's nothing we can do just behave as though we didn't get an + // updated vsync time and let the prediction continue. + const int64_t current_vsync_timestamp = + status ? status.get() : last_vsync_timestamp_; + + const bool vsync_delayed = + last_vsync_timestamp_ == current_vsync_timestamp; + ATRACE_INT("vsync_delayed", vsync_delayed); + + // If vsync was delayed advance the prediction interval and allow the + // fence logic in PostLayers() to skip the frame. + if (vsync_delayed) { + ALOGW( + "HardwareComposer::PostThread: VSYNC timestamp did not advance " + "since last frame: timestamp=%" PRId64 " prediction_interval=%d", + current_vsync_timestamp, vsync_prediction_interval_); + vsync_prediction_interval_++; + } else { + // We have an updated vsync timestamp, reset the prediction interval. + last_vsync_timestamp_ = current_vsync_timestamp; + vsync_prediction_interval_ = 1; + } + } + PostLayers(); } } @@ -860,14 +942,28 @@ void HardwareComposer::SetBacklightBrightness(int brightness) { } } -HardwareComposer::ComposerCallback::ComposerCallback() { - vsync_event_fd_.Reset(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); - LOG_ALWAYS_FATAL_IF(!vsync_event_fd_, "Failed to create vsync event fd : %s", - strerror(errno)); -} - Return<void> HardwareComposer::ComposerCallback::onHotplug( - Hwc2::Display /*display*/, IComposerCallback::Connection /*conn*/) { + Hwc2::Display display, IComposerCallback::Connection /*conn*/) { + // See if the driver supports the vsync_event node in sysfs. + if (display < HWC_NUM_PHYSICAL_DISPLAY_TYPES && + !displays_[display].driver_vsync_event_fd) { + std::array<char, 1024> buffer; + snprintf(buffer.data(), buffer.size(), + "/sys/class/graphics/fb%" PRIu64 "/vsync_event", display); + if (LocalHandle handle{buffer.data(), O_RDONLY}) { + ALOGI( + "HardwareComposer::ComposerCallback::onHotplug: Driver supports " + "vsync_event node for display %" PRIu64, + display); + displays_[display].driver_vsync_event_fd = std::move(handle); + } else { + ALOGI( + "HardwareComposer::ComposerCallback::onHotplug: Driver does not " + "support vsync_event node for display %" PRIu64, + display); + } + } + return Void(); } @@ -878,29 +974,81 @@ Return<void> HardwareComposer::ComposerCallback::onRefresh( Return<void> HardwareComposer::ComposerCallback::onVsync(Hwc2::Display display, int64_t timestamp) { - if (display == HWC_DISPLAY_PRIMARY) { - std::lock_guard<std::mutex> lock(vsync_mutex_); - vsync_time_ = timestamp; - int error = eventfd_write(vsync_event_fd_.Get(), 1); - LOG_ALWAYS_FATAL_IF(error != 0, "Failed writing to vsync event fd"); + TRACE_FORMAT("vsync_callback|display=%" PRIu64 ";timestamp=%" PRId64 "|", + display, timestamp); + if (display < HWC_NUM_PHYSICAL_DISPLAY_TYPES) { + displays_[display].callback_vsync_timestamp = timestamp; + } else { + ALOGW( + "HardwareComposer::ComposerCallback::onVsync: Received vsync on " + "non-physical display: display=%" PRId64, + display); } return Void(); } -const pdx::LocalHandle& HardwareComposer::ComposerCallback::GetVsyncEventFd() - const { - return vsync_event_fd_; -} +Status<int64_t> HardwareComposer::ComposerCallback::GetVsyncTime( + Hwc2::Display display) { + if (display >= HWC_NUM_PHYSICAL_DISPLAY_TYPES) { + ALOGE( + "HardwareComposer::ComposerCallback::GetVsyncTime: Invalid physical " + "display requested: display=%" PRIu64, + display); + return ErrorStatus(EINVAL); + } + + // See if the driver supports direct vsync events. + LocalHandle& event_fd = displays_[display].driver_vsync_event_fd; + if (!event_fd) { + // Fall back to returning the last timestamp returned by the vsync + // callback. + std::lock_guard<std::mutex> autolock(vsync_mutex_); + return displays_[display].callback_vsync_timestamp; + } + + // When the driver supports the vsync_event sysfs node we can use it to + // determine the latest vsync timestamp, even if the HWC callback has been + // delayed. + + // The driver returns data in the form "VSYNC=<timestamp ns>". + std::array<char, 32> data; + data.fill('\0'); + + // Seek back to the beginning of the event file. + int ret = lseek(event_fd.Get(), 0, SEEK_SET); + if (ret < 0) { + const int error = errno; + ALOGE( + "HardwareComposer::ComposerCallback::GetVsyncTime: Failed to seek " + "vsync event fd: %s", + strerror(error)); + return ErrorStatus(error); + } + + // Read the vsync event timestamp. + ret = read(event_fd.Get(), data.data(), data.size()); + if (ret < 0) { + const int error = errno; + ALOGE_IF(error != EAGAIN, + "HardwareComposer::ComposerCallback::GetVsyncTime: Error " + "while reading timestamp: %s", + strerror(error)); + return ErrorStatus(error); + } + + int64_t timestamp; + ret = sscanf(data.data(), "VSYNC=%" PRIu64, + reinterpret_cast<uint64_t*>(×tamp)); + if (ret < 0) { + const int error = errno; + ALOGE( + "HardwareComposer::ComposerCallback::GetVsyncTime: Error while " + "parsing timestamp: %s", + strerror(error)); + return ErrorStatus(error); + } -int64_t HardwareComposer::ComposerCallback::GetVsyncTime() { - std::lock_guard<std::mutex> lock(vsync_mutex_); - eventfd_t event; - eventfd_read(vsync_event_fd_.Get(), &event); - LOG_ALWAYS_FATAL_IF(vsync_time_ < 0, - "Attempt to read vsync time before vsync event"); - int64_t return_val = vsync_time_; - vsync_time_ = -1; - return return_val; + return {timestamp}; } Hwc2::Composer* Layer::composer_{nullptr}; diff --git a/libs/vr/libvrflinger/hardware_composer.h b/libs/vr/libvrflinger/hardware_composer.h index 793c3e887c..8131e50989 100644 --- a/libs/vr/libvrflinger/hardware_composer.h +++ b/libs/vr/libvrflinger/hardware_composer.h @@ -142,9 +142,7 @@ class Layer { bool operator<(const Layer& other) const { return GetSurfaceId() < other.GetSurfaceId(); } - bool operator<(int surface_id) const { - return GetSurfaceId() < surface_id; - } + bool operator<(int surface_id) const { return GetSurfaceId() < surface_id; } // Sets the composer instance used by all Layer instances. static void SetComposer(Hwc2::Composer* composer) { composer_ = composer; } @@ -340,19 +338,23 @@ class HardwareComposer { class ComposerCallback : public Hwc2::IComposerCallback { public: - ComposerCallback(); + ComposerCallback() = default; hardware::Return<void> onHotplug(Hwc2::Display display, Connection conn) override; hardware::Return<void> onRefresh(Hwc2::Display display) override; hardware::Return<void> onVsync(Hwc2::Display display, int64_t timestamp) override; - const pdx::LocalHandle& GetVsyncEventFd() const; - int64_t GetVsyncTime(); + + pdx::Status<int64_t> GetVsyncTime(Hwc2::Display display); private: std::mutex vsync_mutex_; - pdx::LocalHandle vsync_event_fd_; - int64_t vsync_time_ = -1; + + struct Display { + pdx::LocalHandle driver_vsync_event_fd; + int64_t callback_vsync_timestamp{0}; + }; + std::array<Display, HWC_NUM_PHYSICAL_DISPLAY_TYPES> displays_; }; HWC::Error Validate(hwc2_display_t display); @@ -392,11 +394,10 @@ class HardwareComposer { // can be interrupted by a control thread. If interrupted, these calls return // kPostThreadInterrupted. int ReadWaitPPState(); - int WaitForVSync(int64_t* timestamp); + pdx::Status<int64_t> WaitForVSync(); + pdx::Status<int64_t> GetVSyncTime(); int SleepUntil(int64_t wakeup_timestamp); - bool IsFramePendingInDriver() { return false; } - // Reconfigures the layer stack if the display surfaces changed since the last // frame. Called only from the post thread. bool UpdateLayerConfig(); @@ -463,6 +464,9 @@ class HardwareComposer { // The timestamp of the last vsync. int64_t last_vsync_timestamp_ = 0; + // The number of vsync intervals to predict since the last vsync. + int vsync_prediction_interval_ = 1; + // Vsync count since display on. uint32_t vsync_count_ = 0; diff --git a/libs/vr/libvrflinger/vr_flinger.cpp b/libs/vr/libvrflinger/vr_flinger.cpp index fcf94f0865..85dc586eae 100644 --- a/libs/vr/libvrflinger/vr_flinger.cpp +++ b/libs/vr/libvrflinger/vr_flinger.cpp @@ -64,9 +64,6 @@ bool VrFlinger::Init(Hwc2::Composer* hidl, ALOGI("Starting up VrFlinger..."); - setpriority(PRIO_PROCESS, 0, android::PRIORITY_URGENT_DISPLAY); - set_sched_policy(0, SP_FOREGROUND); - // We need to be able to create endpoints with full perms. umask(0000); @@ -100,6 +97,9 @@ bool VrFlinger::Init(Hwc2::Composer* hidl, prctl(PR_SET_NAME, reinterpret_cast<unsigned long>("VrDispatch"), 0, 0, 0); ALOGI("Entering message loop."); + setpriority(PRIO_PROCESS, 0, android::PRIORITY_URGENT_DISPLAY); + set_sched_policy(0, SP_FOREGROUND); + int ret = dispatcher_->EnterDispatchLoop(); if (ret < 0) { ALOGE("Dispatch loop exited because: %s\n", strerror(-ret)); diff --git a/libs/vr/libvrflinger/vsync_service.cpp b/libs/vr/libvrflinger/vsync_service.cpp index 3098b43318..fdeb899f66 100644 --- a/libs/vr/libvrflinger/vsync_service.cpp +++ b/libs/vr/libvrflinger/vsync_service.cpp @@ -110,6 +110,7 @@ void VSyncService::UpdateClients() { } pdx::Status<void> VSyncService::HandleMessage(pdx::Message& message) { + ATRACE_NAME("VSyncService::HandleMessage"); switch (message.GetOp()) { case VSyncProtocol::Wait::Opcode: AddWaiter(message); diff --git a/opengl/tools/glgen/stubs/egl/EGL14cHeader.cpp b/opengl/tools/glgen/stubs/egl/EGL14cHeader.cpp index f6813fdc03..66836b586f 100644 --- a/opengl/tools/glgen/stubs/egl/EGL14cHeader.cpp +++ b/opengl/tools/glgen/stubs/egl/EGL14cHeader.cpp @@ -21,7 +21,7 @@ #pragma GCC diagnostic ignored "-Wunused-function" #include "jni.h" -#include "JNIHelp.h" +#include <nativehelper/JNIHelp.h> #include <android_runtime/AndroidRuntime.h> #include <android_runtime/android_view_Surface.h> #include <android_runtime/android_graphics_SurfaceTexture.h> diff --git a/opengl/tools/glgen/stubs/egl/EGLExtcHeader.cpp b/opengl/tools/glgen/stubs/egl/EGLExtcHeader.cpp index 4df61d3126..fb75d814cf 100644 --- a/opengl/tools/glgen/stubs/egl/EGLExtcHeader.cpp +++ b/opengl/tools/glgen/stubs/egl/EGLExtcHeader.cpp @@ -21,7 +21,7 @@ #pragma GCC diagnostic ignored "-Wunused-function" #include "jni.h" -#include "JNIHelp.h" +#include <nativehelper/JNIHelp.h> #include <android_runtime/AndroidRuntime.h> #include <android_runtime/android_view_Surface.h> #include <android_runtime/android_graphics_SurfaceTexture.h> diff --git a/opengl/tools/glgen/stubs/gles11/common.cpp b/opengl/tools/glgen/stubs/gles11/common.cpp index 7062c5751f..2163d7600d 100644 --- a/opengl/tools/glgen/stubs/gles11/common.cpp +++ b/opengl/tools/glgen/stubs/gles11/common.cpp @@ -1,5 +1,5 @@ #include <jni.h> -#include <JNIHelp.h> +#include <nativehelper/JNIHelp.h> #include <android_runtime/AndroidRuntime.h> #include <utils/misc.h> #include <assert.h> diff --git a/opengl/tools/glgen/stubs/jsr239/GLCHeader.cpp b/opengl/tools/glgen/stubs/jsr239/GLCHeader.cpp index 026cb371aa..03e16e9444 100644 --- a/opengl/tools/glgen/stubs/jsr239/GLCHeader.cpp +++ b/opengl/tools/glgen/stubs/jsr239/GLCHeader.cpp @@ -21,7 +21,7 @@ #pragma GCC diagnostic ignored "-Wunused-function" #include "jni.h" -#include "JNIHelp.h" +#include <nativehelper/JNIHelp.h> #include <android_runtime/AndroidRuntime.h> #include <utils/misc.h> diff --git a/services/inputflinger/InputWindow.cpp b/services/inputflinger/InputWindow.cpp index b54752b08b..3ae7972779 100644 --- a/services/inputflinger/InputWindow.cpp +++ b/services/inputflinger/InputWindow.cpp @@ -49,7 +49,8 @@ bool InputWindowInfo::isTrustedOverlay() const { || layoutParamsType == TYPE_NAVIGATION_BAR_PANEL || layoutParamsType == TYPE_SECURE_SYSTEM_OVERLAY || layoutParamsType == TYPE_DOCK_DIVIDER - || layoutParamsType == TYPE_ACCESSIBILITY_OVERLAY; + || layoutParamsType == TYPE_ACCESSIBILITY_OVERLAY + || layoutParamsType == TYPE_INPUT_CONSUMER; } bool InputWindowInfo::supportsSplitTouch() const { diff --git a/services/inputflinger/InputWindow.h b/services/inputflinger/InputWindow.h index 610290b2e2..9eb27983cd 100644 --- a/services/inputflinger/InputWindow.h +++ b/services/inputflinger/InputWindow.h @@ -101,6 +101,7 @@ struct InputWindowInfo { TYPE_NAVIGATION_BAR = FIRST_SYSTEM_WINDOW+19, TYPE_VOLUME_OVERLAY = FIRST_SYSTEM_WINDOW+20, TYPE_BOOT_PROGRESS = FIRST_SYSTEM_WINDOW+21, + TYPE_INPUT_CONSUMER = FIRST_SYSTEM_WINDOW+22, TYPE_NAVIGATION_BAR_PANEL = FIRST_SYSTEM_WINDOW+24, TYPE_MAGNIFICATION_OVERLAY = FIRST_SYSTEM_WINDOW+27, TYPE_ACCESSIBILITY_OVERLAY = FIRST_SYSTEM_WINDOW+32, diff --git a/services/inputflinger/tests/Android.bp b/services/inputflinger/tests/Android.bp index 29d93f034b..84a63d6511 100644 --- a/services/inputflinger/tests/Android.bp +++ b/services/inputflinger/tests/Android.bp @@ -15,7 +15,6 @@ cc_test { "libhardware", "libhardware_legacy", "libui", - "libskia", "libinput", "libinputflinger", "libinputservice", diff --git a/services/surfaceflinger/Android.mk b/services/surfaceflinger/Android.mk index 38529b6d0a..1f4427a11a 100644 --- a/services/surfaceflinger/Android.mk +++ b/services/surfaceflinger/Android.mk @@ -14,7 +14,7 @@ LOCAL_SRC_FILES := \ FrameTracker.cpp \ GpuService.cpp \ Layer.cpp \ - LayerDim.cpp \ + ColorLayer.cpp \ LayerRejecter.cpp \ LayerVector.cpp \ MessageQueue.cpp \ diff --git a/services/surfaceflinger/LayerDim.cpp b/services/surfaceflinger/ColorLayer.cpp index daebf8abcd..6923782b27 100644 --- a/services/surfaceflinger/LayerDim.cpp +++ b/services/surfaceflinger/ColorLayer.cpp @@ -16,7 +16,7 @@ // #define LOG_NDEBUG 0 #undef LOG_TAG -#define LOG_TAG "LayerDim" +#define LOG_TAG "ColorLayer" #include <stdlib.h> #include <stdint.h> @@ -27,7 +27,7 @@ #include <ui/GraphicBuffer.h> -#include "LayerDim.h" +#include "ColorLayer.h" #include "SurfaceFlinger.h" #include "DisplayDevice.h" #include "RenderEngine/RenderEngine.h" @@ -35,31 +35,29 @@ namespace android { // --------------------------------------------------------------------------- -LayerDim::LayerDim(SurfaceFlinger* flinger, const sp<Client>& client, +ColorLayer::ColorLayer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags) : Layer(flinger, client, name, w, h, flags) { } -LayerDim::~LayerDim() { -} - -void LayerDim::onDraw(const sp<const DisplayDevice>& hw, +void ColorLayer::onDraw(const sp<const DisplayDevice>& hw, const Region& /* clip */, bool useIdentityTransform) const { const State& s(getDrawingState()); - if (s.alpha>0) { + if (s.color.a>0) { Mesh mesh(Mesh::TRIANGLE_FAN, 4, 2); computeGeometry(hw, mesh, useIdentityTransform); RenderEngine& engine(mFlinger->getRenderEngine()); - engine.setupDimLayerBlending(s.alpha); + engine.setupLayerBlending(getPremultipledAlpha(), false /* opaque */, + true /* disableTexture */, s.color); engine.drawMesh(mesh); engine.disableBlending(); } } -bool LayerDim::isVisible() const { +bool ColorLayer::isVisible() const { const Layer::State& s(getDrawingState()); - return !isHiddenByPolicy() && s.alpha; + return !isHiddenByPolicy() && s.color.a; } diff --git a/services/surfaceflinger/LayerDim.h b/services/surfaceflinger/ColorLayer.h index a0cfca98cf..ac3e2a95dc 100644 --- a/services/surfaceflinger/LayerDim.h +++ b/services/surfaceflinger/ColorLayer.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ANDROID_LAYER_DIM_H -#define ANDROID_LAYER_DIM_H +#ifndef ANDROID_COLOR_LAYER_H +#define ANDROID_COLOR_LAYER_H #include <stdint.h> #include <sys/types.h> @@ -26,14 +26,14 @@ namespace android { -class LayerDim : public Layer +class ColorLayer : public Layer { public: - LayerDim(SurfaceFlinger* flinger, const sp<Client>& client, + ColorLayer(SurfaceFlinger* flinger, const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags); - virtual ~LayerDim(); + virtual ~ColorLayer() = default; - virtual const char* getTypeId() const { return "LayerDim"; } + virtual const char* getTypeId() const { return "ColorLayer"; } virtual void onDraw(const sp<const DisplayDevice>& hw, const Region& clip, bool useIdentityTransform) const; virtual bool isOpaque(const Layer::State&) const { return false; } @@ -46,4 +46,4 @@ public: }; // namespace android -#endif // ANDROID_LAYER_DIM_H +#endif // ANDROID_COLOR_LAYER_H diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 9435a187d9..8734ee19fd 100755 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -137,11 +137,7 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client, mCurrentState.requestedFinalCrop = mCurrentState.finalCrop; mCurrentState.requestedCrop = mCurrentState.crop; mCurrentState.z = 0; -#ifdef USE_HWC2 - mCurrentState.alpha = 1.0f; -#else - mCurrentState.alpha = 0xFF; -#endif + mCurrentState.color.a = 1.0f; mCurrentState.layerStack = 0; mCurrentState.flags = layerFlags; mCurrentState.sequence = 0; @@ -334,6 +330,10 @@ const String8& Layer::getName() const { return mName; } +bool Layer::getPremultipledAlpha() const { + return mPremultipliedAlpha; +} + status_t Layer::setBuffers( uint32_t w, uint32_t h, PixelFormat format, uint32_t flags) { @@ -683,7 +683,7 @@ void Layer::setGeometry( " %s (%d)", mName.string(), to_string(blendMode).c_str(), to_string(error).c_str(), static_cast<int32_t>(error)); #else - if (!isOpaque(s) || getAlpha() != 0xFF) { + if (!isOpaque(s) || getAlpha() != 1.0f) { layer.setBlending(mPremultipliedAlpha ? HWC_BLENDING_PREMULT : HWC_BLENDING_COVERAGE); @@ -757,7 +757,7 @@ void Layer::setGeometry( hwcInfo.sourceCrop = sourceCrop; } - float alpha = getAlpha(); + float alpha = static_cast<float>(getAlpha()); error = hwcLayer->setPlaneAlpha(alpha); ALOGE_IF(error != HWC2::Error::None, "[%s] Failed to set plane alpha %.3f: " "%s (%d)", mName.string(), alpha, to_string(error).c_str(), @@ -787,7 +787,7 @@ void Layer::setGeometry( const Transform& tr(hw->getTransform()); layer.setFrame(tr.transform(frame)); layer.setCrop(computeCrop(hw)); - layer.setPlaneAlpha(getAlpha()); + layer.setPlaneAlpha(static_cast<uint8_t>(std::round(255.0f*getAlpha()))); #endif /* @@ -904,8 +904,11 @@ void Layer::setPerFrameData(const sp<const DisplayDevice>& displayDevice) { if (mActiveBuffer == nullptr) { setCompositionType(hwcId, HWC2::Composition::SolidColor); - // For now, we only support black for DimLayer - error = hwcLayer->setColor({0, 0, 0, 255}); + half4 color = getColor(); + error = hwcLayer->setColor({static_cast<uint8_t>(std::round(255.0f*color.r)), + static_cast<uint8_t>(std::round(255.0f * color.g)), + static_cast<uint8_t>(std::round(255.0f * color.b)), + 255}); if (error != HWC2::Error::None) { ALOGE("[%s] Failed to set color: %s (%d)", mName.string(), to_string(error).c_str(), static_cast<int32_t>(error)); @@ -1254,7 +1257,8 @@ void Layer::drawWithOpenGL(const sp<const DisplayDevice>& hw, texCoords[3] = vec2(right, 1.0f - top); RenderEngine& engine(mFlinger->getRenderEngine()); - engine.setupLayerBlending(mPremultipliedAlpha, isOpaque(s), getAlpha()); + engine.setupLayerBlending(mPremultipliedAlpha, isOpaque(s), + false /* disableTexture */, getColor()); #ifdef USE_HWC2 engine.setSourceDataSpace(mCurrentState.dataSpace); #endif @@ -1877,19 +1881,30 @@ bool Layer::setSize(uint32_t w, uint32_t h) { setTransactionFlags(eTransactionNeeded); return true; } -#ifdef USE_HWC2 bool Layer::setAlpha(float alpha) { -#else -bool Layer::setAlpha(uint8_t alpha) { -#endif - if (mCurrentState.alpha == alpha) + if (mCurrentState.color.a == alpha) return false; mCurrentState.sequence++; - mCurrentState.alpha = alpha; + mCurrentState.color.a = alpha; + mCurrentState.modified = true; + setTransactionFlags(eTransactionNeeded); + return true; +} + +bool Layer::setColor(const half3& color) { + if (color.r == mCurrentState.color.r && color.g == mCurrentState.color.g + && color.b == mCurrentState.color.b) + return false; + + mCurrentState.sequence++; + mCurrentState.color.r = color.r; + mCurrentState.color.g = color.g; + mCurrentState.color.b = color.b; mCurrentState.modified = true; setTransactionFlags(eTransactionNeeded); return true; } + bool Layer::setMatrix(const layer_state_t::matrix22_t& matrix) { mCurrentState.sequence++; mCurrentState.requested.transform.set( @@ -2141,13 +2156,8 @@ bool Layer::isHiddenByPolicy() const { } bool Layer::isVisible() const { -#ifdef USE_HWC2 return !(isHiddenByPolicy()) && getAlpha() > 0.0f && (mActiveBuffer != NULL || mSidebandStream != NULL); -#else - return !(isHiddenByPolicy()) && getAlpha() - && (mActiveBuffer != NULL || mSidebandStream != NULL); -#endif } bool Layer::allTransactionsSignaled() { @@ -2439,7 +2449,7 @@ LayerDebugInfo Layer::getLayerDebugInfo() const { info.mHeight = ds.active.h; info.mCrop = ds.crop; info.mFinalCrop = ds.finalCrop; - info.mAlpha = ds.alpha; + info.mColor = ds.color; info.mFlags = ds.flags; info.mPixelFormat = getPixelFormat(); info.mDataSpace = getDataSpace(); @@ -2467,7 +2477,6 @@ LayerDebugInfo Layer::getLayerDebugInfo() const { info.mContentDirty = contentDirty; return info; } - #ifdef USE_HWC2 void Layer::miniDumpHeader(String8& result) { result.append("----------------------------------------"); @@ -2625,8 +2634,8 @@ bool Layer::reparentChildren(const sp<IBinder>& newParentHandle) { return true; } -bool Layer::reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle) { - if (newParentHandle == nullptr || childHandle == nullptr) { +bool Layer::reparent(const sp<IBinder>& newParentHandle) { + if (newParentHandle == nullptr) { return false; } @@ -2637,29 +2646,19 @@ bool Layer::reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& return false; } - handle = static_cast<Handle*>(childHandle.get()); - sp<Layer> child = handle->owner.promote(); - if (child == nullptr) { - ALOGE("Unable to promote child Layer handle"); - return false; - } - - if (mCurrentChildren.indexOf(child) < 0) { - ALOGE("Child layer is not child of current layer"); - return false; + sp<Layer> parent = getParent(); + if (parent != nullptr) { + parent->removeChild(this); } + newParent->addChild(this); - sp<Client> parentClient(mClientRef.promote()); - sp<Client> childClient(child->mClientRef.promote()); + sp<Client> client(mClientRef.promote()); sp<Client> newParentClient(newParent->mClientRef.promote()); - if (parentClient != childClient || childClient != newParentClient) { - ALOGE("Current layer, child layer, and new parent layer must have the same client"); - return false; + if (client != newParentClient) { + client->setParentLayer(newParent); } - newParent->addChild(child); - mCurrentChildren.remove(child); return true; } @@ -2801,23 +2800,17 @@ Transform Layer::getTransform() const { return t * getDrawingState().active.transform; } -#ifdef USE_HWC2 -float Layer::getAlpha() const { +half Layer::getAlpha() const { const auto& p = mDrawingParent.promote(); - float parentAlpha = (p != nullptr) ? p->getAlpha() : 1.0; - return parentAlpha * getDrawingState().alpha; + half parentAlpha = (p != nullptr) ? p->getAlpha() : 1.0_hf; + return parentAlpha * getDrawingState().color.a; } -#else -uint8_t Layer::getAlpha() const { - const auto& p = mDrawingParent.promote(); - float parentAlpha = (p != nullptr) ? (p->getAlpha() / 255.0f) : 1.0; - float drawingAlpha = getDrawingState().alpha / 255.0f; - drawingAlpha = drawingAlpha * parentAlpha; - return static_cast<uint8_t>(std::round(drawingAlpha * 255)); +half4 Layer::getColor() const { + const half4 color(getDrawingState().color); + return half4(color.r, color.g, color.b, getAlpha()); } -#endif void Layer::commitChildList() { for (size_t i = 0; i < mCurrentChildren.size(); i++) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index f94833b32f..921492b210 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -51,6 +51,8 @@ #include "RenderEngine/Mesh.h" #include "RenderEngine/Texture.h" +#include <math/vec4.h> + namespace android { // --------------------------------------------------------------------------- @@ -119,11 +121,6 @@ public: // to achieve mirroring. uint32_t layerStack; -#ifdef USE_HWC2 - float alpha; -#else - uint8_t alpha; -#endif uint8_t flags; uint8_t mask; uint8_t reserved[2]; @@ -158,6 +155,8 @@ public: // A list of surfaces whose Z-order is interpreted relative to ours. SortedVector<wp<Layer>> zOrderRelatives; + + half4 color; }; // ----------------------------------------------------------------------- @@ -225,11 +224,8 @@ public: bool setLayer(int32_t z); bool setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ); -#ifdef USE_HWC2 bool setAlpha(float alpha); -#else - bool setAlpha(uint8_t alpha); -#endif + bool setColor(const half3& color); bool setTransparentRegionHint(const Region& transparent); bool setFlags(uint8_t flags, uint8_t mask); bool setLayerStack(uint32_t layerStack); @@ -241,7 +237,7 @@ public: bool setOverrideScalingMode(int32_t overrideScalingMode); void setInfo(uint32_t type, uint32_t appId); bool reparentChildren(const sp<IBinder>& layer); - bool reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle); + bool reparent(const sp<IBinder>& newParentHandle); bool detachChildren(); // If we have received a new buffer this frame, we will pass its surface @@ -509,11 +505,8 @@ public: // Returns the Alpha of the Surface, accounting for the Alpha // of parent Surfaces in the hierarchy (alpha's will be multiplied // down the hierarchy). -#ifdef USE_HWC2 - float getAlpha() const; -#else - uint8_t getAlpha() const; -#endif + half getAlpha() const; + half4 getColor() const; void traverseInReverseZOrder(LayerVector::StateSet stateSet, const LayerVector::Visitor& visitor); @@ -683,9 +676,8 @@ public: sp<IGraphicBufferProducer> getProducer() const; const String8& getName() const; void notifyAvailableFrames(); - PixelFormat getPixelFormat() const { return mFormat; } - + bool getPremultipledAlpha() const; private: // ----------------------------------------------------------------------- diff --git a/services/surfaceflinger/RenderEngine/Description.cpp b/services/surfaceflinger/RenderEngine/Description.cpp index effd3191c8..706960cafe 100644 --- a/services/surfaceflinger/RenderEngine/Description.cpp +++ b/services/surfaceflinger/RenderEngine/Description.cpp @@ -27,22 +27,15 @@ namespace android { Description::Description() { - mPlaneAlpha = 1.0f; mPremultipliedAlpha = false; mOpaque = true; mTextureEnabled = false; mColorMatrixEnabled = false; - - memset(mColor, 0, sizeof(mColor)); } Description::~Description() { } -void Description::setPlaneAlpha(GLclampf planeAlpha) { - mPlaneAlpha = planeAlpha; -} - void Description::setPremultipliedAlpha(bool premultipliedAlpha) { mPremultipliedAlpha = premultipliedAlpha; } @@ -60,11 +53,8 @@ void Description::disableTexture() { mTextureEnabled = false; } -void Description::setColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha) { - mColor[0] = red; - mColor[1] = green; - mColor[2] = blue; - mColor[3] = alpha; +void Description::setColor(const half4& color) { + mColor = color; } void Description::setProjectionMatrix(const mat4& mtx) { diff --git a/services/surfaceflinger/RenderEngine/Description.h b/services/surfaceflinger/RenderEngine/Description.h index 3beffdf9e1..cbac855ff3 100644 --- a/services/surfaceflinger/RenderEngine/Description.h +++ b/services/surfaceflinger/RenderEngine/Description.h @@ -35,8 +35,6 @@ class Description { friend class Program; friend class ProgramCache; - // value of the plane-alpha, between 0 and 1 - GLclampf mPlaneAlpha; // whether textures are premultiplied bool mPremultipliedAlpha; // whether this layer is marked as opaque @@ -46,8 +44,8 @@ class Description { Texture mTexture; bool mTextureEnabled; - // color used when texturing is disabled - GLclampf mColor[4]; + // color used when texturing is disabled or when setting alpha. + half4 mColor; // projection matrix mat4 mProjectionMatrix; @@ -60,12 +58,11 @@ public: Description(); ~Description(); - void setPlaneAlpha(GLclampf planeAlpha); void setPremultipliedAlpha(bool premultipliedAlpha); void setOpaque(bool opaque); void setTexture(const Texture& texture); void disableTexture(); - void setColor(GLclampf red, GLclampf green, GLclampf blue, GLclampf alpha); + void setColor(const half4& color); void setProjectionMatrix(const mat4& mtx); void setColorMatrix(const mat4& mtx); const mat4& getColorMatrix() const; diff --git a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp index 37a530b33a..daaa11e1d3 100644 --- a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp +++ b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.cpp @@ -204,25 +204,17 @@ void GLES20RenderEngine::setViewportAndProjection( mVpHeight = vph; } -#ifdef USE_HWC2 void GLES20RenderEngine::setupLayerBlending(bool premultipliedAlpha, - bool opaque, float alpha) { -#else -void GLES20RenderEngine::setupLayerBlending( - bool premultipliedAlpha, bool opaque, int alpha) { -#endif - + bool opaque, bool disableTexture, const half4& color) { mState.setPremultipliedAlpha(premultipliedAlpha); mState.setOpaque(opaque); -#ifdef USE_HWC2 - mState.setPlaneAlpha(alpha); + mState.setColor(color); - if (alpha < 1.0f || !opaque) { -#else - mState.setPlaneAlpha(alpha / 255.0f); + if (disableTexture) { + mState.disableTexture(); + } - if (alpha < 0xFF || !opaque) { -#endif + if (color.a < 1.0f || !opaque) { glEnable(GL_BLEND); glBlendFunc(premultipliedAlpha ? GL_ONE : GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); } else { @@ -231,33 +223,6 @@ void GLES20RenderEngine::setupLayerBlending( } #ifdef USE_HWC2 -void GLES20RenderEngine::setupDimLayerBlending(float alpha) { -#else -void GLES20RenderEngine::setupDimLayerBlending(int alpha) { -#endif - mState.setPlaneAlpha(1.0f); - mState.setPremultipliedAlpha(true); - mState.setOpaque(false); -#ifdef USE_HWC2 - mState.setColor(0, 0, 0, alpha); -#else - mState.setColor(0, 0, 0, alpha/255.0f); -#endif - mState.disableTexture(); - -#ifdef USE_HWC2 - if (alpha == 1.0f) { -#else - if (alpha == 0xFF) { -#endif - glDisable(GL_BLEND); - } else { - glEnable(GL_BLEND); - glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); - } -} - -#ifdef USE_HWC2 void GLES20RenderEngine::setColorMode(android_color_mode mode) { ALOGV("setColorMode: %s (0x%x)", decodeColorMode(mode).c_str(), mode); @@ -355,10 +320,9 @@ void GLES20RenderEngine::unbindFramebuffer(uint32_t texName, uint32_t fbName) { } void GLES20RenderEngine::setupFillWithColor(float r, float g, float b, float a) { - mState.setPlaneAlpha(1.0f); mState.setPremultipliedAlpha(true); mState.setOpaque(false); - mState.setColor(r, g, b, a); + mState.setColor(half4(r, g, b, a)); mState.disableTexture(); glDisable(GL_BLEND); } diff --git a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h index eaf94af54c..5ac12fc3d6 100644 --- a/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h +++ b/services/surfaceflinger/RenderEngine/GLES20RenderEngine.h @@ -68,10 +68,9 @@ protected: virtual void setViewportAndProjection(size_t vpw, size_t vph, Rect sourceCrop, size_t hwh, bool yswap, Transform::orientation_flags rotation); -#ifdef USE_HWC2 virtual void setupLayerBlending(bool premultipliedAlpha, bool opaque, - float alpha) override; - virtual void setupDimLayerBlending(float alpha) override; + bool disableTexture, const half4& color) override; +#ifdef USE_HWC2 // Color management related functions and state void setColorMode(android_color_mode mode); @@ -92,10 +91,6 @@ protected: // Currently only supporting sRGB and DisplayP3 color spaces mat4 mSrgbToDisplayP3; -#else - virtual void setupLayerBlending(bool premultipliedAlpha, bool opaque, - int alpha); - virtual void setupDimLayerBlending(int alpha); #endif bool mPlatformHasWideColor = false; diff --git a/services/surfaceflinger/RenderEngine/Program.cpp b/services/surfaceflinger/RenderEngine/Program.cpp index 48a8da5e8e..e95a6c573b 100644 --- a/services/surfaceflinger/RenderEngine/Program.cpp +++ b/services/surfaceflinger/RenderEngine/Program.cpp @@ -22,6 +22,7 @@ #include "Program.h" #include "ProgramCache.h" #include "Description.h" +#include <math/mat4.h> namespace android { @@ -63,7 +64,6 @@ Program::Program(const ProgramCache::Key& /*needs*/, const char* vertex, const c mTextureMatrixLoc = glGetUniformLocation(programId, "texture"); mSamplerLoc = glGetUniformLocation(programId, "sampler"); mColorLoc = glGetUniformLocation(programId, "color"); - mAlphaPlaneLoc = glGetUniformLocation(programId, "alphaPlane"); // set-up the default values for our uniforms glUseProgram(programId); @@ -132,11 +132,9 @@ void Program::setUniforms(const Description& desc) { glUniform1i(mSamplerLoc, 0); glUniformMatrix4fv(mTextureMatrixLoc, 1, GL_FALSE, desc.mTexture.getMatrix().asArray()); } - if (mAlphaPlaneLoc >= 0) { - glUniform1f(mAlphaPlaneLoc, desc.mPlaneAlpha); - } if (mColorLoc >= 0) { - glUniform4fv(mColorLoc, 1, desc.mColor); + const float* color = &static_cast<details::TVec4<float> const &>(desc.mColor)[0]; + glUniform4fv(mColorLoc, 1, color); } if (mColorMatrixLoc >= 0) { glUniformMatrix4fv(mColorMatrixLoc, 1, GL_FALSE, desc.mColorMatrix.asArray()); diff --git a/services/surfaceflinger/RenderEngine/Program.h b/services/surfaceflinger/RenderEngine/Program.h index 36bd120e39..a2ae2ee007 100644 --- a/services/surfaceflinger/RenderEngine/Program.h +++ b/services/surfaceflinger/RenderEngine/Program.h @@ -79,9 +79,6 @@ private: /* location of the sampler uniform */ GLint mSamplerLoc; - /* location of the alpha plane uniform */ - GLint mAlphaPlaneLoc; - /* location of the color uniform */ GLint mColorLoc; }; diff --git a/services/surfaceflinger/RenderEngine/ProgramCache.cpp b/services/surfaceflinger/RenderEngine/ProgramCache.cpp index 06b225299c..b4375454cc 100644 --- a/services/surfaceflinger/RenderEngine/ProgramCache.cpp +++ b/services/surfaceflinger/RenderEngine/ProgramCache.cpp @@ -89,7 +89,7 @@ ProgramCache::~ProgramCache() { void ProgramCache::primeCache() { uint32_t shaderCount = 0; uint32_t keyMask = Key::BLEND_MASK | Key::OPACITY_MASK | - Key::PLANE_ALPHA_MASK | Key::TEXTURE_MASK; + Key::ALPHA_MASK | Key::TEXTURE_MASK; // Prime the cache for all combinations of the above masks, // leaving off the experimental color matrix mask options. @@ -122,8 +122,8 @@ ProgramCache::Key ProgramCache::computeKey(const Description& description) { description.mTexture.getTextureTarget() == GL_TEXTURE_EXTERNAL_OES ? Key::TEXTURE_EXT : description.mTexture.getTextureTarget() == GL_TEXTURE_2D ? Key::TEXTURE_2D : Key::TEXTURE_OFF) - .set(Key::PLANE_ALPHA_MASK, - (description.mPlaneAlpha < 1) ? Key::PLANE_ALPHA_LT_ONE : Key::PLANE_ALPHA_EQ_ONE) + .set(Key::ALPHA_MASK, + (description.mColor.a < 1) ? Key::ALPHA_LT_ONE : Key::ALPHA_EQ_ONE) .set(Key::BLEND_MASK, description.mPremultipliedAlpha ? Key::BLEND_PREMULT : Key::BLEND_NORMAL) .set(Key::OPACITY_MASK, @@ -168,12 +168,12 @@ String8 ProgramCache::generateFragmentShader(const Key& needs) { } else if (needs.getTextureTarget() == Key::TEXTURE_2D) { fs << "uniform sampler2D sampler;" << "varying vec2 outTexCoords;"; - } else if (needs.getTextureTarget() == Key::TEXTURE_OFF) { - fs << "uniform vec4 color;"; } - if (needs.hasPlaneAlpha()) { - fs << "uniform float alphaPlane;"; + + if (needs.getTextureTarget() == Key::TEXTURE_OFF || needs.hasAlpha()) { + fs << "uniform vec4 color;"; } + if (needs.hasColorMatrix()) { fs << "uniform mat4 colorMatrix;"; } @@ -225,18 +225,19 @@ String8 ProgramCache::generateFragmentShader(const Key& needs) { if (needs.isTexturing()) { fs << "gl_FragColor = texture2D(sampler, outTexCoords);"; } else { - fs << "gl_FragColor = color;"; + fs << "gl_FragColor.rgb = color.rgb;"; + fs << "gl_FragColor.a = 1.0;"; } if (needs.isOpaque()) { fs << "gl_FragColor.a = 1.0;"; } - if (needs.hasPlaneAlpha()) { - // modulate the alpha value with planeAlpha + if (needs.hasAlpha()) { + // modulate the current alpha value with alpha set if (needs.isPremultiplied()) { // ... and the color too if we're premultiplied - fs << "gl_FragColor *= alphaPlane;"; + fs << "gl_FragColor *= color.a;"; } else { - fs << "gl_FragColor.a *= alphaPlane;"; + fs << "gl_FragColor.a *= color.a;"; } } diff --git a/services/surfaceflinger/RenderEngine/ProgramCache.h b/services/surfaceflinger/RenderEngine/ProgramCache.h index 5b0fbcd153..ff5cf0f21a 100644 --- a/services/surfaceflinger/RenderEngine/ProgramCache.h +++ b/services/surfaceflinger/RenderEngine/ProgramCache.h @@ -57,9 +57,9 @@ public: OPACITY_TRANSLUCENT = 0x00000000, OPACITY_MASK = 0x00000002, - PLANE_ALPHA_LT_ONE = 0x00000004, - PLANE_ALPHA_EQ_ONE = 0x00000000, - PLANE_ALPHA_MASK = 0x00000004, + ALPHA_LT_ONE = 0x00000004, + ALPHA_EQ_ONE = 0x00000000, + ALPHA_MASK = 0x00000004, TEXTURE_OFF = 0x00000000, TEXTURE_EXT = 0x00000008, @@ -95,8 +95,8 @@ public: inline bool isOpaque() const { return (mKey & OPACITY_MASK) == OPACITY_OPAQUE; } - inline bool hasPlaneAlpha() const { - return (mKey & PLANE_ALPHA_MASK) == PLANE_ALPHA_LT_ONE; + inline bool hasAlpha() const { + return (mKey & ALPHA_MASK) == ALPHA_LT_ONE; } inline bool hasColorMatrix() const { return (mKey & COLOR_MATRIX_MASK) == COLOR_MATRIX_ON; diff --git a/services/surfaceflinger/RenderEngine/RenderEngine.h b/services/surfaceflinger/RenderEngine/RenderEngine.h index 954457946e..fa65979edd 100644 --- a/services/surfaceflinger/RenderEngine/RenderEngine.h +++ b/services/surfaceflinger/RenderEngine/RenderEngine.h @@ -25,6 +25,7 @@ #include <EGL/eglext.h> #include <math/mat4.h> #include <Transform.h> +#include <gui/SurfaceControl.h> #define EGL_NO_CONFIG ((EGLConfig)0) @@ -98,16 +99,13 @@ public: virtual void checkErrors() const; virtual void setViewportAndProjection(size_t vpw, size_t vph, Rect sourceCrop, size_t hwh, bool yswap, Transform::orientation_flags rotation) = 0; + virtual void setupLayerBlending(bool premultipliedAlpha, bool opaque, + bool disableTexture, const half4& color) = 0; #ifdef USE_HWC2 - virtual void setupLayerBlending(bool premultipliedAlpha, bool opaque, float alpha) = 0; - virtual void setupDimLayerBlending(float alpha) = 0; virtual void setColorMode(android_color_mode mode) = 0; virtual void setSourceDataSpace(android_dataspace source) = 0; virtual void setWideColor(bool hasWideColor) = 0; virtual bool usesWideColor() = 0; -#else - virtual void setupLayerBlending(bool premultipliedAlpha, bool opaque, int alpha) = 0; - virtual void setupDimLayerBlending(int alpha) = 0; #endif virtual void setupLayerTexturing(const Texture& texture) = 0; virtual void setupLayerBlackedOut() = 0; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index cae4deacb0..d5c6d3d8bd 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -73,7 +73,7 @@ #include "EventThread.h" #include "Layer.h" #include "LayerVector.h" -#include "LayerDim.h" +#include "ColorLayer.h" #include "MonitoredProducer.h" #include "SurfaceFlinger.h" @@ -1739,7 +1739,7 @@ void SurfaceFlinger::rebuildLayerStacks() { // rebuild the visible layer list per screen if (CC_UNLIKELY(mVisibleRegionsDirty)) { - ATRACE_CALL(); + ATRACE_NAME("rebuildLayerStacks VR Dirty"); mVisibleRegionsDirty = false; invalidateHwcGeometry(); @@ -2437,7 +2437,7 @@ void SurfaceFlinger::computeVisibleRegions(const sp<const DisplayDevice>& displa // compute the opaque region const int32_t layerOrientation = tr.getOrientation(); - if (s.alpha == 1.0f && !translucent && + if (layer->getAlpha() == 1.0f && !translucent && ((layerOrientation & Transform::ROT_INVALID) == false)) { // the opaque region is the layer's footprint opaqueRegion = visibleRegion; @@ -2725,7 +2725,7 @@ bool SurfaceFlinger::doComposeSurfaces( case HWC2::Composition::SolidColor: { const Layer::State& state(layer->getDrawingState()); if (layer->getClearClientTarget(hwcId) && !firstLayer && - layer->isOpaque(state) && (state.alpha == 1.0f) + layer->isOpaque(state) && (state.color.a == 1.0f) && hasClientComposition) { // never clear the very first layer since we're // guaranteed the FB is already cleared @@ -3065,6 +3065,10 @@ uint32_t SurfaceFlinger::setClientStateLocked( if (layer->setAlpha(s.alpha)) flags |= eTraversalNeeded; } + if (what & layer_state_t::eColorChanged) { + if (layer->setColor(s.color)) + flags |= eTraversalNeeded; + } if (what & layer_state_t::eMatrixChanged) { if (layer->setMatrix(s.matrix)) flags |= eTraversalNeeded; @@ -3121,10 +3125,8 @@ uint32_t SurfaceFlinger::setClientStateLocked( // We don't trigger a traversal here because if no other state is // changed, we don't want this to cause any more work } - // Always re-parent the children that explicitly requested to get - // re-parented before the general re-parent of all children. - if (what & layer_state_t::eReparentChild) { - if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) { + if (what & layer_state_t::eReparent) { + if (layer->reparent(s.parentHandleForChild)) { flags |= eTransactionNeeded|eTraversalNeeded; } } @@ -3170,8 +3172,8 @@ status_t SurfaceFlinger::createLayer( uniqueName, w, h, flags, format, handle, gbp, &layer); break; - case ISurfaceComposerClient::eFXSurfaceDim: - result = createDimLayer(client, + case ISurfaceComposerClient::eFXSurfaceColor: + result = createColorLayer(client, uniqueName, w, h, flags, handle, gbp, &layer); break; @@ -3253,11 +3255,11 @@ status_t SurfaceFlinger::createNormalLayer(const sp<Client>& client, return err; } -status_t SurfaceFlinger::createDimLayer(const sp<Client>& client, +status_t SurfaceFlinger::createColorLayer(const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags, sp<IBinder>* handle, sp<IGraphicBufferProducer>* gbp, sp<Layer>* outLayer) { - *outLayer = new LayerDim(this, client, name, w, h, flags); + *outLayer = new ColorLayer(this, client, name, w, h, flags); *handle = (*outLayer)->getHandle(); *gbp = (*outLayer)->getProducer(); return NO_ERROR; @@ -4596,7 +4598,7 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v ALOGE("%c index=%zu, name=%s, layerStack=%d, z=%d, visible=%d, flags=%x, alpha=%.3f", layer->isVisible() ? '+' : '-', i, layer->getName().string(), layer->getLayerStack(), state.z, - layer->isVisible(), state.flags, state.alpha); + layer->isVisible(), state.flags, static_cast<float>(state.color.a)); i++; }); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 1b77aafc58..e87d35f912 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -84,7 +84,7 @@ class Client; class DisplayEventConnection; class EventThread; class Layer; -class LayerDim; +class ColorLayer; class Surface; class RenderEngine; class EventControlThread; @@ -410,7 +410,7 @@ private: sp<IBinder>* outHandle, sp<IGraphicBufferProducer>* outGbp, sp<Layer>* outLayer); - status_t createDimLayer(const sp<Client>& client, const String8& name, + status_t createColorLayer(const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags, sp<IBinder>* outHandle, sp<IGraphicBufferProducer>* outGbp, sp<Layer>* outLayer); diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp index 71aa52dee6..b0021383ff 100644 --- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp +++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp @@ -71,7 +71,7 @@ #include "EventThread.h" #include "Layer.h" #include "LayerVector.h" -#include "LayerDim.h" +#include "ColorLayer.h" #include "MonitoredProducer.h" #include "SurfaceFlinger.h" @@ -179,6 +179,8 @@ SurfaceFlinger::SurfaceFlinger() maxFrameBufferAcquiredBuffers = getInt64< ISurfaceFlingerConfigs, &ISurfaceFlingerConfigs::maxFrameBufferAcquiredBuffers>(2); + mPrimaryDispSync.init(hasSyncFramework, dispSyncPresentTimeOffset); + char value[PROPERTY_VALUE_MAX]; property_get("ro.bq.gpu_to_cpu_unsupported", value, "0"); @@ -2022,7 +2024,7 @@ void SurfaceFlinger::computeVisibleRegions(const sp<const DisplayDevice>& displa // compute the opaque region const int32_t layerOrientation = tr.getOrientation(); - if (s.alpha==255 && !translucent && + if (layer->getAlpha()==1.0f && !translucent && ((layerOrientation & Transform::ROT_INVALID) == false)) { // the opaque region is the layer's footprint opaqueRegion = visibleRegion; @@ -2295,7 +2297,7 @@ bool SurfaceFlinger::doComposeSurfaces(const sp<const DisplayDevice>& hw, const const Layer::State& state(layer->getDrawingState()); if ((cur->getHints() & HWC_HINT_CLEAR_FB) && i - && layer->isOpaque(state) && (state.alpha == 0xFF) + && layer->isOpaque(state) && (state.color.a == 1.0f) && hasGlesComposition) { // never clear the very first layer since we're // guaranteed the FB is already cleared @@ -2620,8 +2622,13 @@ uint32_t SurfaceFlinger::setClientStateLocked( } } if (what & layer_state_t::eAlphaChanged) { - if (layer->setAlpha(uint8_t(255.0f*s.alpha+0.5f))) + if (layer->setAlpha(s.alpha)) + flags |= eTraversalNeeded; + } + if (what & layer_state_t::eColorChanged) { + if (layer->setColor(s.color)) { flags |= eTraversalNeeded; + } } if (what & layer_state_t::eMatrixChanged) { if (layer->setMatrix(s.matrix)) @@ -2679,10 +2686,8 @@ uint32_t SurfaceFlinger::setClientStateLocked( // We don't trigger a traversal here because if no other state is // changed, we don't want this to cause any more work } - // Always re-parent the children that explicitly requested to get - // re-parented before the general re-parent of all children. - if (what & layer_state_t::eReparentChild) { - if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) { + if (what & layer_state_t::eReparent) { + if (layer->reparent(s.parentHandleForChild)) { flags |= eTransactionNeeded|eTraversalNeeded; } } @@ -2728,8 +2733,8 @@ status_t SurfaceFlinger::createLayer( uniqueName, w, h, flags, format, handle, gbp, &layer); break; - case ISurfaceComposerClient::eFXSurfaceDim: - result = createDimLayer(client, + case ISurfaceComposerClient::eFXSurfaceColor: + result = createColorLayer(client, uniqueName, w, h, flags, handle, gbp, &layer); break; @@ -2804,11 +2809,11 @@ status_t SurfaceFlinger::createNormalLayer(const sp<Client>& client, return err; } -status_t SurfaceFlinger::createDimLayer(const sp<Client>& client, +status_t SurfaceFlinger::createColorLayer(const sp<Client>& client, const String8& name, uint32_t w, uint32_t h, uint32_t flags, sp<IBinder>* handle, sp<IGraphicBufferProducer>* gbp, sp<Layer>* outLayer) { - *outLayer = new LayerDim(this, client, name, w, h, flags); + *outLayer = new ColorLayer(this, client, name, w, h, flags); *handle = (*outLayer)->getHandle(); *gbp = (*outLayer)->getProducer(); return NO_ERROR; @@ -4089,10 +4094,10 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v if (layer->getLayerStack() == hw->getLayerStack() && state.z >= minLayerZ && state.z <= maxLayerZ) { layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) { - ALOGE("%c index=%zu, name=%s, layerStack=%d, z=%d, visible=%d, flags=%x, alpha=%x", + ALOGE("%c index=%zu, name=%s, layerStack=%d, z=%d, visible=%d, flags=%x, alpha=%.3f", layer->isVisible() ? '+' : '-', i, layer->getName().string(), layer->getLayerStack(), state.z, - layer->isVisible(), state.flags, state.alpha); + layer->isVisible(), state.flags, static_cast<float>(state.color.a)); i++; }); } diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index db489b2456..eeb492978c 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -98,7 +98,7 @@ void SurfaceInterceptor::addInitialSurfaceStateLocked(Increment* increment, addPositionLocked(transaction, layerId, layer->mCurrentState.active.transform.tx(), layer->mCurrentState.active.transform.ty()); addDepthLocked(transaction, layerId, layer->mCurrentState.z); - addAlphaLocked(transaction, layerId, layer->mCurrentState.alpha); + addAlphaLocked(transaction, layerId, layer->mCurrentState.color.a); addTransparentRegionLocked(transaction, layerId, layer->mCurrentState.activeTransparentRegion); addLayerStackLocked(transaction, layerId, layer->mCurrentState.layerStack); addCropLocked(transaction, layerId, layer->mCurrentState.crop); diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter index 6be708ad1c..5c188dc8a5 100644 --- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter +++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter @@ -1,5 +1,5 @@ { "presubmit": { - "filter": "LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*" + "filter": "LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:LayerColorTest.*" } }
\ No newline at end of file diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp index dea6503e4e..8900a4d258 100644 --- a/services/surfaceflinger/tests/Transaction_test.cpp +++ b/services/surfaceflinger/tests/Transaction_test.cpp @@ -28,6 +28,7 @@ #include <ui/DisplayInfo.h> #include <math.h> +#include <math/vec3.h> namespace android { @@ -1169,7 +1170,7 @@ TEST_F(ChildLayerTest, Bug36858924) { fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0); } -TEST_F(ChildLayerTest, ReparentChild) { +TEST_F(ChildLayerTest, Reparent) { SurfaceComposerClient::openGlobalTransaction(); mChild->show(); mChild->setPosition(10, 10); @@ -1185,7 +1186,7 @@ TEST_F(ChildLayerTest, ReparentChild) { // And 10 more pixels we should be back to the foreground surface mCapture->expectFGColor(84, 84); } - mFGSurfaceControl->reparentChild(mBGSurfaceControl->getHandle(), mChild->getHandle()); + mChild->reparent(mBGSurfaceControl->getHandle()); { ScreenCapture::captureScreen(&mCapture); mCapture->expectFGColor(64, 64); @@ -1198,6 +1199,69 @@ TEST_F(ChildLayerTest, ReparentChild) { } } +TEST_F(ChildLayerTest, ReparentToNoParent) { + SurfaceComposerClient::openGlobalTransaction(); + mChild->show(); + mChild->setPosition(10, 10); + mFGSurfaceControl->setPosition(64, 64); + SurfaceComposerClient::closeGlobalTransaction(true); + + { + ScreenCapture::captureScreen(&mCapture); + // Top left of foreground must now be visible + mCapture->expectFGColor(64, 64); + // But 10 pixels in we should see the child surface + mCapture->expectChildColor(74, 74); + // And 10 more pixels we should be back to the foreground surface + mCapture->expectFGColor(84, 84); + } + mChild->reparent(nullptr); + { + ScreenCapture::captureScreen(&mCapture); + // Nothing should have changed. + mCapture->expectFGColor(64, 64); + mCapture->expectChildColor(74, 74); + mCapture->expectFGColor(84, 84); + } +} + +TEST_F(ChildLayerTest, ReparentFromNoParent) { + sp<SurfaceControl> newSurface = mComposerClient->createSurface( + String8("New Surface"), 10, 10, PIXEL_FORMAT_RGBA_8888, 0); + ASSERT_TRUE(newSurface != NULL); + ASSERT_TRUE(newSurface->isValid()); + + fillSurfaceRGBA8(newSurface, 63, 195, 63); + SurfaceComposerClient::openGlobalTransaction(); + mChild->hide(); + newSurface->show(); + newSurface->setPosition(10, 10); + newSurface->setLayer(INT32_MAX-2); + mFGSurfaceControl->setPosition(64, 64); + SurfaceComposerClient::closeGlobalTransaction(true); + + { + ScreenCapture::captureScreen(&mCapture); + // Top left of foreground must now be visible + mCapture->expectFGColor(64, 64); + // At 10, 10 we should see the new surface + mCapture->checkPixel(10, 10, 63, 195, 63); + } + + SurfaceComposerClient::openGlobalTransaction(); + newSurface->reparent(mFGSurfaceControl->getHandle()); + SurfaceComposerClient::closeGlobalTransaction(true); + + { + ScreenCapture::captureScreen(&mCapture); + // newSurface will now be a child of mFGSurface so it will be 10, 10 offset from + // mFGSurface, putting it at 74, 74. + mCapture->expectFGColor(64, 64); + mCapture->checkPixel(74, 74, 63, 195, 63); + mCapture->expectFGColor(84, 84); + } +} + TEST_F(ChildLayerTest, NestedChildren) { sp<SurfaceControl> grandchild = mComposerClient->createSurface( String8("Grandchild surface"), @@ -1213,4 +1277,97 @@ TEST_F(ChildLayerTest, NestedChildren) { } } +class LayerColorTest : public LayerUpdateTest { + protected: + void SetUp() override { + LayerUpdateTest::SetUp(); + + mLayerColorControl = mComposerClient->createSurface( + String8("Layer color surface"), + 128, 128, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceColor); + + ASSERT_TRUE(mLayerColorControl != NULL); + ASSERT_TRUE(mLayerColorControl->isValid()); + + SurfaceComposerClient::openGlobalTransaction(); + ASSERT_EQ(NO_ERROR, mLayerColorControl->setLayer(INT32_MAX-1)); + ASSERT_EQ(NO_ERROR, mLayerColorControl->setPosition(140, 140)); + ASSERT_EQ(NO_ERROR, mLayerColorControl->hide()); + ASSERT_EQ(NO_ERROR, mFGSurfaceControl->hide()); + SurfaceComposerClient::closeGlobalTransaction(true); + } + + void TearDown() override { + LayerUpdateTest::TearDown(); + mLayerColorControl = 0; + } + + sp<SurfaceControl> mLayerColorControl; +}; + +TEST_F(LayerColorTest, ColorLayerNoAlpha) { + sp<ScreenCapture> sc; + + { + SCOPED_TRACE("before setColor"); + ScreenCapture::captureScreen(&sc); + sc->expectBGColor(145, 145); + } + + + SurfaceComposerClient::openGlobalTransaction(); + half3 color(43.0f/255.0f, 207.0f/255.0f, 131.0f/255.0f); + mLayerColorControl->setColor(color); + mLayerColorControl->show(); + SurfaceComposerClient::closeGlobalTransaction(true); + { + // There should now be a color + SCOPED_TRACE("after setColor"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel(145, 145, 43, 207, 131); + } +} + +TEST_F(LayerColorTest, ColorLayerWithAlpha) { + sp<ScreenCapture> sc; + { + SCOPED_TRACE("before setColor"); + ScreenCapture::captureScreen(&sc); + sc->expectBGColor(145, 145); + } + + SurfaceComposerClient::openGlobalTransaction(); + half3 color(43.0f/255.0f, 207.0f/255.0f, 131.0f/255.0f); + mLayerColorControl->setColor(color); + mLayerColorControl->setAlpha(.75f); + mLayerColorControl->show(); + SurfaceComposerClient::closeGlobalTransaction(true); + { + // There should now be a color with .75 alpha + SCOPED_TRACE("after setColor"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel(145, 145, 48, 171, 147); + } +} + +TEST_F(LayerColorTest, ColorLayerWithNoColor) { + sp<ScreenCapture> sc; + { + SCOPED_TRACE("before setColor"); + ScreenCapture::captureScreen(&sc); + sc->expectBGColor(145, 145); + } + + SurfaceComposerClient::openGlobalTransaction(); + mLayerColorControl->show(); + SurfaceComposerClient::closeGlobalTransaction(true); + { + // There should now be set to 0,0,0 (black) as default. + SCOPED_TRACE("after setColor"); + ScreenCapture::captureScreen(&sc); + sc->checkPixel(145, 145, 0, 0, 0); + } +} + } diff --git a/services/surfaceflinger/tests/hwc2/Android.mk b/services/surfaceflinger/tests/hwc2/Android.mk index 6d20349794..010ac9c890 100644 --- a/services/surfaceflinger/tests/hwc2/Android.mk +++ b/services/surfaceflinger/tests/hwc2/Android.mk @@ -37,7 +37,7 @@ LOCAL_SHARED_LIBRARIES := \ libgui \ liblog \ libsync \ - libskia \ + libhwui \ android.hardware.graphics.common@1.0 LOCAL_STATIC_LIBRARIES := \ libbase \ |