Merge "Ensure stale event does not cause ANR" into tm-dev
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 83e8e83..77bed0e 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -458,8 +458,8 @@
return res;
}
-static int prepare_app_dir(const std::string& path, mode_t target_mode, uid_t uid) {
- if (fs_prepare_dir_strict(path.c_str(), target_mode, uid, uid) != 0) {
+static int prepare_app_dir(const std::string& path, mode_t target_mode, uid_t uid, gid_t gid) {
+ if (fs_prepare_dir_strict(path.c_str(), target_mode, uid, gid) != 0) {
PLOG(ERROR) << "Failed to prepare " << path;
return -1;
}
@@ -599,9 +599,9 @@
}
}
-static binder::Status createAppDataDirs(const std::string& path,
- int32_t uid, int32_t* previousUid, int32_t cacheGid,
- const std::string& seInfo, mode_t targetMode) {
+static binder::Status createAppDataDirs(const std::string& path, int32_t uid, int32_t gid,
+ int32_t* previousUid, int32_t cacheGid,
+ const std::string& seInfo, mode_t targetMode) {
struct stat st{};
bool parent_dir_exists = (stat(path.c_str(), &st) == 0);
@@ -625,9 +625,9 @@
}
// Prepare only the parent app directory
- if (prepare_app_dir(path, targetMode, uid) ||
- prepare_app_cache_dir(path, "cache", 02771, uid, cacheGid) ||
- prepare_app_cache_dir(path, "code_cache", 02771, uid, cacheGid)) {
+ if (prepare_app_dir(path, targetMode, uid, gid) ||
+ prepare_app_cache_dir(path, "cache", 02771, uid, cacheGid) ||
+ prepare_app_cache_dir(path, "code_cache", 02771, uid, cacheGid)) {
return error("Failed to prepare " + path);
}
@@ -686,7 +686,7 @@
if (flags & FLAG_STORAGE_CE) {
auto path = create_data_user_ce_package_path(uuid_, userId, pkgname);
- auto status = createAppDataDirs(path, uid, &previousUid, cacheGid, seInfo, targetMode);
+ auto status = createAppDataDirs(path, uid, uid, &previousUid, cacheGid, seInfo, targetMode);
if (!status.isOk()) {
return status;
}
@@ -711,7 +711,7 @@
if (flags & FLAG_STORAGE_DE) {
auto path = create_data_user_de_package_path(uuid_, userId, pkgname);
- auto status = createAppDataDirs(path, uid, &previousUid, cacheGid, seInfo, targetMode);
+ auto status = createAppDataDirs(path, uid, uid, &previousUid, cacheGid, seInfo, targetMode);
if (!status.isOk()) {
return status;
}
@@ -734,7 +734,7 @@
} else {
// Package does not need sdk storage. Remove it.
- deleteSdkSandboxDataPackageDirectory(uuid, packageName, userId, flags);
+ destroySdkSandboxDataPackageDirectory(uuid, packageName, userId, flags);
}
return ok();
@@ -756,8 +756,7 @@
const char* uuid_ = uuid ? uuid->c_str() : nullptr;
constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
- for (int i = 0; i < 2; i++) {
- int currentFlag = storageFlags[i];
+ for (int currentFlag : storageFlags) {
if ((flags & currentFlag) == 0) {
continue;
}
@@ -773,7 +772,7 @@
LOG(DEBUG) << "Creating app-level sdk data directory: " << packagePath;
#endif
- if (prepare_app_dir(packagePath, 0751, AID_SYSTEM)) {
+ if (prepare_app_dir(packagePath, 0751, AID_SYSTEM, AID_SYSTEM)) {
return error("Failed to prepare " + packagePath);
}
@@ -787,8 +786,8 @@
return exception(binder::Status::EX_ILLEGAL_STATE,
StringPrintf("cacheGid cannot be -1 for sdksandbox data"));
}
- auto status = createAppDataDirs(sharedPath, sdkSandboxUid, &previousSdkSandboxUid, cacheGid,
- seInfo, 0700);
+ auto status = createAppDataDirs(sharedPath, sdkSandboxUid, AID_NOBODY,
+ &previousSdkSandboxUid, cacheGid, seInfo, 0700);
if (!status.isOk()) {
return status;
}
@@ -802,32 +801,6 @@
return ok();
}
-/**
- * Responsible for deleting /data/misc_{ce|de}/user/0/sdksandbox/<package-name> directory
- */
-binder::Status InstalldNativeService::deleteSdkSandboxDataPackageDirectory(
- const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
- int32_t flags) {
- const char* uuid_ = uuid ? uuid->c_str() : nullptr;
- auto res = ok();
-
- constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
- for (int currentFlag : storageFlags) {
- if ((flags & currentFlag) == 0) {
- continue;
- }
- bool isCeData = (currentFlag == FLAG_STORAGE_CE);
-
- const auto& packagePath = create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId,
- packageName.c_str());
- if (delete_dir_contents_and_dir(packagePath, /*ignore_if_missing=*/true) != 0) {
- res = error("Failed to delete sdk package directory: " + packagePath);
- }
- }
-
- return res;
-}
-
binder::Status InstalldNativeService::createAppData(
const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
int32_t flags, int32_t appId, int32_t previousAppId, const std::string& seInfo,
@@ -873,7 +846,6 @@
binder::Status InstalldNativeService::reconcileSdkData(
const android::os::ReconcileSdkDataArgs& args) {
- ENFORCE_UID(AID_SYSTEM);
// Locking is performed depeer in the callstack.
return reconcileSdkData(args.uuid, args.packageName, args.sdkPackageNames, args.randomSuffixes,
@@ -896,6 +868,7 @@
const std::vector<std::string>& sdkPackageNames,
const std::vector<std::string>& randomSuffixes, int userId, int appId, int previousAppId,
const std::string& seInfo, int flags) {
+ ENFORCE_UID(AID_SYSTEM);
CHECK_ARGUMENT_UUID(uuid);
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
for (const auto& sdkPackageName : sdkPackageNames) {
@@ -993,8 +966,8 @@
}
const int32_t sandboxUid = multiuser_get_sdk_sandbox_uid(userId, appId);
int32_t previousSandboxUid = multiuser_get_sdk_sandbox_uid(userId, previousAppId);
- auto status = createAppDataDirs(path, sandboxUid, &previousSandboxUid, cacheGid, seInfo,
- 0700);
+ auto status = createAppDataDirs(path, sandboxUid, AID_NOBODY, &previousSandboxUid,
+ cacheGid, seInfo, 0700);
if (!status.isOk()) {
res = status;
continue;
@@ -1150,6 +1123,47 @@
}
}
}
+ auto status = clearSdkSandboxDataPackageDirectory(uuid, packageName, userId, flags);
+ if (!status.isOk()) {
+ res = status;
+ }
+ return res;
+}
+
+binder::Status InstalldNativeService::clearSdkSandboxDataPackageDirectory(
+ const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
+ int32_t flags) {
+ const char* uuid_ = uuid ? uuid->c_str() : nullptr;
+ const char* pkgname = packageName.c_str();
+
+ binder::Status res = ok();
+ constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
+ for (int i = 0; i < 2; i++) {
+ int currentFlag = storageFlags[i];
+ if ((flags & currentFlag) == 0) {
+ continue;
+ }
+ bool isCeData = (currentFlag == FLAG_STORAGE_CE);
+ std::string suffix;
+ if (flags & FLAG_CLEAR_CACHE_ONLY) {
+ suffix = CACHE_DIR_POSTFIX;
+ } else if (flags & FLAG_CLEAR_CODE_CACHE_ONLY) {
+ suffix = CODE_CACHE_DIR_POSTFIX;
+ }
+
+ auto appPath = create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId, pkgname);
+ if (access(appPath.c_str(), F_OK) != 0) continue;
+ const auto subDirHandler = [&appPath, &res, &suffix](const std::string& filename) {
+ auto filepath = appPath + "/" + filename + suffix;
+ if (delete_dir_contents(filepath, true) != 0) {
+ res = error("Failed to clear contents of " + filepath);
+ }
+ };
+ const int ec = foreach_subdir(appPath, subDirHandler);
+ if (ec != 0) {
+ res = error("Failed to process subdirs for " + appPath);
+ }
+ }
return res;
}
@@ -1246,6 +1260,32 @@
}
}
}
+ auto status = destroySdkSandboxDataPackageDirectory(uuid, packageName, userId, flags);
+ if (!status.isOk()) {
+ res = status;
+ }
+ return res;
+}
+
+binder::Status InstalldNativeService::destroySdkSandboxDataPackageDirectory(
+ const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
+ int32_t flags) {
+ const char* uuid_ = uuid ? uuid->c_str() : nullptr;
+ const char* pkgname = packageName.c_str();
+
+ binder::Status res = ok();
+ constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
+ for (int i = 0; i < 2; i++) {
+ int currentFlag = storageFlags[i];
+ if ((flags & currentFlag) == 0) {
+ continue;
+ }
+ bool isCeData = (currentFlag == FLAG_STORAGE_CE);
+ auto appPath = create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId, pkgname);
+ if (rename_delete_dir_contents_and_dir(appPath) != 0) {
+ res = error("Failed to delete " + appPath);
+ }
+ }
return res;
}
@@ -1731,6 +1771,36 @@
}
}
+ // Copy sdk data for all known users
+ for (auto userId : users) {
+ LOCK_USER();
+
+ constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
+ for (int currentFlag : storageFlags) {
+ const bool isCeData = currentFlag == FLAG_STORAGE_CE;
+
+ const auto from = create_data_misc_sdk_sandbox_package_path(from_uuid, isCeData, userId,
+ package_name);
+ if (access(from.c_str(), F_OK) != 0) {
+ LOG(INFO) << "Missing source " << from;
+ continue;
+ }
+ const auto to = create_data_misc_sdk_sandbox_path(to_uuid, isCeData, userId);
+
+ const int rc = copy_directory_recursive(from.c_str(), to.c_str());
+ if (rc != 0) {
+ res = error(rc, "Failed copying " + from + " to " + to);
+ goto fail;
+ }
+ }
+
+ if (!restoreconSdkDataLocked(toUuid, packageName, userId, FLAG_STORAGE_CE | FLAG_STORAGE_DE,
+ appId, seInfo)
+ .isOk()) {
+ res = error("Failed to restorecon");
+ goto fail;
+ }
+ }
// We let the framework scan the new location and persist that before
// deleting the data in the old location; this ordering ensures that
// we can recover from things like battery pulls.
@@ -1758,6 +1828,18 @@
}
}
}
+ for (auto userId : users) {
+ LOCK_USER();
+ constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
+ for (int currentFlag : storageFlags) {
+ const bool isCeData = currentFlag == FLAG_STORAGE_CE;
+ const auto to = create_data_misc_sdk_sandbox_package_path(to_uuid, isCeData, userId,
+ package_name);
+ if (delete_dir_contents(to.c_str(), 1, nullptr) != 0) {
+ LOG(WARNING) << "Failed to rollback " << to;
+ }
+ }
+ }
return res;
}
@@ -1792,6 +1874,11 @@
if (delete_dir_contents_and_dir(path, true) != 0) {
res = error("Failed to delete " + path);
}
+ auto sdk_sandbox_de_path =
+ create_data_misc_sdk_sandbox_path(uuid_, /*isCeData=*/false, userId);
+ if (delete_dir_contents_and_dir(sdk_sandbox_de_path, true) != 0) {
+ res = error("Failed to delete " + sdk_sandbox_de_path);
+ }
if (uuid_ == nullptr) {
path = create_data_misc_legacy_path(userId);
if (delete_dir_contents_and_dir(path, true) != 0) {
@@ -1808,6 +1895,11 @@
if (delete_dir_contents_and_dir(path, true) != 0) {
res = error("Failed to delete " + path);
}
+ auto sdk_sandbox_ce_path =
+ create_data_misc_sdk_sandbox_path(uuid_, /*isCeData=*/true, userId);
+ if (delete_dir_contents_and_dir(sdk_sandbox_ce_path, true) != 0) {
+ res = error("Failed to delete " + sdk_sandbox_ce_path);
+ }
path = findDataMediaPath(uuid, userId);
if (delete_dir_contents_and_dir(path, true) != 0) {
res = error("Failed to delete " + path);
@@ -3090,6 +3182,49 @@
return res;
}
+binder::Status InstalldNativeService::restoreconSdkDataLocked(
+ const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId,
+ int32_t flags, int32_t appId, const std::string& seInfo) {
+ ENFORCE_UID(AID_SYSTEM);
+ CHECK_ARGUMENT_UUID(uuid);
+ CHECK_ARGUMENT_PACKAGE_NAME(packageName);
+
+ binder::Status res = ok();
+
+ // SELINUX_ANDROID_RESTORECON_DATADATA flag is set by libselinux. Not needed here.
+ unsigned int seflags = SELINUX_ANDROID_RESTORECON_RECURSE;
+ const char* uuid_ = uuid ? uuid->c_str() : nullptr;
+ const char* pkgName = packageName.c_str();
+ const char* seinfo = seInfo.c_str();
+
+ uid_t uid = multiuser_get_sdk_sandbox_uid(userId, appId);
+ constexpr int storageFlags[2] = {FLAG_STORAGE_CE, FLAG_STORAGE_DE};
+ for (int currentFlag : storageFlags) {
+ if ((flags & currentFlag) == 0) {
+ continue;
+ }
+ const bool isCeData = (currentFlag == FLAG_STORAGE_CE);
+ const auto packagePath =
+ create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId, pkgName);
+ if (access(packagePath.c_str(), F_OK) != 0) {
+ LOG(INFO) << "Missing source " << packagePath;
+ continue;
+ }
+ const auto subDirHandler = [&packagePath, &seinfo, &uid, &seflags,
+ &res](const std::string& subDir) {
+ const auto& fullpath = packagePath + "/" + subDir;
+ if (selinux_android_restorecon_pkgdir(fullpath.c_str(), seinfo, uid, seflags) < 0) {
+ res = error("restorecon failed for " + fullpath);
+ }
+ };
+ const auto ec = foreach_subdir(packagePath, subDirHandler);
+ if (ec != 0) {
+ res = error("Failed to restorecon for subdirs of " + packagePath);
+ }
+ }
+ return res;
+}
+
binder::Status InstalldNativeService::createOatDir(const std::string& packageName,
const std::string& oatDir,
const std::string& instructionSet) {
@@ -3439,11 +3574,17 @@
if (flags & FLAG_STORAGE_CE) {
auto ce_path = create_data_user_ce_path(uuid_cstr, userId);
cleanup_invalid_package_dirs_under_path(ce_path);
+ auto sdksandbox_ce_path =
+ create_data_misc_sdk_sandbox_path(uuid_cstr, /*isCeData=*/true, userId);
+ cleanup_invalid_package_dirs_under_path(sdksandbox_ce_path);
}
if (flags & FLAG_STORAGE_DE) {
auto de_path = create_data_user_de_path(uuid_cstr, userId);
cleanup_invalid_package_dirs_under_path(de_path);
+ auto sdksandbox_de_path =
+ create_data_misc_sdk_sandbox_path(uuid_cstr, /*isCeData=*/false, userId);
+ cleanup_invalid_package_dirs_under_path(sdksandbox_de_path);
}
return ok();
diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h
index ecc7f14..1f0fc9c 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -63,9 +63,7 @@
binder::Status restoreconAppData(const std::optional<std::string>& uuid,
const std::string& packageName, int32_t userId, int32_t flags, int32_t appId,
const std::string& seInfo);
- binder::Status restoreconAppDataLocked(const std::optional<std::string>& uuid,
- const std::string& packageName, int32_t userId,
- int32_t flags, int32_t appId, const std::string& seInfo);
+
binder::Status migrateAppData(const std::optional<std::string>& uuid,
const std::string& packageName, int32_t userId, int32_t flags);
binder::Status clearAppData(const std::optional<std::string>& uuid,
@@ -206,23 +204,30 @@
int32_t flags, int32_t appId, int32_t previousAppId,
const std::string& seInfo, int32_t targetSdkVersion,
int64_t* _aidl_return);
+ binder::Status restoreconAppDataLocked(const std::optional<std::string>& uuid,
+ const std::string& packageName, int32_t userId,
+ int32_t flags, int32_t appId, const std::string& seInfo);
binder::Status createSdkSandboxDataPackageDirectory(const std::optional<std::string>& uuid,
const std::string& packageName,
int32_t userId, int32_t appId,
int32_t previousAppId,
const std::string& seInfo, int32_t flags);
-
- binder::Status deleteSdkSandboxDataPackageDirectory(const std::optional<std::string>& uuid,
- const std::string& packageName,
- int32_t userId, int32_t flags);
-
+ binder::Status clearSdkSandboxDataPackageDirectory(const std::optional<std::string>& uuid,
+ const std::string& packageName,
+ int32_t userId, int32_t flags);
+ binder::Status destroySdkSandboxDataPackageDirectory(const std::optional<std::string>& uuid,
+ const std::string& packageName,
+ int32_t userId, int32_t flags);
binder::Status reconcileSdkData(const std::optional<std::string>& uuid,
const std::string& packageName,
const std::vector<std::string>& sdkPackageNames,
const std::vector<std::string>& randomSuffixes, int32_t userId,
int32_t appId, int32_t previousAppId, const std::string& seInfo,
int flags);
+ binder::Status restoreconSdkDataLocked(const std::optional<std::string>& uuid,
+ const std::string& packageName, int32_t userId,
+ int32_t flags, int32_t appId, const std::string& seInfo);
};
} // namespace installd
diff --git a/cmds/installd/tests/Android.bp b/cmds/installd/tests/Android.bp
index 2e63dd6..b3baca5 100644
--- a/cmds/installd/tests/Android.bp
+++ b/cmds/installd/tests/Android.bp
@@ -107,6 +107,7 @@
"libziparchive",
"liblog",
"liblogwrap",
+ "libc++fs",
],
test_config: "installd_service_test.xml",
diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp
index 30a23eb..04558d5 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -32,6 +32,8 @@
#include <android-base/stringprintf.h>
#include <cutils/properties.h>
#include <gtest/gtest.h>
+#include <filesystem>
+#include <fstream>
#include <android/content/pm/IPackageManagerNative.h>
#include <binder/IServiceManager.h>
@@ -43,6 +45,7 @@
#include "utils.h"
using android::base::StringPrintf;
+namespace fs = std::filesystem;
namespace android {
std::string get_package_name(uid_t uid) {
@@ -76,13 +79,15 @@
namespace installd {
static constexpr const char* kTestUuid = "TEST";
-static constexpr const char* kTestPath = "/data/local/tmp/user/0";
+static constexpr const char* kTestPath = "/data/local/tmp";
+static constexpr const uid_t kNobodyUid = 9999;
static constexpr const uid_t kSystemUid = 1000;
static constexpr const int32_t kTestUserId = 0;
static constexpr const uid_t kTestAppId = 19999;
static constexpr const int FLAG_STORAGE_SDK = InstalldNativeService::FLAG_STORAGE_SDK;
const gid_t kTestAppUid = multiuser_get_uid(kTestUserId, kTestAppId);
+const gid_t kTestCacheGid = multiuser_get_cache_gid(kTestUserId, kTestAppId);
const uid_t kTestSdkSandboxUid = multiuser_get_sdk_sandbox_uid(kTestUserId, kTestAppId);
#define FLAG_FORCE InstalldNativeService::FLAG_FORCE
@@ -105,18 +110,18 @@
return create_cache_path_default(path, src, instruction_set);
}
-static std::string get_full_path(const char* path) {
- return StringPrintf("%s/%s", kTestPath, path);
+static std::string get_full_path(const std::string& path) {
+ return StringPrintf("%s/%s", kTestPath, path.c_str());
}
-static void mkdir(const char* path, uid_t owner, gid_t group, mode_t mode) {
+static void mkdir(const std::string& path, uid_t owner, gid_t group, mode_t mode) {
const std::string fullPath = get_full_path(path);
EXPECT_EQ(::mkdir(fullPath.c_str(), mode), 0);
EXPECT_EQ(::chown(fullPath.c_str(), owner, group), 0);
EXPECT_EQ(::chmod(fullPath.c_str(), mode), 0);
}
-static int create(const char* path, uid_t owner, gid_t group, mode_t mode) {
+static int create(const std::string& path, uid_t owner, gid_t group, mode_t mode) {
int fd = ::open(get_full_path(path).c_str(), O_RDWR | O_CREAT, mode);
EXPECT_NE(fd, -1);
EXPECT_EQ(::fchown(fd, owner, group), 0);
@@ -124,8 +129,8 @@
return fd;
}
-static void touch(const char* path, uid_t owner, gid_t group, mode_t mode) {
- EXPECT_EQ(::close(create(path, owner, group, mode)), 0);
+static void touch(const std::string& path, uid_t owner, gid_t group, mode_t mode) {
+ EXPECT_EQ(::close(create(path.c_str(), owner, group, mode)), 0);
}
static int stat_gid(const char* path) {
@@ -140,7 +145,7 @@
return buf.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO | S_ISGID);
}
-static bool exists(const char* path) {
+static bool exists(const std::string& path) {
return ::access(get_full_path(path).c_str(), F_OK) == 0;
}
@@ -163,10 +168,11 @@
return result;
}
-static bool exists_renamed_deleted_dir() {
- return find_file(kTestPath, [](const std::string& name, bool is_dir) {
- return is_dir && is_renamed_deleted_dir(name);
- });
+static bool exists_renamed_deleted_dir(const std::string& rootDirectory) {
+ return find_file((std::string(kTestPath) + rootDirectory).c_str(),
+ [](const std::string& name, bool is_dir) {
+ return is_dir && is_renamed_deleted_dir(name);
+ });
}
class ServiceTest : public testing::Test {
@@ -181,197 +187,205 @@
service = new InstalldNativeService();
testUuid = kTestUuid;
system("rm -rf /data/local/tmp/user");
+ system("rm -rf /data/local/tmp/misc_ce");
+ system("rm -rf /data/local/tmp/misc_de");
system("mkdir -p /data/local/tmp/user/0");
-
+ system("mkdir -p /data/local/tmp/misc_ce/0/sdksandbox");
+ system("mkdir -p /data/local/tmp/misc_de/0/sdksandbox");
init_globals_from_data_and_root();
}
virtual void TearDown() {
delete service;
system("rm -rf /data/local/tmp/user");
+ system("rm -rf /data/local/tmp/misc_ce");
+ system("rm -rf /data/local/tmp/misc_de");
}
};
TEST_F(ServiceTest, FixupAppData_Upgrade) {
LOG(INFO) << "FixupAppData_Upgrade";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/normal", 10000, 10000, 0700);
- mkdir("com.example/cache", 10000, 10000, 0700);
- touch("com.example/cache/file", 10000, 10000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/normal", 10000, 10000, 0700);
+ mkdir("user/0/com.example/cache", 10000, 10000, 0700);
+ touch("user/0/com.example/cache/file", 10000, 10000, 0700);
service->fixupAppData(testUuid, 0);
- EXPECT_EQ(10000, stat_gid("com.example/normal"));
- EXPECT_EQ(20000, stat_gid("com.example/cache"));
- EXPECT_EQ(20000, stat_gid("com.example/cache/file"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/normal"));
+ EXPECT_EQ(20000, stat_gid("user/0/com.example/cache"));
+ EXPECT_EQ(20000, stat_gid("user/0/com.example/cache/file"));
- EXPECT_EQ(0700, stat_mode("com.example/normal"));
- EXPECT_EQ(02771, stat_mode("com.example/cache"));
- EXPECT_EQ(0700, stat_mode("com.example/cache/file"));
+ EXPECT_EQ(0700, stat_mode("user/0/com.example/normal"));
+ EXPECT_EQ(02771, stat_mode("user/0/com.example/cache"));
+ EXPECT_EQ(0700, stat_mode("user/0/com.example/cache/file"));
}
TEST_F(ServiceTest, FixupAppData_Moved) {
LOG(INFO) << "FixupAppData_Moved";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
- mkdir("com.example/bar", 10000, 20000, 0700);
- touch("com.example/bar/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example/bar", 10000, 20000, 0700);
+ touch("user/0/com.example/bar/file", 10000, 20000, 0700);
service->fixupAppData(testUuid, 0);
- EXPECT_EQ(10000, stat_gid("com.example/foo"));
- EXPECT_EQ(20000, stat_gid("com.example/foo/file"));
- EXPECT_EQ(10000, stat_gid("com.example/bar"));
- EXPECT_EQ(10000, stat_gid("com.example/bar/file"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/foo"));
+ EXPECT_EQ(20000, stat_gid("user/0/com.example/foo/file"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/bar"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/bar/file"));
service->fixupAppData(testUuid, FLAG_FORCE);
- EXPECT_EQ(10000, stat_gid("com.example/foo"));
- EXPECT_EQ(10000, stat_gid("com.example/foo/file"));
- EXPECT_EQ(10000, stat_gid("com.example/bar"));
- EXPECT_EQ(10000, stat_gid("com.example/bar/file"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/foo"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/foo/file"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/bar"));
+ EXPECT_EQ(10000, stat_gid("user/0/com.example/bar/file"));
}
TEST_F(ServiceTest, DestroyUserData) {
LOG(INFO) << "DestroyUserData";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
- mkdir("com.example/bar", 10000, 20000, 0700);
- touch("com.example/bar/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example/bar", 10000, 20000, 0700);
+ touch("user/0/com.example/bar/file", 10000, 20000, 0700);
- EXPECT_TRUE(exists("com.example/foo"));
- EXPECT_TRUE(exists("com.example/foo/file"));
- EXPECT_TRUE(exists("com.example/bar"));
- EXPECT_TRUE(exists("com.example/bar/file"));
+ EXPECT_TRUE(exists("user/0/com.example/foo"));
+ EXPECT_TRUE(exists("user/0/com.example/foo/file"));
+ EXPECT_TRUE(exists("user/0/com.example/bar"));
+ EXPECT_TRUE(exists("user/0/com.example/bar/file"));
service->destroyUserData(testUuid, 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE);
- EXPECT_FALSE(exists("com.example/foo"));
- EXPECT_FALSE(exists("com.example/foo/file"));
- EXPECT_FALSE(exists("com.example/bar"));
- EXPECT_FALSE(exists("com.example/bar/file"));
+ EXPECT_FALSE(exists("user/0/com.example/foo"));
+ EXPECT_FALSE(exists("user/0/com.example/foo/file"));
+ EXPECT_FALSE(exists("user/0/com.example/bar"));
+ EXPECT_FALSE(exists("user/0/com.example/bar/file"));
- EXPECT_FALSE(exists_renamed_deleted_dir());
+ EXPECT_FALSE(exists_renamed_deleted_dir("/user/0"));
}
TEST_F(ServiceTest, DestroyAppData) {
LOG(INFO) << "DestroyAppData";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
- mkdir("com.example/bar", 10000, 20000, 0700);
- touch("com.example/bar/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example/bar", 10000, 20000, 0700);
+ touch("user/0/com.example/bar/file", 10000, 20000, 0700);
- EXPECT_TRUE(exists("com.example/foo"));
- EXPECT_TRUE(exists("com.example/foo/file"));
- EXPECT_TRUE(exists("com.example/bar"));
- EXPECT_TRUE(exists("com.example/bar/file"));
+ EXPECT_TRUE(exists("user/0/com.example/foo"));
+ EXPECT_TRUE(exists("user/0/com.example/foo/file"));
+ EXPECT_TRUE(exists("user/0/com.example/bar"));
+ EXPECT_TRUE(exists("user/0/com.example/bar/file"));
service->destroyAppData(testUuid, "com.example", 0, FLAG_STORAGE_DE | FLAG_STORAGE_CE, 0);
- EXPECT_FALSE(exists("com.example/foo"));
- EXPECT_FALSE(exists("com.example/foo/file"));
- EXPECT_FALSE(exists("com.example/bar"));
- EXPECT_FALSE(exists("com.example/bar/file"));
+ EXPECT_FALSE(exists("user/0/com.example/foo"));
+ EXPECT_FALSE(exists("user/0/com.example/foo/file"));
+ EXPECT_FALSE(exists("user/0/com.example/bar"));
+ EXPECT_FALSE(exists("user/0/com.example/bar/file"));
- EXPECT_FALSE(exists_renamed_deleted_dir());
+ EXPECT_FALSE(exists_renamed_deleted_dir("/user/0"));
}
TEST_F(ServiceTest, CleanupInvalidPackageDirs) {
LOG(INFO) << "CleanupInvalidPackageDirs";
- mkdir("5b14b6458a44==deleted==", 10000, 10000, 0700);
- mkdir("5b14b6458a44==deleted==/foo", 10000, 10000, 0700);
- touch("5b14b6458a44==deleted==/foo/file", 10000, 20000, 0700);
- mkdir("5b14b6458a44==deleted==/bar", 10000, 20000, 0700);
- touch("5b14b6458a44==deleted==/bar/file", 10000, 20000, 0700);
+ std::string rootDirectoryPrefix[] = {"user/0", "misc_ce/0/sdksandbox", "misc_de/0/sdksandbox"};
+ for (auto& prefix : rootDirectoryPrefix) {
+ mkdir(prefix + "/5b14b6458a44==deleted==", 10000, 10000, 0700);
+ mkdir(prefix + "/5b14b6458a44==deleted==/foo", 10000, 10000, 0700);
+ touch(prefix + "/5b14b6458a44==deleted==/foo/file", 10000, 20000, 0700);
+ mkdir(prefix + "/5b14b6458a44==deleted==/bar", 10000, 20000, 0700);
+ touch(prefix + "/5b14b6458a44==deleted==/bar/file", 10000, 20000, 0700);
- auto fd = create("5b14b6458a44==deleted==/bar/opened_file", 10000, 20000, 0700);
+ auto fd = create(prefix + "/5b14b6458a44==deleted==/bar/opened_file", 10000, 20000, 0700);
- mkdir("b14b6458a44NOTdeleted", 10000, 10000, 0700);
- mkdir("b14b6458a44NOTdeleted/foo", 10000, 10000, 0700);
- touch("b14b6458a44NOTdeleted/foo/file", 10000, 20000, 0700);
- mkdir("b14b6458a44NOTdeleted/bar", 10000, 20000, 0700);
- touch("b14b6458a44NOTdeleted/bar/file", 10000, 20000, 0700);
+ mkdir(prefix + "/b14b6458a44NOTdeleted", 10000, 10000, 0700);
+ mkdir(prefix + "/b14b6458a44NOTdeleted/foo", 10000, 10000, 0700);
+ touch(prefix + "/b14b6458a44NOTdeleted/foo/file", 10000, 20000, 0700);
+ mkdir(prefix + "/b14b6458a44NOTdeleted/bar", 10000, 20000, 0700);
+ touch(prefix + "/b14b6458a44NOTdeleted/bar/file", 10000, 20000, 0700);
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
- mkdir("com.example/bar", 10000, 20000, 0700);
- touch("com.example/bar/file", 10000, 20000, 0700);
+ mkdir(prefix + "/com.example", 10000, 10000, 0700);
+ mkdir(prefix + "/com.example/foo", 10000, 10000, 0700);
+ touch(prefix + "/com.example/foo/file", 10000, 20000, 0700);
+ mkdir(prefix + "/com.example/bar", 10000, 20000, 0700);
+ touch(prefix + "/com.example/bar/file", 10000, 20000, 0700);
- mkdir("==deleted==", 10000, 10000, 0700);
- mkdir("==deleted==/foo", 10000, 10000, 0700);
- touch("==deleted==/foo/file", 10000, 20000, 0700);
- mkdir("==deleted==/bar", 10000, 20000, 0700);
- touch("==deleted==/bar/file", 10000, 20000, 0700);
+ mkdir(prefix + "/==deleted==", 10000, 10000, 0700);
+ mkdir(prefix + "/==deleted==/foo", 10000, 10000, 0700);
+ touch(prefix + "/==deleted==/foo/file", 10000, 20000, 0700);
+ mkdir(prefix + "/==deleted==/bar", 10000, 20000, 0700);
+ touch(prefix + "/==deleted==/bar/file", 10000, 20000, 0700);
- EXPECT_TRUE(exists("5b14b6458a44==deleted==/foo"));
- EXPECT_TRUE(exists("5b14b6458a44==deleted==/foo/file"));
- EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar"));
- EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar/file"));
- EXPECT_TRUE(exists("5b14b6458a44==deleted==/bar/opened_file"));
+ EXPECT_TRUE(exists(prefix + "/5b14b6458a44==deleted==/foo"));
+ EXPECT_TRUE(exists(prefix + "/5b14b6458a44==deleted==/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/5b14b6458a44==deleted==/bar"));
+ EXPECT_TRUE(exists(prefix + "/5b14b6458a44==deleted==/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/5b14b6458a44==deleted==/bar/opened_file"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo/file"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/foo"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/bar"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/bar/file"));
- EXPECT_TRUE(exists("com.example/foo"));
- EXPECT_TRUE(exists("com.example/foo/file"));
- EXPECT_TRUE(exists("com.example/bar"));
- EXPECT_TRUE(exists("com.example/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/com.example/foo"));
+ EXPECT_TRUE(exists(prefix + "/com.example/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/com.example/bar"));
+ EXPECT_TRUE(exists(prefix + "/com.example/bar/file"));
- EXPECT_TRUE(exists("==deleted==/foo"));
- EXPECT_TRUE(exists("==deleted==/foo/file"));
- EXPECT_TRUE(exists("==deleted==/bar"));
- EXPECT_TRUE(exists("==deleted==/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/==deleted==/foo"));
+ EXPECT_TRUE(exists(prefix + "/==deleted==/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/==deleted==/bar"));
+ EXPECT_TRUE(exists(prefix + "/==deleted==/bar/file"));
- EXPECT_TRUE(exists_renamed_deleted_dir());
+ EXPECT_TRUE(exists_renamed_deleted_dir("/" + prefix));
- service->cleanupInvalidPackageDirs(testUuid, 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE);
+ service->cleanupInvalidPackageDirs(testUuid, 0, FLAG_STORAGE_CE | FLAG_STORAGE_DE);
- EXPECT_EQ(::close(fd), 0);
+ EXPECT_EQ(::close(fd), 0);
- EXPECT_FALSE(exists("5b14b6458a44==deleted==/foo"));
- EXPECT_FALSE(exists("5b14b6458a44==deleted==/foo/file"));
- EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar"));
- EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar/file"));
- EXPECT_FALSE(exists("5b14b6458a44==deleted==/bar/opened_file"));
+ EXPECT_FALSE(exists(prefix + "/5b14b6458a44==deleted==/foo"));
+ EXPECT_FALSE(exists(prefix + "/5b14b6458a44==deleted==/foo/file"));
+ EXPECT_FALSE(exists(prefix + "/5b14b6458a44==deleted==/bar"));
+ EXPECT_FALSE(exists(prefix + "/5b14b6458a44==deleted==/bar/file"));
+ EXPECT_FALSE(exists(prefix + "/5b14b6458a44==deleted==/bar/opened_file"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/foo/file"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar"));
- EXPECT_TRUE(exists("b14b6458a44NOTdeleted/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/foo"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/bar"));
+ EXPECT_TRUE(exists(prefix + "/b14b6458a44NOTdeleted/bar/file"));
- EXPECT_TRUE(exists("com.example/foo"));
- EXPECT_TRUE(exists("com.example/foo/file"));
- EXPECT_TRUE(exists("com.example/bar"));
- EXPECT_TRUE(exists("com.example/bar/file"));
+ EXPECT_TRUE(exists(prefix + "/com.example/foo"));
+ EXPECT_TRUE(exists(prefix + "/com.example/foo/file"));
+ EXPECT_TRUE(exists(prefix + "/com.example/bar"));
+ EXPECT_TRUE(exists(prefix + "/com.example/bar/file"));
- EXPECT_FALSE(exists("==deleted==/foo"));
- EXPECT_FALSE(exists("==deleted==/foo/file"));
- EXPECT_FALSE(exists("==deleted==/bar"));
- EXPECT_FALSE(exists("==deleted==/bar/file"));
+ EXPECT_FALSE(exists(prefix + "/==deleted==/foo"));
+ EXPECT_FALSE(exists(prefix + "/==deleted==/foo/file"));
+ EXPECT_FALSE(exists(prefix + "/==deleted==/bar"));
+ EXPECT_FALSE(exists(prefix + "/==deleted==/bar/file"));
- EXPECT_FALSE(exists_renamed_deleted_dir());
+ EXPECT_FALSE(exists_renamed_deleted_dir(prefix));
+ }
}
TEST_F(ServiceTest, HashSecondaryDex) {
LOG(INFO) << "HashSecondaryDex";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0700);
std::vector<uint8_t> result;
- std::string dexPath = get_full_path("com.example/foo/file");
+ std::string dexPath = get_full_path("user/0/com.example/foo/file");
EXPECT_BINDER_SUCCESS(service->hashSecondaryDexFile(
dexPath, "com.example", 10000, testUuid, FLAG_STORAGE_CE, &result));
@@ -391,7 +405,7 @@
LOG(INFO) << "HashSecondaryDex_NoSuch";
std::vector<uint8_t> result;
- std::string dexPath = get_full_path("com.example/foo/file");
+ std::string dexPath = get_full_path("user/0/com.example/foo/file");
EXPECT_BINDER_SUCCESS(service->hashSecondaryDexFile(
dexPath, "com.example", 10000, testUuid, FLAG_STORAGE_CE, &result));
@@ -401,12 +415,12 @@
TEST_F(ServiceTest, HashSecondaryDex_Unreadable) {
LOG(INFO) << "HashSecondaryDex_Unreadable";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0300);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0300);
std::vector<uint8_t> result;
- std::string dexPath = get_full_path("com.example/foo/file");
+ std::string dexPath = get_full_path("user/0/com.example/foo/file");
EXPECT_BINDER_SUCCESS(service->hashSecondaryDexFile(
dexPath, "com.example", 10000, testUuid, FLAG_STORAGE_CE, &result));
@@ -416,12 +430,12 @@
TEST_F(ServiceTest, HashSecondaryDex_WrongApp) {
LOG(INFO) << "HashSecondaryDex_WrongApp";
- mkdir("com.example", 10000, 10000, 0700);
- mkdir("com.example/foo", 10000, 10000, 0700);
- touch("com.example/foo/file", 10000, 20000, 0700);
+ mkdir("user/0/com.example", 10000, 10000, 0700);
+ mkdir("user/0/com.example/foo", 10000, 10000, 0700);
+ touch("user/0/com.example/foo/file", 10000, 20000, 0700);
std::vector<uint8_t> result;
- std::string dexPath = get_full_path("com.example/foo/file");
+ std::string dexPath = get_full_path("user/0/com.example/foo/file");
EXPECT_BINDER_FAIL(service->hashSecondaryDexFile(
dexPath, "com.wrong", 10000, testUuid, FLAG_STORAGE_CE, &result));
}
@@ -953,20 +967,20 @@
class SdkSandboxDataTest : public testing::Test {
public:
- void CheckFileAccess(const std::string& path, uid_t uid, mode_t mode) {
+ void CheckFileAccess(const std::string& path, uid_t uid, gid_t gid, mode_t mode) {
const auto fullPath = "/data/local/tmp/" + path;
ASSERT_TRUE(exists(fullPath.c_str())) << "For path: " << fullPath;
struct stat st;
ASSERT_EQ(0, stat(fullPath.c_str(), &st));
ASSERT_EQ(uid, st.st_uid) << "For path: " << fullPath;
- ASSERT_EQ(uid, st.st_gid) << "For path: " << fullPath;
+ ASSERT_EQ(gid, st.st_gid) << "For path: " << fullPath;
ASSERT_EQ(mode, st.st_mode) << "For path: " << fullPath;
}
bool exists(const char* path) { return ::access(path, F_OK) == 0; }
// Creates a default CreateAppDataArgs object
- android::os::CreateAppDataArgs createAppDataArgs(std::string packageName) {
+ android::os::CreateAppDataArgs createAppDataArgs(const std::string& packageName) {
android::os::CreateAppDataArgs args;
args.uuid = kTestUuid;
args.packageName = packageName;
@@ -1028,25 +1042,28 @@
}
};
-TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData) {
+TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData) {
android::os::CreateAppDataResult result;
android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
- args.flags = FLAG_STORAGE_CE | FLAG_STORAGE_DE | FLAG_STORAGE_SDK;
// Create the app user data.
ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
const std::string fooCePath = "misc_ce/0/sdksandbox/com.foo";
- CheckFileAccess(fooCePath, kSystemUid, S_IFDIR | 0751);
- CheckFileAccess(fooCePath + "/shared", kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(fooCePath + "/shared/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(fooCePath + "/shared/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(fooCePath, kSystemUid, kSystemUid, S_IFDIR | 0751);
+ CheckFileAccess(fooCePath + "/shared", kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(fooCePath + "/shared/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(fooCePath + "/shared/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
const std::string fooDePath = "misc_de/0/sdksandbox/com.foo";
- CheckFileAccess(fooDePath, kSystemUid, S_IFDIR | 0751);
- CheckFileAccess(fooDePath + "/shared", kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(fooDePath + "/shared/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(fooDePath + "/shared/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(fooDePath, kSystemUid, kSystemUid, S_IFDIR | 0751);
+ CheckFileAccess(fooDePath + "/shared", kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(fooDePath + "/shared/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(fooDePath + "/shared/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
}
TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutSdkFlag) {
@@ -1064,7 +1081,6 @@
TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutSdkFlagDeletesExisting) {
android::os::CreateAppDataResult result;
android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
- args.flags = FLAG_STORAGE_CE | FLAG_STORAGE_DE | FLAG_STORAGE_SDK;
// Create the app user data.
ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
ASSERT_TRUE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo"));
@@ -1085,7 +1101,7 @@
ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
// Only CE paths should exist
- CheckFileAccess("misc_ce/0/sdksandbox/com.foo", kSystemUid, S_IFDIR | 0751);
+ CheckFileAccess("misc_ce/0/sdksandbox/com.foo", kSystemUid, kSystemUid, S_IFDIR | 0751);
// DE paths should not exist
ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo"));
@@ -1103,7 +1119,7 @@
ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo"));
// Only DE paths should exist
- CheckFileAccess("misc_de/0/sdksandbox/com.foo", kSystemUid, S_IFDIR | 0751);
+ CheckFileAccess("misc_de/0/sdksandbox/com.foo", kSystemUid, kSystemUid, S_IFDIR | 0751);
}
TEST_F(SdkSandboxDataTest, ReconcileSdkData) {
@@ -1114,24 +1130,32 @@
ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args));
const std::string barCePath = "misc_ce/0/sdksandbox/com.foo/bar@random1";
- CheckFileAccess(barCePath, kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(barCePath + "/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(barCePath + "/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(barCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(barCePath + "/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(barCePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
const std::string bazCePath = "misc_ce/0/sdksandbox/com.foo/baz@random2";
- CheckFileAccess(bazCePath, kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(bazCePath + "/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(bazCePath + "/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(bazCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(bazCePath + "/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(bazCePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
const std::string barDePath = "misc_de/0/sdksandbox/com.foo/bar@random1";
- CheckFileAccess(barDePath, kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(barDePath + "/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(barDePath + "/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(barDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(barDePath + "/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(barDePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
const std::string bazDePath = "misc_de/0/sdksandbox/com.foo/baz@random2";
- CheckFileAccess(bazDePath, kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess(bazDePath + "/cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
- CheckFileAccess(bazDePath + "/code_cache", kTestSdkSandboxUid, S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(bazDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700);
+ CheckFileAccess(bazDePath + "/cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
+ CheckFileAccess(bazDePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid,
+ S_IFDIR | S_ISGID | 0771);
}
TEST_F(SdkSandboxDataTest, ReconcileSdkData_PackageNameCannotUseRandomSuffixSeparator) {
@@ -1169,8 +1193,10 @@
ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args));
// Previous directories from first attempt should exist
- CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar@random1", kTestSdkSandboxUid, S_IFDIR | 0700);
- CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, S_IFDIR | 0700);
+ CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar@random1", kTestSdkSandboxUid, kNobodyUid,
+ S_IFDIR | 0700);
+ CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, kNobodyUid,
+ S_IFDIR | 0700);
// No new directories should be created on second attempt
ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/bar@r10"));
ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo/bar@r20"));
@@ -1190,12 +1216,211 @@
ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args));
// New directoris should exist
- CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar.diff@random1", kTestSdkSandboxUid,
+ CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar.diff@random1", kTestSdkSandboxUid, kNobodyUid,
S_IFDIR | 0700);
- CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, S_IFDIR | 0700);
+ CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, kNobodyUid,
+ S_IFDIR | 0700);
// Directory for old unreferred sdksandbox package name should be removed
ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/bar@random1"));
}
+class DestroyAppDataTest : public SdkSandboxDataTest {};
+
+TEST_F(DestroyAppDataTest, DestroySdkSandboxDataDirectories_WithCeAndDeFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ // Destroy the app user data.
+ ASSERT_BINDER_SUCCESS(service->destroyAppData(args.uuid, args.packageName, args.userId,
+ args.flags, result.ceDataInode));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo"));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo"));
+}
+
+TEST_F(DestroyAppDataTest, DestroySdkSandboxDataDirectories_WithoutDeFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ // Destroy the app user data.
+ ASSERT_BINDER_SUCCESS(service->destroyAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_CE, result.ceDataInode));
+ ASSERT_TRUE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo"));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo"));
+}
+
+TEST_F(DestroyAppDataTest, DestroySdkSandboxDataDirectories_WithoutCeFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ // Destroy the app user data.
+ ASSERT_BINDER_SUCCESS(service->destroyAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_DE, result.ceDataInode));
+ ASSERT_TRUE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo"));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo"));
+}
+
+class ClearAppDataTest : public SdkSandboxDataTest {
+public:
+ void createTestSdkData(const std::string& packageName, std::vector<std::string> sdkNames) {
+ const auto& cePackagePath = "/data/local/tmp/misc_ce/0/sdksandbox/" + packageName;
+ const auto& dePackagePath = "/data/local/tmp/misc_de/0/sdksandbox/" + packageName;
+ ASSERT_TRUE(mkdirs(cePackagePath + "/shared/cache", 0700));
+ ASSERT_TRUE(mkdirs(cePackagePath + "shared/code_cache", 0700));
+ ASSERT_TRUE(mkdirs(dePackagePath + "/shared/cache", 0700));
+ ASSERT_TRUE(mkdirs(dePackagePath + "/shared/code_cache", 0700));
+ std::ofstream{cePackagePath + "/shared/cache/cachedTestData.txt"};
+ for (auto sdkName : sdkNames) {
+ ASSERT_TRUE(mkdirs(cePackagePath + "/" + sdkName + "/cache", 0700));
+ ASSERT_TRUE(mkdirs(dePackagePath + "/" + sdkName + "/cache", 0700));
+ ASSERT_TRUE(mkdirs(cePackagePath + "/" + sdkName + "/code_cache", 0700));
+ ASSERT_TRUE(mkdirs(dePackagePath + "/" + sdkName + "/code_cache", 0700));
+ std::ofstream{cePackagePath + "/" + sdkName + "/cache/cachedTestData.txt"};
+ std::ofstream{cePackagePath + "/" + sdkName + "/code_cache/cachedTestData.txt"};
+ std::ofstream{dePackagePath + "/" + sdkName + "/cache/cachedTestData.txt"};
+ std::ofstream{dePackagePath + "/" + sdkName + "/code_cache/cachedTestData.txt"};
+ }
+ }
+};
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithCeAndClearCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data.
+ ASSERT_BINDER_SUCCESS(
+ service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_CE | (InstalldNativeService::FLAG_CLEAR_CACHE_ONLY),
+ result.ceDataInode));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/shared/cache")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk1/cache")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk2/cache")));
+}
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithCeAndClearCodeCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data.
+ ASSERT_BINDER_SUCCESS(
+ service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_CE |
+ (InstalldNativeService::FLAG_CLEAR_CODE_CACHE_ONLY),
+ result.ceDataInode));
+ ASSERT_TRUE(fs::is_empty(
+ fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/shared/code_cache")));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk1/code_cache")));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk2/code_cache")));
+}
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithDeAndClearCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data
+ ASSERT_BINDER_SUCCESS(
+ service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_DE | (InstalldNativeService::FLAG_CLEAR_CACHE_ONLY),
+ result.ceDataInode));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/shared/cache")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk1/cache")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk2/cache")));
+}
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithDeAndClearCodeCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data.
+ ASSERT_BINDER_SUCCESS(
+ service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_DE |
+ (InstalldNativeService::FLAG_CLEAR_CODE_CACHE_ONLY),
+ result.ceDataInode));
+ ASSERT_TRUE(fs::is_empty(
+ fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/shared/code_cache")));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk1/code_cache")));
+ ASSERT_TRUE(
+ fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk2/code_cache")));
+}
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithCeAndWithoutAnyCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data.
+ ASSERT_BINDER_SUCCESS(service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_CE, result.ceDataInode));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/shared")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk1")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/sdk2")));
+}
+
+TEST_F(ClearAppDataTest, ClearSdkSandboxDataDirectories_WithDeAndWithoutAnyCacheFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ createTestSdkData("com.foo", {"sdk1", "sdk2"});
+ // Clear the app user data.
+ ASSERT_BINDER_SUCCESS(service->clearAppData(args.uuid, args.packageName, args.userId,
+ FLAG_STORAGE_DE, result.ceDataInode));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/shared")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk1")));
+ ASSERT_TRUE(fs::is_empty(fs::path("/data/local/tmp/misc_de/0/sdksandbox/com.foo/sdk2")));
+}
+
+class DestroyUserDataTest : public SdkSandboxDataTest {};
+
+TEST_F(DestroyUserDataTest, DestroySdkData_WithCeFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ // Destroy user data
+ ASSERT_BINDER_SUCCESS(service->destroyUserData(args.uuid, args.userId, FLAG_STORAGE_CE));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox"));
+ ASSERT_TRUE(exists("/data/local/tmp/misc_de/0/sdksandbox"));
+}
+
+TEST_F(DestroyUserDataTest, DestroySdkData_WithDeFlag) {
+ android::os::CreateAppDataResult result;
+ android::os::CreateAppDataArgs args = createAppDataArgs("com.foo");
+ args.packageName = "com.foo";
+ // Create the app user data.
+ ASSERT_BINDER_SUCCESS(service->createAppData(args, &result));
+ // Destroy user data
+ ASSERT_BINDER_SUCCESS(service->destroyUserData(args.uuid, args.userId, FLAG_STORAGE_DE));
+ ASSERT_TRUE(exists("/data/local/tmp/misc_ce/0/sdksandbox"));
+ ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox"));
+}
+
} // namespace installd
} // namespace android
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 7448308..63d87da 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -166,6 +166,7 @@
"-Wextra-semi",
"-Werror",
"-Wzero-as-null-pointer-constant",
+ "-Wreorder-init-list",
"-DANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION",
"-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION",
],
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 0970ca5..01b25d3 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -19,6 +19,7 @@
#include <atomic>
#include <set>
+#include <android-base/logging.h>
#include <android-base/unique_fd.h>
#include <binder/BpBinder.h>
#include <binder/IInterface.h>
@@ -281,9 +282,11 @@
err = pingBinder();
break;
case EXTENSION_TRANSACTION:
+ CHECK(reply != nullptr);
err = reply->writeStrongBinder(getExtension());
break;
case DEBUG_PID_TRANSACTION:
+ CHECK(reply != nullptr);
err = reply->writeInt32(getDebugPid());
break;
case SET_RPC_CLIENT_TRANSACTION: {
@@ -590,6 +593,7 @@
{
switch (code) {
case INTERFACE_TRANSACTION:
+ CHECK(reply != nullptr);
reply->writeString16(getInterfaceDescriptor());
return NO_ERROR;
diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 056ef0a..921e57c 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -72,29 +72,29 @@
e.cleanupCookie = cleanupCookie;
e.func = func;
- if (ssize_t idx = mObjects.indexOfKey(objectID); idx >= 0) {
+ if (mObjects.find(objectID) != mObjects.end()) {
ALOGI("Trying to attach object ID %p to binder ObjectManager %p with object %p, but object "
"ID already in use",
objectID, this, object);
- return mObjects[idx].object;
+ return mObjects[objectID].object;
}
- mObjects.add(objectID, e);
+ mObjects.insert({objectID, e});
return nullptr;
}
void* BpBinder::ObjectManager::find(const void* objectID) const
{
- const ssize_t i = mObjects.indexOfKey(objectID);
- if (i < 0) return nullptr;
- return mObjects.valueAt(i).object;
+ auto i = mObjects.find(objectID);
+ if (i == mObjects.end()) return nullptr;
+ return i->second.object;
}
void* BpBinder::ObjectManager::detach(const void* objectID) {
- ssize_t idx = mObjects.indexOfKey(objectID);
- if (idx < 0) return nullptr;
- void* value = mObjects[idx].object;
- mObjects.removeItemsAt(idx, 1);
+ auto i = mObjects.find(objectID);
+ if (i == mObjects.end()) return nullptr;
+ void* value = i->second.object;
+ mObjects.erase(i);
return value;
}
@@ -102,10 +102,10 @@
{
const size_t N = mObjects.size();
ALOGV("Killing %zu objects in manager %p", N, this);
- for (size_t i=0; i<N; i++) {
- const entry_t& e = mObjects.valueAt(i);
+ for (auto i : mObjects) {
+ const entry_t& e = i.second;
if (e.func != nullptr) {
- e.func(mObjects.keyAt(i), e.object, e.cleanupCookie);
+ e.func(i.first, e.object, e.cleanupCookie);
}
}
diff --git a/libs/binder/IMemory.cpp b/libs/binder/IMemory.cpp
index bd974b0..9c7ff97 100644
--- a/libs/binder/IMemory.cpp
+++ b/libs/binder/IMemory.cpp
@@ -31,9 +31,10 @@
#include <binder/Parcel.h>
#include <log/log.h>
-#include <utils/KeyedVector.h>
#include <utils/threads.h>
+#include <map>
+
#define VERBOSE 0
namespace android {
@@ -63,7 +64,7 @@
void free_heap(const wp<IBinder>& binder);
Mutex mHeapCacheLock; // Protects entire vector below.
- KeyedVector< wp<IBinder>, heap_info_t > mHeapCache;
+ std::map<wp<IBinder>, heap_info_t> mHeapCache;
// We do not use the copy-on-write capabilities of KeyedVector.
// TODO: Reimplemement based on standard C++ container?
};
@@ -434,9 +435,9 @@
sp<IMemoryHeap> HeapCache::find_heap(const sp<IBinder>& binder)
{
Mutex::Autolock _l(mHeapCacheLock);
- ssize_t i = mHeapCache.indexOfKey(binder);
- if (i>=0) {
- heap_info_t& info = mHeapCache.editValueAt(i);
+ auto i = mHeapCache.find(binder);
+ if (i != mHeapCache.end()) {
+ heap_info_t& info = i->second;
ALOGD_IF(VERBOSE,
"found binder=%p, heap=%p, size=%zu, fd=%d, count=%d",
binder.get(), info.heap.get(),
@@ -452,7 +453,7 @@
info.count = 1;
//ALOGD("adding binder=%p, heap=%p, count=%d",
// binder.get(), info.heap.get(), info.count);
- mHeapCache.add(binder, info);
+ mHeapCache.insert({binder, info});
return info.heap;
}
}
@@ -466,9 +467,9 @@
sp<IMemoryHeap> rel;
{
Mutex::Autolock _l(mHeapCacheLock);
- ssize_t i = mHeapCache.indexOfKey(binder);
- if (i>=0) {
- heap_info_t& info(mHeapCache.editValueAt(i));
+ auto i = mHeapCache.find(binder);
+ if (i != mHeapCache.end()) {
+ heap_info_t& info = i->second;
if (--info.count == 0) {
ALOGD_IF(VERBOSE,
"removing binder=%p, heap=%p, size=%zu, fd=%d, count=%d",
@@ -477,8 +478,8 @@
static_cast<BpMemoryHeap*>(info.heap.get())
->mHeapId.load(memory_order_relaxed),
info.count);
- rel = mHeapCache.valueAt(i).heap;
- mHeapCache.removeItemsAt(i);
+ rel = i->second.heap;
+ mHeapCache.erase(i);
}
} else {
ALOGE("free_heap binder=%p not found!!!", binder.unsafe_get());
@@ -490,23 +491,23 @@
{
sp<IMemoryHeap> realHeap;
Mutex::Autolock _l(mHeapCacheLock);
- ssize_t i = mHeapCache.indexOfKey(binder);
- if (i>=0) realHeap = mHeapCache.valueAt(i).heap;
- else realHeap = interface_cast<IMemoryHeap>(binder);
+ auto i = mHeapCache.find(binder);
+ if (i != mHeapCache.end())
+ realHeap = i->second.heap;
+ else
+ realHeap = interface_cast<IMemoryHeap>(binder);
return realHeap;
}
void HeapCache::dump_heaps()
{
Mutex::Autolock _l(mHeapCacheLock);
- int c = mHeapCache.size();
- for (int i=0 ; i<c ; i++) {
- const heap_info_t& info = mHeapCache.valueAt(i);
+ for (const auto& i : mHeapCache) {
+ const heap_info_t& info = i.second;
BpMemoryHeap const* h(static_cast<BpMemoryHeap const *>(info.heap.get()));
- ALOGD("hey=%p, heap=%p, count=%d, (fd=%d, base=%p, size=%zu)",
- mHeapCache.keyAt(i).unsafe_get(),
- info.heap.get(), info.count,
- h->mHeapId.load(memory_order_relaxed), h->mBase, h->mSize);
+ ALOGD("hey=%p, heap=%p, count=%d, (fd=%d, base=%p, size=%zu)", i.first.unsafe_get(),
+ info.heap.get(), info.count, h->mHeapId.load(memory_order_relaxed), h->mBase,
+ h->mSize);
}
}
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 13f0a4c..f79075d 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -1199,7 +1199,8 @@
case BR_DECREFS:
refs = (RefBase::weakref_type*)mIn.readPointer();
- obj = (BBinder*)mIn.readPointer();
+ // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
+ obj = (BBinder*)mIn.readPointer(); // consume
// NOTE: This assertion is not valid, because the object may no
// longer exist (thus the (BBinder*)cast above resulting in a different
// memory address).
@@ -1409,7 +1410,7 @@
uint32_t *async_received)
{
int ret = 0;
- binder_frozen_status_info info;
+ binder_frozen_status_info info = {};
info.pid = pid;
#if defined(__ANDROID__)
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 6a138e3..504c6c2 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -63,7 +63,7 @@
// This macro should never be used at runtime, as a too large value
// of s could cause an integer overflow. Instead, you should always
// use the wrapper function pad_size()
-#define PAD_SIZE_UNSAFE(s) (((s)+3)&~3)
+#define PAD_SIZE_UNSAFE(s) (((s) + 3) & ~3UL)
static size_t pad_size(size_t s) {
if (s > (std::numeric_limits<size_t>::max() - 3)) {
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index baa817c..b14a838 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -409,6 +409,28 @@
return 0;
}
+#define DRIVER_FEATURES_PATH "/dev/binderfs/features/"
+bool ProcessState::isDriverFeatureEnabled(const DriverFeature feature) {
+ static const char* const names[] = {
+ [static_cast<int>(DriverFeature::ONEWAY_SPAM_DETECTION)] =
+ DRIVER_FEATURES_PATH "oneway_spam_detection",
+ };
+ int fd = open(names[static_cast<int>(feature)], O_RDONLY | O_CLOEXEC);
+ char on;
+ if (fd == -1) {
+ ALOGE_IF(errno != ENOENT, "%s: cannot open %s: %s", __func__,
+ names[static_cast<int>(feature)], strerror(errno));
+ return false;
+ }
+ if (read(fd, &on, sizeof(on)) == -1) {
+ ALOGE("%s: error reading to %s: %s", __func__,
+ names[static_cast<int>(feature)], strerror(errno));
+ return false;
+ }
+ close(fd);
+ return on == '1';
+}
+
status_t ProcessState::enableOnewaySpamDetection(bool enable) {
uint32_t enableDetection = enable ? 1 : 0;
if (ioctl(mDriverFD, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enableDetection) == -1) {
@@ -452,7 +474,9 @@
uint32_t enable = DEFAULT_ENABLE_ONEWAY_SPAM_DETECTION;
result = ioctl(fd, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enable);
if (result == -1) {
- ALOGV("Binder ioctl to enable oneway spam detection failed: %s", strerror(errno));
+ ALOGE_IF(ProcessState::isDriverFeatureEnabled(
+ ProcessState::DriverFeature::ONEWAY_SPAM_DETECTION),
+ "Binder ioctl to enable oneway spam detection failed: %s", strerror(errno));
}
return fd;
}
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index b84395e..e79cb86 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -152,8 +152,13 @@
}
status_t RpcSession::setupPreconnectedClient(unique_fd fd, std::function<unique_fd()>&& request) {
- return setupClient([&](const std::vector<uint8_t>& sessionId, bool incoming) -> status_t {
- // std::move'd from fd becomes -1 (!ok())
+ // Why passing raw fd? When fd is passed as reference, Clang analyzer sees that the variable
+ // `fd` is a moved-from object. To work-around the issue, unwrap the raw fd from the outer `fd`,
+ // pass the raw fd by value to the lambda, and then finally wrap it in unique_fd inside the
+ // lambda.
+ return setupClient([&, raw = fd.release()](const std::vector<uint8_t>& sessionId,
+ bool incoming) -> status_t {
+ unique_fd fd(raw);
if (!fd.ok()) {
fd = request();
if (!fd.ok()) return BAD_VALUE;
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 4ddbce7..2e7084e 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -125,8 +125,8 @@
auto&& [it, inserted] = mNodeForAddress.insert({RpcWireAddress::toRaw(address),
BinderNode{
.binder = binder,
- .timesSent = 1,
.sentRef = binder,
+ .timesSent = 1,
}});
if (inserted) {
*outAddress = it->first;
diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index c0454b6..8deb2fe 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -17,10 +17,10 @@
#pragma once
#include <binder/IBinder.h>
-#include <utils/KeyedVector.h>
#include <utils/Mutex.h>
#include <utils/threads.h>
+#include <map>
#include <unordered_map>
#include <variant>
@@ -110,7 +110,7 @@
IBinder::object_cleanup_func func;
};
- KeyedVector<const void*, entry_t> mObjects;
+ std::map<const void*, entry_t> mObjects;
};
class PrivateAccessor {
diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h
index ea40db8..bb55831 100644
--- a/libs/binder/include/binder/IServiceManager.h
+++ b/libs/binder/include/binder/IServiceManager.h
@@ -56,8 +56,16 @@
static const int DUMP_FLAG_PROTO = 1 << 4;
/**
- * Retrieve an existing service, blocking for a few seconds
- * if it doesn't yet exist.
+ * Retrieve an existing service, blocking for a few seconds if it doesn't yet exist. This
+ * does polling. A more efficient way to make sure you unblock as soon as the service is
+ * available is to use waitForService or to use service notifications.
+ *
+ * Warning: when using this API, typically, you should call it in a loop. It's dangerous to
+ * assume that nullptr could mean that the service is not available. The service could just
+ * be starting. Generally, whether a service exists, this information should be declared
+ * externally (for instance, an Android feature might imply the existence of a service,
+ * a system property, or in the case of services in the VINTF manifest, it can be checked
+ * with isDeclared).
*/
virtual sp<IBinder> getService( const String16& name) const = 0;
diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h
index cf8d8e4..0deee73 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -91,6 +91,12 @@
*/
size_t getThreadPoolMaxThreadCount() const;
+ enum class DriverFeature {
+ ONEWAY_SPAM_DETECTION,
+ };
+ // Determine whether a feature is supported by the binder driver.
+ static bool isDriverFeatureEnabled(const DriverFeature feature);
+
private:
static sp<ProcessState> init(const char* defaultDriver, bool requireDefault);
diff --git a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
index 2c471c6..7ea9be7 100644
--- a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
@@ -268,7 +268,11 @@
const char* getMessage() const { return AStatus_getMessage(get()); }
std::string getDescription() const {
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 30, *)) {
+#else
+ if (__ANDROID_API__ >= 30) {
+#endif
const char* cStr = AStatus_getDescription(get());
std::string ret = cStr;
AStatus_deleteDescription(cStr);
diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
index 09411e7..b3bc7f4 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -256,7 +256,11 @@
// ourselves. The defaults are harmless.
AIBinder_Class_setOnDump(clazz, ICInterfaceData::onDump);
#ifdef HAS_BINDER_SHELL_COMMAND
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 30, *)) {
+#else
+ if (__ANDROID_API__ >= 30) {
+#endif
AIBinder_Class_setHandleShellCommand(clazz, ICInterfaceData::handleShellCommand);
}
#endif
diff --git a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
index 972eca7..28819bb 100644
--- a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
@@ -51,7 +51,11 @@
AParcelableHolder(const AParcelableHolder& other)
: mParcel(AParcel_create()), mStability(other.mStability) {
// AParcelableHolder has been introduced in 31.
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
AParcel_appendFrom(other.mParcel.get(), this->mParcel.get(), 0,
AParcel_getDataSize(other.mParcel.get()));
}
@@ -63,13 +67,21 @@
binder_status_t writeToParcel(AParcel* parcel) const {
RETURN_ON_FAILURE(AParcel_writeInt32(parcel, static_cast<int32_t>(this->mStability)));
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
int32_t size = AParcel_getDataSize(this->mParcel.get());
RETURN_ON_FAILURE(AParcel_writeInt32(parcel, size));
} else {
return STATUS_INVALID_OPERATION;
}
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
int32_t size = AParcel_getDataSize(this->mParcel.get());
RETURN_ON_FAILURE(AParcel_appendFrom(this->mParcel.get(), parcel, 0, size));
} else {
@@ -79,7 +91,11 @@
}
binder_status_t readFromParcel(const AParcel* parcel) {
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
AParcel_reset(mParcel.get());
} else {
return STATUS_INVALID_OPERATION;
@@ -99,7 +115,11 @@
return STATUS_BAD_VALUE;
}
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
status = AParcel_appendFrom(parcel, mParcel.get(), dataStartPos, dataSize);
} else {
status = STATUS_INVALID_OPERATION;
@@ -115,7 +135,11 @@
if (this->mStability > T::_aidl_stability) {
return STATUS_BAD_VALUE;
}
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
AParcel_reset(mParcel.get());
} else {
return STATUS_INVALID_OPERATION;
@@ -129,7 +153,11 @@
binder_status_t getParcelable(std::optional<T>* ret) const {
const std::string parcelableDesc(T::descriptor);
AParcel_setDataPosition(mParcel.get(), 0);
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
if (AParcel_getDataSize(mParcel.get()) == 0) {
*ret = std::nullopt;
return STATUS_OK;
@@ -153,7 +181,11 @@
}
void reset() {
+#ifdef __ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__
if (__builtin_available(android 31, *)) {
+#else
+ if (__ANDROID_API__ >= 31) {
+#endif
AParcel_reset(mParcel.get());
}
}
diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h
index 2a66941..dfa8ea2 100644
--- a/libs/binder/ndk/include_platform/android/binder_manager.h
+++ b/libs/binder/ndk/include_platform/android/binder_manager.h
@@ -53,11 +53,19 @@
/**
* Gets a binder object with this specific instance name. Blocks for a couple of seconds waiting on
* it. This also implicitly calls AIBinder_incStrong (so the caller of this function is responsible
- * for calling AIBinder_decStrong).
+ * for calling AIBinder_decStrong). This does polling. A more efficient way to make sure you
+ * unblock as soon as the service is available is to use AIBinder_waitForService.
*
* WARNING: when using this API across an APEX boundary, do not use with unstable
* AIDL services. TODO(b/139325195)
*
+ * WARNING: when using this API, typically, you should call it in a loop. It's dangerous to
+ * assume that nullptr could mean that the service is not available. The service could just
+ * be starting. Generally, whether a service exists, this information should be declared
+ * externally (for instance, an Android feature might imply the existence of a service,
+ * a system property, or in the case of services in the VINTF manifest, it can be checked
+ * with AServiceManager_isDeclared).
+ *
* \param instance identifier of the service used to lookup the service.
*/
__attribute__((warn_unused_result)) AIBinder* AServiceManager_getService(const char* instance)
diff --git a/libs/cputimeinstate/testtimeinstate.cpp b/libs/cputimeinstate/testtimeinstate.cpp
index 1513eca..45a6d47 100644
--- a/libs/cputimeinstate/testtimeinstate.cpp
+++ b/libs/cputimeinstate/testtimeinstate.cpp
@@ -31,6 +31,7 @@
#include <android-base/unique_fd.h>
#include <bpf/BpfMap.h>
#include <cputimeinstate.h>
+#include <cutils/android_filesystem_config.h>
#include <libbpf.h>
namespace android {
@@ -219,6 +220,7 @@
uint32_t totalFreqsCount = totalTimes.size();
std::vector<uint64_t> allUidTimes(totalFreqsCount, 0);
for (auto const &[uid, uidTimes]: *allUid) {
+ if (uid == AID_SDK_SANDBOX) continue;
for (uint32_t freqIdx = 0; freqIdx < uidTimes[policyIdx].size(); ++freqIdx) {
allUidTimes[std::min(freqIdx, totalFreqsCount - 1)] += uidTimes[policyIdx][freqIdx];
}
@@ -646,5 +648,55 @@
}
}
+void *forceSwitchWithUid(void *uidPtr) {
+ if (!uidPtr) return nullptr;
+ setuid(*(uint32_t *)uidPtr);
+
+ // Sleep briefly to trigger a context switch, ensuring we see at least one update.
+ struct timespec ts;
+ ts.tv_sec = 0;
+ ts.tv_nsec = 1000000;
+ nanosleep(&ts, NULL);
+ return nullptr;
+}
+
+TEST_F(TimeInStateTest, SdkSandboxUid) {
+ // Find an unused app UID and its corresponding SDK sandbox uid.
+ uint32_t appUid = AID_APP_START, sandboxUid;
+ {
+ auto times = getUidsCpuFreqTimes();
+ ASSERT_TRUE(times.has_value());
+ ASSERT_FALSE(times->empty());
+ for (const auto &kv : *times) {
+ if (kv.first > AID_APP_END) break;
+ appUid = std::max(appUid, kv.first);
+ }
+ appUid++;
+ sandboxUid = appUid + (AID_SDK_SANDBOX_PROCESS_START - AID_APP_START);
+ }
+
+ // Create a thread to run with the fake sandbox uid.
+ pthread_t thread;
+ ASSERT_EQ(pthread_create(&thread, NULL, &forceSwitchWithUid, &sandboxUid), 0);
+ pthread_join(thread, NULL);
+
+ // Confirm we recorded stats for appUid and AID_SDK_SANDBOX but not sandboxUid
+ auto allTimes = getUidsCpuFreqTimes();
+ ASSERT_TRUE(allTimes.has_value());
+ ASSERT_FALSE(allTimes->empty());
+ ASSERT_NE(allTimes->find(appUid), allTimes->end());
+ ASSERT_NE(allTimes->find(AID_SDK_SANDBOX), allTimes->end());
+ ASSERT_EQ(allTimes->find(sandboxUid), allTimes->end());
+
+ auto allConcurrentTimes = getUidsConcurrentTimes();
+ ASSERT_TRUE(allConcurrentTimes.has_value());
+ ASSERT_FALSE(allConcurrentTimes->empty());
+ ASSERT_NE(allConcurrentTimes->find(appUid), allConcurrentTimes->end());
+ ASSERT_NE(allConcurrentTimes->find(AID_SDK_SANDBOX), allConcurrentTimes->end());
+ ASSERT_EQ(allConcurrentTimes->find(sandboxUid), allConcurrentTimes->end());
+
+ ASSERT_TRUE(clearUidTimes(appUid));
+}
+
} // namespace bpf
} // namespace android
diff --git a/services/inputflinger/tests/fuzzers/Android.bp b/services/inputflinger/tests/fuzzers/Android.bp
index df4db19..455a1e2 100644
--- a/services/inputflinger/tests/fuzzers/Android.bp
+++ b/services/inputflinger/tests/fuzzers/Android.bp
@@ -42,4 +42,7 @@
srcs: [
"LatencyTrackerFuzzer.cpp",
],
+ fuzz_config: {
+ cc: ["android-framework-input@google.com"],
+ },
}
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/GpuCompositionResult.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/GpuCompositionResult.h
index 2b1f50f..ed1ddc1 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/GpuCompositionResult.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/GpuCompositionResult.h
@@ -22,6 +22,9 @@
namespace android::compositionengine::impl {
struct GpuCompositionResult {
+ // True if composition strategy was predicted successfully.
+ bool succeeded = false;
+
// Composition ready fence.
base::unique_fd fence{};
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index ade9b25..92f22b6 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -141,18 +141,6 @@
// This is slightly distinct from nits, in that nits cannot be passed to hw composer.
std::optional<float> displayBrightness = std::nullopt;
- enum class CompositionStrategyPredictionState : uint32_t {
- // Composition strategy prediction did not run for this frame.
- DISABLED = 0,
- // Composition strategy predicted successfully for this frame.
- SUCCESS = 1,
- // Composition strategy prediction failed for this frame.
- FAIL = 2,
- };
-
- CompositionStrategyPredictionState strategyPrediction =
- CompositionStrategyPredictionState::DISABLED;
-
// Debugging
void dump(std::string& result) const;
};
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index e4bd325..cd10bc1 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -58,8 +58,7 @@
Output::~Output() = default;
namespace impl {
-using CompositionStrategyPredictionState =
- OutputCompositionState::CompositionStrategyPredictionState;
+
namespace {
template <typename T>
@@ -967,7 +966,6 @@
}
auto changes = chooseCompositionStrategy();
- outputState.strategyPrediction = CompositionStrategyPredictionState::DISABLED;
outputState.previousDeviceRequestedChanges = changes;
if (changes) {
applyCompositionStrategy(changes);
@@ -1004,8 +1002,7 @@
auto changes = hwcResult.valid() ? hwcResult.get() : std::nullopt;
const bool predictionSucceeded = dequeueSucceeded && changes == previousChanges;
- state.strategyPrediction = predictionSucceeded ? CompositionStrategyPredictionState::SUCCESS
- : CompositionStrategyPredictionState::FAIL;
+ compositionResult.succeeded = predictionSucceeded;
if (!predictionSucceeded) {
ATRACE_NAME("CompositionStrategyPredictionMiss");
if (changes) {
@@ -1047,15 +1044,15 @@
void Output::finishFrame(const CompositionRefreshArgs& refreshArgs, GpuCompositionResult&& result) {
ATRACE_CALL();
ALOGV(__FUNCTION__);
- const auto& outputState = getState();
- if (!outputState.isEnabled) {
+
+ if (!getState().isEnabled) {
return;
}
std::optional<base::unique_fd> optReadyFence;
std::shared_ptr<renderengine::ExternalTexture> buffer;
base::unique_fd bufferFence;
- if (outputState.strategyPrediction == CompositionStrategyPredictionState::SUCCESS) {
+ if (result.succeeded) {
optReadyFence = std::move(result.fence);
} else {
if (result.bufferAvailable()) {
@@ -1203,8 +1200,12 @@
// because high frequency consumes extra battery.
const bool expensiveBlurs =
refreshArgs.blursAreExpensive && mLayerRequestingBackgroundBlur != nullptr;
- const bool expensiveRenderingExpected =
- clientCompositionDisplay.outputDataspace == ui::Dataspace::DISPLAY_P3 || expensiveBlurs;
+ const bool expensiveRenderingExpected = expensiveBlurs ||
+ std::any_of(clientCompositionLayers.begin(), clientCompositionLayers.end(),
+ [outputDataspace =
+ clientCompositionDisplay.outputDataspace](const auto& layer) {
+ return layer.sourceDataspace != outputDataspace;
+ });
if (expensiveRenderingExpected) {
setExpensiveRenderingExpected(true);
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
index 7188281..482250a 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp
@@ -18,19 +18,6 @@
#include <compositionengine/impl/OutputCompositionState.h>
namespace android::compositionengine::impl {
-using CompositionStrategyPredictionState =
- OutputCompositionState::CompositionStrategyPredictionState;
-
-std::string toString(CompositionStrategyPredictionState state) {
- switch (state) {
- case CompositionStrategyPredictionState::DISABLED:
- return "Disabled";
- case CompositionStrategyPredictionState::SUCCESS:
- return "Success";
- case CompositionStrategyPredictionState::FAIL:
- return "Fail";
- }
-}
void OutputCompositionState::dump(std::string& out) const {
out.append(" ");
@@ -69,7 +56,6 @@
dumpVal(out, "sdrWhitePointNits", sdrWhitePointNits);
dumpVal(out, "clientTargetBrightness", clientTargetBrightness);
dumpVal(out, "displayBrightness", displayBrightness);
- dumpVal(out, "compositionStrategyPredictionState", toString(strategyPrediction));
out.append("\n");
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 3e983f3..723593d 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -325,7 +325,8 @@
// For hdr content, treat the white point as the display brightness - HDR content should not be
// boosted or dimmed.
if (isHdrDataspace(state.dataspace) ||
- getOutput().getState().displayBrightnessNits == getOutput().getState().sdrWhitePointNits) {
+ getOutput().getState().displayBrightnessNits == getOutput().getState().sdrWhitePointNits ||
+ getOutput().getState().displayBrightnessNits == 0.f) {
state.dimmingRatio = 1.f;
state.whitePointNits = getOutput().getState().displayBrightnessNits;
} else {
@@ -506,9 +507,18 @@
to_string(error).c_str(), static_cast<int32_t>(error));
}
- // Don't dim cached layers
- const auto dimmingRatio =
- outputDependentState.overrideInfo.buffer ? 1.f : outputDependentState.dimmingRatio;
+ // Cached layers are not dimmed, which means that composer should attempt to dim.
+ // Note that if the dimming ratio is large, then this may cause the cached layer
+ // to kick back into GPU composition :(
+ // Also note that this assumes that there are no HDR layers that are able to be cached.
+ // Otherwise, this could cause HDR layers to be dimmed twice.
+ const auto dimmingRatio = outputDependentState.overrideInfo.buffer
+ ? (getOutput().getState().displayBrightnessNits != 0.f
+ ? std::clamp(getOutput().getState().sdrWhitePointNits /
+ getOutput().getState().displayBrightnessNits,
+ 0.f, 1.f)
+ : 1.f)
+ : outputDependentState.dimmingRatio;
if (auto error = hwcLayer->setBrightness(dimmingRatio); error != hal::Error::NONE) {
ALOGE("[%s] Failed to set brightness %f: %s (%d)", getLayerFE().getDebugName(),
@@ -788,6 +798,7 @@
}};
settings.sourceDataspace = getState().overrideInfo.dataspace;
settings.alpha = 1.0f;
+ settings.whitePointNits = getOutput().getState().sdrWhitePointNits;
return {static_cast<LayerFE::LayerSettings>(settings)};
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index 2203f22..42c1263 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -20,6 +20,7 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
#include <android-base/properties.h>
+#include <android-base/stringprintf.h>
#include <compositionengine/impl/OutputCompositionState.h>
#include <compositionengine/impl/planner/CachedSet.h>
#include <math/HashCombine.h>
@@ -216,9 +217,8 @@
renderengine::LayerSettings holePunchSettings;
renderengine::LayerSettings holePunchBackgroundSettings;
if (mHolePunchLayer) {
- auto clientCompositionList =
- mHolePunchLayer->getOutputLayer()->getLayerFE().prepareClientCompositionList(
- targetSettings);
+ auto& layerFE = mHolePunchLayer->getOutputLayer()->getLayerFE();
+ auto clientCompositionList = layerFE.prepareClientCompositionList(targetSettings);
// Assume that the final layer contains the buffer that we want to
// replace with a hole punch.
holePunchSettings = clientCompositionList.back();
@@ -227,7 +227,8 @@
holePunchSettings.source.solidColor = half3(0.0f, 0.0f, 0.0f);
holePunchSettings.disableBlending = true;
holePunchSettings.alpha = 0.0f;
- holePunchSettings.name = std::string("hole punch layer");
+ holePunchSettings.name =
+ android::base::StringPrintf("hole punch layer for %s", layerFE.getDebugName());
layerSettings.push_back(holePunchSettings);
// Add a solid background as the first layer in case there is no opaque
@@ -390,7 +391,10 @@
const auto b = mTexture ? mTexture->get()->getBuffer().get() : nullptr;
base::StringAppendF(&result, " Override buffer: %p\n", b);
}
- base::StringAppendF(&result, " HolePunchLayer: %p\n", mHolePunchLayer);
+ base::StringAppendF(&result, " HolePunchLayer: %p\t%s\n", mHolePunchLayer,
+ mHolePunchLayer
+ ? mHolePunchLayer->getOutputLayer()->getLayerFE().getDebugName()
+ : "");
if (mLayers.size() == 1) {
base::StringAppendF(&result, " Layer [%s]\n", mLayers[0].getName().c_str());
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index dda0822..8eb1946 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -657,7 +657,7 @@
EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace);
}
-TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsCorrectly) {
+TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsAndDimmingRatioCorrectly) {
mOutputState.sdrWhitePointNits = 200.f;
mOutputState.displayBrightnessNits = 800.f;
@@ -665,12 +665,15 @@
mLayerFEState.isColorspaceAgnostic = false;
mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0);
EXPECT_EQ(mOutputState.sdrWhitePointNits, mOutputLayer.getState().whitePointNits);
+ EXPECT_EQ(mOutputState.sdrWhitePointNits / mOutputState.displayBrightnessNits,
+ mOutputLayer.getState().dimmingRatio);
mLayerFEState.dataspace = ui::Dataspace::BT2020_ITU_PQ;
mLayerFEState.isColorspaceAgnostic = false;
mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0);
EXPECT_EQ(mOutputState.displayBrightnessNits, mOutputLayer.getState().whitePointNits);
+ EXPECT_EQ(1.f, mOutputLayer.getState().dimmingRatio);
}
TEST_F(OutputLayerUpdateCompositionStateTest, doesNotRecomputeGeometryIfNotRequested) {
@@ -750,9 +753,10 @@
static constexpr bool kLayerGenericMetadata1Mandatory = true;
static constexpr bool kLayerGenericMetadata2Mandatory = true;
static constexpr float kWhitePointNits = 200.f;
+ static constexpr float kSdrWhitePointNits = 100.f;
static constexpr float kDisplayBrightnessNits = 400.f;
static constexpr float kLayerBrightness = kWhitePointNits / kDisplayBrightnessNits;
- static constexpr float kFullLayerBrightness = 1.f;
+ static constexpr float kOverrideLayerBrightness = kSdrWhitePointNits / kDisplayBrightnessNits;
static const half4 kColor;
static const Rect kDisplayFrame;
@@ -798,6 +802,7 @@
mLayerFEState.acquireFence = kFence;
mOutputState.displayBrightnessNits = kDisplayBrightnessNits;
+ mOutputState.sdrWhitePointNits = kSdrWhitePointNits;
EXPECT_CALL(mOutput, getDisplayColorProfile())
.WillRepeatedly(Return(&mDisplayColorProfile));
@@ -1117,7 +1122,7 @@
expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform,
kOverrideBlendMode, kSkipAlpha);
expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion,
- kOverrideSurfaceDamage, kFullLayerBrightness);
+ kOverrideSurfaceDamage, kOverrideLayerBrightness);
expectSetHdrMetadataAndBufferCalls();
expectSetCompositionTypeCall(Composition::DEVICE);
EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
@@ -1133,7 +1138,7 @@
expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform,
kOverrideBlendMode, kSkipAlpha);
expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion,
- kOverrideSurfaceDamage, kFullLayerBrightness);
+ kOverrideSurfaceDamage, kOverrideLayerBrightness);
expectSetHdrMetadataAndBufferCalls();
expectSetCompositionTypeCall(Composition::DEVICE);
EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
@@ -1149,7 +1154,7 @@
expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform,
kOverrideBlendMode, kOverrideAlpha);
expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion,
- kOverrideSurfaceDamage, kFullLayerBrightness);
+ kOverrideSurfaceDamage, kOverrideLayerBrightness);
expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence);
expectSetCompositionTypeCall(Composition::DEVICE);
EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
@@ -1165,7 +1170,7 @@
expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform,
kOverrideBlendMode, kOverrideAlpha);
expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion,
- kOverrideSurfaceDamage, kFullLayerBrightness);
+ kOverrideSurfaceDamage, kOverrideLayerBrightness);
expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence);
expectSetCompositionTypeCall(Composition::DEVICE);
EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false));
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 0c5ea79..af172d8 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -74,9 +74,6 @@
constexpr OutputColorSetting kVendorSpecifiedOutputColorSetting =
static_cast<OutputColorSetting>(0x100);
-using CompositionStrategyPredictionState = android::compositionengine::impl::
- OutputCompositionState::CompositionStrategyPredictionState;
-
struct OutputPartialMockBase : public impl::Output {
// compositionengine::Output overrides
const OutputCompositionState& getState() const override { return mState; }
@@ -1024,7 +1021,6 @@
EXPECT_CALL(*mRenderSurface, prepareFrame(false, true));
mOutput.prepareFrame();
- EXPECT_EQ(mOutput.getState().strategyPrediction, CompositionStrategyPredictionState::DISABLED);
}
// Note: Use OutputTest and not OutputPrepareFrameTest, so the real
@@ -1040,7 +1036,6 @@
EXPECT_TRUE(mOutput->getState().usesClientComposition);
EXPECT_FALSE(mOutput->getState().usesDeviceComposition);
- EXPECT_EQ(mOutput->getState().strategyPrediction, CompositionStrategyPredictionState::DISABLED);
}
struct OutputPrepareFrameAsyncTest : public testing::Test {
@@ -1089,7 +1084,7 @@
EXPECT_CALL(mOutput, composeSurfaces(_, Ref(mRefreshArgs), _, _));
impl::GpuCompositionResult result = mOutput.prepareFrameAsync(mRefreshArgs);
- EXPECT_EQ(mOutput.getState().strategyPrediction, CompositionStrategyPredictionState::SUCCESS);
+ EXPECT_TRUE(result.succeeded);
EXPECT_FALSE(result.bufferAvailable());
}
@@ -1109,7 +1104,7 @@
EXPECT_CALL(mOutput, chooseCompositionStrategyAsync()).WillOnce([&] { return p.get_future(); });
impl::GpuCompositionResult result = mOutput.prepareFrameAsync(mRefreshArgs);
- EXPECT_EQ(mOutput.getState().strategyPrediction, CompositionStrategyPredictionState::FAIL);
+ EXPECT_FALSE(result.succeeded);
EXPECT_FALSE(result.bufferAvailable());
}
@@ -1137,7 +1132,7 @@
EXPECT_CALL(mOutput, composeSurfaces(_, Ref(mRefreshArgs), _, _));
impl::GpuCompositionResult result = mOutput.prepareFrameAsync(mRefreshArgs);
- EXPECT_EQ(mOutput.getState().strategyPrediction, CompositionStrategyPredictionState::FAIL);
+ EXPECT_FALSE(result.succeeded);
EXPECT_TRUE(result.bufferAvailable());
}
@@ -1166,7 +1161,7 @@
EXPECT_CALL(mOutput, composeSurfaces(_, Ref(mRefreshArgs), _, _));
impl::GpuCompositionResult result = mOutput.prepareFrameAsync(mRefreshArgs);
- EXPECT_EQ(mOutput.getState().strategyPrediction, CompositionStrategyPredictionState::FAIL);
+ EXPECT_FALSE(result.succeeded);
EXPECT_TRUE(result.bufferAvailable());
}
@@ -3010,21 +3005,22 @@
TEST_F(OutputFinishFrameTest, predictionSucceeded) {
mOutput.mState.isEnabled = true;
- mOutput.mState.strategyPrediction = CompositionStrategyPredictionState::SUCCESS;
+
InSequence seq;
EXPECT_CALL(*mRenderSurface, queueBuffer(_));
impl::GpuCompositionResult result;
+ result.succeeded = true;
mOutput.finishFrame(mRefreshArgs, std::move(result));
}
TEST_F(OutputFinishFrameTest, predictionFailedAndBufferIsReused) {
mOutput.mState.isEnabled = true;
- mOutput.mState.strategyPrediction = CompositionStrategyPredictionState::FAIL;
InSequence seq;
impl::GpuCompositionResult result;
+ result.succeeded = false;
result.buffer =
std::make_shared<renderengine::mock::FakeExternalTexture>(1, 1,
HAL_PIXEL_FORMAT_RGBA_8888, 1,
@@ -3986,8 +3982,9 @@
TEST_F(OutputComposeSurfacesTest_SetsExpensiveRendering, IfExepensiveOutputDataspaceIsUsed) {
mOutput.mState.dataspace = kExpensiveOutputDataspace;
+ LayerFE::LayerSettings layerSettings;
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, kExpensiveOutputDataspace, _))
- .WillOnce(Return(std::vector<LayerFE::LayerSettings>{}));
+ .WillOnce(Return(std::vector<LayerFE::LayerSettings>{layerSettings}));
// For this test, we also check the call order of key functions.
InSequence seq;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d1ac1d3..efaa975 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -169,8 +169,6 @@
using aidl::android::hardware::graphics::common::DisplayDecorationSupport;
using aidl::android::hardware::graphics::composer3::Capability;
using aidl::android::hardware::graphics::composer3::DisplayCapability;
-using CompositionStrategyPredictionState = android::compositionengine::impl::
- OutputCompositionState::CompositionStrategyPredictionState;
namespace android {
@@ -482,7 +480,7 @@
property_get("debug.sf.disable_client_composition_cache", value, "0");
mDisableClientCompositionCache = atoi(value);
- property_get("debug.sf.predict_hwc_composition_strategy", value, "1");
+ property_get("debug.sf.predict_hwc_composition_strategy", value, "0");
mPredictCompositionStrategy = atoi(value);
// We should be reading 'persist.sys.sf.color_saturation' here
@@ -2272,24 +2270,24 @@
const bool prevFrameHadClientComposition = mHadClientComposition;
- mHadClientComposition = mHadDeviceComposition = mReusedClientComposition = false;
- TimeStats::ClientCompositionRecord clientCompositionRecord;
- for (const auto& [_, display] : displays) {
- const auto& state = display->getCompositionDisplay()->getState();
- mHadClientComposition |= state.usesClientComposition && !state.reusedClientComposition;
- mHadDeviceComposition |= state.usesDeviceComposition;
- mReusedClientComposition |= state.reusedClientComposition;
- clientCompositionRecord.predicted |=
- (state.strategyPrediction != CompositionStrategyPredictionState::DISABLED);
- clientCompositionRecord.predictionSucceeded |=
- (state.strategyPrediction == CompositionStrategyPredictionState::SUCCESS);
+ mHadClientComposition = std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+ const auto& state = pair.second->getCompositionDisplay()->getState();
+ return state.usesClientComposition && !state.reusedClientComposition;
+ });
+ mHadDeviceComposition = std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+ const auto& state = pair.second->getCompositionDisplay()->getState();
+ return state.usesDeviceComposition;
+ });
+ mReusedClientComposition =
+ std::any_of(displays.cbegin(), displays.cend(), [](const auto& pair) {
+ const auto& state = pair.second->getCompositionDisplay()->getState();
+ return state.reusedClientComposition;
+ });
+ // Only report a strategy change if we move in and out of client composition
+ if (prevFrameHadClientComposition != mHadClientComposition) {
+ mTimeStats->incrementCompositionStrategyChanges();
}
- clientCompositionRecord.hadClientComposition = mHadClientComposition;
- clientCompositionRecord.reused = mReusedClientComposition;
- clientCompositionRecord.changed = prevFrameHadClientComposition != mHadClientComposition;
- mTimeStats->pushCompositionStrategyState(clientCompositionRecord);
-
// TODO: b/160583065 Enable skip validation when SF caches all client composition layers
const bool usedGpuComposition = mHadClientComposition || mReusedClientComposition;
modulateVsync(&VsyncModulator::onDisplayRefresh, usedGpuComposition);
@@ -2544,6 +2542,13 @@
}
mTimeStats->incrementTotalFrames();
+ if (mHadClientComposition) {
+ mTimeStats->incrementClientCompositionFrames();
+ }
+
+ if (mReusedClientComposition) {
+ mTimeStats->incrementClientCompositionReusedFrames();
+ }
mTimeStats->setPresentFenceGlobal(mPreviousPresentFences[0].fenceTime);
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index e5a9dd4..b1a2bda 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -321,19 +321,22 @@
mTimeStats.missedFramesLegacy++;
}
-void TimeStats::pushCompositionStrategyState(const TimeStats::ClientCompositionRecord& record) {
- if (!mEnabled.load() || !record.hasInterestingData()) {
- return;
- }
+void TimeStats::incrementClientCompositionFrames() {
+ if (!mEnabled.load()) return;
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- if (record.changed) mTimeStats.compositionStrategyChangesLegacy++;
- if (record.hadClientComposition) mTimeStats.clientCompositionFramesLegacy++;
- if (record.reused) mTimeStats.clientCompositionReusedFramesLegacy++;
- if (record.predicted) mTimeStats.compositionStrategyPredictedLegacy++;
- if (record.predictionSucceeded) mTimeStats.compositionStrategyPredictionSucceededLegacy++;
+ mTimeStats.clientCompositionFramesLegacy++;
+}
+
+void TimeStats::incrementClientCompositionReusedFrames() {
+ if (!mEnabled.load()) return;
+
+ ATRACE_CALL();
+
+ std::lock_guard<std::mutex> lock(mMutex);
+ mTimeStats.clientCompositionReusedFramesLegacy++;
}
void TimeStats::incrementRefreshRateSwitches() {
@@ -345,6 +348,15 @@
mTimeStats.refreshRateSwitchesLegacy++;
}
+void TimeStats::incrementCompositionStrategyChanges() {
+ if (!mEnabled.load()) return;
+
+ ATRACE_CALL();
+
+ std::lock_guard<std::mutex> lock(mMutex);
+ mTimeStats.compositionStrategyChangesLegacy++;
+}
+
void TimeStats::recordDisplayEventConnectionCount(int32_t count) {
if (!mEnabled.load()) return;
@@ -1050,10 +1062,8 @@
mTimeStats.missedFramesLegacy = 0;
mTimeStats.clientCompositionFramesLegacy = 0;
mTimeStats.clientCompositionReusedFramesLegacy = 0;
- mTimeStats.compositionStrategyChangesLegacy = 0;
- mTimeStats.compositionStrategyPredictedLegacy = 0;
- mTimeStats.compositionStrategyPredictionSucceededLegacy = 0;
mTimeStats.refreshRateSwitchesLegacy = 0;
+ mTimeStats.compositionStrategyChangesLegacy = 0;
mTimeStats.displayEventConnectionsCountLegacy = 0;
mTimeStats.displayOnTimeLegacy = 0;
mTimeStats.presentToPresentLegacy.hist.clear();
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 7a159b8..77c7973 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -45,7 +45,7 @@
virtual ~TimeStats() = default;
// Process a pull request from statsd.
- virtual bool onPullAtom(const int atomId, std::string* pulledData) = 0;
+ virtual bool onPullAtom(const int atomId, std::string* pulledData);
virtual void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) = 0;
virtual bool isEnabled() = 0;
@@ -53,8 +53,14 @@
virtual void incrementTotalFrames() = 0;
virtual void incrementMissedFrames() = 0;
+ virtual void incrementClientCompositionFrames() = 0;
+ virtual void incrementClientCompositionReusedFrames() = 0;
// Increments the number of times the display refresh rate changed.
virtual void incrementRefreshRateSwitches() = 0;
+ // Increments the number of changes in composition strategy
+ // The intention is to reflect the number of changes between hwc and gpu
+ // composition, where "gpu composition" may also include mixed composition.
+ virtual void incrementCompositionStrategyChanges() = 0;
// Records the most up-to-date count of display event connections.
// The stored count will be the maximum ever recoded.
virtual void recordDisplayEventConnectionCount(int32_t count) = 0;
@@ -152,24 +158,6 @@
}
};
- struct ClientCompositionRecord {
- // Frame had client composition or mixed composition
- bool hadClientComposition = false;
- // Composition changed between hw composition and mixed/client composition
- bool changed = false;
- // Frame reused the client composition result from a previous frame
- bool reused = false;
- // Composition strategy predicted for frame
- bool predicted = false;
- // Composition strategy prediction succeeded
- bool predictionSucceeded = false;
-
- // Whether there is data we want to record.
- bool hasInterestingData() const {
- return hadClientComposition || changed || reused || predicted;
- }
- };
-
virtual void incrementJankyFrames(const JankyFramesInfo& info) = 0;
// Clean up the layer record
virtual void onDestroy(int32_t layerId) = 0;
@@ -181,7 +169,6 @@
// Source of truth is RefrehRateStats.
virtual void recordRefreshRate(uint32_t fps, nsecs_t duration) = 0;
virtual void setPresentFenceGlobal(const std::shared_ptr<FenceTime>& presentFence) = 0;
- virtual void pushCompositionStrategyState(const ClientCompositionRecord&) = 0;
};
namespace impl {
@@ -249,7 +236,10 @@
void incrementTotalFrames() override;
void incrementMissedFrames() override;
+ void incrementClientCompositionFrames() override;
+ void incrementClientCompositionReusedFrames() override;
void incrementRefreshRateSwitches() override;
+ void incrementCompositionStrategyChanges() override;
void recordDisplayEventConnectionCount(int32_t count) override;
void recordFrameDuration(nsecs_t startTime, nsecs_t endTime) override;
@@ -285,8 +275,6 @@
void recordRefreshRate(uint32_t fps, nsecs_t duration) override;
void setPresentFenceGlobal(const std::shared_ptr<FenceTime>& presentFence) override;
- void pushCompositionStrategyState(const ClientCompositionRecord&) override;
-
static const size_t MAX_NUM_TIME_RECORDS = 64;
private:
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
index cf1ca65..69afa2a 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
+++ b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
@@ -143,14 +143,6 @@
clientCompositionReusedFramesLegacy);
StringAppendF(&result, "refreshRateSwitches = %d\n", refreshRateSwitchesLegacy);
StringAppendF(&result, "compositionStrategyChanges = %d\n", compositionStrategyChangesLegacy);
- StringAppendF(&result, "compositionStrategyPredicted = %d\n",
- compositionStrategyPredictedLegacy);
- StringAppendF(&result, "compositionStrategyPredictionSucceeded = %d\n",
- compositionStrategyPredictionSucceededLegacy);
- StringAppendF(&result, "compositionStrategyPredictionFailed = %d\n",
- compositionStrategyPredictedLegacy -
- compositionStrategyPredictionSucceededLegacy);
-
StringAppendF(&result, "displayOnTime = %" PRId64 " ms\n", displayOnTimeLegacy);
StringAppendF(&result, "displayConfigStats is as below:\n");
for (const auto& [fps, duration] : refreshRateStatsLegacy) {
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
index 237ae8d..438561c 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
+++ b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
@@ -178,8 +178,6 @@
Histogram frameDurationLegacy;
Histogram renderEngineTimingLegacy;
std::unordered_map<uint32_t, nsecs_t> refreshRateStatsLegacy;
- int32_t compositionStrategyPredictedLegacy = 0;
- int32_t compositionStrategyPredictionSucceededLegacy = 0;
std::unordered_map<TimelineStatsKey, TimelineStats, TimelineStatsKey::Hasher> stats;
diff --git a/services/surfaceflinger/Tracing/LocklessStack.h b/services/surfaceflinger/Tracing/LocklessStack.h
new file mode 100644
index 0000000..20f2aa8
--- /dev/null
+++ b/services/surfaceflinger/Tracing/LocklessStack.h
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+#include <atomic>
+
+template <typename T>
+// Single consumer multi producer stack. We can understand the two operations independently to see
+// why they are without race condition.
+//
+// push is responsible for maintaining a linked list stored in mPush, and called from multiple
+// threads without lock. We can see that if two threads never observe the same value from
+// mPush.load, it just functions as a normal linked list. In the case where two threads observe the
+// same value, one of them has to execute the compare_exchange first. The one that doesn't execute
+// the compare exchange first, will receive false from compare_exchange. previousHead is updated (by
+// compare_exchange) to the most recent value of mPush, and we try again. It's relatively clear to
+// see that the process can repeat with an arbitrary number of threads.
+//
+// Pop is much simpler. If mPop is empty (as it begins) it atomically exchanges
+// the entire push list with null. This is safe, since the only other reader (push)
+// of mPush will retry if it changes in between it's read and atomic compare. We
+// then store the list and pop one element.
+//
+// If we already had something in the pop list we just pop directly.
+class LocklessStack {
+public:
+ class Entry {
+ public:
+ T* mValue;
+ std::atomic<Entry*> mNext;
+ Entry(T* value): mValue(value) {}
+ };
+ std::atomic<Entry*> mPush = nullptr;
+ std::atomic<Entry*> mPop = nullptr;
+ void push(T* value) {
+ Entry* entry = new Entry(value);
+ Entry* previousHead = mPush.load(/*std::memory_order_relaxed*/);
+ do {
+ entry->mNext = previousHead;
+ } while (!mPush.compare_exchange_weak(previousHead, entry)); /*std::memory_order_release*/
+ }
+ T* pop() {
+ Entry* popped = mPop.load(/*std::memory_order_acquire*/);
+ if (popped) {
+ // Single consumer so this is fine
+ mPop.store(popped->mNext/* , std::memory_order_release */);
+ auto value = popped->mValue;
+ delete popped;
+ return value;
+ } else {
+ Entry *grabbedList = mPush.exchange(nullptr/* , std::memory_order_acquire */);
+ if (!grabbedList) return nullptr;
+ mPop.store(grabbedList->mNext/* , std::memory_order_release */);
+ auto value = grabbedList->mValue;
+ delete grabbedList;
+ return value;
+ }
+ }
+};
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
index 1e5c3e7..d249b60 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp
@@ -180,13 +180,13 @@
}
if (layer.what & layer_state_t::eReparent) {
- int32_t layerId = layer.parentSurfaceControlForChild
+ int64_t layerId = layer.parentSurfaceControlForChild
? mMapper->getLayerId(layer.parentSurfaceControlForChild->getHandle())
: -1;
proto.set_parent_id(layerId);
}
if (layer.what & layer_state_t::eRelativeLayerChanged) {
- int32_t layerId = layer.relativeLayerSurfaceControl
+ int64_t layerId = layer.relativeLayerSurfaceControl
? mMapper->getLayerId(layer.relativeLayerSurfaceControl->getHandle())
: -1;
proto.set_relative_parent_id(layerId);
@@ -342,13 +342,13 @@
outState.merge(state);
if (state.what & layer_state_t::eReparent) {
- outState.parentId = proto.parent_id();
+ outState.parentId = static_cast<int32_t>(proto.parent_id());
}
if (state.what & layer_state_t::eRelativeLayerChanged) {
- outState.relativeParentId = proto.relative_parent_id();
+ outState.relativeParentId = static_cast<int32_t>(proto.relative_parent_id());
}
if (state.what & layer_state_t::eInputInfoChanged) {
- outState.inputCropId = proto.window_info_handle().crop_layer_id();
+ outState.inputCropId = static_cast<int32_t>(proto.window_info_handle().crop_layer_id());
}
if (state.what & layer_state_t::eBufferChanged) {
const proto::LayerState_BufferData& bufferProto = proto.buffer_data();
@@ -364,7 +364,7 @@
}
void TransactionProtoParser::fromProto(const proto::LayerState& proto, layer_state_t& layer) {
- layer.layerId = proto.layer_id();
+ layer.layerId = (int32_t)proto.layer_id();
layer.what |= proto.what();
layer.surface = mMapper->getLayerHandle(layer.layerId);
@@ -431,6 +431,7 @@
layer.bufferData->frameNumber = bufferProto.frame_number();
layer.bufferData->flags = Flags<BufferData::BufferDataChange>(bufferProto.flags());
layer.bufferData->cachedBuffer.id = bufferProto.cached_buffer_id();
+ layer.bufferData->acquireFence = Fence::NO_FENCE;
}
if (proto.what() & layer_state_t::eApiChanged) {
@@ -450,23 +451,25 @@
}
if (proto.what() & layer_state_t::eReparent) {
- int32_t layerId = proto.parent_id();
+ int64_t layerId = proto.parent_id();
if (layerId == -1) {
layer.parentSurfaceControlForChild = nullptr;
} else {
layer.parentSurfaceControlForChild =
new SurfaceControl(SurfaceComposerClient::getDefault(),
- mMapper->getLayerHandle(layerId), nullptr, layerId);
+ mMapper->getLayerHandle(static_cast<int32_t>(layerId)),
+ nullptr, static_cast<int32_t>(layerId));
}
}
if (proto.what() & layer_state_t::eRelativeLayerChanged) {
- int32_t layerId = proto.relative_parent_id();
+ int64_t layerId = proto.relative_parent_id();
if (layerId == -1) {
layer.relativeLayerSurfaceControl = nullptr;
} else {
layer.relativeLayerSurfaceControl =
new SurfaceControl(SurfaceComposerClient::getDefault(),
- mMapper->getLayerHandle(layerId), nullptr, layerId);
+ mMapper->getLayerHandle(static_cast<int32_t>(layerId)),
+ nullptr, static_cast<int32_t>(layerId));
}
layer.z = proto.z();
}
@@ -493,8 +496,9 @@
inputInfo.transform.set(transformProto.tx(), transformProto.ty());
inputInfo.replaceTouchableRegionWithCrop =
windowInfoProto.replace_touchable_region_with_crop();
- int32_t layerId = windowInfoProto.crop_layer_id();
- inputInfo.touchableRegionCropHandle = mMapper->getLayerHandle(layerId);
+ int64_t layerId = windowInfoProto.crop_layer_id();
+ inputInfo.touchableRegionCropHandle =
+ mMapper->getLayerHandle(static_cast<int32_t>(layerId));
layer.windowInfoHandle = sp<gui::WindowInfoHandle>::make(inputInfo);
}
if (proto.what() & layer_state_t::eBackgroundColorChanged) {
diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.h b/services/surfaceflinger/Tracing/TransactionProtoParser.h
index 2f70b27..872a901 100644
--- a/services/surfaceflinger/Tracing/TransactionProtoParser.h
+++ b/services/surfaceflinger/Tracing/TransactionProtoParser.h
@@ -80,7 +80,8 @@
public:
virtual ~FlingerDataMapper() = default;
virtual sp<IBinder> getLayerHandle(int32_t /* layerId */) const { return nullptr; }
- virtual int32_t getLayerId(const sp<IBinder>& /* layerHandle */) const { return -1; }
+ virtual int64_t getLayerId(const sp<IBinder>& /* layerHandle */) const { return -1; }
+ virtual int64_t getLayerId(BBinder* /* layerHandle */) const { return -1; }
virtual sp<IBinder> getDisplayHandle(int32_t /* displayId */) const { return nullptr; }
virtual int32_t getDisplayId(const sp<IBinder>& /* displayHandle */) const { return -1; }
virtual std::shared_ptr<BufferData> getGraphicData(uint64_t bufferId, uint32_t width,
@@ -106,6 +107,7 @@
TransactionState fromProto(const proto::TransactionState&);
void mergeFromProto(const proto::LayerState&, TracingLayerState& outState);
void fromProto(const proto::LayerCreationArgs&, TracingLayerCreationArgs& outArgs);
+ std::unique_ptr<FlingerDataMapper> mMapper;
private:
proto::LayerState toProto(const layer_state_t&);
@@ -113,7 +115,6 @@
void fromProto(const proto::LayerState&, layer_state_t& out);
DisplayState fromProto(const proto::DisplayState&);
- std::unique_ptr<FlingerDataMapper> mMapper;
};
} // namespace android::surfaceflinger
diff --git a/services/surfaceflinger/Tracing/TransactionTracing.cpp b/services/surfaceflinger/Tracing/TransactionTracing.cpp
index d5e837f..6381758 100644
--- a/services/surfaceflinger/Tracing/TransactionTracing.cpp
+++ b/services/surfaceflinger/Tracing/TransactionTracing.cpp
@@ -29,23 +29,16 @@
namespace android {
-class FlingerDataMapper : public TransactionProtoParser::FlingerDataMapper {
- std::unordered_map<BBinder* /* layerHandle */, int32_t /* layerId */>& mLayerHandles;
-
+// Keeps the binder address as the layer id so we can avoid holding the tracing lock in the
+// binder thread.
+class FlatDataMapper : public TransactionProtoParser::FlingerDataMapper {
public:
- FlingerDataMapper(std::unordered_map<BBinder* /* handle */, int32_t /* id */>& layerHandles)
- : mLayerHandles(layerHandles) {}
-
- int32_t getLayerId(const sp<IBinder>& layerHandle) const override {
+ virtual int64_t getLayerId(const sp<IBinder>& layerHandle) const {
if (layerHandle == nullptr) {
return -1;
}
- auto it = mLayerHandles.find(layerHandle->localBinder());
- if (it == mLayerHandles.end()) {
- ALOGW("Could not find layer handle %p", layerHandle->localBinder());
- return -1;
- }
- return it->second;
+
+ return reinterpret_cast<int64_t>(layerHandle->localBinder());
}
void getGraphicBufferPropertiesFromCache(client_cache_t cachedBuffer, uint64_t* outBufferId,
@@ -72,8 +65,33 @@
}
};
+class FlingerDataMapper : public FlatDataMapper {
+ std::unordered_map<BBinder* /* layerHandle */, int32_t /* layerId */>& mLayerHandles;
+
+public:
+ FlingerDataMapper(std::unordered_map<BBinder* /* handle */, int32_t /* id */>& layerHandles)
+ : mLayerHandles(layerHandles) {}
+
+ int64_t getLayerId(const sp<IBinder>& layerHandle) const override {
+ if (layerHandle == nullptr) {
+ return -1;
+ }
+ return getLayerId(layerHandle->localBinder());
+ }
+
+ int64_t getLayerId(BBinder* localBinder) const {
+ auto it = mLayerHandles.find(localBinder);
+ if (it == mLayerHandles.end()) {
+ ALOGW("Could not find layer handle %p", localBinder);
+ return -1;
+ }
+ return it->second;
+ }
+};
+
TransactionTracing::TransactionTracing()
- : mProtoParser(std::make_unique<FlingerDataMapper>(mLayerHandles)) {
+ : mProtoParser(std::make_unique<FlingerDataMapper>(mLayerHandles)),
+ mLockfreeProtoParser(std::make_unique<FlatDataMapper>()) {
std::scoped_lock lock(mTraceLock);
mBuffer.setSize(mBufferSizeInBytes);
@@ -129,9 +147,9 @@
}
void TransactionTracing::addQueuedTransaction(const TransactionState& transaction) {
- std::scoped_lock lock(mTraceLock);
- ATRACE_CALL();
- mQueuedTransactions[transaction.id] = mProtoParser.toProto(transaction);
+ proto::TransactionState* state =
+ new proto::TransactionState(mLockfreeProtoParser.toProto(transaction));
+ mTransactionQueue.push(state);
}
void TransactionTracing::addCommittedTransactions(std::vector<TransactionState>& transactions,
@@ -182,6 +200,38 @@
std::scoped_lock lock(mTraceLock);
std::vector<std::string> removedEntries;
proto::TransactionTraceEntry entryProto;
+
+ while (auto incomingTransaction = mTransactionQueue.pop()) {
+ auto transaction = *incomingTransaction;
+ int32_t layerCount = transaction.layer_changes_size();
+ for (int i = 0; i < layerCount; i++) {
+ auto layer = transaction.mutable_layer_changes(i);
+ layer->set_layer_id(
+ mProtoParser.mMapper->getLayerId(reinterpret_cast<BBinder*>(layer->layer_id())));
+ if ((layer->what() & layer_state_t::eReparent) && layer->parent_id() != -1) {
+ layer->set_parent_id(
+ mProtoParser.mMapper->getLayerId(reinterpret_cast<BBinder*>(
+ layer->parent_id())));
+ }
+
+ if ((layer->what() & layer_state_t::eRelativeLayerChanged) &&
+ layer->relative_parent_id() != -1) {
+ layer->set_relative_parent_id(
+ mProtoParser.mMapper->getLayerId(reinterpret_cast<BBinder*>(
+ layer->relative_parent_id())));
+ }
+
+ if (layer->has_window_info_handle() &&
+ layer->window_info_handle().crop_layer_id() != -1) {
+ auto input = layer->mutable_window_info_handle();
+ input->set_crop_layer_id(
+ mProtoParser.mMapper->getLayerId(reinterpret_cast<BBinder*>(
+ input->crop_layer_id())));
+ }
+ }
+ mQueuedTransactions[incomingTransaction->transaction_id()] = transaction;
+ delete incomingTransaction;
+ }
for (const CommittedTransactions& entry : committedTransactions) {
entryProto.set_elapsed_realtime_nanos(entry.timestamp);
entryProto.set_vsync_id(entry.vsyncId);
@@ -317,9 +367,9 @@
// Merge layer states to starting transaction state.
for (const proto::TransactionState& transaction : removedEntry.transactions()) {
for (const proto::LayerState& layerState : transaction.layer_changes()) {
- auto it = mStartingStates.find(layerState.layer_id());
+ auto it = mStartingStates.find((int32_t)layerState.layer_id());
if (it == mStartingStates.end()) {
- ALOGW("Could not find layer id %d", layerState.layer_id());
+ ALOGW("Could not find layer id %d", (int32_t)layerState.layer_id());
continue;
}
mProtoParser.mergeFromProto(layerState, it->second);
diff --git a/services/surfaceflinger/Tracing/TransactionTracing.h b/services/surfaceflinger/Tracing/TransactionTracing.h
index 95256c4..4c291f9 100644
--- a/services/surfaceflinger/Tracing/TransactionTracing.h
+++ b/services/surfaceflinger/Tracing/TransactionTracing.h
@@ -26,6 +26,7 @@
#include <thread>
#include "RingBuffer.h"
+#include "LocklessStack.h"
#include "TransactionProtoParser.h"
using namespace android::surfaceflinger;
@@ -78,6 +79,7 @@
size_t mBufferSizeInBytes GUARDED_BY(mTraceLock) = CONTINUOUS_TRACING_BUFFER_SIZE;
std::unordered_map<uint64_t, proto::TransactionState> mQueuedTransactions
GUARDED_BY(mTraceLock);
+ LocklessStack<proto::TransactionState> mTransactionQueue;
nsecs_t mStartingTimestamp GUARDED_BY(mTraceLock);
std::vector<proto::LayerCreationArgs> mCreatedLayers GUARDED_BY(mTraceLock);
std::unordered_map<BBinder* /* layerHandle */, int32_t /* layerId */> mLayerHandles
@@ -85,6 +87,9 @@
std::vector<int32_t /* layerId */> mRemovedLayerHandles GUARDED_BY(mTraceLock);
std::map<int32_t /* layerId */, TracingLayerState> mStartingStates GUARDED_BY(mTraceLock);
TransactionProtoParser mProtoParser GUARDED_BY(mTraceLock);
+ // Parses the transaction to proto without holding any tracing locks so we can generate proto
+ // in the binder thread without any contention.
+ TransactionProtoParser mLockfreeProtoParser;
// We do not want main thread to block so main thread will try to acquire mMainThreadLock,
// otherwise will push data to temporary container.
diff --git a/services/surfaceflinger/layerproto/transactions.proto b/services/surfaceflinger/layerproto/transactions.proto
index fcf4499..4f99b19 100644
--- a/services/surfaceflinger/layerproto/transactions.proto
+++ b/services/surfaceflinger/layerproto/transactions.proto
@@ -70,7 +70,7 @@
// Keep insync with layer_state_t
message LayerState {
- int32 layer_id = 1;
+ int64 layer_id = 1;
// Changes are split into ChangesLsb and ChangesMsb. First 32 bits are in ChangesLsb
// and the next 32 bits are in ChangesMsb. This is needed because enums have to be
// 32 bits and there's no nice way to put 64bit constants into .proto files.
@@ -161,8 +161,8 @@
Matrix22 matrix = 11;
float corner_radius = 12;
uint32 background_blur_radius = 13;
- int32 parent_id = 14;
- int32 relative_parent_id = 15;
+ int64 parent_id = 14;
+ int64 relative_parent_id = 15;
float alpha = 16;
message Color3 {
@@ -233,7 +233,7 @@
bool focusable = 5;
bool has_wallpaper = 6;
float global_scale_factor = 7;
- int32 crop_layer_id = 8;
+ int64 crop_layer_id = 8;
bool replace_touchable_region_with_crop = 9;
RectProto touchable_region_crop = 10;
Transform transform = 11;
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index 6ffc039..0ef8456 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -268,11 +268,8 @@
for (size_t i = 0; i < MISSED_FRAMES; i++) {
ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementMissedFrames());
}
- TimeStats::ClientCompositionRecord record;
- record.hadClientComposition = true;
-
for (size_t i = 0; i < CLIENT_COMPOSITION_FRAMES; i++) {
- ASSERT_NO_FATAL_FAILURE(mTimeStats->pushCompositionStrategyState(record));
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementClientCompositionFrames());
}
SFTimeStatsGlobalProto globalProto;
@@ -462,49 +459,19 @@
EXPECT_THAT(result, HasSubstr(expectedResult));
}
-TEST_F(TimeStatsTest, canIncreaseClientCompositionStats) {
+TEST_F(TimeStatsTest, canIncreaseClientCompositionReusedFrames) {
// this stat is not in the proto so verify by checking the string dump
- constexpr size_t COMPOSITION_STRATEGY_CHANGED_FRAMES = 1;
- constexpr size_t HAD_CLIENT_COMPOSITION_FRAMES = 2;
- constexpr size_t REUSED_CLIENT_COMPOSITION_FRAMES = 3;
- constexpr size_t COMPOSITION_STRATEGY_PREDICTION_SUCCEEDED_FRAMES = 4;
- constexpr size_t COMPOSITION_STRATEGY_PREDICTED_FRAMES = 5;
+ constexpr size_t CLIENT_COMPOSITION_REUSED_FRAMES = 2;
EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
- for (size_t i = 0; i <= COMPOSITION_STRATEGY_PREDICTED_FRAMES; i++) {
- TimeStats::ClientCompositionRecord record;
- record.hadClientComposition = i < HAD_CLIENT_COMPOSITION_FRAMES;
- record.changed = i < COMPOSITION_STRATEGY_CHANGED_FRAMES;
- record.reused = i < REUSED_CLIENT_COMPOSITION_FRAMES;
- record.predicted = i < COMPOSITION_STRATEGY_PREDICTED_FRAMES;
- record.predictionSucceeded = i < COMPOSITION_STRATEGY_PREDICTION_SUCCEEDED_FRAMES;
- mTimeStats->pushCompositionStrategyState(record);
+ for (size_t i = 0; i < CLIENT_COMPOSITION_REUSED_FRAMES; i++) {
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementClientCompositionReusedFrames());
}
const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING));
- std::string expected =
- "compositionStrategyChanges = " + std::to_string(COMPOSITION_STRATEGY_CHANGED_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
-
- expected = "clientCompositionFrames = " + std::to_string(HAD_CLIENT_COMPOSITION_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
-
- expected =
- "clientCompositionReusedFrames = " + std::to_string(REUSED_CLIENT_COMPOSITION_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
-
- expected = "compositionStrategyPredicted = " +
- std::to_string(COMPOSITION_STRATEGY_PREDICTED_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
-
- expected = "compositionStrategyPredictionSucceeded = " +
- std::to_string(COMPOSITION_STRATEGY_PREDICTION_SUCCEEDED_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
-
- expected = "compositionStrategyPredictionFailed = " +
- std::to_string(COMPOSITION_STRATEGY_PREDICTED_FRAMES -
- COMPOSITION_STRATEGY_PREDICTION_SUCCEEDED_FRAMES);
- EXPECT_THAT(result, HasSubstr(expected));
+ const std::string expectedResult =
+ "clientCompositionReusedFrames = " + std::to_string(CLIENT_COMPOSITION_REUSED_FRAMES);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
}
TEST_F(TimeStatsTest, canIncreaseRefreshRateSwitches) {
@@ -522,6 +489,21 @@
EXPECT_THAT(result, HasSubstr(expectedResult));
}
+TEST_F(TimeStatsTest, canIncreaseCompositionStrategyChanges) {
+ // this stat is not in the proto so verify by checking the string dump
+ constexpr size_t COMPOSITION_STRATEGY_CHANGES = 2;
+
+ EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
+ for (size_t i = 0; i < COMPOSITION_STRATEGY_CHANGES; i++) {
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementCompositionStrategyChanges());
+ }
+
+ const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING));
+ const std::string expectedResult =
+ "compositionStrategyChanges = " + std::to_string(COMPOSITION_STRATEGY_CHANGES);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+}
+
TEST_F(TimeStatsTest, canAverageFrameDuration) {
EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
mTimeStats->setPowerMode(PowerMode::ON);
@@ -854,7 +836,7 @@
ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementTotalFrames());
ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementMissedFrames());
- ASSERT_NO_FATAL_FAILURE(mTimeStats->pushCompositionStrategyState({}));
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementClientCompositionFrames());
ASSERT_NO_FATAL_FAILURE(mTimeStats->setPowerMode(PowerMode::ON));
mTimeStats->recordFrameDuration(std::chrono::nanoseconds(3ms).count(),
@@ -885,8 +867,9 @@
TEST_F(TimeStatsTest, canClearDumpOnlyTimeStats) {
// These stats are not in the proto so verify by checking the string dump.
EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
- ASSERT_NO_FATAL_FAILURE(mTimeStats->pushCompositionStrategyState({}));
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementClientCompositionReusedFrames());
ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementRefreshRateSwitches());
+ ASSERT_NO_FATAL_FAILURE(mTimeStats->incrementCompositionStrategyChanges());
mTimeStats->setPowerMode(PowerMode::ON);
mTimeStats->recordFrameDuration(std::chrono::nanoseconds(1ms).count(),
std::chrono::nanoseconds(5ms).count());
@@ -1049,10 +1032,8 @@
for (size_t i = 0; i < MISSED_FRAMES; i++) {
mTimeStats->incrementMissedFrames();
}
- TimeStats::ClientCompositionRecord record;
- record.hadClientComposition = true;
for (size_t i = 0; i < CLIENT_COMPOSITION_FRAMES; i++) {
- mTimeStats->pushCompositionStrategyState(record);
+ mTimeStats->incrementClientCompositionFrames();
}
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000);
diff --git a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
index ab893a3..f5e3b77 100644
--- a/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionProtoParserTest.cpp
@@ -81,7 +81,7 @@
sp<IBinder> getLayerHandle(int32_t id) const override {
return (id == 42) ? layerHandle : nullptr;
}
- int32_t getLayerId(const sp<IBinder>& handle) const override {
+ int64_t getLayerId(const sp<IBinder>& handle) const override {
return (handle == layerHandle) ? 42 : -1;
}
sp<IBinder> getDisplayHandle(int32_t id) const {
diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
index 0dee800..0a69b56 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
@@ -33,7 +33,10 @@
MOCK_METHOD0(miniDump, std::string());
MOCK_METHOD0(incrementTotalFrames, void());
MOCK_METHOD0(incrementMissedFrames, void());
+ MOCK_METHOD0(incrementClientCompositionFrames, void());
+ MOCK_METHOD0(incrementClientCompositionReusedFrames, void());
MOCK_METHOD0(incrementRefreshRateSwitches, void());
+ MOCK_METHOD0(incrementCompositionStrategyChanges, void());
MOCK_METHOD1(recordDisplayEventConnectionCount, void(int32_t));
MOCK_METHOD2(recordFrameDuration, void(nsecs_t, nsecs_t));
MOCK_METHOD2(recordRenderEngineDuration, void(nsecs_t, nsecs_t));
@@ -60,8 +63,6 @@
void(hardware::graphics::composer::V2_4::IComposerClient::PowerMode));
MOCK_METHOD2(recordRefreshRate, void(uint32_t, nsecs_t));
MOCK_METHOD1(setPresentFenceGlobal, void(const std::shared_ptr<FenceTime>&));
- MOCK_METHOD(void, pushCompositionStrategyState,
- (const android::TimeStats::ClientCompositionRecord&), (override));
};
} // namespace android::mock