diff options
19 files changed, 200 insertions, 339 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 7eb5cb4172..f32bb2403d 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -726,8 +726,7 @@ binder::Status InstalldNativeService::createAppDataLocked( if (flags & FLAG_STORAGE_SDK) { // Safe to ignore status since we can retry creating this by calling reconcileSdkData - auto ignore = createSdkSandboxDataPackageDirectory(uuid, packageName, userId, appId, - previousAppId, seInfo, flags); + auto ignore = createSdkSandboxDataPackageDirectory(uuid, packageName, userId, appId, flags); if (!ignore.isOk()) { PLOG(WARNING) << "Failed to create sdk data package directory for " << packageName; } @@ -746,7 +745,7 @@ binder::Status InstalldNativeService::createAppDataLocked( */ binder::Status InstalldNativeService::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) { + int32_t appId, int32_t flags) { int32_t sdkSandboxUid = multiuser_get_sdk_sandbox_uid(userId, appId); if (sdkSandboxUid == -1) { // There no valid sdk sandbox process for this app. Skip creation of data directory @@ -765,7 +764,7 @@ binder::Status InstalldNativeService::createSdkSandboxDataPackageDirectory( // /data/misc_{ce,de}/<user-id>/sdksandbox directory gets created by vold // during user creation - // Prepare the app directory + // Prepare the package directory auto packagePath = create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId, packageName.c_str()); #if SDK_DEBUG @@ -775,27 +774,6 @@ binder::Status InstalldNativeService::createSdkSandboxDataPackageDirectory( if (prepare_app_dir(packagePath, 0751, AID_SYSTEM, AID_SYSTEM)) { return error("Failed to prepare " + packagePath); } - - // Now prepare the shared directory which will be accessible by all codes - auto sharedPath = create_data_misc_sdk_sandbox_shared_path(uuid_, isCeData, userId, - packageName.c_str()); - - int32_t previousSdkSandboxUid = multiuser_get_sdk_sandbox_uid(userId, previousAppId); - int32_t cacheGid = multiuser_get_cache_gid(userId, appId); - if (cacheGid == -1) { - return exception(binder::Status::EX_ILLEGAL_STATE, - StringPrintf("cacheGid cannot be -1 for sdksandbox data")); - } - auto status = createAppDataDirs(sharedPath, sdkSandboxUid, AID_NOBODY, - &previousSdkSandboxUid, cacheGid, seInfo, 0700 | S_ISGID); - if (!status.isOk()) { - return status; - } - - // TODO(b/211763739): We also need to handle art profile creations - - // TODO(b/211763739): And return the CE inode of the sdksandbox root directory and - // app directory under it so we can clear contents while CE storage is locked } return ok(); @@ -848,8 +826,8 @@ binder::Status InstalldNativeService::reconcileSdkData( const android::os::ReconcileSdkDataArgs& args) { // Locking is performed depeer in the callstack. - return reconcileSdkData(args.uuid, args.packageName, args.sdkPackageNames, args.randomSuffixes, - args.userId, args.appId, args.previousAppId, args.seInfo, args.flags); + return reconcileSdkData(args.uuid, args.packageName, args.subDirNames, args.userId, args.appId, + args.previousAppId, args.seInfo, args.flags); } /** @@ -863,17 +841,14 @@ binder::Status InstalldNativeService::reconcileSdkData( * is to avoid having same per-sdk directory with different suffix. * - If a sdk level directory exist which is absent from sdkPackageNames, we remove it. */ -binder::Status InstalldNativeService::reconcileSdkData( - const std::optional<std::string>& uuid, const std::string& packageName, - const std::vector<std::string>& sdkPackageNames, - const std::vector<std::string>& randomSuffixes, int userId, int appId, int previousAppId, - const std::string& seInfo, int flags) { +binder::Status InstalldNativeService::reconcileSdkData(const std::optional<std::string>& uuid, + const std::string& packageName, + const std::vector<std::string>& subDirNames, + 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) { - CHECK_ARGUMENT_PACKAGE_NAME(sdkPackageName); - } LOCK_PACKAGE_USER(); #if SDK_DEBUG @@ -882,16 +857,9 @@ binder::Status InstalldNativeService::reconcileSdkData( const char* uuid_ = uuid ? uuid->c_str() : nullptr; - // Validate we have enough randomSuffixStrings - if (randomSuffixes.size() != sdkPackageNames.size()) { - return exception(binder::Status::EX_ILLEGAL_ARGUMENT, - StringPrintf("Not enough random suffix. Required %d, received %d.", - (int)sdkPackageNames.size(), (int)randomSuffixes.size())); - } - // Prepare the sdk package directory in case it's missing - const auto status = createSdkSandboxDataPackageDirectory(uuid, packageName, userId, appId, - previousAppId, seInfo, flags); + const auto status = + createSdkSandboxDataPackageDirectory(uuid, packageName, userId, appId, flags); if (!status.isOk()) { return status; } @@ -905,37 +873,22 @@ binder::Status InstalldNativeService::reconcileSdkData( } const bool isCeData = (currentFlag == FLAG_STORAGE_CE); - // Since random suffix provided will be random every time, we need to ensure we don't end up - // creating multuple directories for same sdk package with different suffixes. This - // is ensured by fetching all the existing sub directories and storing them so that we can - // check for existence later. We also remove unconsumed sdk directories in this check. const auto packagePath = create_data_misc_sdk_sandbox_package_path(uuid_, isCeData, userId, packageName.c_str()); - const std::unordered_set<std::string> expectedSdkNames(sdkPackageNames.begin(), - sdkPackageNames.end()); - // Store paths of per-sdk directory for sdk that already exists - std::unordered_map<std::string, std::string> sdkNamesThatExist; - - const auto subDirHandler = [&packagePath, &expectedSdkNames, &sdkNamesThatExist, - &res](const std::string& filename) { - auto filepath = packagePath + "/" + filename; - auto tokens = Split(filename, "@"); - if (tokens.size() != 2) { - // Not a per-sdk directory with random suffix - return; - } - auto sdkName = tokens[0]; + // Remove existing sub-directories not referred in subDirNames + const std::unordered_set<std::string> expectedSubDirNames(subDirNames.begin(), + subDirNames.end()); + const auto subDirHandler = [&packagePath, &expectedSubDirNames, + &res](const std::string& subDirName) { // Remove the per-sdk directory if it is not referred in - // expectedSdkNames - if (expectedSdkNames.find(sdkName) == expectedSdkNames.end()) { - if (delete_dir_contents_and_dir(filepath) != 0) { - res = error("Failed to delete " + filepath); + // expectedSubDirNames + if (expectedSubDirNames.find(subDirName) == expectedSubDirNames.end()) { + auto path = packagePath + "/" + subDirName; + if (delete_dir_contents_and_dir(path) != 0) { + res = error("Failed to delete " + path); return; } - } else { - // Otherwise, store it as existing sdk level directory - sdkNamesThatExist[sdkName] = filepath; } }; const int ec = foreach_subdir(packagePath, subDirHandler); @@ -944,19 +897,11 @@ binder::Status InstalldNativeService::reconcileSdkData( continue; } - // Create sdksandbox data directory for each sdksandbox package - for (int i = 0, size = sdkPackageNames.size(); i < size; i++) { - const std::string& sdkName = sdkPackageNames[i]; - const std::string& randomSuffix = randomSuffixes[i]; - std::string path; - if (const auto& it = sdkNamesThatExist.find(sdkName); it != sdkNamesThatExist.end()) { - // Already exists. Use existing path instead of creating a new one - path = it->second; - } else { - path = create_data_misc_sdk_sandbox_sdk_path(uuid_, isCeData, userId, - packageName.c_str(), sdkName.c_str(), - randomSuffix.c_str()); - } + // Now create the subDirNames + for (const auto& subDirName : subDirNames) { + const std::string path = + create_data_misc_sdk_sandbox_sdk_path(uuid_, isCeData, userId, + packageName.c_str(), subDirName.c_str()); // Create the directory along with cache and code_cache const int32_t cacheGid = multiuser_get_cache_gid(userId, appId); diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 1f0fc9cf0f..5c2184252c 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -211,8 +211,7 @@ private: 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); + int32_t flags); binder::Status clearSdkSandboxDataPackageDirectory(const std::optional<std::string>& uuid, const std::string& packageName, int32_t userId, int32_t flags); @@ -221,8 +220,7 @@ private: 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, + const std::vector<std::string>& subDirNames, int32_t userId, int32_t appId, int32_t previousAppId, const std::string& seInfo, int flags); binder::Status restoreconSdkDataLocked(const std::optional<std::string>& uuid, diff --git a/cmds/installd/TEST_MAPPING b/cmds/installd/TEST_MAPPING index 3f0fb6d2ba..8ccab4cc2d 100644 --- a/cmds/installd/TEST_MAPPING +++ b/cmds/installd/TEST_MAPPING @@ -30,6 +30,9 @@ }, { "name": "CtsCompilationTestCases" + }, + { + "name": "SdkSandboxStorageHostTest" } ] } diff --git a/cmds/installd/binder/android/os/ReconcileSdkDataArgs.aidl b/cmds/installd/binder/android/os/ReconcileSdkDataArgs.aidl index 2f794b16cc..583a36d580 100644 --- a/cmds/installd/binder/android/os/ReconcileSdkDataArgs.aidl +++ b/cmds/installd/binder/android/os/ReconcileSdkDataArgs.aidl @@ -20,8 +20,7 @@ package android.os; parcelable ReconcileSdkDataArgs { @nullable @utf8InCpp String uuid; @utf8InCpp String packageName; - @utf8InCpp List<String> sdkPackageNames; - @utf8InCpp List<String> randomSuffixes; + @utf8InCpp List<String> subDirNames; int userId; int appId; int previousAppId; diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp index 672ca7f52b..38cb37046d 100644 --- a/cmds/installd/tests/installd_service_test.cpp +++ b/cmds/installd/tests/installd_service_test.cpp @@ -45,7 +45,7 @@ #include "utils.h" using android::base::StringPrintf; -namespace fs = std::filesystem; +using std::filesystem::is_empty; namespace android { std::string get_package_name(uid_t uid) { @@ -79,12 +79,15 @@ std::string get_package_name(uid_t uid) { namespace installd { static constexpr const char* kTestUuid = "TEST"; -static constexpr const char* kTestPath = "/data/local/tmp"; +static const std::string 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; +static constexpr const int FLAG_CLEAR_CACHE_ONLY = InstalldNativeService::FLAG_CLEAR_CACHE_ONLY; +static constexpr const int FLAG_CLEAR_CODE_CACHE_ONLY = + InstalldNativeService::FLAG_CLEAR_CODE_CACHE_ONLY; const gid_t kTestAppUid = multiuser_get_uid(kTestUserId, kTestAppId); const gid_t kTestCacheGid = multiuser_get_cache_gid(kTestUserId, kTestAppId); @@ -111,7 +114,7 @@ bool create_cache_path(char path[PKG_PATH_MAX], const char *src, const char *ins } static std::string get_full_path(const std::string& path) { - return StringPrintf("%s/%s", kTestPath, path.c_str()); + return StringPrintf("%s/%s", kTestPath.c_str(), path.c_str()); } static void mkdir(const std::string& path, uid_t owner, gid_t group, mode_t mode) { @@ -169,10 +172,9 @@ static bool find_file(const char* path, Pred&& pred) { } 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); - }); + return find_file((kTestPath + rootDirectory).c_str(), [](const std::string& name, bool is_dir) { + return is_dir && is_renamed_deleted_dir(name); + }); } class ServiceTest : public testing::Test { @@ -992,16 +994,12 @@ public: } android::os::ReconcileSdkDataArgs reconcileSdkDataArgs( - std::string packageName, std::vector<std::string> codeNames, - std::vector<std::string> randomSuffixes) { + const std::string& packageName, const std::vector<std::string>& subDirNames) { android::os::ReconcileSdkDataArgs args; args.uuid = kTestUuid; args.packageName = packageName; - for (const auto& codeName : codeNames) { - args.sdkPackageNames.push_back(codeName); - } - for (const auto& randomSuffix : randomSuffixes) { - args.randomSuffixes.push_back(randomSuffix); + for (const auto& subDirName : subDirNames) { + args.subDirNames.push_back(subDirName); } args.userId = kTestUserId; args.appId = kTestAppId; @@ -1051,24 +1049,12 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData) { const std::string fooCePath = "misc_ce/0/sdksandbox/com.foo"; CheckFileAccess(fooCePath, kSystemUid, kSystemUid, S_IFDIR | 0751); - CheckFileAccess(fooCePath + "/shared", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | S_ISGID | 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, kSystemUid, S_IFDIR | 0751); - CheckFileAccess(fooDePath + "/shared", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | S_ISGID | 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) { +TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData_WithoutSdkFlag) { android::os::CreateAppDataResult result; android::os::CreateAppDataArgs args = createAppDataArgs("com.foo"); args.flags = FLAG_STORAGE_CE | FLAG_STORAGE_DE; @@ -1080,7 +1066,7 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutSdkFlag) ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo")); } -TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutSdkFlagDeletesExisting) { +TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData_WithoutSdkFlagDeletesExisting) { android::os::CreateAppDataResult result; android::os::CreateAppDataArgs args = createAppDataArgs("com.foo"); // Create the app user data. @@ -1094,7 +1080,7 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutSdkFlagDe ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo")); } -TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutDeFlag) { +TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData_WithoutDeFlag) { android::os::CreateAppDataResult result; android::os::CreateAppDataArgs args = createAppDataArgs("com.foo"); args.flags = FLAG_STORAGE_CE | FLAG_STORAGE_SDK; @@ -1109,7 +1095,7 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutDeFlag) { ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo")); } -TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutCeFlag) { +TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData_WithoutCeFlag) { android::os::CreateAppDataResult result; android::os::CreateAppDataArgs args = createAppDataArgs("com.foo"); args.flags = FLAG_STORAGE_DE | FLAG_STORAGE_SDK; @@ -1126,7 +1112,7 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkAppLevelData_WithoutCeFlag) { TEST_F(SdkSandboxDataTest, ReconcileSdkData) { android::os::ReconcileSdkDataArgs args = - reconcileSdkDataArgs("com.foo", {"bar", "baz"}, {"random1", "random2"}); + reconcileSdkDataArgs("com.foo", {"bar@random1", "baz@random2"}); // Create the sdk data. ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); @@ -1160,59 +1146,15 @@ TEST_F(SdkSandboxDataTest, ReconcileSdkData) { S_IFDIR | S_ISGID | 0771); } -TEST_F(SdkSandboxDataTest, ReconcileSdkData_PackageNameCannotUseRandomSuffixSeparator) { - android::os::ReconcileSdkDataArgs args = - reconcileSdkDataArgs("com.foo", {"bar@illegal"}, {"random1"}); - - // Create the sdksandbox data. - auto status = service->reconcileSdkData(args); - ASSERT_EQ(status.exceptionCode(), binder::Status::EX_ILLEGAL_ARGUMENT); - ASSERT_EQ(status.exceptionMessage(), "Package name bar@illegal is malformed"); -} - -TEST_F(SdkSandboxDataTest, ReconcileSdkData_NotEnoughRandomSuffix) { - android::os::ReconcileSdkDataArgs args = - reconcileSdkDataArgs("com.foo", {"bar", "baz"}, {"random1"}); - - // Create the sdksandbox data. - auto status = service->reconcileSdkData(args); - ASSERT_EQ(status.exceptionCode(), binder::Status::EX_ILLEGAL_ARGUMENT); - ASSERT_EQ(status.exceptionMessage(), "Not enough random suffix. Required 2, received 1."); -} - -TEST_F(SdkSandboxDataTest, ReconcileSdkData_DirectoryNotCreatedIfAlreadyExistsIgnoringSuffix) { - android::os::ReconcileSdkDataArgs args = - reconcileSdkDataArgs("com.foo", {"bar", "baz"}, {"random1", "random2"}); - - // Create the sdksandbox data. - ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); - - // Retry with different random suffix - args.randomSuffixes[0] = "r10"; - args.randomSuffixes[1] = "r20"; - - // Create the sdksandbox data again - ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); - - // Previous directories from first attempt should exist - CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar@random1", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | S_ISGID | 0700); - CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | S_ISGID | 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")); -} - TEST_F(SdkSandboxDataTest, ReconcileSdkData_ExtraCodeDirectoriesAreDeleted) { android::os::ReconcileSdkDataArgs args = - reconcileSdkDataArgs("com.foo", {"bar", "baz"}, {"random1", "random2"}); + reconcileSdkDataArgs("com.foo", {"bar@random1", "baz@random2"}); // Create the sdksandbox data. ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); // Retry with different package name - args.sdkPackageNames[0] = "bar.diff"; + args.subDirNames[0] = "bar.diff@random1"; // Create the sdksandbox data again ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); @@ -1272,130 +1214,90 @@ 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"}; + ASSERT_TRUE(mkdirs(cePackagePath, 0700)); + ASSERT_TRUE(mkdirs(dePackagePath, 0700)); + const std::vector<std::string> packagePaths = {cePackagePath, dePackagePath}; + for (const auto& packagePath : packagePaths) { + for (auto sdkName : sdkNames) { + ASSERT_TRUE(mkdirs(packagePath + "/" + sdkName + "/cache", 0700)); + ASSERT_TRUE(mkdirs(packagePath + "/" + sdkName + "/code_cache", 0700)); + std::ofstream{packagePath + "/" + sdkName + "/cache/cachedTestData.txt"}; + std::ofstream{packagePath + "/" + 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"}); + createTestSdkData("com.foo", {"shared", "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"))); + ASSERT_BINDER_SUCCESS(service->clearAppData(kTestUuid, "com.foo", 0, + FLAG_STORAGE_CE | FLAG_CLEAR_CACHE_ONLY, -1)); + + const std::string packagePath = kTestPath + "/misc_ce/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared/cache")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1/cache")); + ASSERT_TRUE(is_empty(packagePath + "/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"}); + createTestSdkData("com.foo", {"shared", "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"))); + ASSERT_BINDER_SUCCESS(service->clearAppData(kTestUuid, "com.foo", 0, + FLAG_STORAGE_CE | FLAG_CLEAR_CODE_CACHE_ONLY, -1)); + + const std::string packagePath = kTestPath + "/misc_ce/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared/code_cache")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1/code_cache")); + ASSERT_TRUE(is_empty(packagePath + "/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"}); + createTestSdkData("com.foo", {"shared", "sdk1", "sdk2"}); // Clear the app user data ASSERT_BINDER_SUCCESS( - service->clearAppData(args.uuid, args.packageName, args.userId, + service->clearAppData(kTestUuid, "com.foo", 0, 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"))); + -1)); + + const std::string packagePath = kTestPath + "/misc_de/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared/cache")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1/cache")); + ASSERT_TRUE(is_empty(packagePath + "/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"}); + createTestSdkData("com.foo", {"shared", "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"))); + ASSERT_BINDER_SUCCESS(service->clearAppData(kTestUuid, "com.foo", 0, + FLAG_STORAGE_DE | FLAG_CLEAR_CODE_CACHE_ONLY, -1)); + + const std::string packagePath = kTestPath + "/misc_de/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared/code_cache")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1/code_cache")); + ASSERT_TRUE(is_empty(packagePath + "/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"}); + createTestSdkData("com.foo", {"shared", "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"))); + ASSERT_BINDER_SUCCESS(service->clearAppData(kTestUuid, "com.foo", 0, FLAG_STORAGE_CE, -1)); + + const std::string packagePath = kTestPath + "/misc_ce/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1")); + ASSERT_TRUE(is_empty(packagePath + "/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"}); + createTestSdkData("com.foo", {"shared", "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"))); + ASSERT_BINDER_SUCCESS(service->clearAppData(kTestUuid, "com.foo", 0, FLAG_STORAGE_DE, -1)); + + const std::string packagePath = kTestPath + "/misc_de/0/sdksandbox/com.foo"; + ASSERT_TRUE(is_empty(packagePath + "/shared")); + ASSERT_TRUE(is_empty(packagePath + "/sdk1")); + ASSERT_TRUE(is_empty(packagePath + "/sdk2")); } class DestroyUserDataTest : public SdkSandboxDataTest {}; diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp index 38c1c05c53..910cd630f3 100644 --- a/cmds/installd/tests/installd_utils_test.cpp +++ b/cmds/installd/tests/installd_utils_test.cpp @@ -690,11 +690,11 @@ TEST_F(UtilsTest, TestSdkSandboxDataPaths) { create_data_misc_sdk_sandbox_package_path(nullptr, true, 10, "com.foo")); EXPECT_EQ("/data/misc_ce/0/sdksandbox/com.foo/shared", - create_data_misc_sdk_sandbox_shared_path(nullptr, true, 0, "com.foo")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, true, 0, "com.foo", "shared")); EXPECT_EQ("/data/misc_ce/10/sdksandbox/com.foo/shared", - create_data_misc_sdk_sandbox_shared_path(nullptr, true, 10, "com.foo")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, true, 10, "com.foo", "shared")); EXPECT_EQ("/data/misc_ce/10/sdksandbox/com.foo/bar@random", - create_data_misc_sdk_sandbox_sdk_path(nullptr, true, 10, "com.foo", "bar", "random")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, true, 10, "com.foo", "bar@random")); // De data paths EXPECT_EQ("/data/misc_de/0/sdksandbox", @@ -707,12 +707,11 @@ TEST_F(UtilsTest, TestSdkSandboxDataPaths) { create_data_misc_sdk_sandbox_package_path(nullptr, false, 10, "com.foo")); EXPECT_EQ("/data/misc_de/0/sdksandbox/com.foo/shared", - create_data_misc_sdk_sandbox_shared_path(nullptr, false, 0, "com.foo")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, false, 0, "com.foo", "shared")); EXPECT_EQ("/data/misc_de/10/sdksandbox/com.foo/shared", - create_data_misc_sdk_sandbox_shared_path(nullptr, false, 10, "com.foo")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, false, 10, "com.foo", "shared")); EXPECT_EQ("/data/misc_de/10/sdksandbox/com.foo/bar@random", - create_data_misc_sdk_sandbox_sdk_path(nullptr, false, 10, "com.foo", "bar", - "random")); + create_data_misc_sdk_sandbox_sdk_path(nullptr, false, 10, "com.foo", "bar@random")); } TEST_F(UtilsTest, WaitChild) { diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp index 8cfd12313b..123e3d4181 100644 --- a/cmds/installd/utils.cpp +++ b/cmds/installd/utils.cpp @@ -223,28 +223,17 @@ std::string create_data_misc_sdk_sandbox_package_path(const char* volume_uuid, b } /** - * Create the path name where shared code data for a particular app will be stored. - * E.g. /data/misc_ce/0/sdksandbox/<package-name>/shared - */ -std::string create_data_misc_sdk_sandbox_shared_path(const char* volume_uuid, bool isCeData, - userid_t user, const char* package_name) { - return StringPrintf("%s/shared", - create_data_misc_sdk_sandbox_package_path(volume_uuid, isCeData, user, - package_name) - .c_str()); -} - -/** - * Create the path name where per-code level data for a particular app will be stored. - * E.g. /data/misc_ce/0/sdksandbox/<package-name>/<sdk-name>-<random-suffix> + * Create the path name where sdk data for a particular sdk will be stored. + * E.g. /data/misc_ce/0/sdksandbox/<package-name>/com.foo@randomstrings */ std::string create_data_misc_sdk_sandbox_sdk_path(const char* volume_uuid, bool isCeData, userid_t user, const char* package_name, - const char* sdk_name, const char* randomSuffix) { - check_package_name(sdk_name); - auto package_path = - create_data_misc_sdk_sandbox_package_path(volume_uuid, isCeData, user, package_name); - return StringPrintf("%s/%s@%s", package_path.c_str(), sdk_name, randomSuffix); + const char* sub_dir_name) { + return StringPrintf("%s/%s", + create_data_misc_sdk_sandbox_package_path(volume_uuid, isCeData, user, + package_name) + .c_str(), + sub_dir_name); } std::string create_data_misc_ce_rollback_base_path(const char* volume_uuid, userid_t user) { diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h index 54d77f95d6..cb3099337c 100644 --- a/cmds/installd/utils.h +++ b/cmds/installd/utils.h @@ -65,11 +65,9 @@ std::string create_data_misc_sdk_sandbox_path(const char* volume_uuid, bool isCe userid_t userid); std::string create_data_misc_sdk_sandbox_package_path(const char* volume_uuid, bool isCeData, userid_t userid, const char* package_name); -std::string create_data_misc_sdk_sandbox_shared_path(const char* volume_uuid, bool isCeData, - userid_t userid, const char* package_name); std::string create_data_misc_sdk_sandbox_sdk_path(const char* volume_uuid, bool isCeData, userid_t userid, const char* package_name, - const char* sdk_name, const char* randomSuffix); + const char* sub_dir_name); std::string create_data_misc_ce_rollback_base_path(const char* volume_uuid, userid_t user); std::string create_data_misc_de_rollback_base_path(const char* volume_uuid, userid_t user); diff --git a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_parcel.h b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_parcel.h index 749bf212e6..633626ca44 100644 --- a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_parcel.h +++ b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_parcel.h @@ -19,13 +19,18 @@ #include <binder/Parcel.h> #include <fuzzer/FuzzedDataProvider.h> +#include <functional> + namespace android { /** * Fill parcel data, including some random binder objects and FDs + * + * p - the Parcel to fill + * provider - takes ownership and completely consumes provider + * writeHeader - optional function to write a specific header once the format of the parcel is + * picked (for instance, to write an interface header) */ -void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider); -/** - * Fill parcel data, but don't fill any objects. - */ -void fillRandomParcelData(Parcel* p, FuzzedDataProvider&& provider); +void fillRandomParcel( + Parcel* p, FuzzedDataProvider&& provider, + std::function<void(Parcel* p, FuzzedDataProvider& provider)> writeHeader = nullptr); } // namespace android diff --git a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp index e849c9bbce..be39bb9195 100644 --- a/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp +++ b/libs/binder/tests/parcel_fuzzer/libbinder_driver.cpp @@ -27,7 +27,14 @@ void fuzzService(const sp<IBinder>& binder, FuzzedDataProvider&& provider) { std::vector<uint8_t> subData = provider.ConsumeBytes<uint8_t>( provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); - fillRandomParcel(&data, FuzzedDataProvider(subData.data(), subData.size())); + fillRandomParcel(&data, FuzzedDataProvider(subData.data(), subData.size()), + [&binder](Parcel* p, FuzzedDataProvider& provider) { + // most code will be behind checks that the head of the Parcel + // is exactly this, so make it easier for fuzzers to reach this + if (provider.ConsumeBool()) { + p->writeInterfaceToken(binder->getInterfaceDescriptor()); + } + }); Parcel reply; (void)binder->transact(code, data, &reply, flags); diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp index 8bf04ccae0..cfabc1e6b5 100644 --- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp +++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp @@ -34,15 +34,26 @@ private: String16 mDescriptor; }; -void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) { +static void fillRandomParcelData(Parcel* p, FuzzedDataProvider&& provider) { + std::vector<uint8_t> data = provider.ConsumeBytes<uint8_t>(provider.remaining_bytes()); + CHECK(OK == p->write(data.data(), data.size())); +} + +void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider, + std::function<void(Parcel* p, FuzzedDataProvider& provider)> writeHeader) { if (provider.ConsumeBool()) { auto session = RpcSession::make(RpcTransportCtxFactoryRaw::make()); CHECK_EQ(OK, session->addNullDebuggingClient()); p->markForRpc(session); + + writeHeader(p, provider); + fillRandomParcelData(p, std::move(provider)); return; } + writeHeader(p, provider); + while (provider.remaining_bytes() > 0) { auto fillFunc = provider.PickValueInArray<const std::function<void()>>({ // write data @@ -85,9 +96,4 @@ void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) { } } -void fillRandomParcelData(Parcel* p, FuzzedDataProvider&& provider) { - std::vector<uint8_t> data = provider.ConsumeBytes<uint8_t>(provider.remaining_bytes()); - CHECK(OK == p->write(data.data(), data.size())); -} - } // namespace android diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 0fb16f2f12..48a9bc50c4 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -1,4 +1,3 @@ - /* * Copyright (C) 2007 The Android Open Source Project * diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp index a1e14559e9..896f25404d 100644 --- a/services/surfaceflinger/LayerRenderArea.cpp +++ b/services/surfaceflinger/LayerRenderArea.cpp @@ -26,18 +26,12 @@ namespace android { namespace { -struct ReparentForDrawing { - const sp<Layer>& oldParent; - - ReparentForDrawing(const sp<Layer>& oldParent, const sp<Layer>& newParent, - const Rect& drawingBounds) - : oldParent(oldParent) { +void reparentForDrawing(const sp<Layer>& oldParent, const sp<Layer>& newParent, + const Rect& drawingBounds) { // Compute and cache the bounds for the new parent layer. newParent->computeBounds(drawingBounds.toFloatRect(), ui::Transform(), - 0.f /* shadowRadius */); + 0.f /* shadowRadius */); oldParent->setChildrenDrawingParent(newParent); - } - ~ReparentForDrawing() { oldParent->setChildrenDrawingParent(oldParent); } }; } // namespace @@ -114,11 +108,19 @@ void LayerRenderArea::render(std::function<void()> drawLayers) { } else { // In the "childrenOnly" case we reparent the children to a screenshot // layer which has no properties set and which does not draw. + // We hold the statelock as the reparent-for-drawing operation modifies the + // hierarchy and there could be readers on Binder threads, like dump. sp<ContainerLayer> screenshotParentLayer = mFlinger.getFactory().createContainerLayer( - {&mFlinger, nullptr, "Screenshot Parent"s, 0, LayerMetadata()}); - - ReparentForDrawing reparent(mLayer, screenshotParentLayer, sourceCrop); + {&mFlinger, nullptr, "Screenshot Parent"s, 0, LayerMetadata()}); + { + Mutex::Autolock _l(mFlinger.mStateLock); + reparentForDrawing(mLayer, screenshotParentLayer, sourceCrop); + } drawLayers(); + { + Mutex::Autolock _l(mFlinger.mStateLock); + mLayer->setChildrenDrawingParent(mLayer); + } } } diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h index 6a906944a7..41273e01c1 100644 --- a/services/surfaceflinger/LayerRenderArea.h +++ b/services/surfaceflinger/LayerRenderArea.h @@ -46,6 +46,7 @@ public: Rect getSourceCrop() const override; void render(std::function<void()> drawLayers) override; + virtual sp<Layer> getParentLayer() const { return mLayer; } private: const sp<Layer> mLayer; @@ -58,4 +59,4 @@ private: const bool mChildrenOnly; }; -} // namespace android
\ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index c9f7f46953..387364c03a 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -4,6 +4,7 @@ #include <ui/Transform.h> #include <functional> +#include "Layer.h" namespace android { @@ -85,6 +86,10 @@ public: // Returns the source display viewport. const Rect& getLayerStackSpaceRect() const { return mLayerStackSpaceRect; } + // If this is a LayerRenderArea, return the root layer of the + // capture operation. + virtual sp<Layer> getParentLayer() const { return nullptr; } + protected: const bool mAllowSecureLayers; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 425b78bd68..268036ce0c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6499,19 +6499,6 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, // and failed if display is not in native mode. This provide a way to force using native // colors when capture. dataspace = args.dataspace; - if (dataspace == ui::Dataspace::UNKNOWN) { - auto display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { - return display.getLayerStack() == layerStack; - }); - if (!display) { - // If the layer is not on a display, use the dataspace for the default display. - display = getDefaultDisplayDeviceLocked(); - } - - const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode; - dataspace = pickDataspaceFromColorMode(colorMode); - } - } // mStateLock // really small crop or frameScale @@ -6640,7 +6627,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre renderArea->render([&] { renderEngineResultFuture = - renderScreenImplLocked(*renderArea, traverseLayers, buffer, + renderScreenImpl(*renderArea, traverseLayers, buffer, canCaptureBlackoutContent, regionSampling, grayscale, captureResults); }); @@ -6673,7 +6660,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre } } -std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScreenImplLocked( +std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScreenImpl( const RenderArea& renderArea, TraverseLayersFunction traverseLayers, const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool canCaptureBlackoutContent, bool regionSampling, bool grayscale, @@ -6697,7 +6684,22 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree } captureResults.buffer = buffer->getBuffer(); - captureResults.capturedDataspace = renderArea.getReqDataSpace(); + auto dataspace = renderArea.getReqDataSpace(); + auto parent = renderArea.getParentLayer(); + if ((dataspace == ui::Dataspace::UNKNOWN) && (parent != nullptr)) { + Mutex::Autolock lock(mStateLock); + auto display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { + return display.getLayerStack() == layerStack; + }); + if (!display) { + // If the layer is not on a display, use the dataspace for the default display. + display = getDefaultDisplayDeviceLocked(); + } + + const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode; + dataspace = pickDataspaceFromColorMode(colorMode); + } + captureResults.capturedDataspace = dataspace; const auto reqWidth = renderArea.getReqWidth(); const auto reqHeight = renderArea.getReqHeight(); @@ -6715,7 +6717,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree clientCompositionDisplay.clip = sourceCrop; clientCompositionDisplay.orientation = rotation; - clientCompositionDisplay.outputDataspace = renderArea.getReqDataSpace(); + clientCompositionDisplay.outputDataspace = dataspace; clientCompositionDisplay.maxLuminance = DisplayDevice::sDefaultMaxLumiance; const float colorSaturation = grayscale ? 0 : 1; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index fa6580342b..81afa9b0fa 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -375,6 +375,7 @@ private: friend class MonitoredProducer; friend class RefreshRateOverlay; friend class RegionSamplingThread; + friend class LayerRenderArea; friend class LayerTracing; // For unit tests @@ -863,10 +864,10 @@ private: RenderAreaFuture, TraverseLayersFunction, const std::shared_ptr<renderengine::ExternalTexture>&, bool regionSampling, bool grayscale, const sp<IScreenCaptureListener>&); - std::shared_future<renderengine::RenderEngineResult> renderScreenImplLocked( + std::shared_future<renderengine::RenderEngineResult> renderScreenImpl( const RenderArea&, TraverseLayersFunction, const std::shared_ptr<renderengine::ExternalTexture>&, bool canCaptureBlackoutContent, - bool regionSampling, bool grayscale, ScreenCaptureResults&); + bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock); // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a // matching ownerUid diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 16690757e9..15c9d199cd 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -244,8 +244,8 @@ void CompositionTest::captureScreenComposition() { HAL_PIXEL_FORMAT_RGBA_8888, 1, usage); - auto result = mFlinger.renderScreenImplLocked(*renderArea, traverseLayers, mCaptureScreenBuffer, - forSystem, regionSampling); + auto result = mFlinger.renderScreenImpl(*renderArea, traverseLayers, mCaptureScreenBuffer, + forSystem, regionSampling); EXPECT_TRUE(result.valid()); auto& [status, drawFence] = result.get(); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index fe0564e0d3..6780108c4f 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -400,12 +400,12 @@ public: return mFlinger->setPowerModeInternal(display, mode); } - auto renderScreenImplLocked(const RenderArea& renderArea, + auto renderScreenImpl(const RenderArea& renderArea, SurfaceFlinger::TraverseLayersFunction traverseLayers, const std::shared_ptr<renderengine::ExternalTexture>& buffer, bool forSystem, bool regionSampling) { ScreenCaptureResults captureResults; - return mFlinger->renderScreenImplLocked(renderArea, traverseLayers, buffer, forSystem, + return mFlinger->renderScreenImpl(renderArea, traverseLayers, buffer, forSystem, regionSampling, false /* grayscale */, captureResults); } |