diff options
94 files changed, 1727 insertions, 661 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index d7c9b40d92..e14af77194 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -471,6 +471,49 @@ status_t InstalldNativeService::dump(int fd, const Vector<String16>& /* args */) return NO_ERROR; } +constexpr const char kXattrRestoreconInProgress[] = "user.restorecon_in_progress"; + +static std::string lgetfilecon(const std::string& path) { + char* context; + if (::lgetfilecon(path.c_str(), &context) < 0) { + PLOG(ERROR) << "Failed to lgetfilecon for " << path; + return {}; + } + std::string result{context}; + free(context); + return result; +} + +static bool getRestoreconInProgress(const std::string& path) { + bool inProgress = false; + if (getxattr(path.c_str(), kXattrRestoreconInProgress, &inProgress, sizeof(inProgress)) != + sizeof(inProgress)) { + if (errno != ENODATA) { + PLOG(ERROR) << "Failed to check in-progress restorecon for " << path; + } + return false; + } + return inProgress; +} + +struct RestoreconInProgress { + explicit RestoreconInProgress(const std::string& path) : mPath(path) { + bool inProgress = true; + if (setxattr(mPath.c_str(), kXattrRestoreconInProgress, &inProgress, sizeof(inProgress), + 0) != 0) { + PLOG(ERROR) << "Failed to set in-progress restorecon for " << path; + } + } + ~RestoreconInProgress() { + if (removexattr(mPath.c_str(), kXattrRestoreconInProgress) < 0) { + PLOG(ERROR) << "Failed to clear in-progress restorecon for " << mPath; + } + } + +private: + const std::string& mPath; +}; + /** * Perform restorecon of the given path, but only perform recursive restorecon * if the label of that top-level file actually changed. This can save us @@ -479,56 +522,56 @@ status_t InstalldNativeService::dump(int fd, const Vector<String16>& /* args */) static int restorecon_app_data_lazy(const std::string& path, const std::string& seInfo, uid_t uid, bool existing) { ScopedTrace tracer("restorecon-lazy"); - int res = 0; - char* before = nullptr; - char* after = nullptr; if (!existing) { ScopedTrace tracer("new-path"); if (selinux_android_restorecon_pkgdir(path.c_str(), seInfo.c_str(), uid, SELINUX_ANDROID_RESTORECON_RECURSE) < 0) { PLOG(ERROR) << "Failed recursive restorecon for " << path; - goto fail; + return -1; } - return res; + return 0; } - // Note that SELINUX_ANDROID_RESTORECON_DATADATA flag is set by - // libselinux. Not needed here. - if (lgetfilecon(path.c_str(), &before) < 0) { - PLOG(ERROR) << "Failed before getfilecon for " << path; - goto fail; - } - if (selinux_android_restorecon_pkgdir(path.c_str(), seInfo.c_str(), uid, 0) < 0) { - PLOG(ERROR) << "Failed top-level restorecon for " << path; - goto fail; - } - if (lgetfilecon(path.c_str(), &after) < 0) { - PLOG(ERROR) << "Failed after getfilecon for " << path; - goto fail; + // Note that SELINUX_ANDROID_RESTORECON_DATADATA flag is set by libselinux. Not needed here. + + // Check to see if there was an interrupted operation. + bool inProgress = getRestoreconInProgress(path); + std::string before, after; + if (!inProgress) { + if (before = lgetfilecon(path); before.empty()) { + PLOG(ERROR) << "Failed before getfilecon for " << path; + return -1; + } + if (selinux_android_restorecon_pkgdir(path.c_str(), seInfo.c_str(), uid, 0) < 0) { + PLOG(ERROR) << "Failed top-level restorecon for " << path; + return -1; + } + if (after = lgetfilecon(path); after.empty()) { + PLOG(ERROR) << "Failed after getfilecon for " << path; + return -1; + } } // If the initial top-level restorecon above changed the label, then go // back and restorecon everything recursively - if (strcmp(before, after)) { + if (inProgress || before != after) { ScopedTrace tracer("label-change"); if (existing) { LOG(DEBUG) << "Detected label change from " << before << " to " << after << " at " << path << "; running recursive restorecon"; } + + // Temporary mark the folder as "in-progress" to resume in case of reboot/other failure. + RestoreconInProgress fence(path); + if (selinux_android_restorecon_pkgdir(path.c_str(), seInfo.c_str(), uid, SELINUX_ANDROID_RESTORECON_RECURSE) < 0) { PLOG(ERROR) << "Failed recursive restorecon for " << path; - goto fail; + return -1; } } - goto done; -fail: - res = -1; -done: - free(before); - free(after); - return res; + return 0; } static bool internal_storage_has_project_id() { // The following path is populated in setFirstBoot, so if this file is present @@ -3283,7 +3326,7 @@ binder::Status InstalldNativeService::linkNativeLibraryDirectory( } char *con = nullptr; - if (lgetfilecon(pkgdir, &con) < 0) { + if (::lgetfilecon(pkgdir, &con) < 0) { return error("Failed to lgetfilecon " + _pkgdir); } diff --git a/cmds/sfdo/sfdo.cpp b/cmds/sfdo/sfdo.cpp index 55326ea737..de0e1718ab 100644 --- a/cmds/sfdo/sfdo.cpp +++ b/cmds/sfdo/sfdo.cpp @@ -16,7 +16,7 @@ #include <inttypes.h> #include <stdint.h> #include <any> -#include <unordered_map> +#include <map> #include <cutils/properties.h> #include <sys/resource.h> @@ -29,18 +29,28 @@ using namespace android; -std::unordered_map<std::string, std::any> g_functions; +std::map<std::string, std::any> g_functions; -const std::unordered_map<std::string, std::string> g_function_details = { - {"DebugFlash", "[optional(delay)] Perform a debug flash."}, - {"FrameRateIndicator", "[hide | show] displays the framerate in the top left corner."}, - {"scheduleComposite", "Force composite ahead of next VSYNC."}, - {"scheduleCommit", "Force commit ahead of next VSYNC."}, - {"scheduleComposite", "PENDING - if you have a good understanding let me know!"}, +enum class ParseToggleResult { + kError, + kFalse, + kTrue, +}; + +const std::map<std::string, std::string> g_function_details = { + {"debugFlash", "[optional(delay)] Perform a debug flash."}, + {"frameRateIndicator", "[hide | show] displays the framerate in the top left corner."}, + {"scheduleComposite", "Force composite ahead of next VSYNC."}, + {"scheduleCommit", "Force commit ahead of next VSYNC."}, + {"scheduleComposite", "PENDING - if you have a good understanding let me know!"}, + {"forceClientComposition", + "[enabled | disabled] When enabled, it disables " + "Hardware Overlays, and routes all window composition to the GPU. This can " + "help check if there is a bug in HW Composer."}, }; static void ShowUsage() { - std::cout << "usage: sfdo [help, FrameRateIndicator show, DebugFlash enabled, ...]\n\n"; + std::cout << "usage: sfdo [help, frameRateIndicator show, debugFlash enabled, ...]\n\n"; for (const auto& sf : g_functions) { const std::string fn = sf.first; std::string fdetails = "TODO"; @@ -50,7 +60,26 @@ static void ShowUsage() { } } -int FrameRateIndicator([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { +// Returns 1 for positive keywords and 0 for negative keywords. +// If the string does not match any it will return -1. +ParseToggleResult parseToggle(const char* str) { + const std::unordered_set<std::string> positive{"1", "true", "y", "yes", + "on", "enabled", "show"}; + const std::unordered_set<std::string> negative{"0", "false", "n", "no", + "off", "disabled", "hide"}; + + const std::string word(str); + if (positive.count(word)) { + return ParseToggleResult::kTrue; + } + if (negative.count(word)) { + return ParseToggleResult::kFalse; + } + + return ParseToggleResult::kError; +} + +int frameRateIndicator(int argc, char** argv) { bool hide = false, show = false; if (argc == 3) { show = strcmp(argv[2], "show") == 0; @@ -60,13 +89,13 @@ int FrameRateIndicator([[maybe_unused]] int argc, [[maybe_unused]] char** argv) if (show || hide) { ComposerServiceAIDL::getComposerService()->enableRefreshRateOverlay(show); } else { - std::cerr << "Incorrect usage of FrameRateIndicator. Missing [hide | show].\n"; + std::cerr << "Incorrect usage of frameRateIndicator. Missing [hide | show].\n"; return -1; } return 0; } -int DebugFlash([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { +int debugFlash(int argc, char** argv) { int delay = 0; if (argc == 3) { delay = atoi(argv[2]) == 0; @@ -86,14 +115,40 @@ int scheduleCommit([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { return 0; } +int forceClientComposition(int argc, char** argv) { + bool enabled = true; + // A valid command looks like this: + // adb shell sfdo forceClientComposition enabled + if (argc >= 3) { + const ParseToggleResult toggle = parseToggle(argv[2]); + if (toggle == ParseToggleResult::kError) { + std::cerr << "Incorrect usage of forceClientComposition. " + "Missing [enabled | disabled].\n"; + return -1; + } + if (argc > 3) { + std::cerr << "Too many arguments after [enabled | disabled]. " + "Ignoring extra arguments.\n"; + } + enabled = (toggle == ParseToggleResult::kTrue); + } else { + std::cerr << "Incorrect usage of forceClientComposition. Missing [enabled | disabled].\n"; + return -1; + } + + ComposerServiceAIDL::getComposerService()->forceClientComposition(enabled); + return 0; +} + int main(int argc, char** argv) { std::cout << "Execute SurfaceFlinger internal commands.\n"; std::cout << "sfdo requires to be run with root permissions..\n"; - g_functions["FrameRateIndicator"] = FrameRateIndicator; - g_functions["DebugFlash"] = DebugFlash; + g_functions["frameRateIndicator"] = frameRateIndicator; + g_functions["debugFlash"] = debugFlash; g_functions["scheduleComposite"] = scheduleComposite; g_functions["scheduleCommit"] = scheduleCommit; + g_functions["forceClientComposition"] = forceClientComposition; if (argc > 1 && g_functions.find(argv[1]) != g_functions.end()) { std::cout << "Running: " << argv[1] << "\n"; diff --git a/include/android/performance_hint.h b/include/android/performance_hint.h index ba8b02d597..9d2c79139f 100644 --- a/include/android/performance_hint.h +++ b/include/android/performance_hint.h @@ -60,6 +60,27 @@ __BEGIN_DECLS struct APerformanceHintManager; struct APerformanceHintSession; +struct AWorkDuration; + +/** + * {@link AWorkDuration} is an opaque type that represents the breakdown of the + * actual workload duration in each component internally. + * + * A new {@link AWorkDuration} can be obtained using + * {@link AWorkDuration_create()}, when the client finishes using + * {@link AWorkDuration}, {@link AWorkDuration_release()} must be + * called to destroy and free up the resources associated with + * {@link AWorkDuration}. + * + * This file provides a set of functions to allow clients to set the measured + * work duration of each component on {@link AWorkDuration}. + * + * - AWorkDuration_setWorkPeriodStartTimestampNanos() + * - AWorkDuration_setActualTotalDurationNanos() + * - AWorkDuration_setActualCpuDurationNanos() + * - AWorkDuration_setActualGpuDurationNanos() + */ +typedef struct AWorkDuration AWorkDuration; /** * An opaque type representing a handle to a performance hint manager. @@ -102,7 +123,7 @@ typedef struct APerformanceHintSession APerformanceHintSession; * * @return APerformanceHintManager instance on success, nullptr on failure. */ -APerformanceHintManager* APerformanceHint_getManager() __INTRODUCED_IN(__ANDROID_API_T__); +APerformanceHintManager* _Nullable APerformanceHint_getManager() __INTRODUCED_IN(__ANDROID_API_T__); /** * Creates a session for the given set of threads and sets their initial target work @@ -116,9 +137,9 @@ APerformanceHintManager* APerformanceHint_getManager() __INTRODUCED_IN(__ANDROID * This must be positive if using the work duration API, or 0 otherwise. * @return APerformanceHintManager instance on success, nullptr on failure. */ -APerformanceHintSession* APerformanceHint_createSession( - APerformanceHintManager* manager, - const int32_t* threadIds, size_t size, +APerformanceHintSession* _Nullable APerformanceHint_createSession( + APerformanceHintManager* _Nonnull manager, + const int32_t* _Nonnull threadIds, size_t size, int64_t initialTargetWorkDurationNanos) __INTRODUCED_IN(__ANDROID_API_T__); /** @@ -128,7 +149,7 @@ APerformanceHintSession* APerformanceHint_createSession( * @return the preferred update rate supported by device software. */ int64_t APerformanceHint_getPreferredUpdateRateNanos( - APerformanceHintManager* manager) __INTRODUCED_IN(__ANDROID_API_T__); + APerformanceHintManager* _Nonnull manager) __INTRODUCED_IN(__ANDROID_API_T__); /** * Updates this session's target duration for each cycle of work. @@ -140,7 +161,7 @@ int64_t APerformanceHint_getPreferredUpdateRateNanos( * EPIPE if communication with the system service has failed. */ int APerformanceHint_updateTargetWorkDuration( - APerformanceHintSession* session, + APerformanceHintSession* _Nonnull session, int64_t targetDurationNanos) __INTRODUCED_IN(__ANDROID_API_T__); /** @@ -157,7 +178,7 @@ int APerformanceHint_updateTargetWorkDuration( * EPIPE if communication with the system service has failed. */ int APerformanceHint_reportActualWorkDuration( - APerformanceHintSession* session, + APerformanceHintSession* _Nonnull session, int64_t actualDurationNanos) __INTRODUCED_IN(__ANDROID_API_T__); /** @@ -167,7 +188,7 @@ int APerformanceHint_reportActualWorkDuration( * @param session The performance hint session instance to release. */ void APerformanceHint_closeSession( - APerformanceHintSession* session) __INTRODUCED_IN(__ANDROID_API_T__); + APerformanceHintSession* _Nonnull session) __INTRODUCED_IN(__ANDROID_API_T__); /** * Set a list of threads to the performance hint session. This operation will replace @@ -184,8 +205,8 @@ void APerformanceHint_closeSession( * EPERM if any thread id doesn't belong to the application. */ int APerformanceHint_setThreads( - APerformanceHintSession* session, - const pid_t* threadIds, + APerformanceHintSession* _Nonnull session, + const pid_t* _Nonnull threadIds, size_t size) __INTRODUCED_IN(__ANDROID_API_U__); /** @@ -198,11 +219,92 @@ int APerformanceHint_setThreads( * EPIPE if communication with the system service has failed. */ int APerformanceHint_setPreferPowerEfficiency( - APerformanceHintSession* session, + APerformanceHintSession* _Nonnull session, bool enabled) __INTRODUCED_IN(__ANDROID_API_V__); +/** + * Reports the durations for the last cycle of work. + * + * The system will attempt to adjust the scheduling and performance of the + * threads within the thread group to bring the actual duration close to the target duration. + * + * @param session The {@link APerformanceHintSession} instance to update. + * @param workDuration The {@link AWorkDuration} structure of times the thread group took to + * complete its last task in nanoseconds breaking down into different components. + * + * The work period start timestamp, actual total duration and actual CPU duration must be + * positive. + * + * The actual GPU duration must be non-negative. If the actual GPU duration is 0, it means + * the actual GPU duration is not measured. + * + * @return 0 on success. + * EINVAL if session is nullptr or any duration is an invalid number. + * EPIPE if communication with the system service has failed. + */ +int APerformanceHint_reportActualWorkDuration2( + APerformanceHintSession* _Nonnull session, + AWorkDuration* _Nonnull workDuration) __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Creates a new AWorkDuration. When the client finishes using {@link AWorkDuration}, it should + * call {@link AWorkDuration_release()} to destroy {@link AWorkDuration} and release all resources + * associated with it. + * + * @return AWorkDuration on success and nullptr otherwise. + */ +AWorkDuration* _Nonnull AWorkDuration_create() __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Destroys {@link AWorkDuration} and free all resources associated to it. + * + * @param aWorkDuration The {@link AWorkDuration} created by calling {@link AWorkDuration_create()} + */ +void AWorkDuration_release(AWorkDuration* _Nonnull WorkDuration) __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Sets the work period start timestamp in nanoseconds. + * + * @param aWorkDuration The {@link AWorkDuration} created by calling {@link AWorkDuration_create()} + * @param workPeriodStartTimestampNanos The work period start timestamp in nanoseconds based on + * CLOCK_MONOTONIC about when the work starts, the timestamp must be positive. + */ +void AWorkDuration_setWorkPeriodStartTimestampNanos(AWorkDuration* _Nonnull aWorkDuration, + int64_t workPeriodStartTimestampNanos) __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Sets the actual total work duration in nanoseconds. + * + * @param aWorkDuration The {@link AWorkDuration} created by calling {@link AWorkDuration_create()} + * @param actualTotalDurationNanos The actual total work duration in nanoseconds, the number must be + * positive. + */ +void AWorkDuration_setActualTotalDurationNanos(AWorkDuration* _Nonnull aWorkDuration, + int64_t actualTotalDurationNanos) __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Sets the actual CPU work duration in nanoseconds. + * + * @param aWorkDuration The {@link AWorkDuration} created by calling {@link AWorkDuration_create()} + * @param actualCpuDurationNanos The actual CPU work duration in nanoseconds, the number must be + * positive. + */ +void AWorkDuration_setActualCpuDurationNanos(AWorkDuration* _Nonnull aWorkDuration, + int64_t actualCpuDurationNanos) __INTRODUCED_IN(__ANDROID_API_V__); + +/** + * Sets the actual GPU work duration in nanoseconds. + * + * @param aWorkDuration The {@link AWorkDuration} created by calling {@link AWorkDuration_create()}. + * @param actualGpuDurationNanos The actual GPU work duration in nanoseconds, the number must be + * non-negative. If the actual GPU duration is 0, it means the actual GPU duration is + * measured. + */ +void AWorkDuration_setActualGpuDurationNanos(AWorkDuration* _Nonnull aWorkDuration, + int64_t actualGpuDurationNanos) __INTRODUCED_IN(__ANDROID_API_V__); + __END_DECLS #endif // ANDROID_NATIVE_PERFORMANCE_HINT_H -/** @} */
\ No newline at end of file +/** @} */ diff --git a/include/android/thermal.h b/include/android/thermal.h index 1f477f8233..0b57e9376d 100644 --- a/include/android/thermal.h +++ b/include/android/thermal.h @@ -111,7 +111,7 @@ typedef struct AThermalManager AThermalManager; * It's passed the updated thermal status as parameter, as well as the * pointer provided by the client that registered a callback. */ -typedef void (*AThermal_StatusCallback)(void *data, AThermalStatus status); +typedef void (*AThermal_StatusCallback)(void* data, AThermalStatus status); /** * Acquire an instance of the thermal manager. This must be freed using @@ -222,6 +222,70 @@ int AThermal_unregisterThermalStatusListener(AThermalManager *manager, float AThermal_getThermalHeadroom(AThermalManager *manager, int forecastSeconds) __INTRODUCED_IN(31); +/** + * This struct defines an instance of headroom threshold value and its status. + * <p> + * The value should be monotonically non-decreasing as the thermal status increases. + * For {@link ATHERMAL_STATUS_SEVERE}, its headroom threshold is guaranteed to + * be 1.0f. For status below severe status, the value should be lower or equal + * to 1.0f, and for status above severe, the value should be larger or equal to 1.0f. + * <p> + * Also see {@link AThermal_getThermalHeadroom} for explanation on headroom, and + * {@link AThermal_getThermalHeadroomThresholds} for how to use this. + */ +struct AThermalHeadroomThreshold { + float headroom; + AThermalStatus thermalStatus; +}; + +/** + * Gets the thermal headroom thresholds for all available thermal status. + * + * A thermal status will only exist in output if the device manufacturer has the + * corresponding threshold defined for at least one of its slow-moving skin temperature + * sensors. If it's set, one should also expect to get it from + * {@link #AThermal_getCurrentThermalStatus} or {@link AThermal_StatusCallback}. + * <p> + * The headroom threshold is used to interpret the possible thermal throttling status based on + * the headroom prediction. For example, if the headroom threshold for + * {@link ATHERMAL_STATUS_LIGHT} is 0.7, and a headroom prediction in 10s returns 0.75 + * (or {@code AThermal_getThermalHeadroom(10)=0.75}), one can expect that in 10 seconds the system + * could be in lightly throttled state if the workload remains the same. The app can consider + * taking actions according to the nearest throttling status the difference between the headroom and + * the threshold. + * <p> + * For new devices it's guaranteed to have a single sensor, but for older devices with multiple + * sensors reporting different threshold values, the minimum threshold is taken to be conservative + * on predictions. Thus, when reading real-time headroom, it's not guaranteed that a real-time value + * of 0.75 (or {@code AThermal_getThermalHeadroom(0)}=0.75) exceeding the threshold of 0.7 above + * will always come with lightly throttled state + * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_LIGHT}) but it can be lower + * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_NONE}). + * While it's always guaranteed that the device won't be throttled heavier than the unmet + * threshold's state, so a real-time headroom of 0.75 will never come with + * {@link #ATHERMAL_STATUS_MODERATE} but always lower, and 0.65 will never come with + * {@link ATHERMAL_STATUS_LIGHT} but {@link #ATHERMAL_STATUS_NONE}. + * <p> + * The returned list of thresholds is cached on first successful query and owned by the thermal + * manager, which will not change between calls to this function. The caller should only need to + * free the manager with {@link AThermal_releaseManager}. + * + * @param manager The manager instance to use. + * Acquired via {@link AThermal_acquireManager}. + * @param outThresholds non-null output pointer to null AThermalHeadroomThreshold pointer, which + * will be set to the cached array of thresholds if thermal thresholds are supported + * by the system or device, otherwise nullptr or unmodified. + * @param size non-null output pointer whose value will be set to the size of the threshold array + * or 0 if it's not supported. + * @return 0 on success + * EINVAL if outThresholds or size_t is nullptr, or *outThresholds is not nullptr. + * EPIPE if communication with the system service has failed. + * ENOSYS if the feature is disabled by the current system. + */ +int AThermal_getThermalHeadroomThresholds(AThermalManager* manager, + const AThermalHeadroomThreshold ** outThresholds, + size_t* size) __INTRODUCED_IN(35); + #ifdef __cplusplus } #endif diff --git a/include/private/thermal_private.h b/include/private/thermal_private.h new file mode 100644 index 0000000000..951d953267 --- /dev/null +++ b/include/private/thermal_private.h @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2023 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. + */ + +#ifndef ANDROID_PRIVATE_NATIVE_THERMAL_H +#define ANDROID_PRIVATE_NATIVE_THERMAL_H + +#include <stdint.h> + +__BEGIN_DECLS + +/** + * For testing only. + */ +void AThermal_setIThermalServiceForTesting(void* iThermalService); + +__END_DECLS + +#endif // ANDROID_PRIVATE_NATIVE_THERMAL_H
\ No newline at end of file diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 620c23c1bb..f90c618a94 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -251,13 +251,15 @@ cc_library_shared { srcs: [ // Trusty-specific files "OS_android.cpp", - "trusty/logging.cpp", "trusty/OS.cpp", "trusty/RpcServerTrusty.cpp", "trusty/RpcTransportTipcTrusty.cpp", "trusty/TrustyStatus.cpp", "trusty/socket.cpp", ], + shared_libs: [ + "liblog", + ], } cc_defaults { diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp index 824323857d..301dbf6cdf 100644 --- a/libs/binder/Binder.cpp +++ b/libs/binder/Binder.cpp @@ -19,7 +19,6 @@ #include <atomic> #include <set> -#include <android-base/logging.h> #include <android-base/unique_fd.h> #include <binder/BpBinder.h> #include <binder/IInterface.h> @@ -308,7 +307,7 @@ status_t BBinder::startRecordingTransactions(const Parcel& data) { Extras* e = getOrCreateExtras(); RpcMutexUniqueLock lock(e->mLock); if (mRecordingOn) { - LOG(INFO) << "Could not start Binder recording. Another is already in progress."; + ALOGI("Could not start Binder recording. Another is already in progress."); return INVALID_OPERATION; } else { status_t readStatus = data.readUniqueFileDescriptor(&(e->mRecordingFd)); @@ -316,7 +315,7 @@ status_t BBinder::startRecordingTransactions(const Parcel& data) { return readStatus; } mRecordingOn = true; - LOG(INFO) << "Started Binder recording."; + ALOGI("Started Binder recording."); return NO_ERROR; } } @@ -340,10 +339,10 @@ status_t BBinder::stopRecordingTransactions() { if (mRecordingOn) { e->mRecordingFd.reset(); mRecordingOn = false; - LOG(INFO) << "Stopped Binder recording."; + ALOGI("Stopped Binder recording."); return NO_ERROR; } else { - LOG(INFO) << "Could not stop Binder recording. One is not in progress."; + ALOGI("Could not stop Binder recording. One is not in progress."); return INVALID_OPERATION; } } @@ -377,11 +376,11 @@ status_t BBinder::transact( err = stopRecordingTransactions(); break; case EXTENSION_TRANSACTION: - CHECK(reply != nullptr); + LOG_ALWAYS_FATAL_IF(reply == nullptr, "reply == nullptr"); err = reply->writeStrongBinder(getExtension()); break; case DEBUG_PID_TRANSACTION: - CHECK(reply != nullptr); + LOG_ALWAYS_FATAL_IF(reply == nullptr, "reply == nullptr"); err = reply->writeInt32(getDebugPid()); break; case SET_RPC_CLIENT_TRANSACTION: { @@ -414,10 +413,10 @@ status_t BBinder::transact( reply ? *reply : emptyReply, err); if (transaction) { if (status_t err = transaction->dumpToFile(e->mRecordingFd); err != NO_ERROR) { - LOG(INFO) << "Failed to dump RecordedTransaction to file with error " << err; + ALOGI("Failed to dump RecordedTransaction to file with error %d", err); } } else { - LOG(INFO) << "Failed to create RecordedTransaction object."; + ALOGI("Failed to create RecordedTransaction object."); } } } @@ -705,7 +704,7 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd, return status; } rpcServer->setMaxThreads(binderThreadPoolMaxCount); - LOG(INFO) << "RpcBinder: Started Binder debug on " << getInterfaceDescriptor(); + ALOGI("RpcBinder: Started Binder debug on %s", String8(getInterfaceDescriptor()).c_str()); rpcServer->start(); e->mRpcServerLinks.emplace(link); LOG_RPC_DETAIL("%s(fd=%d) successful", __PRETTY_FUNCTION__, socketFdForPrint); @@ -723,20 +722,20 @@ BBinder::~BBinder() { if (!wasParceled()) { if (getExtension()) { - ALOGW("Binder %p destroyed with extension attached before being parceled.", this); + ALOGW("Binder %p destroyed with extension attached before being parceled.", this); } if (isRequestingSid()) { - ALOGW("Binder %p destroyed when requesting SID before being parceled.", this); + ALOGW("Binder %p destroyed when requesting SID before being parceled.", this); } if (isInheritRt()) { - ALOGW("Binder %p destroyed after setInheritRt before being parceled.", this); + ALOGW("Binder %p destroyed after setInheritRt before being parceled.", this); } #ifdef __linux__ if (getMinSchedulerPolicy() != SCHED_NORMAL) { - ALOGW("Binder %p destroyed after setMinSchedulerPolicy before being parceled.", this); + ALOGW("Binder %p destroyed after setMinSchedulerPolicy before being parceled.", this); } if (getMinSchedulerPriority() != 0) { - ALOGW("Binder %p destroyed after setMinSchedulerPolicy before being parceled.", this); + ALOGW("Binder %p destroyed after setMinSchedulerPolicy before being parceled.", this); } #endif // __linux__ } @@ -752,7 +751,7 @@ status_t BBinder::onTransact( { switch (code) { case INTERFACE_TRANSACTION: - CHECK(reply != nullptr); + LOG_ALWAYS_FATAL_IF(reply == nullptr, "reply == nullptr"); reply->writeString16(getInterfaceDescriptor()); return NO_ERROR; diff --git a/libs/binder/FdTrigger.cpp b/libs/binder/FdTrigger.cpp index a1fbbf321c..895690f9b8 100644 --- a/libs/binder/FdTrigger.cpp +++ b/libs/binder/FdTrigger.cpp @@ -53,7 +53,7 @@ bool FdTrigger::isTriggered() { #ifdef BINDER_RPC_SINGLE_THREADED return mTriggered; #else - return mWrite == -1; + return !mWrite.ok(); #endif } diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp index cedd3af289..f7b5a55a31 100644 --- a/libs/binder/RecordedTransaction.cpp +++ b/libs/binder/RecordedTransaction.cpp @@ -15,10 +15,10 @@ */ #include <android-base/file.h> -#include <android-base/logging.h> #include <android-base/unique_fd.h> #include <binder/Functional.h> #include <binder/RecordedTransaction.h> +#include <inttypes.h> #include <sys/mman.h> #include <algorithm> @@ -127,18 +127,17 @@ std::optional<RecordedTransaction> RecordedTransaction::fromDetails( t.mData.mInterfaceName = std::string(String8(interfaceName).c_str()); if (interfaceName.size() != t.mData.mInterfaceName.size()) { - LOG(ERROR) << "Interface Name is not valid. Contains characters that aren't single byte " - "utf-8."; + ALOGE("Interface Name is not valid. Contains characters that aren't single byte utf-8."); return std::nullopt; } if (t.mSent.setData(dataParcel.data(), dataParcel.dataBufferSize()) != android::NO_ERROR) { - LOG(ERROR) << "Failed to set sent parcel data."; + ALOGE("Failed to set sent parcel data."); return std::nullopt; } if (t.mReply.setData(replyParcel.data(), replyParcel.dataBufferSize()) != android::NO_ERROR) { - LOG(ERROR) << "Failed to set reply parcel data."; + ALOGE("Failed to set reply parcel data."); return std::nullopt; } @@ -168,38 +167,37 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd const long pageSize = sysconf(_SC_PAGE_SIZE); struct stat fileStat; if (fstat(fd.get(), &fileStat) != 0) { - LOG(ERROR) << "Unable to get file information"; + ALOGE("Unable to get file information"); return std::nullopt; } off_t fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR); if (fdCurrentPosition == -1) { - LOG(ERROR) << "Invalid offset in file descriptor."; + ALOGE("Invalid offset in file descriptor."); return std::nullopt; } do { if (fileStat.st_size < (fdCurrentPosition + (off_t)sizeof(ChunkDescriptor))) { - LOG(ERROR) << "Not enough file remains to contain expected chunk descriptor"; + ALOGE("Not enough file remains to contain expected chunk descriptor"); return std::nullopt; } if (!android::base::ReadFully(fd, &chunk, sizeof(ChunkDescriptor))) { - LOG(ERROR) << "Failed to read ChunkDescriptor from fd " << fd.get() << ". " - << strerror(errno); + ALOGE("Failed to read ChunkDescriptor from fd %d. %s", fd.get(), strerror(errno)); return std::nullopt; } transaction_checksum_t checksum = *reinterpret_cast<transaction_checksum_t*>(&chunk); fdCurrentPosition = lseek(fd.get(), 0, SEEK_CUR); if (fdCurrentPosition == -1) { - LOG(ERROR) << "Invalid offset in file descriptor."; + ALOGE("Invalid offset in file descriptor."); return std::nullopt; } off_t mmapPageAlignedStart = (fdCurrentPosition / pageSize) * pageSize; off_t mmapPayloadStartOffset = fdCurrentPosition - mmapPageAlignedStart; if (chunk.dataSize > kMaxChunkDataSize) { - LOG(ERROR) << "Chunk data exceeds maximum size."; + ALOGE("Chunk data exceeds maximum size."); return std::nullopt; } @@ -207,12 +205,12 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd chunk.dataSize + PADDING8(chunk.dataSize) + sizeof(transaction_checksum_t); if (chunkPayloadSize > (size_t)(fileStat.st_size - fdCurrentPosition)) { - LOG(ERROR) << "Chunk payload exceeds remaining file size."; + ALOGE("Chunk payload exceeds remaining file size."); return std::nullopt; } if (PADDING8(chunkPayloadSize) != 0) { - LOG(ERROR) << "Invalid chunk size, not aligned " << chunkPayloadSize; + ALOGE("Invalid chunk size, not aligned %zu", chunkPayloadSize); return std::nullopt; } @@ -228,8 +226,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd sizeof(transaction_checksum_t); // Skip chunk descriptor and required mmap // page-alignment if (payloadMap == MAP_FAILED) { - LOG(ERROR) << "Memory mapping failed for fd " << fd.get() << ": " << errno << " " - << strerror(errno); + ALOGE("Memory mapping failed for fd %d: %d %s", fd.get(), errno, strerror(errno)); return std::nullopt; } for (size_t checksumIndex = 0; @@ -237,21 +234,21 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd checksum ^= payloadMap[checksumIndex]; } if (checksum != 0) { - LOG(ERROR) << "Checksum failed."; + ALOGE("Checksum failed."); return std::nullopt; } fdCurrentPosition = lseek(fd.get(), chunkPayloadSize, SEEK_CUR); if (fdCurrentPosition == -1) { - LOG(ERROR) << "Invalid offset in file descriptor."; + ALOGE("Invalid offset in file descriptor."); return std::nullopt; } switch (chunk.chunkType) { case HEADER_CHUNK: { if (chunk.dataSize != static_cast<uint32_t>(sizeof(TransactionHeader))) { - LOG(ERROR) << "Header Chunk indicated size " << chunk.dataSize << "; Expected " - << sizeof(TransactionHeader) << "."; + ALOGE("Header Chunk indicated size %" PRIu32 "; Expected %zu.", chunk.dataSize, + sizeof(TransactionHeader)); return std::nullopt; } t.mData.mHeader = *reinterpret_cast<TransactionHeader*>(payloadMap); @@ -265,7 +262,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd case DATA_PARCEL_CHUNK: { if (t.mSent.setData(reinterpret_cast<const unsigned char*>(payloadMap), chunk.dataSize) != android::NO_ERROR) { - LOG(ERROR) << "Failed to set sent parcel data."; + ALOGE("Failed to set sent parcel data."); return std::nullopt; } break; @@ -273,7 +270,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd case REPLY_PARCEL_CHUNK: { if (t.mReply.setData(reinterpret_cast<const unsigned char*>(payloadMap), chunk.dataSize) != android::NO_ERROR) { - LOG(ERROR) << "Failed to set reply parcel data."; + ALOGE("Failed to set reply parcel data."); return std::nullopt; } break; @@ -281,7 +278,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd case END_CHUNK: break; default: - LOG(INFO) << "Unrecognized chunk."; + ALOGI("Unrecognized chunk."); break; } } while (chunk.chunkType != END_CHUNK); @@ -292,7 +289,7 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd android::status_t RecordedTransaction::writeChunk(borrowed_fd fd, uint32_t chunkType, size_t byteCount, const uint8_t* data) const { if (byteCount > kMaxChunkDataSize) { - LOG(ERROR) << "Chunk data exceeds maximum size"; + ALOGE("Chunk data exceeds maximum size"); return BAD_VALUE; } ChunkDescriptor descriptor = {.chunkType = chunkType, @@ -321,7 +318,7 @@ android::status_t RecordedTransaction::writeChunk(borrowed_fd fd, uint32_t chunk // Write buffer to file if (!android::base::WriteFully(fd, buffer.data(), buffer.size())) { - LOG(ERROR) << "Failed to write chunk fd " << fd.get(); + ALOGE("Failed to write chunk fd %d", fd.get()); return UNKNOWN_ERROR; } return NO_ERROR; @@ -331,26 +328,26 @@ android::status_t RecordedTransaction::dumpToFile(const unique_fd& fd) const { if (NO_ERROR != writeChunk(fd, HEADER_CHUNK, sizeof(TransactionHeader), reinterpret_cast<const uint8_t*>(&(mData.mHeader)))) { - LOG(ERROR) << "Failed to write transactionHeader to fd " << fd.get(); + ALOGE("Failed to write transactionHeader to fd %d", fd.get()); return UNKNOWN_ERROR; } if (NO_ERROR != writeChunk(fd, INTERFACE_NAME_CHUNK, mData.mInterfaceName.size() * sizeof(uint8_t), reinterpret_cast<const uint8_t*>(mData.mInterfaceName.c_str()))) { - LOG(INFO) << "Failed to write Interface Name Chunk to fd " << fd.get(); + ALOGI("Failed to write Interface Name Chunk to fd %d", fd.get()); return UNKNOWN_ERROR; } if (NO_ERROR != writeChunk(fd, DATA_PARCEL_CHUNK, mSent.dataBufferSize(), mSent.data())) { - LOG(ERROR) << "Failed to write sent Parcel to fd " << fd.get(); + ALOGE("Failed to write sent Parcel to fd %d", fd.get()); return UNKNOWN_ERROR; } if (NO_ERROR != writeChunk(fd, REPLY_PARCEL_CHUNK, mReply.dataBufferSize(), mReply.data())) { - LOG(ERROR) << "Failed to write reply Parcel to fd " << fd.get(); + ALOGE("Failed to write reply Parcel to fd %d", fd.get()); return UNKNOWN_ERROR; } if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, NULL)) { - LOG(ERROR) << "Failed to write end chunk to fd " << fd.get(); + ALOGE("Failed to write end chunk to fd %d", fd.get()); return UNKNOWN_ERROR; } return NO_ERROR; diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp index 1ba20b3103..fefaa810d2 100644 --- a/libs/binder/RpcServer.cpp +++ b/libs/binder/RpcServer.cpp @@ -168,7 +168,7 @@ void RpcServer::setConnectionFilter(std::function<bool(const void*, size_t)>&& f void RpcServer::setServerSocketModifier(std::function<void(base::borrowed_fd)>&& modifier) { RpcMutexLockGuard _l(mLock); - LOG_ALWAYS_FATAL_IF(mServer.fd != -1, "Already started"); + LOG_ALWAYS_FATAL_IF(mServer.fd.ok(), "Already started"); mServerSocketModifier = std::move(modifier); } @@ -200,7 +200,7 @@ void RpcServer::start() { status_t RpcServer::acceptSocketConnection(const RpcServer& server, RpcTransportFd* out) { RpcTransportFd clientSocket(unique_fd(TEMP_FAILURE_RETRY( accept4(server.mServer.fd.get(), nullptr, nullptr, SOCK_CLOEXEC | SOCK_NONBLOCK)))); - if (clientSocket.fd < 0) { + if (!clientSocket.fd.ok()) { int savedErrno = errno; ALOGE("Could not accept4 socket: %s", strerror(savedErrno)); return -savedErrno; diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp index c895b21f91..cd8f41711f 100644 --- a/libs/binder/RpcSession.cpp +++ b/libs/binder/RpcSession.cpp @@ -208,7 +208,7 @@ status_t RpcSession::addNullDebuggingClient() { unique_fd serverFd(TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY | O_CLOEXEC))); - if (serverFd == -1) { + if (!serverFd.ok()) { int savedErrno = errno; ALOGE("Could not connect to /dev/null: %s", strerror(savedErrno)); return -savedErrno; @@ -594,7 +594,7 @@ status_t RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, unique_fd serverFd(TEMP_FAILURE_RETRY( socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0))); - if (serverFd == -1) { + if (!serverFd.ok()) { int savedErrno = errno; ALOGE("Could not create socket at %s: %s", addr.toString().c_str(), strerror(savedErrno)); diff --git a/libs/binder/RpcTrusty.cpp b/libs/binder/RpcTrusty.cpp index 3b53b05991..2f2fe7d276 100644 --- a/libs/binder/RpcTrusty.cpp +++ b/libs/binder/RpcTrusty.cpp @@ -16,7 +16,6 @@ #define LOG_TAG "RpcTrusty" -#include <android-base/logging.h> #include <android-base/unique_fd.h> #include <binder/RpcSession.h> #include <binder/RpcTransportTipcAndroid.h> @@ -35,13 +34,13 @@ sp<RpcSession> RpcTrustyConnectWithSessionInitializer( auto request = [=] { int tipcFd = tipc_connect(device, port); if (tipcFd < 0) { - LOG(ERROR) << "Failed to connect to Trusty service. Error code: " << tipcFd; + ALOGE("Failed to connect to Trusty service. Error code: %d", tipcFd); return unique_fd(); } return unique_fd(tipcFd); }; if (status_t status = session->setupPreconnectedClient(unique_fd{}, request); status != OK) { - LOG(ERROR) << "Failed to set up Trusty client. Error: " << statusToString(status).c_str(); + ALOGE("Failed to set up Trusty client. Error: %s", statusToString(status).c_str()); return nullptr; } return session; diff --git a/libs/binder/Utils.cpp b/libs/binder/Utils.cpp index 47fd17dcb1..d9a96af80a 100644 --- a/libs/binder/Utils.cpp +++ b/libs/binder/Utils.cpp @@ -16,7 +16,6 @@ #include "Utils.h" -#include <android-base/logging.h> #include <string.h> namespace android { @@ -26,7 +25,7 @@ void zeroMemory(uint8_t* data, size_t size) { } std::string HexString(const void* bytes, size_t len) { - CHECK(bytes != nullptr || len == 0) << bytes << " " << len; + LOG_ALWAYS_FATAL_IF(len > 0 && bytes == nullptr, "%p %zu", bytes, len); // b/132916539: Doing this the 'C way', std::setfill triggers ubsan implicit conversion const uint8_t* bytes8 = static_cast<const uint8_t*>(bytes); diff --git a/libs/binder/Utils.h b/libs/binder/Utils.h index c8431aab43..eec09eb859 100644 --- a/libs/binder/Utils.h +++ b/libs/binder/Utils.h @@ -14,6 +14,8 @@ * limitations under the License. */ +#pragma once + #include <stddef.h> #include <sys/uio.h> #include <cstdint> @@ -22,8 +24,16 @@ #include <log/log.h> #include <utils/Errors.h> -#define PLOGE_VA_ARGS(...) , ##__VA_ARGS__ -#define PLOGE(fmt, ...) ALOGE(fmt ": %s" PLOGE_VA_ARGS(__VA_ARGS__), strerror(errno)) +#define PLOGE(fmt, ...) \ + do { \ + auto savedErrno = errno; \ + ALOGE(fmt ": %s" __VA_OPT__(, ) __VA_ARGS__, strerror(savedErrno)); \ + } while (0) +#define PLOGF(fmt, ...) \ + do { \ + auto savedErrno = errno; \ + LOG_ALWAYS_FATAL(fmt ": %s" __VA_OPT__(, ) __VA_ARGS__, strerror(savedErrno)); \ + } while (0) /* TEMP_FAILURE_RETRY is not available on macOS and Trusty. */ #ifndef TEMP_FAILURE_RETRY diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp index f51cd9bc99..118409eeff 100644 --- a/libs/binder/libbinder_rpc_unstable.cpp +++ b/libs/binder/libbinder_rpc_unstable.cpp @@ -16,7 +16,6 @@ #include <binder_rpc_unstable.hpp> -#include <android-base/logging.h> #include <android-base/unique_fd.h> #include <android/binder_libbinder.h> #include <binder/RpcServer.h> @@ -85,8 +84,8 @@ ARpcServer* ARpcServer_newVsock(AIBinder* service, unsigned int cid, unsigned in } if (status_t status = server->setupVsockServer(bindCid, port); status != OK) { - LOG(ERROR) << "Failed to set up vsock server with port " << port - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up vsock server with port %u error: %s", port, + statusToString(status).c_str()); return nullptr; } if (cid != VMADDR_CID_ANY) { @@ -95,7 +94,7 @@ ARpcServer* ARpcServer_newVsock(AIBinder* service, unsigned int cid, unsigned in const sockaddr_vm* vaddr = reinterpret_cast<const sockaddr_vm*>(addr); LOG_ALWAYS_FATAL_IF(vaddr->svm_family != AF_VSOCK, "address is not a vsock"); if (cid != vaddr->svm_cid) { - LOG(ERROR) << "Rejected vsock connection from CID " << vaddr->svm_cid; + ALOGE("Rejected vsock connection from CID %u", vaddr->svm_cid); return false; } return true; @@ -109,12 +108,12 @@ ARpcServer* ARpcServer_newBoundSocket(AIBinder* service, int socketFd) { auto server = RpcServer::make(); auto fd = unique_fd(socketFd); if (!fd.ok()) { - LOG(ERROR) << "Invalid socket fd " << socketFd; + ALOGE("Invalid socket fd %d", socketFd); return nullptr; } if (status_t status = server->setupRawSocketServer(std::move(fd)); status != OK) { - LOG(ERROR) << "Failed to set up RPC server with fd " << socketFd - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up RPC server with fd %d error: %s", socketFd, + statusToString(status).c_str()); return nullptr; } server->setRootObject(AIBinder_toPlatformBinder(service)); @@ -125,13 +124,13 @@ ARpcServer* ARpcServer_newUnixDomainBootstrap(AIBinder* service, int bootstrapFd auto server = RpcServer::make(); auto fd = unique_fd(bootstrapFd); if (!fd.ok()) { - LOG(ERROR) << "Invalid bootstrap fd " << bootstrapFd; + ALOGE("Invalid bootstrap fd %d", bootstrapFd); return nullptr; } if (status_t status = server->setupUnixDomainSocketBootstrapServer(std::move(fd)); status != OK) { - LOG(ERROR) << "Failed to set up Unix Domain RPC server with bootstrap fd " << bootstrapFd - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up Unix Domain RPC server with bootstrap fd %d error: %s", bootstrapFd, + statusToString(status).c_str()); return nullptr; } server->setRootObject(AIBinder_toPlatformBinder(service)); @@ -141,8 +140,8 @@ ARpcServer* ARpcServer_newUnixDomainBootstrap(AIBinder* service, int bootstrapFd ARpcServer* ARpcServer_newInet(AIBinder* service, const char* address, unsigned int port) { auto server = RpcServer::make(); if (status_t status = server->setupInetServer(address, port, nullptr); status != OK) { - LOG(ERROR) << "Failed to set up inet RPC server with address " << address << " and port " - << port << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up inet RPC server with address %s and port %u error: %s", address, + port, statusToString(status).c_str()); return nullptr; } server->setRootObject(AIBinder_toPlatformBinder(service)); @@ -191,8 +190,8 @@ void ARpcSession_free(ARpcSession* handle) { AIBinder* ARpcSession_setupVsockClient(ARpcSession* handle, unsigned int cid, unsigned int port) { auto session = handleToStrongPointer<RpcSession>(handle); if (status_t status = session->setupVsockClient(cid, port); status != OK) { - LOG(ERROR) << "Failed to set up vsock client with CID " << cid << " and port " << port - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up vsock client with CID %u and port %u error: %s", cid, port, + statusToString(status).c_str()); return nullptr; } return AIBinder_fromPlatformBinder(session->getRootObject()); @@ -203,8 +202,8 @@ AIBinder* ARpcSession_setupUnixDomainClient(ARpcSession* handle, const char* nam pathname = ANDROID_SOCKET_DIR "/" + pathname; auto session = handleToStrongPointer<RpcSession>(handle); if (status_t status = session->setupUnixDomainClient(pathname.c_str()); status != OK) { - LOG(ERROR) << "Failed to set up Unix Domain RPC client with path: " << pathname - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up Unix Domain RPC client with path: %s error: %s", pathname.c_str(), + statusToString(status).c_str()); return nullptr; } return AIBinder_fromPlatformBinder(session->getRootObject()); @@ -214,13 +213,13 @@ AIBinder* ARpcSession_setupUnixDomainBootstrapClient(ARpcSession* handle, int bo auto session = handleToStrongPointer<RpcSession>(handle); auto fd = unique_fd(dup(bootstrapFd)); if (!fd.ok()) { - LOG(ERROR) << "Invalid bootstrap fd " << bootstrapFd; + ALOGE("Invalid bootstrap fd %d", bootstrapFd); return nullptr; } if (status_t status = session->setupUnixDomainSocketBootstrapClient(std::move(fd)); status != OK) { - LOG(ERROR) << "Failed to set up Unix Domain RPC client with bootstrap fd: " << bootstrapFd - << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up Unix Domain RPC client with bootstrap fd: %d error: %s", + bootstrapFd, statusToString(status).c_str()); return nullptr; } return AIBinder_fromPlatformBinder(session->getRootObject()); @@ -229,8 +228,8 @@ AIBinder* ARpcSession_setupUnixDomainBootstrapClient(ARpcSession* handle, int bo AIBinder* ARpcSession_setupInet(ARpcSession* handle, const char* address, unsigned int port) { auto session = handleToStrongPointer<RpcSession>(handle); if (status_t status = session->setupInetClient(address, port); status != OK) { - LOG(ERROR) << "Failed to set up inet RPC client with address " << address << " and port " - << port << " error: " << statusToString(status).c_str(); + ALOGE("Failed to set up inet RPC client with address %s and port %u error: %s", address, + port, statusToString(status).c_str()); return nullptr; } return AIBinder_fromPlatformBinder(session->getRootObject()); @@ -241,7 +240,7 @@ AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* handle, int (*request auto session = handleToStrongPointer<RpcSession>(handle); auto request = [=] { return unique_fd{requestFd(param)}; }; if (status_t status = session->setupPreconnectedClient(unique_fd{}, request); status != OK) { - LOG(ERROR) << "Failed to set up vsock client. error: " << statusToString(status).c_str(); + ALOGE("Failed to set up vsock client. error: %s", statusToString(status).c_str()); return nullptr; } return AIBinder_fromPlatformBinder(session->getRootObject()); diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index 47da296b70..bf7a0ba5f0 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -14,7 +14,6 @@ * limitations under the License. */ -#include <android-base/logging.h> #include <android/binder_ibinder.h> #include <android/binder_ibinder_platform.h> #include <android/binder_stability.h> @@ -48,8 +47,8 @@ static void* kValue = static_cast<void*>(new bool{true}); void clean(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */}; static void attach(const sp<IBinder>& binder) { - // can only attach once - CHECK_EQ(nullptr, binder->attachObject(kId, kValue, nullptr /*cookie*/, clean)); + auto alreadyAttached = binder->attachObject(kId, kValue, nullptr /*cookie*/, clean); + LOG_ALWAYS_FATAL_IF(alreadyAttached != nullptr, "can only attach once"); } static bool has(const sp<IBinder>& binder) { return binder != nullptr && binder->findObject(kId) == kValue; @@ -65,9 +64,9 @@ struct Value { }; void clean(const void* id, void* obj, void* cookie) { // be weary of leaks! - // LOG(INFO) << "Deleting an ABpBinder"; + // ALOGI("Deleting an ABpBinder"); - CHECK(id == kId) << id << " " << obj << " " << cookie; + LOG_ALWAYS_FATAL_IF(id != kId, "%p %p %p", id, obj, cookie); delete static_cast<Value*>(obj); }; @@ -121,14 +120,13 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) { if (mClazz != nullptr && !asABpBinder()) { const String16& currentDescriptor = mClazz->getInterfaceDescriptor(); if (newDescriptor == currentDescriptor) { - LOG(ERROR) << __func__ << ": Class descriptors '" << currentDescriptor - << "' match during associateClass, but they are different class objects (" - << clazz << " vs " << mClazz << "). Class descriptor collision?"; + ALOGE("Class descriptors '%s' match during associateClass, but they are different class" + " objects (%p vs %p). Class descriptor collision?", + String8(currentDescriptor).c_str(), clazz, mClazz); } else { - LOG(ERROR) << __func__ - << ": Class cannot be associated on object which already has a class. " - "Trying to associate to '" - << newDescriptor << "' but already set to '" << currentDescriptor << "'."; + ALOGE("%s: Class cannot be associated on object which already has a class. " + "Trying to associate to '%s' but already set to '%s'.", + __func__, String8(newDescriptor).c_str(), String8(currentDescriptor).c_str()); } // always a failure because we know mClazz != clazz @@ -141,13 +139,12 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) { // more flake-proof. However, the check is not dependent on the lock. if (descriptor != newDescriptor && !(asABpBinder() && asABpBinder()->isServiceFuzzing())) { if (getBinder()->isBinderAlive()) { - LOG(ERROR) << __func__ << ": Expecting binder to have class '" << newDescriptor - << "' but descriptor is actually '" << SanitizeString(descriptor) << "'."; + ALOGE("%s: Expecting binder to have class '%s' but descriptor is actually '%s'.", + __func__, String8(newDescriptor).c_str(), SanitizeString(descriptor).c_str()); } else { // b/155793159 - LOG(ERROR) << __func__ << ": Cannot associate class '" << newDescriptor - << "' to dead binder with cached descriptor '" << SanitizeString(descriptor) - << "'."; + ALOGE("%s: Cannot associate class '%s' to dead binder with cached descriptor '%s'.", + __func__, String8(newDescriptor).c_str(), SanitizeString(descriptor).c_str()); } return false; } @@ -164,7 +161,7 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) { ABBinder::ABBinder(const AIBinder_Class* clazz, void* userData) : AIBinder(clazz), BBinder(), mUserData(userData) { - CHECK(clazz != nullptr); + LOG_ALWAYS_FATAL_IF(clazz == nullptr, "clazz == nullptr"); } ABBinder::~ABBinder() { getClass()->onDestroy(mUserData); @@ -184,7 +181,7 @@ status_t ABBinder::dump(int fd, const ::android::Vector<String16>& args) { // technically UINT32_MAX would be okay here, but INT32_MAX is expected since this may be // null in Java if (args.size() > INT32_MAX) { - LOG(ERROR) << "ABBinder::dump received too many arguments: " << args.size(); + ALOGE("ABBinder::dump received too many arguments: %zu", args.size()); return STATUS_BAD_VALUE; } @@ -263,7 +260,7 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce ABpBinder::ABpBinder(const ::android::sp<::android::IBinder>& binder) : AIBinder(nullptr /*clazz*/), mRemote(binder) { - CHECK(binder != nullptr); + LOG_ALWAYS_FATAL_IF(binder == nullptr, "binder == nullptr"); } ABpBinder::~ABpBinder() {} @@ -373,27 +370,27 @@ AIBinder_Class* AIBinder_Class_define(const char* interfaceDescriptor, } void AIBinder_Class_setOnDump(AIBinder_Class* clazz, AIBinder_onDump onDump) { - CHECK(clazz != nullptr) << "setOnDump requires non-null clazz"; + LOG_ALWAYS_FATAL_IF(clazz == nullptr, "setOnDump requires non-null clazz"); // this is required to be called before instances are instantiated clazz->onDump = onDump; } void AIBinder_Class_disableInterfaceTokenHeader(AIBinder_Class* clazz) { - CHECK(clazz != nullptr) << "disableInterfaceTokenHeader requires non-null clazz"; + LOG_ALWAYS_FATAL_IF(clazz == nullptr, "disableInterfaceTokenHeader requires non-null clazz"); clazz->writeHeader = false; } void AIBinder_Class_setHandleShellCommand(AIBinder_Class* clazz, AIBinder_handleShellCommand handleShellCommand) { - CHECK(clazz != nullptr) << "setHandleShellCommand requires non-null clazz"; + LOG_ALWAYS_FATAL_IF(clazz == nullptr, "setHandleShellCommand requires non-null clazz"); clazz->handleShellCommand = handleShellCommand; } const char* AIBinder_Class_getDescriptor(const AIBinder_Class* clazz) { - CHECK(clazz != nullptr) << "getDescriptor requires non-null clazz"; + LOG_ALWAYS_FATAL_IF(clazz == nullptr, "getDescriptor requires non-null clazz"); return clazz->getInterfaceDescriptorUtf8(); } @@ -405,8 +402,8 @@ AIBinder_DeathRecipient::TransferDeathRecipient::~TransferDeathRecipient() { } void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinder>& who) { - CHECK(who == mWho) << who.unsafe_get() << "(" << who.get_refs() << ") vs " << mWho.unsafe_get() - << " (" << mWho.get_refs() << ")"; + LOG_ALWAYS_FATAL_IF(who != mWho, "%p (%p) vs %p (%p)", who.unsafe_get(), who.get_refs(), + mWho.unsafe_get(), mWho.get_refs()); mOnDied(mCookie); @@ -417,7 +414,7 @@ void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinde if (recipient != nullptr && strongWho != nullptr) { status_t result = recipient->unlinkToDeath(strongWho, mCookie); if (result != ::android::DEAD_OBJECT) { - LOG(WARNING) << "Unlinking to dead binder resulted in: " << result; + ALOGW("Unlinking to dead binder resulted in: %d", result); } } @@ -426,7 +423,7 @@ void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinde AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied) : mOnDied(onDied), mOnUnlinked(nullptr) { - CHECK(onDied != nullptr); + LOG_ALWAYS_FATAL_IF(onDied == nullptr, "onDied == nullptr"); } void AIBinder_DeathRecipient::pruneDeadTransferEntriesLocked() { @@ -438,7 +435,7 @@ void AIBinder_DeathRecipient::pruneDeadTransferEntriesLocked() { } binder_status_t AIBinder_DeathRecipient::linkToDeath(const sp<IBinder>& binder, void* cookie) { - CHECK(binder != nullptr); + LOG_ALWAYS_FATAL_IF(binder == nullptr, "binder == nullptr"); std::lock_guard<std::mutex> l(mDeathRecipientsMutex); @@ -459,7 +456,7 @@ binder_status_t AIBinder_DeathRecipient::linkToDeath(const sp<IBinder>& binder, } binder_status_t AIBinder_DeathRecipient::unlinkToDeath(const sp<IBinder>& binder, void* cookie) { - CHECK(binder != nullptr); + LOG_ALWAYS_FATAL_IF(binder == nullptr, "binder == nullptr"); std::lock_guard<std::mutex> l(mDeathRecipientsMutex); @@ -471,9 +468,8 @@ binder_status_t AIBinder_DeathRecipient::unlinkToDeath(const sp<IBinder>& binder status_t status = binder->unlinkToDeath(recipient, cookie, 0 /*flags*/); if (status != ::android::OK) { - LOG(ERROR) << __func__ - << ": removed reference to death recipient but unlink failed: " - << statusToString(status); + ALOGE("%s: removed reference to death recipient but unlink failed: %s", __func__, + statusToString(status).c_str()); } return PruneStatusT(status); } @@ -490,7 +486,7 @@ void AIBinder_DeathRecipient::setOnUnlinked(AIBinder_DeathRecipient_onBinderUnli AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args) { if (clazz == nullptr) { - LOG(ERROR) << __func__ << ": Must provide class to construct local binder."; + ALOGE("%s: Must provide class to construct local binder.", __func__); return nullptr; } @@ -554,8 +550,7 @@ binder_status_t AIBinder_dump(AIBinder* binder, int fd, const char** args, uint3 binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient, void* cookie) { if (binder == nullptr || recipient == nullptr) { - LOG(ERROR) << __func__ << ": Must provide binder (" << binder << ") and recipient (" - << recipient << ")"; + ALOGE("%s: Must provide binder (%p) and recipient (%p)", __func__, binder, recipient); return STATUS_UNEXPECTED_NULL; } @@ -566,8 +561,7 @@ binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient, void* cookie) { if (binder == nullptr || recipient == nullptr) { - LOG(ERROR) << __func__ << ": Must provide binder (" << binder << ") and recipient (" - << recipient << ")"; + ALOGE("%s: Must provide binder (%p) and recipient (%p)", __func__, binder, recipient); return STATUS_UNEXPECTED_NULL; } @@ -596,7 +590,7 @@ void AIBinder_incStrong(AIBinder* binder) { } void AIBinder_decStrong(AIBinder* binder) { if (binder == nullptr) { - LOG(ERROR) << __func__ << ": on null binder"; + ALOGE("%s: on null binder", __func__); return; } @@ -604,7 +598,7 @@ void AIBinder_decStrong(AIBinder* binder) { } int32_t AIBinder_debugGetRefCount(AIBinder* binder) { if (binder == nullptr) { - LOG(ERROR) << __func__ << ": on null binder"; + ALOGE("%s: on null binder", __func__); return -1; } @@ -642,15 +636,14 @@ void* AIBinder_getUserData(AIBinder* binder) { binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in) { if (binder == nullptr || in == nullptr) { - LOG(ERROR) << __func__ << ": requires non-null parameters binder (" << binder - << ") and in (" << in << ")."; + ALOGE("%s: requires non-null parameters binder (%p) and in (%p).", __func__, binder, in); return STATUS_UNEXPECTED_NULL; } const AIBinder_Class* clazz = binder->getClass(); if (clazz == nullptr) { - LOG(ERROR) << __func__ - << ": Class must be defined for a remote binder transaction. See " - "AIBinder_associateClass."; + ALOGE("%s: Class must be defined for a remote binder transaction. See " + "AIBinder_associateClass.", + __func__); return STATUS_INVALID_OPERATION; } @@ -683,7 +676,7 @@ static void DestroyParcel(AParcel** parcel) { binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, AParcel** in, AParcel** out, binder_flags_t flags) { if (in == nullptr) { - LOG(ERROR) << __func__ << ": requires non-null in parameter"; + ALOGE("%s: requires non-null in parameter", __func__); return STATUS_UNEXPECTED_NULL; } @@ -693,27 +686,26 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa AutoParcelDestroyer forIn(in, DestroyParcel); if (!isUserCommand(code)) { - LOG(ERROR) << __func__ - << ": Only user-defined transactions can be made from the NDK, but requested: " - << code; + ALOGE("%s: Only user-defined transactions can be made from the NDK, but requested: %d", + __func__, code); return STATUS_UNKNOWN_TRANSACTION; } constexpr binder_flags_t kAllFlags = FLAG_PRIVATE_VENDOR | FLAG_ONEWAY | FLAG_CLEAR_BUF; if ((flags & ~kAllFlags) != 0) { - LOG(ERROR) << __func__ << ": Unrecognized flags sent: " << flags; + ALOGE("%s: Unrecognized flags sent: %d", __func__, flags); return STATUS_BAD_VALUE; } if (binder == nullptr || *in == nullptr || out == nullptr) { - LOG(ERROR) << __func__ << ": requires non-null parameters binder (" << binder << "), in (" - << in << "), and out (" << out << ")."; + ALOGE("%s: requires non-null parameters binder (%p), in (%p), and out (%p).", __func__, + binder, in, out); return STATUS_UNEXPECTED_NULL; } if ((*in)->getBinder() != binder) { - LOG(ERROR) << __func__ << ": parcel is associated with binder object " << binder - << " but called with " << (*in)->getBinder(); + ALOGE("%s: parcel is associated with binder object %p but called with %p", __func__, binder, + (*in)->getBinder()); return STATUS_BAD_VALUE; } @@ -733,7 +725,7 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa AIBinder_DeathRecipient* AIBinder_DeathRecipient_new( AIBinder_DeathRecipient_onBinderDied onBinderDied) { if (onBinderDied == nullptr) { - LOG(ERROR) << __func__ << ": requires non-null onBinderDied parameter."; + ALOGE("%s: requires non-null onBinderDied parameter.", __func__); return nullptr; } auto ret = new AIBinder_DeathRecipient(onBinderDied); @@ -799,9 +791,8 @@ binder_status_t AIBinder_setExtension(AIBinder* binder, AIBinder* ext) { void AIBinder_setRequestingSid(AIBinder* binder, bool requestingSid) { ABBinder* localBinder = binder->asABBinder(); - if (localBinder == nullptr) { - LOG(FATAL) << "AIBinder_setRequestingSid must be called on a local binder"; - } + LOG_ALWAYS_FATAL_IF(localBinder == nullptr, + "AIBinder_setRequestingSid must be called on a local binder"); localBinder->setRequestingSid(requestingSid); } @@ -816,9 +807,8 @@ void AIBinder_setMinSchedulerPolicy(AIBinder* binder, int policy, int priority) void AIBinder_setInheritRt(AIBinder* binder, bool inheritRt) { ABBinder* localBinder = binder->asABBinder(); - if (localBinder == nullptr) { - LOG(FATAL) << "AIBinder_setInheritRt must be called on a local binder"; - } + LOG_ALWAYS_FATAL_IF(localBinder == nullptr, + "AIBinder_setInheritRt must be called on a local binder"); localBinder->setInheritRt(inheritRt); } diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp index 037aa2e120..c15bcf968d 100644 --- a/libs/binder/ndk/parcel.cpp +++ b/libs/binder/ndk/parcel.cpp @@ -14,21 +14,20 @@ * limitations under the License. */ +#include <android-base/unique_fd.h> #include <android/binder_parcel.h> #include <android/binder_parcel_platform.h> -#include "parcel_internal.h" - -#include "ibinder_internal.h" -#include "status_internal.h" - -#include <limits> - -#include <android-base/logging.h> -#include <android-base/unique_fd.h> #include <binder/Parcel.h> #include <binder/ParcelFileDescriptor.h> +#include <inttypes.h> #include <utils/Unicode.h> +#include <limits> + +#include "ibinder_internal.h" +#include "parcel_internal.h" +#include "status_internal.h" + using ::android::IBinder; using ::android::Parcel; using ::android::sp; @@ -52,11 +51,11 @@ static binder_status_t WriteAndValidateArraySize(AParcel* parcel, bool isNullArr if (length < -1) return STATUS_BAD_VALUE; if (!isNullArray && length < 0) { - LOG(ERROR) << __func__ << ": non-null array but length is " << length; + ALOGE("non-null array but length is %" PRIi32, length); return STATUS_BAD_VALUE; } if (isNullArray && length > 0) { - LOG(ERROR) << __func__ << ": null buffer cannot be for size " << length << " array."; + ALOGE("null buffer cannot be for size %" PRIi32 " array.", length); return STATUS_BAD_VALUE; } @@ -325,7 +324,7 @@ binder_status_t AParcel_readStatusHeader(const AParcel* parcel, AStatus** status binder_status_t AParcel_writeString(AParcel* parcel, const char* string, int32_t length) { if (string == nullptr) { if (length != -1) { - LOG(WARNING) << __func__ << ": null string must be used with length == -1."; + ALOGW("null string must be used with length == -1."); return STATUS_BAD_VALUE; } @@ -334,7 +333,7 @@ binder_status_t AParcel_writeString(AParcel* parcel, const char* string, int32_t } if (length < 0) { - LOG(WARNING) << __func__ << ": Negative string length: " << length; + ALOGW("Negative string length: %" PRIi32, length); return STATUS_BAD_VALUE; } @@ -342,7 +341,7 @@ binder_status_t AParcel_writeString(AParcel* parcel, const char* string, int32_t const ssize_t len16 = utf8_to_utf16_length(str8, length); if (len16 < 0 || len16 >= std::numeric_limits<int32_t>::max()) { - LOG(WARNING) << __func__ << ": Invalid string length: " << len16; + ALOGW("Invalid string length: %zd", len16); return STATUS_BAD_VALUE; } @@ -383,7 +382,7 @@ binder_status_t AParcel_readString(const AParcel* parcel, void* stringData, } if (len8 <= 0 || len8 > std::numeric_limits<int32_t>::max()) { - LOG(WARNING) << __func__ << ": Invalid string length: " << len8; + ALOGW("Invalid string length: %zd", len8); return STATUS_BAD_VALUE; } @@ -391,7 +390,7 @@ binder_status_t AParcel_readString(const AParcel* parcel, void* stringData, bool success = allocator(stringData, len8, &str8); if (!success || str8 == nullptr) { - LOG(WARNING) << __func__ << ": AParcel_stringAllocator failed to allocate."; + ALOGW("AParcel_stringAllocator failed to allocate."); return STATUS_NO_MEMORY; } diff --git a/libs/binder/ndk/process.cpp b/libs/binder/ndk/process.cpp index 0fea57b0a7..0072ac3d3e 100644 --- a/libs/binder/ndk/process.cpp +++ b/libs/binder/ndk/process.cpp @@ -15,12 +15,10 @@ */ #include <android/binder_process.h> +#include <binder/IPCThreadState.h> #include <mutex> -#include <android-base/logging.h> -#include <binder/IPCThreadState.h> - using ::android::IPCThreadState; using ::android::ProcessState; diff --git a/libs/binder/ndk/service_manager.cpp b/libs/binder/ndk/service_manager.cpp index 29777866e2..3bfdc59ec2 100644 --- a/libs/binder/ndk/service_manager.cpp +++ b/libs/binder/ndk/service_manager.cpp @@ -15,14 +15,12 @@ */ #include <android/binder_manager.h> +#include <binder/IServiceManager.h> +#include <binder/LazyServiceRegistrar.h> #include "ibinder_internal.h" #include "status_internal.h" -#include <android-base/logging.h> -#include <binder/IServiceManager.h> -#include <binder/LazyServiceRegistrar.h> - using ::android::defaultServiceManager; using ::android::IBinder; using ::android::IServiceManager; @@ -115,7 +113,8 @@ struct AServiceManager_NotificationRegistration std::lock_guard<std::mutex> l(m); if (onRegister == nullptr) return; - CHECK_EQ(String8(smInstance), instance); + LOG_ALWAYS_FATAL_IF(String8(smInstance) != instance, "onServiceRegistration: %s != %s", + String8(smInstance).c_str(), instance); sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(binder); AIBinder_incStrong(ret.get()); @@ -135,8 +134,8 @@ __attribute__((warn_unused_result)) AServiceManager_NotificationRegistration* AServiceManager_registerForServiceNotifications(const char* instance, AServiceManager_onRegister onRegister, void* cookie) { - CHECK_NE(instance, nullptr); - CHECK_NE(onRegister, nullptr) << instance; + LOG_ALWAYS_FATAL_IF(instance == nullptr, "instance == nullptr"); + LOG_ALWAYS_FATAL_IF(onRegister == nullptr, "onRegister == nullptr for %s", instance); // cookie can be nullptr auto cb = sp<AServiceManager_NotificationRegistration>::make(); @@ -146,8 +145,8 @@ AServiceManager_registerForServiceNotifications(const char* instance, sp<IServiceManager> sm = defaultServiceManager(); if (status_t res = sm->registerForNotifications(String16(instance), cb); res != STATUS_OK) { - LOG(ERROR) << "Failed to register for service notifications for " << instance << ": " - << statusToString(res); + ALOGE("Failed to register for service notifications for %s: %s", instance, + statusToString(res).c_str()); return nullptr; } @@ -157,7 +156,7 @@ AServiceManager_registerForServiceNotifications(const char* instance, void AServiceManager_NotificationRegistration_delete( AServiceManager_NotificationRegistration* notification) { - CHECK_NE(notification, nullptr); + LOG_ALWAYS_FATAL_IF(notification == nullptr, "notification == nullptr"); notification->clear(); notification->decStrong(nullptr); } @@ -172,9 +171,9 @@ bool AServiceManager_isDeclared(const char* instance) { } void AServiceManager_forEachDeclaredInstance(const char* interface, void* context, void (*callback)(const char*, void*)) { - CHECK(interface != nullptr); + LOG_ALWAYS_FATAL_IF(interface == nullptr, "interface == nullptr"); // context may be nullptr - CHECK(callback != nullptr); + LOG_ALWAYS_FATAL_IF(callback == nullptr, "callback == nullptr"); sp<IServiceManager> sm = defaultServiceManager(); for (const String16& instance : sm->getDeclaredInstances(String16(interface))) { @@ -191,9 +190,9 @@ bool AServiceManager_isUpdatableViaApex(const char* instance) { } void AServiceManager_getUpdatableApexName(const char* instance, void* context, void (*callback)(const char*, void*)) { - CHECK_NE(instance, nullptr); + LOG_ALWAYS_FATAL_IF(instance == nullptr, "instance == nullptr"); // context may be nullptr - CHECK_NE(callback, nullptr); + LOG_ALWAYS_FATAL_IF(callback == nullptr, "callback == nullptr"); sp<IServiceManager> sm = defaultServiceManager(); std::optional<String16> updatableViaApex = sm->updatableViaApex(String16(instance)); diff --git a/libs/binder/ndk/status.cpp b/libs/binder/ndk/status.cpp index 8ed91a5314..3aac3c0545 100644 --- a/libs/binder/ndk/status.cpp +++ b/libs/binder/ndk/status.cpp @@ -17,8 +17,6 @@ #include <android/binder_status.h> #include "status_internal.h" -#include <android-base/logging.h> - using ::android::status_t; using ::android::statusToString; using ::android::binder::Status; @@ -127,8 +125,8 @@ binder_status_t PruneStatusT(status_t status) { return STATUS_UNKNOWN_ERROR; default: - LOG(WARNING) << __func__ << ": Unknown status_t (" << statusToString(status) - << ") pruned into STATUS_UNKNOWN_ERROR"; + ALOGW("%s: Unknown status_t (%s) pruned into STATUS_UNKNOWN_ERROR", __func__, + statusToString(status).c_str()); return STATUS_UNKNOWN_ERROR; } } @@ -159,8 +157,8 @@ binder_exception_t PruneException(int32_t exception) { return EX_TRANSACTION_FAILED; default: - LOG(WARNING) << __func__ << ": Unknown binder exception (" << exception - << ") pruned into EX_TRANSACTION_FAILED"; + ALOGW("%s: Unknown binder exception (%d) pruned into EX_TRANSACTION_FAILED", __func__, + exception); return EX_TRANSACTION_FAILED; } } diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 78f8877c1d..6d122c5388 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -27,7 +27,7 @@ use std::cmp::Ordering; use std::convert::TryFrom; use std::ffi::{c_void, CStr, CString}; use std::fmt; -use std::fs::File; +use std::io::Write; use std::marker::PhantomData; use std::ops::Deref; use std::os::raw::c_char; @@ -62,7 +62,7 @@ pub trait Interface: Send + Sync + DowncastSync { /// /// This handler is a no-op by default and should be implemented for each /// Binder service struct that wishes to respond to dump transactions. - fn dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + fn dump(&self, _writer: &mut dyn Write, _args: &[&CStr]) -> Result<()> { Ok(()) } } @@ -165,7 +165,7 @@ pub trait Remotable: Send + Sync + 'static { /// Handle a request to invoke the dump transaction on this /// object. - fn on_dump(&self, file: &File, args: &[&CStr]) -> Result<()>; + fn on_dump(&self, file: &mut dyn Write, args: &[&CStr]) -> Result<()>; /// Retrieve the class of this remote object. /// @@ -934,8 +934,8 @@ macro_rules! declare_binder_interface { } } - fn on_dump(&self, file: &std::fs::File, args: &[&std::ffi::CStr]) -> std::result::Result<(), $crate::StatusCode> { - self.0.dump(file, args) + fn on_dump(&self, writer: &mut dyn std::io::Write, args: &[&std::ffi::CStr]) -> std::result::Result<(), $crate::StatusCode> { + self.0.dump(writer, args) } fn get_class() -> $crate::binder_impl::InterfaceClass { diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs index b248f5eb28..b250012801 100644 --- a/libs/binder/rust/src/native.rs +++ b/libs/binder/rust/src/native.rs @@ -25,6 +25,7 @@ use crate::sys; use std::convert::TryFrom; use std::ffi::{c_void, CStr, CString}; use std::fs::File; +use std::io::Write; use std::mem::ManuallyDrop; use std::ops::Deref; use std::os::raw::c_char; @@ -341,7 +342,7 @@ impl<T: Remotable> InterfaceClassMethods for Binder<T> { } // Safety: Our caller promised that fd is a file descriptor. We don't // own this file descriptor, so we need to be careful not to drop it. - let file = unsafe { ManuallyDrop::new(File::from_raw_fd(fd)) }; + let mut file = unsafe { ManuallyDrop::new(File::from_raw_fd(fd)) }; if args.is_null() && num_args != 0 { return StatusCode::UNEXPECTED_NULL as status_t; @@ -366,7 +367,7 @@ impl<T: Remotable> InterfaceClassMethods for Binder<T> { // Safety: Our caller promised that the binder has a `T` pointer in its // user data. let binder: &T = unsafe { &*(object as *const T) }; - let res = binder.on_dump(&file, &args); + let res = binder.on_dump(&mut *file, &args); match res { Ok(()) => 0, @@ -569,7 +570,7 @@ impl Remotable for () { Ok(()) } - fn on_dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + fn on_dump(&self, _writer: &mut dyn Write, _args: &[&CStr]) -> Result<()> { Ok(()) } diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index c049b807df..c87fa89756 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -26,7 +26,7 @@ use binder::binder_impl::{ use std::convert::{TryFrom, TryInto}; use std::ffi::CStr; -use std::fs::File; +use std::io::Write; use std::sync::Mutex; /// Name of service runner. @@ -118,7 +118,7 @@ impl TryFrom<u32> for TestTransactionCode { } impl Interface for TestService { - fn dump(&self, _file: &File, args: &[&CStr]) -> Result<(), StatusCode> { + fn dump(&self, _writer: &mut dyn Write, args: &[&CStr]) -> Result<(), StatusCode> { let mut dump_args = self.dump_args.lock().unwrap(); dump_args.extend(args.iter().map(|s| s.to_str().unwrap().to_owned())); Ok(()) diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index bbb310a538..624edba9cd 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -87,8 +87,8 @@ public: android::base::borrowed_fd /* readEnd */)>& f) { android::base::unique_fd childWriteEnd; android::base::unique_fd childReadEnd; - CHECK(android::base::Pipe(&mReadEnd, &childWriteEnd, 0)) << strerror(errno); - CHECK(android::base::Pipe(&childReadEnd, &mWriteEnd, 0)) << strerror(errno); + if (!android::base::Pipe(&mReadEnd, &childWriteEnd, 0)) PLOGF("child write pipe failed"); + if (!android::base::Pipe(&childReadEnd, &mWriteEnd, 0)) PLOGF("child read pipe failed"); if (0 == (mPid = fork())) { // racey: assume parent doesn't crash before this is set prctl(PR_SET_PDEATHSIG, SIGHUP); @@ -146,8 +146,10 @@ static base::unique_fd initUnixSocket(std::string addr) { auto socket_addr = UnixSocketAddress(addr.c_str()); base::unique_fd fd( TEMP_FAILURE_RETRY(socket(socket_addr.addr()->sa_family, SOCK_STREAM, AF_UNIX))); - CHECK(fd.ok()); - CHECK_EQ(0, TEMP_FAILURE_RETRY(bind(fd.get(), socket_addr.addr(), socket_addr.addrSize()))); + if (!fd.ok()) PLOGF("initUnixSocket failed to create socket"); + if (0 != TEMP_FAILURE_RETRY(bind(fd.get(), socket_addr.addr(), socket_addr.addrSize()))) { + PLOGF("initUnixSocket failed to bind"); + } return fd; } @@ -205,14 +207,12 @@ public: static base::unique_fd connectTo(const RpcSocketAddress& addr) { base::unique_fd serverFd( TEMP_FAILURE_RETRY(socket(addr.addr()->sa_family, SOCK_STREAM | SOCK_CLOEXEC, 0))); - int savedErrno = errno; - CHECK(serverFd.ok()) << "Could not create socket " << addr.toString() << ": " - << strerror(savedErrno); + if (!serverFd.ok()) { + PLOGF("Could not create socket %s", addr.toString().c_str()); + } if (0 != TEMP_FAILURE_RETRY(connect(serverFd.get(), addr.addr(), addr.addrSize()))) { - int savedErrno = errno; - LOG(FATAL) << "Could not connect to socket " << addr.toString() << ": " - << strerror(savedErrno); + PLOGF("Could not connect to socket %s", addr.toString().c_str()); } return serverFd; } @@ -221,8 +221,7 @@ static base::unique_fd connectTo(const RpcSocketAddress& addr) { static base::unique_fd connectToUnixBootstrap(const RpcTransportFd& transportFd) { base::unique_fd sockClient, sockServer; if (!base::Socketpair(SOCK_STREAM, &sockClient, &sockServer)) { - int savedErrno = errno; - LOG(FATAL) << "Failed socketpair(): " << strerror(savedErrno); + PLOGF("Failed socketpair()"); } int zero = 0; @@ -231,8 +230,7 @@ static base::unique_fd connectToUnixBootstrap(const RpcTransportFd& transportFd) fds.emplace_back(std::move(sockServer)); if (binder::os::sendMessageOnSocket(transportFd, &iov, 1, &fds) < 0) { - int savedErrno = errno; - LOG(FATAL) << "Failed sendMessageOnSocket: " << strerror(savedErrno); + PLOGF("Failed sendMessageOnSocket"); } return std::move(sockClient); } @@ -246,10 +244,12 @@ std::unique_ptr<RpcTransportCtxFactory> BinderRpc::newFactory(RpcSecurity rpcSec // threads. std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( const BinderRpcOptions& options) { - CHECK_GE(options.numSessions, 1) << "Must have at least one session to a server"; + LOG_ALWAYS_FATAL_IF(options.numSessions < 1, "Must have at least one session to a server"); if (options.numIncomingConnectionsBySession.size() != 0) { - CHECK_EQ(options.numIncomingConnectionsBySession.size(), options.numSessions); + LOG_ALWAYS_FATAL_IF(options.numIncomingConnectionsBySession.size() != options.numSessions, + "%s: %zu != %zu", __func__, + options.numIncomingConnectionsBySession.size(), options.numSessions); } SocketType socketType = GetParam().type; @@ -274,8 +274,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( // Do not set O_CLOEXEC, bootstrapServerFd needs to survive fork/exec. // This is because we cannot pass ParcelFileDescriptor over a pipe. if (!base::Socketpair(SOCK_STREAM, &bootstrapClientFd, &socketFd)) { - int savedErrno = errno; - LOG(FATAL) << "Failed socketpair(): " << strerror(savedErrno); + PLOGF("Failed socketpair()"); } } @@ -288,8 +287,10 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( auto writeFd = std::to_string(writeEnd.get()); auto readFd = std::to_string(readEnd.get()); - execl(servicePath.c_str(), servicePath.c_str(), writeFd.c_str(), readFd.c_str(), - NULL); + auto status = execl(servicePath.c_str(), servicePath.c_str(), writeFd.c_str(), + readFd.c_str(), NULL); + PLOGF("execl('%s', _, %s, %s) should not return at all, but it returned %d", + servicePath.c_str(), writeFd.c_str(), readFd.c_str(), status); })); BinderRpcTestServerConfig serverConfig; @@ -334,16 +335,16 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( } writeToFd(ret->host.writeEnd(), clientInfo); - CHECK_LE(serverInfo.port, std::numeric_limits<unsigned int>::max()); + LOG_ALWAYS_FATAL_IF(serverInfo.port > std::numeric_limits<unsigned int>::max()); if (socketType == SocketType::INET) { - CHECK_NE(0, serverInfo.port); + LOG_ALWAYS_FATAL_IF(0 == serverInfo.port); } if (rpcSecurity == RpcSecurity::TLS) { const auto& serverCert = serverInfo.cert.data; - CHECK_EQ(OK, - certVerifier->addTrustedPeerCertificate(RpcCertificateFormat::PEM, - serverCert)); + LOG_ALWAYS_FATAL_IF( + OK != + certVerifier->addTrustedPeerCertificate(RpcCertificateFormat::PEM, serverCert)); } } @@ -356,7 +357,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( ? options.numIncomingConnectionsBySession.at(i) : 0; - CHECK(session->setProtocolVersion(clientVersion)); + LOG_ALWAYS_FATAL_IF(!session->setProtocolVersion(clientVersion)); session->setMaxIncomingThreads(numIncoming); session->setMaxOutgoingConnections(options.numOutgoingConnections); session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode); @@ -408,7 +409,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( ret->sessions.clear(); break; } - CHECK_EQ(status, OK) << "Could not connect: " << statusToString(status); + LOG_ALWAYS_FATAL_IF(status != OK, "Could not connect: %s", statusToString(status).c_str()); ret->sessions.push_back({session, session->getRootObject()}); } return ret; @@ -589,12 +590,12 @@ TEST_P(BinderRpc, OnewayCallQueueingWithFds) { android::os::ParcelFileDescriptor fdA; EXPECT_OK(proc.rootIface->blockingRecvFd(&fdA)); std::string result; - CHECK(android::base::ReadFdToString(fdA.get(), &result)); + ASSERT_TRUE(android::base::ReadFdToString(fdA.get(), &result)); EXPECT_EQ(result, "a"); android::os::ParcelFileDescriptor fdB; EXPECT_OK(proc.rootIface->blockingRecvFd(&fdB)); - CHECK(android::base::ReadFdToString(fdB.get(), &result)); + ASSERT_TRUE(android::base::ReadFdToString(fdB.get(), &result)); EXPECT_EQ(result, "b"); saturateThreadPool(kNumServerThreads, proc.rootIface); @@ -951,8 +952,8 @@ TEST_P(BinderRpc, ReceiveFile) { ASSERT_TRUE(status.isOk()) << status; std::string result; - CHECK(android::base::ReadFdToString(out.get(), &result)); - EXPECT_EQ(result, "hello"); + ASSERT_TRUE(android::base::ReadFdToString(out.get(), &result)); + ASSERT_EQ(result, "hello"); } TEST_P(BinderRpc, SendFiles) { @@ -981,7 +982,7 @@ TEST_P(BinderRpc, SendFiles) { ASSERT_TRUE(status.isOk()) << status; std::string result; - CHECK(android::base::ReadFdToString(out.get(), &result)); + EXPECT_TRUE(android::base::ReadFdToString(out.get(), &result)); EXPECT_EQ(result, "123abcd"); } @@ -1006,7 +1007,7 @@ TEST_P(BinderRpc, SendMaxFiles) { ASSERT_TRUE(status.isOk()) << status; std::string result; - CHECK(android::base::ReadFdToString(out.get(), &result)); + EXPECT_TRUE(android::base::ReadFdToString(out.get(), &result)); EXPECT_EQ(result, std::string(253, 'a')); } @@ -1158,7 +1159,7 @@ bool testSupportVsockLoopback() { return false; } - LOG_ALWAYS_FATAL_IF(serverFd == -1, "Could not create socket: %s", strerror(errno)); + LOG_ALWAYS_FATAL_IF(!serverFd.ok(), "Could not create socket: %s", strerror(errno)); sockaddr_vm serverAddr{ .svm_family = AF_VSOCK, @@ -1180,7 +1181,7 @@ bool testSupportVsockLoopback() { // and they return ETIMEDOUT after that. android::base::unique_fd connectFd( TEMP_FAILURE_RETRY(socket(AF_VSOCK, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0))); - LOG_ALWAYS_FATAL_IF(connectFd == -1, "Could not create socket for port %u: %s", vsockPort, + LOG_ALWAYS_FATAL_IF(!connectFd.ok(), "Could not create socket for port %u: %s", vsockPort, strerror(errno)); bool success = false; @@ -1359,7 +1360,11 @@ private: }; TEST(BinderRpc, Java) { -#if !defined(__ANDROID__) + bool expectDebuggable = false; +#if defined(__ANDROID__) + expectDebuggable = android::base::GetBoolProperty("ro.debuggable", false) && + android::base::GetProperty("ro.build.type", "") != "user"; +#else GTEST_SKIP() << "This test is only run on Android. Though it can technically run on host on" "createRpcDelegateServiceManager() with a device attached, such test belongs " "to binderHostDeviceTest. Hence, just disable this test on host."; @@ -1387,8 +1392,7 @@ TEST(BinderRpc, Java) { auto keepAlive = sp<BBinder>::make(); auto setRpcClientDebugStatus = binder->setRpcClientDebug(std::move(socket), keepAlive); - if (!android::base::GetBoolProperty("ro.debuggable", false) || - android::base::GetProperty("ro.build.type", "") == "user") { + if (!expectDebuggable) { ASSERT_EQ(INVALID_OPERATION, setRpcClientDebugStatus) << "setRpcClientDebug should return INVALID_OPERATION on non-debuggable or user " "builds, but get " @@ -1593,12 +1597,10 @@ public: iovec iov{&buf, sizeof(buf)}; if (binder::os::receiveMessageFromSocket(mFd, &iov, 1, &fds) < 0) { - int savedErrno = errno; - LOG(FATAL) << "Failed receiveMessage: " << strerror(savedErrno); - } - if (fds.size() != 1) { - LOG(FATAL) << "Expected one FD from receiveMessage(), got " << fds.size(); + PLOGF("Failed receiveMessage"); } + LOG_ALWAYS_FATAL_IF(fds.size() != 1, "Expected one FD from receiveMessage(), got %zu", + fds.size()); return std::move(std::get<base::unique_fd>(fds[0])); } @@ -2083,7 +2085,7 @@ INSTANTIATE_TEST_CASE_P( int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); - android::base::InitLogging(argv, android::base::StderrLogger, android::base::DefaultAborter); + __android_log_set_logger(__android_log_stderr_logger); return RUN_ALL_TESTS(); } diff --git a/libs/binder/tests/binderRpcTestCommon.h b/libs/binder/tests/binderRpcTestCommon.h index c3070ddd14..b2b63e4bf3 100644 --- a/libs/binder/tests/binderRpcTestCommon.h +++ b/libs/binder/tests/binderRpcTestCommon.h @@ -36,10 +36,12 @@ #include <string> #include <vector> +#ifdef __ANDROID__ +#include <android-base/properties.h> +#endif + #ifndef __TRUSTY__ #include <android-base/file.h> -#include <android-base/logging.h> -#include <android-base/properties.h> #include <android/binder_auto_utils.h> #include <android/binder_libbinder.h> #include <binder/ProcessState.h> @@ -156,21 +158,21 @@ struct BinderRpcOptions { #ifndef __TRUSTY__ static inline void writeString(android::base::borrowed_fd fd, std::string_view str) { uint64_t length = str.length(); - CHECK(android::base::WriteFully(fd, &length, sizeof(length))); - CHECK(android::base::WriteFully(fd, str.data(), str.length())); + LOG_ALWAYS_FATAL_IF(!android::base::WriteFully(fd, &length, sizeof(length))); + LOG_ALWAYS_FATAL_IF(!android::base::WriteFully(fd, str.data(), str.length())); } static inline std::string readString(android::base::borrowed_fd fd) { uint64_t length; - CHECK(android::base::ReadFully(fd, &length, sizeof(length))); + LOG_ALWAYS_FATAL_IF(!android::base::ReadFully(fd, &length, sizeof(length))); std::string ret(length, '\0'); - CHECK(android::base::ReadFully(fd, ret.data(), length)); + LOG_ALWAYS_FATAL_IF(!android::base::ReadFully(fd, ret.data(), length)); return ret; } static inline void writeToFd(android::base::borrowed_fd fd, const Parcelable& parcelable) { Parcel parcel; - CHECK_EQ(OK, parcelable.writeToParcel(&parcel)); + LOG_ALWAYS_FATAL_IF(OK != parcelable.writeToParcel(&parcel)); writeString(fd, std::string(reinterpret_cast<const char*>(parcel.data()), parcel.dataSize())); } @@ -178,9 +180,10 @@ template <typename T> static inline T readFromFd(android::base::borrowed_fd fd) { std::string data = readString(fd); Parcel parcel; - CHECK_EQ(OK, parcel.setData(reinterpret_cast<const uint8_t*>(data.data()), data.size())); + LOG_ALWAYS_FATAL_IF(OK != + parcel.setData(reinterpret_cast<const uint8_t*>(data.data()), data.size())); T object; - CHECK_EQ(OK, object.readFromParcel(&parcel)); + LOG_ALWAYS_FATAL_IF(OK != object.readFromParcel(&parcel)); return object; } @@ -207,7 +210,7 @@ static inline std::unique_ptr<RpcTransportCtxFactory> newTlsFactory( // Create an FD that returns `contents` when read. static inline base::unique_fd mockFileDescriptor(std::string contents) { android::base::unique_fd readFd, writeFd; - CHECK(android::base::Pipe(&readFd, &writeFd)) << strerror(errno); + LOG_ALWAYS_FATAL_IF(!android::base::Pipe(&readFd, &writeFd), "%s", strerror(errno)); RpcMaybeThread([writeFd = std::move(writeFd), contents = std::move(contents)]() { signal(SIGPIPE, SIG_IGN); // ignore possible SIGPIPE from the write if (!WriteStringToFd(contents, writeFd)) { diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp index 7435f308d1..5025bd617f 100644 --- a/libs/binder/tests/binderRpcTestService.cpp +++ b/libs/binder/tests/binderRpcTestService.cpp @@ -65,7 +65,7 @@ public: std::string acc; for (const auto& file : files) { std::string result; - CHECK(android::base::ReadFdToString(file.get(), &result)); + LOG_ALWAYS_FATAL_IF(!android::base::ReadFdToString(file.get(), &result)); acc.append(result); } out->reset(mockFileDescriptor(acc)); @@ -98,7 +98,7 @@ public: }; int main(int argc, char* argv[]) { - android::base::InitLogging(argv, android::base::StderrLogger, android::base::DefaultAborter); + __android_log_set_logger(__android_log_stderr_logger); LOG_ALWAYS_FATAL_IF(argc != 3, "Invalid number of arguments: %d", argc); base::unique_fd writeEnd(atoi(argv[1])); @@ -118,7 +118,7 @@ int main(int argc, char* argv[]) { auto certVerifier = std::make_shared<RpcCertificateVerifierSimple>(); sp<RpcServer> server = RpcServer::make(newTlsFactory(rpcSecurity, certVerifier)); - CHECK(server->setProtocolVersion(serverConfig.serverVersion)); + LOG_ALWAYS_FATAL_IF(!server->setProtocolVersion(serverConfig.serverVersion)); server->setMaxThreads(serverConfig.numThreads); server->setSupportedFileDescriptorTransportModes(serverSupportedFileDescriptorTransportModes); @@ -129,22 +129,25 @@ int main(int argc, char* argv[]) { case SocketType::PRECONNECTED: [[fallthrough]]; case SocketType::UNIX: - CHECK_EQ(OK, server->setupUnixDomainServer(serverConfig.addr.c_str())) - << serverConfig.addr; + LOG_ALWAYS_FATAL_IF(OK != server->setupUnixDomainServer(serverConfig.addr.c_str()), + "%s", serverConfig.addr.c_str()); break; case SocketType::UNIX_BOOTSTRAP: - CHECK_EQ(OK, server->setupUnixDomainSocketBootstrapServer(std::move(socketFd))); + LOG_ALWAYS_FATAL_IF(OK != + server->setupUnixDomainSocketBootstrapServer(std::move(socketFd))); break; case SocketType::UNIX_RAW: - CHECK_EQ(OK, server->setupRawSocketServer(std::move(socketFd))); + LOG_ALWAYS_FATAL_IF(OK != server->setupRawSocketServer(std::move(socketFd))); break; case SocketType::VSOCK: - CHECK_EQ(OK, server->setupVsockServer(VMADDR_CID_LOCAL, serverConfig.vsockPort)) - << "Need `sudo modprobe vsock_loopback`?"; + LOG_ALWAYS_FATAL_IF(OK != + server->setupVsockServer(VMADDR_CID_LOCAL, + serverConfig.vsockPort), + "Need `sudo modprobe vsock_loopback`?"); break; case SocketType::INET: { - CHECK_EQ(OK, server->setupInetServer(kLocalInetAddress, 0, &outPort)); - CHECK_NE(0, outPort); + LOG_ALWAYS_FATAL_IF(OK != server->setupInetServer(kLocalInetAddress, 0, &outPort)); + LOG_ALWAYS_FATAL_IF(0 == outPort); break; } default: @@ -159,21 +162,21 @@ int main(int argc, char* argv[]) { if (rpcSecurity == RpcSecurity::TLS) { for (const auto& clientCert : clientInfo.certs) { - CHECK_EQ(OK, - certVerifier->addTrustedPeerCertificate(RpcCertificateFormat::PEM, - clientCert.data)); + LOG_ALWAYS_FATAL_IF(OK != + certVerifier->addTrustedPeerCertificate(RpcCertificateFormat::PEM, + clientCert.data)); } } server->setPerSessionRootObject([&](wp<RpcSession> session, const void* addrPtr, size_t len) { { sp<RpcSession> spSession = session.promote(); - CHECK_NE(nullptr, spSession.get()); + LOG_ALWAYS_FATAL_IF(nullptr == spSession.get()); } // UNIX sockets with abstract addresses return // sizeof(sa_family_t)==2 in addrlen - CHECK_GE(len, sizeof(sa_family_t)); + LOG_ALWAYS_FATAL_IF(len < sizeof(sa_family_t)); const sockaddr* addr = reinterpret_cast<const sockaddr*>(addrPtr); sp<MyBinderRpcTestAndroid> service = sp<MyBinderRpcTestAndroid>::make(); switch (addr->sa_family) { @@ -181,15 +184,15 @@ int main(int argc, char* argv[]) { // nothing to save break; case AF_VSOCK: - CHECK_EQ(len, sizeof(sockaddr_vm)); + LOG_ALWAYS_FATAL_IF(len != sizeof(sockaddr_vm)); service->port = reinterpret_cast<const sockaddr_vm*>(addr)->svm_port; break; case AF_INET: - CHECK_EQ(len, sizeof(sockaddr_in)); + LOG_ALWAYS_FATAL_IF(len != sizeof(sockaddr_in)); service->port = ntohs(reinterpret_cast<const sockaddr_in*>(addr)->sin_port); break; case AF_INET6: - CHECK_EQ(len, sizeof(sockaddr_in)); + LOG_ALWAYS_FATAL_IF(len != sizeof(sockaddr_in)); service->port = ntohs(reinterpret_cast<const sockaddr_in6*>(addr)->sin6_port); break; default: diff --git a/libs/binder/trusty/kernel/rules.mk b/libs/binder/trusty/kernel/rules.mk index 1f05ef757a..d2b37aa8f6 100644 --- a/libs/binder/trusty/kernel/rules.mk +++ b/libs/binder/trusty/kernel/rules.mk @@ -24,7 +24,6 @@ LIBUTILS_DIR := system/core/libutils FMTLIB_DIR := external/fmtlib MODULE_SRCS := \ - $(LOCAL_DIR)/../logging.cpp \ $(LOCAL_DIR)/../TrustyStatus.cpp \ $(LIBBINDER_DIR)/Binder.cpp \ $(LIBBINDER_DIR)/BpBinder.cpp \ diff --git a/libs/binder/trusty/logging.cpp b/libs/binder/trusty/logging.cpp deleted file mode 100644 index 88a1075f60..0000000000 --- a/libs/binder/trusty/logging.cpp +++ /dev/null @@ -1,165 +0,0 @@ -/* - * Copyright (C) 2022 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. - */ - -#define TLOG_TAG "libbinder" - -#include "android-base/logging.h" - -#include <trusty_log.h> -#include <iostream> -#include <string> - -#include <android-base/strings.h> - -namespace android { -namespace base { - -static const char* GetFileBasename(const char* file) { - const char* last_slash = strrchr(file, '/'); - if (last_slash != nullptr) { - return last_slash + 1; - } - return file; -} - -// This splits the message up line by line, by calling log_function with a pointer to the start of -// each line and the size up to the newline character. It sends size = -1 for the final line. -template <typename F, typename... Args> -static void SplitByLines(const char* msg, const F& log_function, Args&&... args) { - const char* newline; - while ((newline = strchr(msg, '\n')) != nullptr) { - log_function(msg, newline - msg, args...); - msg = newline + 1; - } - - log_function(msg, -1, args...); -} - -void DefaultAborter(const char* abort_message) { - TLOGC("aborting: %s\n", abort_message); - abort(); -} - -static void TrustyLogLine(const char* msg, int /*length*/, android::base::LogSeverity severity, - const char* tag) { - switch (severity) { - case VERBOSE: - case DEBUG: - TLOGD("%s: %s\n", tag, msg); - break; - case INFO: - TLOGI("%s: %s\n", tag, msg); - break; - case WARNING: - TLOGW("%s: %s\n", tag, msg); - break; - case ERROR: - TLOGE("%s: %s\n", tag, msg); - break; - case FATAL_WITHOUT_ABORT: - case FATAL: - TLOGC("%s: %s\n", tag, msg); - break; - } -} - -void TrustyLogger(android::base::LogId, android::base::LogSeverity severity, const char* tag, - const char*, unsigned int, const char* full_message) { - SplitByLines(full_message, TrustyLogLine, severity, tag); -} - -// This indirection greatly reduces the stack impact of having lots of -// checks/logging in a function. -class LogMessageData { -public: - LogMessageData(const char* file, unsigned int line, LogSeverity severity, const char* tag, - int error) - : file_(GetFileBasename(file)), - line_number_(line), - severity_(severity), - tag_(tag), - error_(error) {} - - const char* GetFile() const { return file_; } - - unsigned int GetLineNumber() const { return line_number_; } - - LogSeverity GetSeverity() const { return severity_; } - - const char* GetTag() const { return tag_; } - - int GetError() const { return error_; } - - std::ostream& GetBuffer() { return buffer_; } - - std::string ToString() const { return buffer_.str(); } - -private: - std::ostringstream buffer_; - const char* const file_; - const unsigned int line_number_; - const LogSeverity severity_; - const char* const tag_; - const int error_; - - DISALLOW_COPY_AND_ASSIGN(LogMessageData); -}; - -LogMessage::LogMessage(const char* file, unsigned int line, LogId, LogSeverity severity, - const char* tag, int error) - : LogMessage(file, line, severity, tag, error) {} - -LogMessage::LogMessage(const char* file, unsigned int line, LogSeverity severity, const char* tag, - int error) - : data_(new LogMessageData(file, line, severity, tag, error)) {} - -LogMessage::~LogMessage() { - // Check severity again. This is duplicate work wrt/ LOG macros, but not LOG_STREAM. - if (!WOULD_LOG(data_->GetSeverity())) { - return; - } - - // Finish constructing the message. - if (data_->GetError() != -1) { - data_->GetBuffer() << ": " << strerror(data_->GetError()); - } - std::string msg(data_->ToString()); - - LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetSeverity(), data_->GetTag(), - msg.c_str()); - - // Abort if necessary. - if (data_->GetSeverity() == FATAL) { - DefaultAborter(msg.c_str()); - } -} - -std::ostream& LogMessage::stream() { - return data_->GetBuffer(); -} - -void LogMessage::LogLine(const char* file, unsigned int line, LogSeverity severity, const char* tag, - const char* message) { - TrustyLogger(DEFAULT, severity, tag ?: "<unknown>", file, line, message); -} - -bool ShouldLog(LogSeverity /*severity*/, const char* /*tag*/) { - // This is controlled by Trusty's log level. - return true; -} - -} // namespace base -} // namespace android diff --git a/libs/binder/trusty/rules.mk b/libs/binder/trusty/rules.mk index 2e56cbd21b..9cad556711 100644 --- a/libs/binder/trusty/rules.mk +++ b/libs/binder/trusty/rules.mk @@ -24,7 +24,6 @@ LIBUTILS_DIR := system/core/libutils FMTLIB_DIR := external/fmtlib MODULE_SRCS := \ - $(LOCAL_DIR)/logging.cpp \ $(LOCAL_DIR)/OS.cpp \ $(LOCAL_DIR)/RpcServerTrusty.cpp \ $(LOCAL_DIR)/RpcTransportTipcTrusty.cpp \ diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 8d0331ebb5..f317a2eea0 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -41,6 +41,8 @@ #include <android-base/thread_annotations.h> #include <chrono> +#include <com_android_graphics_libgui_flags.h> + using namespace com::android::graphics::libgui; using namespace std::chrono_literals; @@ -102,12 +104,11 @@ void BLASTBufferItemConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry } } -void BLASTBufferItemConsumer::updateFrameTimestamps(uint64_t frameNumber, nsecs_t refreshStartTime, - const sp<Fence>& glDoneFence, - const sp<Fence>& presentFence, - const sp<Fence>& prevReleaseFence, - CompositorTiming compositorTiming, - nsecs_t latchTime, nsecs_t dequeueReadyTime) { +void BLASTBufferItemConsumer::updateFrameTimestamps( + uint64_t frameNumber, uint64_t previousFrameNumber, nsecs_t refreshStartTime, + const sp<Fence>& glDoneFence, const sp<Fence>& presentFence, + const sp<Fence>& prevReleaseFence, CompositorTiming compositorTiming, nsecs_t latchTime, + nsecs_t dequeueReadyTime) { Mutex::Autolock lock(mMutex); // if the producer is not connected, don't bother updating, @@ -118,7 +119,15 @@ void BLASTBufferItemConsumer::updateFrameTimestamps(uint64_t frameNumber, nsecs_ std::shared_ptr<FenceTime> releaseFenceTime = std::make_shared<FenceTime>(prevReleaseFence); mFrameEventHistory.addLatch(frameNumber, latchTime); - mFrameEventHistory.addRelease(frameNumber, dequeueReadyTime, std::move(releaseFenceTime)); + if (flags::frametimestamps_previousrelease()) { + if (previousFrameNumber > 0) { + mFrameEventHistory.addRelease(previousFrameNumber, dequeueReadyTime, + std::move(releaseFenceTime)); + } + } else { + mFrameEventHistory.addRelease(frameNumber, dequeueReadyTime, std::move(releaseFenceTime)); + } + mFrameEventHistory.addPreComposition(frameNumber, refreshStartTime); mFrameEventHistory.addPostComposition(frameNumber, glDoneFenceTime, presentFenceTime, compositorTiming); @@ -364,6 +373,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence if (stat.latchTime > 0) { mBufferItemConsumer ->updateFrameTimestamps(stat.frameEventStats.frameNumber, + stat.frameEventStats.previousFrameNumber, stat.frameEventStats.refreshStartTime, stat.frameEventStats.gpuCompositionDoneFence, stat.presentFence, stat.previousReleaseFence, diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index 29d64afb24..f5d19aac78 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -25,6 +25,10 @@ #include <gui/LayerState.h> #include <private/gui/ParcelUtils.h> +#include <com_android_graphics_libgui_flags.h> + +using namespace com::android::graphics::libgui; + namespace android { namespace { // Anonymous @@ -49,6 +53,11 @@ status_t FrameEventHistoryStats::writeToParcel(Parcel* output) const { status_t err = output->writeUint64(frameNumber); if (err != NO_ERROR) return err; + if (flags::frametimestamps_previousrelease()) { + err = output->writeUint64(previousFrameNumber); + if (err != NO_ERROR) return err; + } + if (gpuCompositionDoneFence) { err = output->writeBool(true); if (err != NO_ERROR) return err; @@ -79,6 +88,11 @@ status_t FrameEventHistoryStats::readFromParcel(const Parcel* input) { status_t err = input->readUint64(&frameNumber); if (err != NO_ERROR) return err; + if (flags::frametimestamps_previousrelease()) { + err = input->readUint64(&previousFrameNumber); + if (err != NO_ERROR) return err; + } + bool hasFence = false; err = input->readBool(&hasFence); if (err != NO_ERROR) return err; diff --git a/libs/gui/WindowInfo.cpp b/libs/gui/WindowInfo.cpp index 2eb6bd670d..6a4460b650 100644 --- a/libs/gui/WindowInfo.cpp +++ b/libs/gui/WindowInfo.cpp @@ -66,8 +66,9 @@ bool WindowInfo::overlaps(const WindowInfo* other) const { bool WindowInfo::operator==(const WindowInfo& info) const { return info.token == token && info.id == id && info.name == name && info.dispatchingTimeout == dispatchingTimeout && info.frame == frame && - info.surfaceInset == surfaceInset && info.globalScaleFactor == globalScaleFactor && - info.transform == transform && info.touchableRegion.hasSameRects(touchableRegion) && + info.contentSize == contentSize && info.surfaceInset == surfaceInset && + info.globalScaleFactor == globalScaleFactor && info.transform == transform && + info.touchableRegion.hasSameRects(touchableRegion) && info.touchOcclusionMode == touchOcclusionMode && info.ownerPid == ownerPid && info.ownerUid == ownerUid && info.packageName == packageName && info.inputConfig == inputConfig && info.displayId == displayId && @@ -101,6 +102,8 @@ status_t WindowInfo::writeToParcel(android::Parcel* parcel) const { parcel->writeInt32( static_cast<std::underlying_type_t<WindowInfo::Type>>(layoutParamsType)) ?: parcel->write(frame) ?: + parcel->writeInt32(contentSize.width) ?: + parcel->writeInt32(contentSize.height) ?: parcel->writeInt32(surfaceInset) ?: parcel->writeFloat(globalScaleFactor) ?: parcel->writeFloat(alpha) ?: @@ -150,6 +153,8 @@ status_t WindowInfo::readFromParcel(const android::Parcel* parcel) { status = parcel->readInt32(&lpFlags) ?: parcel->readInt32(&lpType) ?: parcel->read(frame) ?: + parcel->readInt32(&contentSize.width) ?: + parcel->readInt32(&contentSize.height) ?: parcel->readInt32(&surfaceInset) ?: parcel->readFloat(&globalScaleFactor) ?: parcel->readFloat(&alpha) ?: diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index 265373c6e0..d24f8eefd5 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -516,6 +516,13 @@ interface ISurfaceComposer { void scheduleCommit(); /** + * Force all window composition to the GPU (i.e. disable Hardware Overlays). + * This can help check if there is a bug in HW Composer. + * Requires root or android.permission.HARDWARE_TEST + */ + void forceClientComposition(boolean enabled); + + /** * Gets priority of the RenderEngine in SurfaceFlinger. */ int getGpuContextPriority(); diff --git a/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp b/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp index 17f4c630ce..2e270b721f 100644 --- a/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp +++ b/libs/gui/fuzzer/libgui_bufferQueue_fuzzer.cpp @@ -141,8 +141,8 @@ void BufferQueueFuzzer::invokeBlastBufferQueue() { CompositorTiming compTiming; sp<Fence> previousFence = new Fence(memfd_create("pfd", MFD_ALLOW_SEALING)); sp<Fence> gpuFence = new Fence(memfd_create("gfd", MFD_ALLOW_SEALING)); - FrameEventHistoryStats frameStats(frameNumber, gpuFence, compTiming, - mFdp.ConsumeIntegral<int64_t>(), + FrameEventHistoryStats frameStats(frameNumber, mFdp.ConsumeIntegral<uint64_t>(), gpuFence, + compTiming, mFdp.ConsumeIntegral<int64_t>(), mFdp.ConsumeIntegral<int64_t>()); std::vector<SurfaceControlStats> stats; sp<Fence> presentFence = new Fence(memfd_create("fd", MFD_ALLOW_SEALING)); diff --git a/libs/gui/fuzzer/libgui_fuzzer_utils.h b/libs/gui/fuzzer/libgui_fuzzer_utils.h index 065ba06e38..ffe7e4123b 100644 --- a/libs/gui/fuzzer/libgui_fuzzer_utils.h +++ b/libs/gui/fuzzer/libgui_fuzzer_utils.h @@ -154,6 +154,7 @@ public: MOCK_METHOD(binder::Status, setDebugFlash, (int), (override)); MOCK_METHOD(binder::Status, scheduleComposite, (), (override)); MOCK_METHOD(binder::Status, scheduleCommit, (), (override)); + MOCK_METHOD(binder::Status, forceClientComposition, (bool), (override)); MOCK_METHOD(binder::Status, updateSmallAreaDetection, (const std::vector<int32_t>&, const std::vector<float>&), (override)); MOCK_METHOD(binder::Status, setSmallAreaDetectionThreshold, (int32_t, float), (override)); diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 892215ec32..0e1a505c69 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -50,8 +50,8 @@ public: void onDisconnect() override EXCLUDES(mMutex); void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) override EXCLUDES(mMutex); - void updateFrameTimestamps(uint64_t frameNumber, nsecs_t refreshStartTime, - const sp<Fence>& gpuCompositionDoneFence, + void updateFrameTimestamps(uint64_t frameNumber, uint64_t previousFrameNumber, + nsecs_t refreshStartTime, const sp<Fence>& gpuCompositionDoneFence, const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence, CompositorTiming compositorTiming, nsecs_t latchTime, nsecs_t dequeueReadyTime) EXCLUDES(mMutex); diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h index 364a57b8dd..bc97cd0828 100644 --- a/libs/gui/include/gui/ITransactionCompletedListener.h +++ b/libs/gui/include/gui/ITransactionCompletedListener.h @@ -95,15 +95,18 @@ public: status_t readFromParcel(const Parcel* input) override; FrameEventHistoryStats() = default; - FrameEventHistoryStats(uint64_t fn, const sp<Fence>& gpuCompFence, CompositorTiming compTiming, + FrameEventHistoryStats(uint64_t frameNumber, uint64_t previousFrameNumber, + const sp<Fence>& gpuCompFence, CompositorTiming compTiming, nsecs_t refreshTime, nsecs_t dequeueReadyTime) - : frameNumber(fn), + : frameNumber(frameNumber), + previousFrameNumber(previousFrameNumber), gpuCompositionDoneFence(gpuCompFence), compositorTiming(compTiming), refreshStartTime(refreshTime), dequeueReadyTime(dequeueReadyTime) {} uint64_t frameNumber; + uint64_t previousFrameNumber; sp<Fence> gpuCompositionDoneFence; CompositorTiming compositorTiming; nsecs_t refreshStartTime; diff --git a/libs/gui/include/gui/WindowInfo.h b/libs/gui/include/gui/WindowInfo.h index bd2eb7413b..dcc38d7564 100644 --- a/libs/gui/include/gui/WindowInfo.h +++ b/libs/gui/include/gui/WindowInfo.h @@ -26,6 +26,7 @@ #include <gui/constants.h> #include <ui/Rect.h> #include <ui/Region.h> +#include <ui/Size.h> #include <ui/Transform.h> #include <utils/RefBase.h> #include <utils/Timers.h> @@ -196,6 +197,9 @@ struct WindowInfo : public Parcelable { /* These values are filled in by SurfaceFlinger. */ Rect frame = Rect::INVALID_RECT; + // The real size of the content, excluding any crop. If no buffer is rendered, this is 0,0 + ui::Size contentSize = ui::Size(0, 0); + /* * SurfaceFlinger consumes this value to shrink the computed frame. This is * different from shrinking the touchable region in that it DOES shift the coordinate diff --git a/libs/gui/libgui_flags.aconfig b/libs/gui/libgui_flags.aconfig index a16be786be..b081030c9f 100644 --- a/libs/gui/libgui_flags.aconfig +++ b/libs/gui/libgui_flags.aconfig @@ -8,3 +8,10 @@ flag { is_fixed_read_only: true } +flag { + name: "frametimestamps_previousrelease" + namespace: "core_graphics" + description: "Controls a fence fixup for timestamp apis" + bug: "310927247" + is_fixed_read_only: true +} diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 9893c7146e..ea7078dd05 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -42,9 +42,12 @@ #include <gtest/gtest.h> +#include <com_android_graphics_libgui_flags.h> + using namespace std::chrono_literals; namespace android { +using namespace com::android::graphics::libgui; using Transaction = SurfaceComposerClient::Transaction; using android::hardware::graphics::common::V1_2::BufferUsage; @@ -1581,6 +1584,9 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) { nsecs_t postedTimeB = 0; setUpAndQueueBuffer(igbProducer, &requestedPresentTimeB, &postedTimeB, &qbOutput, true); history.applyDelta(qbOutput.frameTimestamps); + + adapter.waitForCallback(2); + events = history.getFrame(1); ASSERT_NE(nullptr, events); @@ -1590,7 +1596,9 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) { ASSERT_GE(events->postedTime, postedTimeA); ASSERT_GE(events->latchTime, postedTimeA); - ASSERT_GE(events->dequeueReadyTime, events->latchTime); + if (flags::frametimestamps_previousrelease()) { + ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING); + } ASSERT_NE(nullptr, events->gpuCompositionDoneFence); ASSERT_NE(nullptr, events->displayPresentFence); ASSERT_NE(nullptr, events->releaseFence); @@ -1602,6 +1610,50 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_Basic) { ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime); ASSERT_GE(events->postedTime, postedTimeB); + // Now do the same as above with a third buffer, so that timings related to + // buffer releases make it back to the first frame. + nsecs_t requestedPresentTimeC = 0; + nsecs_t postedTimeC = 0; + setUpAndQueueBuffer(igbProducer, &requestedPresentTimeC, &postedTimeC, &qbOutput, true); + history.applyDelta(qbOutput.frameTimestamps); + + adapter.waitForCallback(3); + + // Check the first frame... + events = history.getFrame(1); + ASSERT_NE(nullptr, events); + ASSERT_EQ(1, events->frameNumber); + ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime); + ASSERT_GE(events->postedTime, postedTimeA); + ASSERT_GE(events->latchTime, postedTimeA); + // Now dequeueReadyTime is valid, because the release timings finally + // propaged to queueBuffer() + ASSERT_GE(events->dequeueReadyTime, events->latchTime); + ASSERT_NE(nullptr, events->gpuCompositionDoneFence); + ASSERT_NE(nullptr, events->displayPresentFence); + ASSERT_NE(nullptr, events->releaseFence); + + // ...and the second + events = history.getFrame(2); + ASSERT_NE(nullptr, events); + ASSERT_EQ(2, events->frameNumber); + ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime); + ASSERT_GE(events->postedTime, postedTimeB); + ASSERT_GE(events->latchTime, postedTimeB); + if (flags::frametimestamps_previousrelease()) { + ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING); + } + ASSERT_NE(nullptr, events->gpuCompositionDoneFence); + ASSERT_NE(nullptr, events->displayPresentFence); + ASSERT_NE(nullptr, events->releaseFence); + + // ...and finally the third! + events = history.getFrame(3); + ASSERT_NE(nullptr, events); + ASSERT_EQ(3, events->frameNumber); + ASSERT_EQ(requestedPresentTimeC, events->requestedPresentTime); + ASSERT_GE(events->postedTime, postedTimeC); + // wait for any callbacks that have not been received adapter.waitForCallbacks(); } @@ -1660,6 +1712,8 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_DroppedFrame) { setUpAndQueueBuffer(igbProducer, &requestedPresentTimeC, &postedTimeC, &qbOutput, true); history.applyDelta(qbOutput.frameTimestamps); + adapter.waitForCallback(3); + // frame number, requestedPresentTime, and postTime should not have changed ASSERT_EQ(1, events->frameNumber); ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime); @@ -1679,6 +1733,42 @@ TEST_F(BLASTFrameEventHistoryTest, FrameEventHistory_DroppedFrame) { ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime); ASSERT_GE(events->postedTime, postedTimeB); ASSERT_GE(events->latchTime, postedTimeB); + + if (flags::frametimestamps_previousrelease()) { + ASSERT_EQ(events->dequeueReadyTime, FrameEvents::TIMESTAMP_PENDING); + } + ASSERT_NE(nullptr, events->gpuCompositionDoneFence); + ASSERT_NE(nullptr, events->displayPresentFence); + ASSERT_NE(nullptr, events->releaseFence); + + // Queue another buffer to check for timestamps that came late + nsecs_t requestedPresentTimeD = 0; + nsecs_t postedTimeD = 0; + setUpAndQueueBuffer(igbProducer, &requestedPresentTimeD, &postedTimeD, &qbOutput, true); + history.applyDelta(qbOutput.frameTimestamps); + + adapter.waitForCallback(4); + + // frame number, requestedPresentTime, and postTime should not have changed + events = history.getFrame(1); + ASSERT_EQ(1, events->frameNumber); + ASSERT_EQ(requestedPresentTimeA, events->requestedPresentTime); + ASSERT_GE(events->postedTime, postedTimeA); + + // a valid latchtime and pre and post composition info should not be set for the dropped frame + ASSERT_FALSE(events->hasLatchInfo()); + ASSERT_FALSE(events->hasDequeueReadyInfo()); + ASSERT_FALSE(events->hasGpuCompositionDoneInfo()); + ASSERT_FALSE(events->hasDisplayPresentInfo()); + ASSERT_FALSE(events->hasReleaseInfo()); + + // we should also have gotten values for the presented frame + events = history.getFrame(2); + ASSERT_NE(nullptr, events); + ASSERT_EQ(2, events->frameNumber); + ASSERT_EQ(requestedPresentTimeB, events->requestedPresentTime); + ASSERT_GE(events->postedTime, postedTimeB); + ASSERT_GE(events->latchTime, postedTimeB); ASSERT_GE(events->dequeueReadyTime, events->latchTime); ASSERT_NE(nullptr, events->gpuCompositionDoneFence); ASSERT_NE(nullptr, events->displayPresentFence); diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 8d3eacba90..60221aa30a 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -935,6 +935,10 @@ public: binder::Status scheduleCommit() override { return binder::Status::ok(); } + binder::Status forceClientComposition(bool /*enabled*/) override { + return binder::Status::ok(); + } + binder::Status updateSmallAreaDetection(const std::vector<int32_t>& /*appIds*/, const std::vector<float>& /*thresholds*/) { return binder::Status::ok(); diff --git a/libs/gui/tests/WindowInfo_test.cpp b/libs/gui/tests/WindowInfo_test.cpp index f2feaefa13..5eb5d3bff0 100644 --- a/libs/gui/tests/WindowInfo_test.cpp +++ b/libs/gui/tests/WindowInfo_test.cpp @@ -28,6 +28,7 @@ namespace android { using gui::InputApplicationInfo; using gui::TouchOcclusionMode; using gui::WindowInfo; +using ui::Size; namespace test { @@ -53,6 +54,7 @@ TEST(WindowInfo, Parcelling) { i.layoutParamsType = WindowInfo::Type::INPUT_METHOD; i.dispatchingTimeout = 12s; i.frame = Rect(93, 34, 16, 19); + i.contentSize = Size(10, 40); i.surfaceInset = 17; i.globalScaleFactor = 0.3; i.alpha = 0.7; @@ -83,6 +85,7 @@ TEST(WindowInfo, Parcelling) { ASSERT_EQ(i.layoutParamsType, i2.layoutParamsType); ASSERT_EQ(i.dispatchingTimeout, i2.dispatchingTimeout); ASSERT_EQ(i.frame, i2.frame); + ASSERT_EQ(i.contentSize, i2.contentSize); ASSERT_EQ(i.surfaceInset, i2.surfaceInset); ASSERT_EQ(i.globalScaleFactor, i2.globalScaleFactor); ASSERT_EQ(i.alpha, i2.alpha); diff --git a/libs/input/Android.bp b/libs/input/Android.bp index c37db1693a..dd8dc8dc9d 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -50,7 +50,7 @@ cc_aconfig_library { // overrides for flags, without having to link against a separate version of libinput or of this // library. Bundling this library directly into libinput prevents us from having to add this // library as a shared lib dependency everywhere where libinput is used. - test: true, + mode: "test", shared: { enabled: false, }, diff --git a/libs/input/VirtualInputDevice.cpp b/libs/input/VirtualInputDevice.cpp index 9a459b135c..db7031ab03 100644 --- a/libs/input/VirtualInputDevice.cpp +++ b/libs/input/VirtualInputDevice.cpp @@ -193,6 +193,7 @@ const std::map<int, int> VirtualKeyboard::KEY_CODE_MAPPING = { {AKEYCODE_NUMPAD_ENTER, KEY_KPENTER}, {AKEYCODE_NUMPAD_EQUALS, KEY_KPEQUAL}, {AKEYCODE_NUMPAD_COMMA, KEY_KPCOMMA}, + {AKEYCODE_LANGUAGE_SWITCH, KEY_LANGUAGE}, }; VirtualKeyboard::VirtualKeyboard(unique_fd fd) : VirtualInputDevice(std::move(fd)) {} VirtualKeyboard::~VirtualKeyboard() {} diff --git a/opengl/Android.bp b/opengl/Android.bp index b15694bf50..4454f36b67 100644 --- a/opengl/Android.bp +++ b/opengl/Android.bp @@ -72,6 +72,10 @@ cc_library_headers { llndk: { llndk_headers: true, }, + apex_available: [ + "//apex_available:platform", + "com.android.virt", + ], } subdirs = [ diff --git a/opengl/libs/EGL/MultifileBlobCache.cpp b/opengl/libs/EGL/MultifileBlobCache.cpp index ed3c616b92..9905210014 100644 --- a/opengl/libs/EGL/MultifileBlobCache.cpp +++ b/opengl/libs/EGL/MultifileBlobCache.cpp @@ -18,6 +18,7 @@ #include "MultifileBlobCache.h" +#include <android-base/properties.h> #include <dirent.h> #include <fcntl.h> #include <inttypes.h> @@ -62,12 +63,15 @@ void freeHotCacheEntry(android::MultifileHotCache& entry) { namespace android { MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, size_t maxTotalSize, - const std::string& baseDir) + size_t maxTotalEntries, const std::string& baseDir) : mInitialized(false), + mCacheVersion(0), mMaxKeySize(maxKeySize), mMaxValueSize(maxValueSize), mMaxTotalSize(maxTotalSize), + mMaxTotalEntries(maxTotalEntries), mTotalCacheSize(0), + mTotalCacheEntries(0), mHotCacheLimit(0), mHotCacheSize(0), mWorkerThreadIdle(true) { @@ -76,6 +80,26 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s return; } + // Set the cache version, override if debug value set + mCacheVersion = kMultifileBlobCacheVersion; + int debugCacheVersion = base::GetIntProperty("debug.egl.blobcache.cache_version", -1); + if (debugCacheVersion >= 0) { + ALOGV("INIT: Using %u as cacheVersion instead of %u", debugCacheVersion, mCacheVersion); + mCacheVersion = debugCacheVersion; + } + + // Set the platform build ID, override if debug value set + mBuildId = base::GetProperty("ro.build.id", ""); + std::string debugBuildId = base::GetProperty("debug.egl.blobcache.build_id", ""); + if (!debugBuildId.empty()) { + ALOGV("INIT: Using %s as buildId instead of %s", debugBuildId.c_str(), mBuildId.c_str()); + if (debugBuildId.length() > PROP_VALUE_MAX) { + ALOGV("INIT: debugBuildId is too long (%zu), reduce it to %u", debugBuildId.length(), + PROP_VALUE_MAX); + } + mBuildId = debugBuildId; + } + // Establish the name of our multifile directory mMultifileDirName = baseDir + ".multifile"; @@ -93,14 +117,30 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s mTaskThread = std::thread(&MultifileBlobCache::processTasks, this); // See if the dir exists, and initialize using its contents + bool statusGood = false; + + // Check that our cacheVersion and buildId match struct stat st; if (stat(mMultifileDirName.c_str(), &st) == 0) { + if (checkStatus(mMultifileDirName.c_str())) { + statusGood = true; + } else { + ALOGV("INIT: Cache status has changed, clearing the cache"); + if (!clearCache()) { + ALOGE("INIT: Unable to clear cache"); + return; + } + } + } + + if (statusGood) { // Read all the files and gather details, then preload their contents DIR* dir; struct dirent* entry; if ((dir = opendir(mMultifileDirName.c_str())) != nullptr) { while ((entry = readdir(dir)) != nullptr) { - if (entry->d_name == "."s || entry->d_name == ".."s) { + if (entry->d_name == "."s || entry->d_name == ".."s || + strcmp(entry->d_name, kMultifileBlobCacheStatusFile) == 0) { continue; } @@ -123,7 +163,8 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s if (st.st_size <= 0 || st.st_atime <= 0) { ALOGE("INIT: Entry %u has invalid stats! Removing.", entryHash); if (remove(fullPath.c_str()) != 0) { - ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno)); + ALOGE("INIT: Error removing %s: %s", fullPath.c_str(), + std::strerror(errno)); } continue; } @@ -140,7 +181,7 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s MultifileHeader header; size_t result = read(fd, static_cast<void*>(&header), sizeof(MultifileHeader)); if (result != sizeof(MultifileHeader)) { - ALOGE("Error reading MultifileHeader from cache entry (%s): %s", + ALOGE("INIT: Error reading MultifileHeader from cache entry (%s): %s", fullPath.c_str(), std::strerror(errno)); close(fd); return; @@ -150,7 +191,8 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s if (header.magic != kMultifileMagic) { ALOGE("INIT: Entry %u has bad magic (%u)! Removing.", entryHash, header.magic); if (remove(fullPath.c_str()) != 0) { - ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno)); + ALOGE("INIT: Error removing %s: %s", fullPath.c_str(), + std::strerror(errno)); } close(fd); continue; @@ -175,7 +217,7 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s if (header.crc != crc32c(mappedEntry + sizeof(MultifileHeader), fileSize - sizeof(MultifileHeader))) { - ALOGE("INIT: Entry %u failed CRC check! Removing.", entryHash); + ALOGV("INIT: Entry %u failed CRC check! Removing.", entryHash); if (remove(fullPath.c_str()) != 0) { ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno)); } @@ -184,11 +226,12 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s // If the cache entry is damaged or no good, remove it if (header.keySize <= 0 || header.valueSize <= 0) { - ALOGE("INIT: Entry %u has a bad header keySize (%lu) or valueSize (%lu), " + ALOGV("INIT: Entry %u has a bad header keySize (%lu) or valueSize (%lu), " "removing.", entryHash, header.keySize, header.valueSize); if (remove(fullPath.c_str()) != 0) { - ALOGE("Error removing %s: %s", fullPath.c_str(), std::strerror(errno)); + ALOGE("INIT: Error removing %s: %s", fullPath.c_str(), + std::strerror(errno)); } continue; } @@ -226,9 +269,17 @@ MultifileBlobCache::MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, s // If the multifile directory does not exist, create it and start from scratch if (mkdir(mMultifileDirName.c_str(), 0755) != 0 && (errno != EEXIST)) { ALOGE("Unable to create directory (%s), errno (%i)", mMultifileDirName.c_str(), errno); + return; + } + + // Create new status file + if (!createStatus(mMultifileDirName.c_str())) { + ALOGE("INIT: Failed to create status file!"); + return; } } + ALOGV("INIT: Multifile BlobCache initialization succeeded"); mInitialized = true; } @@ -270,7 +321,7 @@ void MultifileBlobCache::set(const void* key, EGLsizeiANDROID keySize, const voi size_t fileSize = sizeof(MultifileHeader) + keySize + valueSize; // If we're going to be over the cache limit, kick off a trim to clear space - if (getTotalSize() + fileSize > mMaxTotalSize) { + if (getTotalSize() + fileSize > mMaxTotalSize || getTotalEntries() + 1 > mMaxTotalEntries) { ALOGV("SET: Cache is full, calling trimCache to clear space"); trimCache(); } @@ -469,6 +520,112 @@ void MultifileBlobCache::finish() { } } +bool MultifileBlobCache::createStatus(const std::string& baseDir) { + // Populate the status struct + struct MultifileStatus status; + memset(&status, 0, sizeof(status)); + status.magic = kMultifileMagic; + status.cacheVersion = mCacheVersion; + + // Copy the buildId string in, up to our allocated space + strncpy(status.buildId, mBuildId.c_str(), + mBuildId.length() > PROP_VALUE_MAX ? PROP_VALUE_MAX : mBuildId.length()); + + // Finally update the crc, using cacheVersion and everything the follows + status.crc = + crc32c(reinterpret_cast<uint8_t*>(&status) + offsetof(MultifileStatus, cacheVersion), + sizeof(status) - offsetof(MultifileStatus, cacheVersion)); + + // Create the status file + std::string cacheStatus = baseDir + "/" + kMultifileBlobCacheStatusFile; + int fd = open(cacheStatus.c_str(), O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR); + if (fd == -1) { + ALOGE("STATUS(CREATE): Unable to create status file: %s, error: %s", cacheStatus.c_str(), + std::strerror(errno)); + return false; + } + + // Write the buffer contents to disk + ssize_t result = write(fd, &status, sizeof(status)); + close(fd); + if (result != sizeof(status)) { + ALOGE("STATUS(CREATE): Error writing cache status file: %s, error %s", cacheStatus.c_str(), + std::strerror(errno)); + return false; + } + + ALOGV("STATUS(CREATE): Created status file: %s", cacheStatus.c_str()); + return true; +} + +bool MultifileBlobCache::checkStatus(const std::string& baseDir) { + std::string cacheStatus = baseDir + "/" + kMultifileBlobCacheStatusFile; + + // Does status exist + struct stat st; + if (stat(cacheStatus.c_str(), &st) != 0) { + ALOGV("STATUS(CHECK): Status file (%s) missing", cacheStatus.c_str()); + return false; + } + + // If the status entry is damaged or no good, remove it + if (st.st_size <= 0 || st.st_atime <= 0) { + ALOGE("STATUS(CHECK): Cache status has invalid stats!"); + return false; + } + + // Open the file so we can read its header + int fd = open(cacheStatus.c_str(), O_RDONLY); + if (fd == -1) { + ALOGE("STATUS(CHECK): Cache error - failed to open cacheStatus: %s, error: %s", + cacheStatus.c_str(), std::strerror(errno)); + return false; + } + + // Read in the status header + MultifileStatus status; + size_t result = read(fd, static_cast<void*>(&status), sizeof(MultifileStatus)); + close(fd); + if (result != sizeof(MultifileStatus)) { + ALOGE("STATUS(CHECK): Error reading cache status (%s): %s", cacheStatus.c_str(), + std::strerror(errno)); + return false; + } + + // Verify header magic + if (status.magic != kMultifileMagic) { + ALOGE("STATUS(CHECK): Cache status has bad magic (%u)!", status.magic); + return false; + } + + // Ensure we have a good CRC + if (status.crc != + crc32c(reinterpret_cast<uint8_t*>(&status) + offsetof(MultifileStatus, cacheVersion), + sizeof(status) - offsetof(MultifileStatus, cacheVersion))) { + ALOGE("STATUS(CHECK): Cache status failed CRC check!"); + return false; + } + + // Check cacheVersion + if (status.cacheVersion != mCacheVersion) { + ALOGV("STATUS(CHECK): Cache version has changed! old(%u) new(%u)", status.cacheVersion, + mCacheVersion); + return false; + } + + // Check buildId + if (strcmp(status.buildId, mBuildId.c_str()) != 0) { + ALOGV("STATUS(CHECK): BuildId has changed! old(%s) new(%s)", status.buildId, + mBuildId.c_str()); + return false; + } + + // All checks passed! + ALOGV("STATUS(CHECK): Status file is good! cacheVersion(%u), buildId(%s) file(%s)", + status.cacheVersion, status.buildId, cacheStatus.c_str()); + return true; +} + void MultifileBlobCache::trackEntry(uint32_t entryHash, EGLsizeiANDROID valueSize, size_t fileSize, time_t accessTime) { mEntries.insert(entryHash); @@ -485,10 +642,12 @@ MultifileEntryStats MultifileBlobCache::getEntryStats(uint32_t entryHash) { void MultifileBlobCache::increaseTotalCacheSize(size_t fileSize) { mTotalCacheSize += fileSize; + mTotalCacheEntries++; } void MultifileBlobCache::decreaseTotalCacheSize(size_t fileSize) { mTotalCacheSize -= fileSize; + mTotalCacheEntries--; } bool MultifileBlobCache::addToHotCache(uint32_t newEntryHash, int newFd, uint8_t* newEntryBuffer, @@ -557,7 +716,7 @@ bool MultifileBlobCache::removeFromHotCache(uint32_t entryHash) { return false; } -bool MultifileBlobCache::applyLRU(size_t cacheLimit) { +bool MultifileBlobCache::applyLRU(size_t cacheSizeLimit, size_t cacheEntryLimit) { // Walk through our map of sorted last access times and remove files until under the limit for (auto cacheEntryIter = mEntryStats.begin(); cacheEntryIter != mEntryStats.end();) { uint32_t entryHash = cacheEntryIter->first; @@ -590,9 +749,10 @@ bool MultifileBlobCache::applyLRU(size_t cacheLimit) { // See if it has been reduced enough size_t totalCacheSize = getTotalSize(); - if (totalCacheSize <= cacheLimit) { + size_t totalCacheEntries = getTotalEntries(); + if (totalCacheSize <= cacheSizeLimit && totalCacheEntries <= cacheEntryLimit) { // Success - ALOGV("LRU: Reduced cache to %zu", totalCacheSize); + ALOGV("LRU: Reduced cache to size %zu entries %zu", totalCacheSize, totalCacheEntries); return true; } } @@ -601,8 +761,43 @@ bool MultifileBlobCache::applyLRU(size_t cacheLimit) { return false; } +// Clear the cache by removing all entries and deleting the directory +bool MultifileBlobCache::clearCache() { + DIR* dir; + struct dirent* entry; + dir = opendir(mMultifileDirName.c_str()); + if (dir == nullptr) { + ALOGE("CLEAR: Unable to open multifile dir: %s", mMultifileDirName.c_str()); + return false; + } + + // Delete all entries and the status file + while ((entry = readdir(dir)) != nullptr) { + if (entry->d_name == "."s || entry->d_name == ".."s) { + continue; + } + + std::string entryName = entry->d_name; + std::string fullPath = mMultifileDirName + "/" + entryName; + if (remove(fullPath.c_str()) != 0) { + ALOGE("CLEAR: Error removing %s: %s", fullPath.c_str(), std::strerror(errno)); + return false; + } + } + + // Delete the directory + if (remove(mMultifileDirName.c_str()) != 0) { + ALOGE("CLEAR: Error removing %s: %s", mMultifileDirName.c_str(), std::strerror(errno)); + return false; + } + + ALOGV("CLEAR: Cleared the multifile blobcache"); + return true; +} + // When removing files, what fraction of the overall limit should be reached when removing files // A divisor of two will decrease the cache to 50%, four to 25% and so on +// We use the same limit to manage size and entry count constexpr uint32_t kCacheLimitDivisor = 2; // Calculate the cache size and remove old entries until under the limit @@ -611,8 +806,9 @@ void MultifileBlobCache::trimCache() { ALOGV("TRIM: Waiting for work to complete."); waitForWorkComplete(); - ALOGV("TRIM: Reducing multifile cache size to %zu", mMaxTotalSize / kCacheLimitDivisor); - if (!applyLRU(mMaxTotalSize / kCacheLimitDivisor)) { + ALOGV("TRIM: Reducing multifile cache size to %zu, entries %zu", + mMaxTotalSize / kCacheLimitDivisor, mMaxTotalEntries / kCacheLimitDivisor); + if (!applyLRU(mMaxTotalSize / kCacheLimitDivisor, mMaxTotalEntries / kCacheLimitDivisor)) { ALOGE("Error when clearing multifile shader cache"); return; } diff --git a/opengl/libs/EGL/MultifileBlobCache.h b/opengl/libs/EGL/MultifileBlobCache.h index 5e527dcf35..18566c2892 100644 --- a/opengl/libs/EGL/MultifileBlobCache.h +++ b/opengl/libs/EGL/MultifileBlobCache.h @@ -21,6 +21,7 @@ #include <EGL/eglext.h> #include <android-base/thread_annotations.h> +#include <cutils/properties.h> #include <future> #include <map> #include <queue> @@ -33,6 +34,9 @@ namespace android { +constexpr uint32_t kMultifileBlobCacheVersion = 1; +constexpr char kMultifileBlobCacheStatusFile[] = "cache.status"; + struct MultifileHeader { uint32_t magic; uint32_t crc; @@ -46,6 +50,13 @@ struct MultifileEntryStats { time_t accessTime; }; +struct MultifileStatus { + uint32_t magic; + uint32_t crc; + uint32_t cacheVersion; + char buildId[PROP_VALUE_MAX]; +}; + struct MultifileHotCache { int entryFd; uint8_t* entryBuffer; @@ -92,7 +103,7 @@ private: class MultifileBlobCache { public: MultifileBlobCache(size_t maxKeySize, size_t maxValueSize, size_t maxTotalSize, - const std::string& baseDir); + size_t maxTotalEntries, const std::string& baseDir); ~MultifileBlobCache(); void set(const void* key, EGLsizeiANDROID keySize, const void* value, @@ -103,6 +114,13 @@ public: void finish(); size_t getTotalSize() const { return mTotalCacheSize; } + size_t getTotalEntries() const { return mTotalCacheEntries; } + + const std::string& getCurrentBuildId() const { return mBuildId; } + void setCurrentBuildId(const std::string& buildId) { mBuildId = buildId; } + + uint32_t getCurrentCacheVersion() const { return mCacheVersion; } + void setCurrentCacheVersion(uint32_t cacheVersion) { mCacheVersion = cacheVersion; } private: void trackEntry(uint32_t entryHash, EGLsizeiANDROID valueSize, size_t fileSize, @@ -111,6 +129,9 @@ private: bool removeEntry(uint32_t entryHash); MultifileEntryStats getEntryStats(uint32_t entryHash); + bool createStatus(const std::string& baseDir); + bool checkStatus(const std::string& baseDir); + size_t getFileSize(uint32_t entryHash); size_t getValueSize(uint32_t entryHash); @@ -120,12 +141,16 @@ private: bool addToHotCache(uint32_t entryHash, int fd, uint8_t* entryBufer, size_t entrySize); bool removeFromHotCache(uint32_t entryHash); + bool clearCache(); void trimCache(); - bool applyLRU(size_t cacheLimit); + bool applyLRU(size_t cacheSizeLimit, size_t cacheEntryLimit); bool mInitialized; std::string mMultifileDirName; + std::string mBuildId; + uint32_t mCacheVersion; + std::unordered_set<uint32_t> mEntries; std::unordered_map<uint32_t, MultifileEntryStats> mEntryStats; std::unordered_map<uint32_t, MultifileHotCache> mHotCache; @@ -133,7 +158,9 @@ private: size_t mMaxKeySize; size_t mMaxValueSize; size_t mMaxTotalSize; + size_t mMaxTotalEntries; size_t mTotalCacheSize; + size_t mTotalCacheEntries; size_t mHotCacheLimit; size_t mHotCacheEntryLimit; size_t mHotCacheSize; diff --git a/opengl/libs/EGL/MultifileBlobCache_test.cpp b/opengl/libs/EGL/MultifileBlobCache_test.cpp index 1639be6480..90a0f1ee06 100644 --- a/opengl/libs/EGL/MultifileBlobCache_test.cpp +++ b/opengl/libs/EGL/MultifileBlobCache_test.cpp @@ -16,13 +16,17 @@ #include "MultifileBlobCache.h" +#include <android-base/properties.h> #include <android-base/test_utils.h> #include <fcntl.h> #include <gtest/gtest.h> #include <stdio.h> +#include <fstream> #include <memory> +using namespace std::literals; + namespace android { template <typename T> @@ -31,23 +35,40 @@ using sp = std::shared_ptr<T>; constexpr size_t kMaxKeySize = 2 * 1024; constexpr size_t kMaxValueSize = 6 * 1024; constexpr size_t kMaxTotalSize = 32 * 1024; +constexpr size_t kMaxTotalEntries = 64; class MultifileBlobCacheTest : public ::testing::Test { protected: virtual void SetUp() { + clearProperties(); mTempFile.reset(new TemporaryFile()); mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, - &mTempFile->path[0])); + kMaxTotalEntries, &mTempFile->path[0])); } - virtual void TearDown() { mMBC.reset(); } + virtual void TearDown() { + clearProperties(); + mMBC.reset(); + } int getFileDescriptorCount(); + std::vector<std::string> getCacheEntries(); + + void clearProperties(); std::unique_ptr<TemporaryFile> mTempFile; std::unique_ptr<MultifileBlobCache> mMBC; }; +void MultifileBlobCacheTest::clearProperties() { + // Clear any debug properties used in the tests + base::SetProperty("debug.egl.blobcache.cache_version", ""); + base::WaitForProperty("debug.egl.blobcache.cache_version", ""); + + base::SetProperty("debug.egl.blobcache.build_id", ""); + base::WaitForProperty("debug.egl.blobcache.build_id", ""); +} + TEST_F(MultifileBlobCacheTest, CacheSingleValueSucceeds) { unsigned char buf[4] = {0xee, 0xee, 0xee, 0xee}; mMBC->set("abcd", 4, "efgh", 4); @@ -211,6 +232,23 @@ TEST_F(MultifileBlobCacheTest, CacheMaxKeyAndValueSizeSucceeds) { } } +TEST_F(MultifileBlobCacheTest, CacheMaxEntrySucceeds) { + // Fill the cache with max entries + int i = 0; + for (i = 0; i < kMaxTotalEntries; i++) { + mMBC->set(std::to_string(i).c_str(), sizeof(i), std::to_string(i).c_str(), sizeof(i)); + } + + // Ensure it is full + ASSERT_EQ(mMBC->getTotalEntries(), kMaxTotalEntries); + + // Add another entry + mMBC->set(std::to_string(i).c_str(), sizeof(i), std::to_string(i).c_str(), sizeof(i)); + + // Ensure total entries is cut in half + 1 + ASSERT_EQ(mMBC->getTotalEntries(), kMaxTotalEntries / 2 + 1); +} + TEST_F(MultifileBlobCacheTest, CacheMinKeyAndValueSizeSucceeds) { unsigned char buf[1] = {0xee}; mMBC->set("x", 1, "y", 1); @@ -234,8 +272,7 @@ int MultifileBlobCacheTest::getFileDescriptorCount() { TEST_F(MultifileBlobCacheTest, EnsureFileDescriptorsClosed) { // Populate the cache with a bunch of entries - size_t kLargeNumberOfEntries = 1024; - for (int i = 0; i < kLargeNumberOfEntries; i++) { + for (int i = 0; i < kMaxTotalEntries; i++) { // printf("Caching: %i", i); // Use the index as the key and value @@ -247,27 +284,223 @@ TEST_F(MultifileBlobCacheTest, EnsureFileDescriptorsClosed) { } // Ensure we don't have a bunch of open fds - ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2); + ASSERT_LT(getFileDescriptorCount(), kMaxTotalEntries / 2); // Close the cache so everything writes out mMBC->finish(); mMBC.reset(); // Now open it again and ensure we still don't have a bunch of open fds - mMBC.reset( - new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, &mTempFile->path[0])); + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); // Check after initialization - ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2); + ASSERT_LT(getFileDescriptorCount(), kMaxTotalEntries / 2); - for (int i = 0; i < kLargeNumberOfEntries; i++) { + for (int i = 0; i < kMaxTotalEntries; i++) { int result = 0; ASSERT_EQ(sizeof(i), mMBC->get(&i, sizeof(i), &result, sizeof(result))); ASSERT_EQ(i, result); } // And again after we've actually used it - ASSERT_LT(getFileDescriptorCount(), kLargeNumberOfEntries / 2); + ASSERT_LT(getFileDescriptorCount(), kMaxTotalEntries / 2); +} + +std::vector<std::string> MultifileBlobCacheTest::getCacheEntries() { + std::string cachePath = &mTempFile->path[0]; + std::string multifileDirName = cachePath + ".multifile"; + std::vector<std::string> cacheEntries; + + struct stat info; + if (stat(multifileDirName.c_str(), &info) == 0) { + // We have a multifile dir. Skip the status file and return the only entry. + DIR* dir; + struct dirent* entry; + if ((dir = opendir(multifileDirName.c_str())) != nullptr) { + while ((entry = readdir(dir)) != nullptr) { + if (entry->d_name == "."s || entry->d_name == ".."s) { + continue; + } + if (strcmp(entry->d_name, kMultifileBlobCacheStatusFile) == 0) { + continue; + } + cacheEntries.push_back(multifileDirName + "/" + entry->d_name); + } + } else { + printf("Unable to open %s, error: %s\n", multifileDirName.c_str(), + std::strerror(errno)); + } + } else { + printf("Unable to stat %s, error: %s\n", multifileDirName.c_str(), std::strerror(errno)); + } + + return cacheEntries; +} + +TEST_F(MultifileBlobCacheTest, CacheContainsStatus) { + struct stat info; + std::stringstream statusFile; + statusFile << &mTempFile->path[0] << ".multifile/" << kMultifileBlobCacheStatusFile; + + // After INIT, cache should have a status + ASSERT_TRUE(stat(statusFile.str().c_str(), &info) == 0); + + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure status lives after closing the cache + ASSERT_TRUE(stat(statusFile.str().c_str(), &info) == 0); + + // Open the cache again + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we still have a status + ASSERT_TRUE(stat(statusFile.str().c_str(), &info) == 0); +} + +// Verify missing cache status file causes cache the be cleared +TEST_F(MultifileBlobCacheTest, MissingCacheStatusClears) { + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure there is one cache entry + ASSERT_EQ(getCacheEntries().size(), 1); + + // Delete the status file + std::stringstream statusFile; + statusFile << &mTempFile->path[0] << ".multifile/" << kMultifileBlobCacheStatusFile; + remove(statusFile.str().c_str()); + + // Open the cache again and ensure no cache hits + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we have no entries + ASSERT_EQ(getCacheEntries().size(), 0); +} + +// Verify modified cache status file BEGIN causes cache to be cleared +TEST_F(MultifileBlobCacheTest, ModifiedCacheStatusBeginClears) { + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure there is one cache entry + ASSERT_EQ(getCacheEntries().size(), 1); + + // Modify the status file + std::stringstream statusFile; + statusFile << &mTempFile->path[0] << ".multifile/" << kMultifileBlobCacheStatusFile; + + // Stomp on the beginning of the cache file + const char* stomp = "BADF00D"; + std::fstream fs(statusFile.str()); + fs.seekp(0, std::ios_base::beg); + fs.write(stomp, strlen(stomp)); + fs.flush(); + fs.close(); + + // Open the cache again and ensure no cache hits + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we have no entries + ASSERT_EQ(getCacheEntries().size(), 0); +} + +// Verify modified cache status file END causes cache to be cleared +TEST_F(MultifileBlobCacheTest, ModifiedCacheStatusEndClears) { + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure there is one cache entry + ASSERT_EQ(getCacheEntries().size(), 1); + + // Modify the status file + std::stringstream statusFile; + statusFile << &mTempFile->path[0] << ".multifile/" << kMultifileBlobCacheStatusFile; + + // Stomp on the END of the cache status file, modifying its contents + const char* stomp = "BADF00D"; + std::fstream fs(statusFile.str()); + fs.seekp(-strlen(stomp), std::ios_base::end); + fs.write(stomp, strlen(stomp)); + fs.flush(); + fs.close(); + + // Open the cache again and ensure no cache hits + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we have no entries + ASSERT_EQ(getCacheEntries().size(), 0); +} + +// Verify mismatched cacheVersion causes cache to be cleared +TEST_F(MultifileBlobCacheTest, MismatchedCacheVersionClears) { + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure there is one cache entry + ASSERT_EQ(getCacheEntries().size(), 1); + + // Set a debug cacheVersion + std::string newCacheVersion = std::to_string(kMultifileBlobCacheVersion + 1); + ASSERT_TRUE(base::SetProperty("debug.egl.blobcache.cache_version", newCacheVersion.c_str())); + ASSERT_TRUE( + base::WaitForProperty("debug.egl.blobcache.cache_version", newCacheVersion.c_str())); + + // Open the cache again and ensure no cache hits + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we have no entries + ASSERT_EQ(getCacheEntries().size(), 0); +} + +// Verify mismatched buildId causes cache to be cleared +TEST_F(MultifileBlobCacheTest, MismatchedBuildIdClears) { + // Set one entry + mMBC->set("abcd", 4, "efgh", 4); + + // Close the cache so everything writes out + mMBC->finish(); + mMBC.reset(); + + // Ensure there is one cache entry + ASSERT_EQ(getCacheEntries().size(), 1); + + // Set a debug buildId + base::SetProperty("debug.egl.blobcache.build_id", "foo"); + base::WaitForProperty("debug.egl.blobcache.build_id", "foo"); + + // Open the cache again and ensure no cache hits + mMBC.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &mTempFile->path[0])); + + // Ensure we have no entries + ASSERT_EQ(getCacheEntries().size(), 0); } } // namespace android diff --git a/opengl/libs/EGL/egl_cache.cpp b/opengl/libs/EGL/egl_cache.cpp index 1b68344cb1..98ff1d12cc 100644 --- a/opengl/libs/EGL/egl_cache.cpp +++ b/opengl/libs/EGL/egl_cache.cpp @@ -41,6 +41,7 @@ static const unsigned int kDeferredMonolithicSaveDelay = 4; constexpr uint32_t kMaxMultifileKeySize = 1 * 1024 * 1024; constexpr uint32_t kMaxMultifileValueSize = 8 * 1024 * 1024; constexpr uint32_t kMaxMultifileTotalSize = 32 * 1024 * 1024; +constexpr uint32_t kMaxMultifileTotalEntries = 4 * 1024; namespace android { @@ -277,7 +278,7 @@ MultifileBlobCache* egl_cache_t::getMultifileBlobCacheLocked() { if (mMultifileBlobCache == nullptr) { mMultifileBlobCache.reset(new MultifileBlobCache(kMaxMultifileKeySize, kMaxMultifileValueSize, mCacheByteLimit, - mFilename)); + kMaxMultifileTotalEntries, mFilename)); } return mMultifileBlobCache.get(); } diff --git a/opengl/libs/EGL/fuzzer/MultifileBlobCache_fuzzer.cpp b/opengl/libs/EGL/fuzzer/MultifileBlobCache_fuzzer.cpp index 633cc9c51b..2d9ee3a49e 100644 --- a/opengl/libs/EGL/fuzzer/MultifileBlobCache_fuzzer.cpp +++ b/opengl/libs/EGL/fuzzer/MultifileBlobCache_fuzzer.cpp @@ -28,6 +28,7 @@ namespace android { constexpr size_t kMaxKeySize = 2 * 1024; constexpr size_t kMaxValueSize = 6 * 1024; constexpr size_t kMaxTotalSize = 32 * 1024; +constexpr size_t kMaxTotalEntries = 64; extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { // To fuzz this, we're going to create a key/value pair from data @@ -79,8 +80,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { std::unique_ptr<MultifileBlobCache> mbc; tempFile.reset(new TemporaryFile()); - mbc.reset( - new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, &tempFile->path[0])); + mbc.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &tempFile->path[0])); // With remaining data, select different paths below int loopCount = 1; uint8_t bumpCount = 0; @@ -131,8 +132,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { // Place the maxKey/maxValue twice // The first will fit, the second will trigger hot cache trimming tempFile.reset(new TemporaryFile()); - mbc.reset( - new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, &tempFile->path[0])); + mbc.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, kMaxTotalSize, kMaxTotalEntries, + &tempFile->path[0])); uint8_t* buffer = new uint8_t[kMaxValueSize]; mbc->set(maxKey1.data(), kMaxKeySize, maxValue1.data(), kMaxValueSize); mbc->set(maxKey2.data(), kMaxKeySize, maxValue2.data(), kMaxValueSize); @@ -145,7 +146,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { // overflow tempFile.reset(new TemporaryFile()); mbc.reset(new MultifileBlobCache(kMaxKeySize, kMaxValueSize, 2 * (kMaxKeySize + kMaxValueSize), - &tempFile->path[0])); + kMaxTotalEntries, &tempFile->path[0])); mbc->set(maxKey1.data(), kMaxKeySize, maxValue1.data(), kMaxValueSize); mbc->set(maxKey2.data(), kMaxKeySize, maxValue2.data(), kMaxValueSize); mbc->get(maxKey1.data(), kMaxKeySize, buffer, kMaxValueSize); diff --git a/opengl/tests/EGLTest/egl_cache_test.cpp b/opengl/tests/EGLTest/egl_cache_test.cpp index f81c68f66e..ce5818229a 100644 --- a/opengl/tests/EGLTest/egl_cache_test.cpp +++ b/opengl/tests/EGLTest/egl_cache_test.cpp @@ -114,25 +114,26 @@ std::string EGLCacheTest::getCachefileName() { struct stat info; if (stat(multifileDirName.c_str(), &info) == 0) { // Ensure we only have one file to manage - int realFileCount = 0; + int entryFileCount = 0; - // We have a multifile dir. Return the only real file in it. + // We have a multifile dir. Return the only entry file in it. DIR* dir; struct dirent* entry; if ((dir = opendir(multifileDirName.c_str())) != nullptr) { while ((entry = readdir(dir)) != nullptr) { - if (entry->d_name == "."s || entry->d_name == ".."s) { + if (entry->d_name == "."s || entry->d_name == ".."s || + strcmp(entry->d_name, kMultifileBlobCacheStatusFile) == 0) { continue; } cachefileName = multifileDirName + "/" + entry->d_name; - realFileCount++; + entryFileCount++; } } else { printf("Unable to open %s, error: %s\n", multifileDirName.c_str(), std::strerror(errno)); } - if (realFileCount != 1) { + if (entryFileCount != 1) { // If there was more than one real file in the directory, this // violates test assumptions cachefileName = ""; diff --git a/services/inputflinger/Android.bp b/services/inputflinger/Android.bp index b213f9aeba..b5f5df95a9 100644 --- a/services/inputflinger/Android.bp +++ b/services/inputflinger/Android.bp @@ -243,6 +243,9 @@ phony { "Bug-115739809", "StructLayout_test", + // jni + "libservices.core", + // rust targets "libinput_rust_test", diff --git a/services/inputflinger/UnwantedInteractionBlocker.cpp b/services/inputflinger/UnwantedInteractionBlocker.cpp index 0f6232477e..1e2b9b3ad3 100644 --- a/services/inputflinger/UnwantedInteractionBlocker.cpp +++ b/services/inputflinger/UnwantedInteractionBlocker.cpp @@ -67,6 +67,16 @@ const bool DEBUG_OUTBOUND_MOTION = const bool DEBUG_MODEL = __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG "Model", ANDROID_LOG_INFO); +/** + * When multi-device input is enabled, we shouldn't use PreferStylusOverTouchBlocker at all. + * However, multi-device input has the following default behaviour: hovering stylus rejects touch. + * Therefore, if we want to disable that behaviour (and go back to a place where stylus down + * blocks touch, but hovering stylus doesn't interact with touch), we should just disable the entire + * multi-device input feature. + */ +const bool ENABLE_MULTI_DEVICE_INPUT = input_flags::enable_multi_device_input() && + !input_flags::disable_reject_touch_on_stylus_hover(); + // Category (=namespace) name for the input settings that are applied at boot time static const char* INPUT_NATIVE_BOOT = "input_native_boot"; /** @@ -347,7 +357,7 @@ void UnwantedInteractionBlocker::notifyMotion(const NotifyMotionArgs& args) { ALOGD_IF(DEBUG_INBOUND_MOTION, "%s: %s", __func__, args.dump().c_str()); { // acquire lock std::scoped_lock lock(mLock); - if (input_flags::enable_multi_device_input()) { + if (ENABLE_MULTI_DEVICE_INPUT) { notifyMotionLocked(args); } else { const std::vector<NotifyMotionArgs> processedArgs = diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 1958c357d2..23ac088f7f 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -6927,11 +6927,12 @@ sp<WindowInfoHandle> InputDispatcher::findWallpaperWindowBelow( return nullptr; } -void InputDispatcher::setKeyRepeatConfiguration(nsecs_t timeout, nsecs_t delay) { +void InputDispatcher::setKeyRepeatConfiguration(std::chrono::nanoseconds timeout, + std::chrono::nanoseconds delay) { std::scoped_lock _l(mLock); - mConfig.keyRepeatTimeout = timeout; - mConfig.keyRepeatDelay = delay; + mConfig.keyRepeatTimeout = timeout.count(); + mConfig.keyRepeatDelay = delay.count(); } } // namespace android::inputdispatcher diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index e9d52cd4ec..574cf8bee2 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -145,7 +145,8 @@ public: // Public to allow tests to verify that a Monitor can get ANR. void setMonitorDispatchingTimeoutForTest(std::chrono::nanoseconds timeout); - void setKeyRepeatConfiguration(nsecs_t timeout, nsecs_t delay) override; + void setKeyRepeatConfiguration(std::chrono::nanoseconds timeout, + std::chrono::nanoseconds delay) override; private: enum class DropReason { diff --git a/services/inputflinger/dispatcher/InputState.cpp b/services/inputflinger/dispatcher/InputState.cpp index 16cc266938..17f0b8737b 100644 --- a/services/inputflinger/dispatcher/InputState.cpp +++ b/services/inputflinger/dispatcher/InputState.cpp @@ -24,19 +24,6 @@ namespace android::inputdispatcher { -namespace { -bool isHoverAction(int32_t action) { - switch (MotionEvent::getActionMasked(action)) { - case AMOTION_EVENT_ACTION_HOVER_ENTER: - case AMOTION_EVENT_ACTION_HOVER_MOVE: - case AMOTION_EVENT_ACTION_HOVER_EXIT: { - return true; - } - } - return false; -} -} // namespace - InputState::InputState(const IdGenerator& idGenerator) : mIdGenerator(idGenerator) {} InputState::~InputState() {} @@ -113,13 +100,6 @@ bool InputState::trackMotion(const MotionEntry& entry, int32_t action, int32_t f if (isStylusEvent(lastMemento.source, lastMemento.pointerProperties) && !isStylusEvent(entry.source, entry.pointerProperties)) { // We already have a stylus stream, and the new event is not from stylus. - if (!lastMemento.hovering) { - // If stylus is currently down, reject the new event unconditionally. - return false; - } - } - if (!lastMemento.hovering && isHoverAction(action)) { - // Reject hovers if already down return false; } } @@ -366,19 +346,13 @@ bool InputState::shouldCancelPreviousStream(const MotionEntry& motionEntry, return false; } - // We want stylus down to block touch and other source types, but stylus hover should not - // have such an effect. - if (isHoverAction(motionEntry.action) && !lastMemento.hovering) { - // New event is a hover. Keep the current non-hovering gesture instead - return false; - } - - if (isStylusEvent(lastMemento.source, lastMemento.pointerProperties) && !lastMemento.hovering) { - // We have non-hovering stylus already active. + if (isStylusEvent(lastMemento.source, lastMemento.pointerProperties)) { + // A stylus is already active. if (isStylusEvent(motionEntry.source, motionEntry.pointerProperties) && actionMasked == AMOTION_EVENT_ACTION_DOWN) { - // If this new event is a stylus from a different device going down, then cancel the old - // stylus and allow the new stylus to take over + // If this new event is from a different device, then cancel the old + // stylus and allow the new stylus to take over, but only if it's going down. + // Otherwise, they will start to race each other. return true; } diff --git a/services/inputflinger/dispatcher/include/InputDispatcherInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherInterface.h index d099b44d91..bc7b6445ff 100644 --- a/services/inputflinger/dispatcher/include/InputDispatcherInterface.h +++ b/services/inputflinger/dispatcher/include/InputDispatcherInterface.h @@ -221,7 +221,8 @@ public: /* * Updates key repeat configuration timeout and delay. */ - virtual void setKeyRepeatConfiguration(nsecs_t timeout, nsecs_t delay) = 0; + virtual void setKeyRepeatConfiguration(std::chrono::nanoseconds timeout, + std::chrono::nanoseconds delay) = 0; }; } // namespace android diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 91eceb0c4e..71362e3be9 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -2593,9 +2593,9 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusDownWithSpyBlocksTouchDown) { /** * One window. Stylus hover on the window. Next, touch from another device goes down. Ensure that - * touch is not dropped, because stylus hover should be ignored. + * touch is dropped, because stylus hover takes precedence. */ -TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDoesNotBlockTouchDown) { +TEST_F(InputDispatcherMultiDeviceTest, StylusHoverBlocksTouchDown) { std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> window = sp<FakeWindowHandle>::make(application, mDispatcher, "Window", ADISPLAY_ID_DEFAULT); @@ -2624,34 +2624,29 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDoesNotBlockTouchDown) { .pointer(PointerBuilder(0, ToolType::FINGER).x(141).y(146)) .build()); - // Stylus hover is canceled because touch is down - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_HOVER_EXIT), - WithDeviceId(stylusDeviceId), WithCoords(100, 110))); - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_DOWN), WithDeviceId(touchDeviceId), - WithCoords(140, 145))); - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_MOVE), WithDeviceId(touchDeviceId), - WithCoords(141, 146))); + // Touch is ignored because stylus is hovering - // Subsequent stylus movements are ignored + // Subsequent stylus movements are delivered correctly mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_MOVE, AINPUT_SOURCE_STYLUS) .deviceId(stylusDeviceId) .pointer(PointerBuilder(0, ToolType::STYLUS).x(101).y(111)) .build()); + window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_HOVER_MOVE), + WithDeviceId(stylusDeviceId), WithCoords(101, 111))); - // but subsequent touches continue to be delivered + // and subsequent touches continue to be ignored mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_MOVE, AINPUT_SOURCE_TOUCHSCREEN) .deviceId(touchDeviceId) .pointer(PointerBuilder(0, ToolType::FINGER).x(142).y(147)) .build()); - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_MOVE), WithDeviceId(touchDeviceId), - WithCoords(142, 147))); + window->assertNoEvents(); } /** * One window. Touch down on the window. Then, stylus hover on the window from another device. - * Ensure that touch is not canceled, because stylus hover should be dropped. + * Ensure that touch is canceled, because stylus hover should take precedence. */ -TEST_F(InputDispatcherMultiDeviceTest, TouchIsNotCanceledByStylusHover) { +TEST_F(InputDispatcherMultiDeviceTest, TouchIsCanceledByStylusHover) { std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> window = sp<FakeWindowHandle>::make(application, mDispatcher, "Window", ADISPLAY_ID_DEFAULT); @@ -2683,15 +2678,21 @@ TEST_F(InputDispatcherMultiDeviceTest, TouchIsNotCanceledByStylusHover) { .deviceId(stylusDeviceId) .pointer(PointerBuilder(0, ToolType::STYLUS).x(101).y(111)) .build()); - // Stylus hover movement is dropped + // Stylus hover movement causes touch to be canceled + window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_CANCEL), WithDeviceId(touchDeviceId), + WithCoords(141, 146))); + window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_HOVER_ENTER), + WithDeviceId(stylusDeviceId), WithCoords(100, 110))); + window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_HOVER_MOVE), + WithDeviceId(stylusDeviceId), WithCoords(101, 111))); + // Subsequent touch movements are ignored mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_MOVE, AINPUT_SOURCE_TOUCHSCREEN) .deviceId(touchDeviceId) .pointer(PointerBuilder(0, ToolType::FINGER).x(142).y(147)) .build()); - // Subsequent touch movements are delivered correctly - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_MOVE), WithDeviceId(touchDeviceId), - WithCoords(142, 147))); + + window->assertNoEvents(); } /** @@ -3008,11 +3009,11 @@ TEST_F(InputDispatcherMultiDeviceTest, MultiDeviceWithSpy) { * Three windows: a window on the left, a window on the right, and a spy window positioned above * both. * Check hover in left window and touch down in the right window. - * At first, spy should receive hover, but the touch down should cancel hovering inside spy. + * At first, spy should receive hover. Spy shouldn't receive touch while stylus is hovering. * At the same time, left and right should be getting independent streams of hovering and touch, * respectively. */ -TEST_F(InputDispatcherMultiDeviceTest, MultiDeviceHoverBlockedByTouchWithSpy) { +TEST_F(InputDispatcherMultiDeviceTest, MultiDeviceHoverBlocksTouchWithSpy) { std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> spyWindow = @@ -3052,28 +3053,25 @@ TEST_F(InputDispatcherMultiDeviceTest, MultiDeviceHoverBlockedByTouchWithSpy) { .pointer(PointerBuilder(0, ToolType::FINGER).x(300).y(100)) .build()); leftWindow->assertNoEvents(); - spyWindow->consumeMotionEvent( - AllOf(WithMotionAction(ACTION_HOVER_EXIT), WithDeviceId(stylusDeviceId))); - spyWindow->consumeMotionEvent( - AllOf(WithMotionAction(ACTION_DOWN), WithDeviceId(touchDeviceId))); + spyWindow->assertNoEvents(); rightWindow->consumeMotionEvent( AllOf(WithMotionAction(ACTION_DOWN), WithDeviceId(touchDeviceId))); - // Stylus movements continue. They should be delivered to the left window only. + // Stylus movements continue. They should be delivered to the left window and the spy. mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_MOVE, AINPUT_SOURCE_STYLUS) .deviceId(stylusDeviceId) .pointer(PointerBuilder(0, ToolType::STYLUS).x(110).y(110)) .build()); leftWindow->consumeMotionEvent( AllOf(WithMotionAction(ACTION_HOVER_MOVE), WithDeviceId(stylusDeviceId))); + spyWindow->consumeMotionEvent( + AllOf(WithMotionAction(ACTION_HOVER_MOVE), WithDeviceId(stylusDeviceId))); - // Touch movements continue. They should be delivered to the right window and to the spy + // Touch movements continue. They should be delivered to the right window only mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_MOVE, AINPUT_SOURCE_TOUCHSCREEN) .deviceId(touchDeviceId) .pointer(PointerBuilder(0, ToolType::FINGER).x(301).y(101)) .build()); - spyWindow->consumeMotionEvent( - AllOf(WithMotionAction(ACTION_MOVE), WithDeviceId(touchDeviceId))); rightWindow->consumeMotionEvent( AllOf(WithMotionAction(ACTION_MOVE), WithDeviceId(touchDeviceId))); @@ -3288,7 +3286,7 @@ TEST_F(InputDispatcherMultiDeviceTest, HoverTapAndSplitTouch) { * While the touch is down, new hover events from the stylus device should be ignored. After the * touch is gone, stylus hovering should start working again. */ -TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDroppedWhenTouchTap) { +TEST_F(InputDispatcherMultiDeviceTest, StylusHoverIgnoresTouchTap) { std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> window = sp<FakeWindowHandle>::make(application, mDispatcher, "Window", ADISPLAY_ID_DEFAULT); @@ -3314,10 +3312,7 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDroppedWhenTouchTap) { .deviceId(touchDeviceId) .pointer(PointerBuilder(0, ToolType::FINGER).x(100).y(100)) .build())); - // The touch device should cause hover to stop! - window->consumeMotionEvent( - AllOf(WithMotionAction(ACTION_HOVER_EXIT), WithDeviceId(stylusDeviceId))); - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_DOWN), WithDeviceId(touchDeviceId))); + // The touch device should be ignored! // Continue hovering with stylus. ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, @@ -3327,7 +3322,9 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDroppedWhenTouchTap) { .deviceId(stylusDeviceId) .pointer(PointerBuilder(0, ToolType::STYLUS).x(60).y(60)) .build())); - // Hovers are now ignored + // Hovers continue to work + window->consumeMotionEvent( + AllOf(WithMotionAction(ACTION_HOVER_MOVE), WithDeviceId(stylusDeviceId))); // Lift up the finger ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, @@ -3337,7 +3334,6 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDroppedWhenTouchTap) { .deviceId(touchDeviceId) .pointer(PointerBuilder(0, ToolType::FINGER).x(100).y(100)) .build())); - window->consumeMotionEvent(AllOf(WithMotionAction(ACTION_UP), WithDeviceId(touchDeviceId))); ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectMotionEvent(*mDispatcher, @@ -3346,8 +3342,8 @@ TEST_F(InputDispatcherMultiDeviceTest, StylusHoverDroppedWhenTouchTap) { .deviceId(stylusDeviceId) .pointer(PointerBuilder(0, ToolType::STYLUS).x(70).y(70)) .build())); - window->consumeMotionEvent(AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_ENTER), - WithDeviceId(stylusDeviceId))); + window->consumeMotionEvent( + AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithDeviceId(stylusDeviceId))); window->assertNoEvents(); } @@ -6752,8 +6748,8 @@ TEST_F(InputDispatcherFallbackKeyTest, CanceledKeyCancelsFallback) { class InputDispatcherKeyRepeatTest : public InputDispatcherTest { protected: - static constexpr nsecs_t KEY_REPEAT_TIMEOUT = 40 * 1000000; // 40 ms - static constexpr nsecs_t KEY_REPEAT_DELAY = 40 * 1000000; // 40 ms + static constexpr std::chrono::nanoseconds KEY_REPEAT_TIMEOUT = 40ms; + static constexpr std::chrono::nanoseconds KEY_REPEAT_DELAY = 40ms; std::shared_ptr<FakeApplicationHandle> mApp; sp<FakeWindowHandle> mWindow; @@ -8135,7 +8131,8 @@ TEST_F(InputDispatcherSingleWindowAnr, // Injection is async, so it will succeed ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectKey(*mDispatcher, AKEY_EVENT_ACTION_DOWN, /*repeatCount=*/0, - ADISPLAY_ID_DEFAULT, InputEventInjectionSync::NONE)); + ADISPLAY_ID_DEFAULT, InputEventInjectionSync::NONE, INJECT_EVENT_TIMEOUT, + /*allowKeyRepeat=*/false)); // At this point, key is still pending, and should not be sent to the application yet. // Make sure the `assertNoEvents` check doesn't take too long. It uses // CONSUME_TIMEOUT_NO_EVENT_EXPECTED under the hood. diff --git a/services/powermanager/Android.bp b/services/powermanager/Android.bp index 8b16890a45..1f72e8ba2c 100644 --- a/services/powermanager/Android.bp +++ b/services/powermanager/Android.bp @@ -19,6 +19,7 @@ cc_library_shared { "PowerHalWrapper.cpp", "PowerSaveState.cpp", "Temperature.cpp", + "WorkDuration.cpp", "WorkSource.cpp", ":libpowermanager_aidl", ], diff --git a/services/powermanager/WorkDuration.cpp b/services/powermanager/WorkDuration.cpp new file mode 100644 index 0000000000..ef723c229c --- /dev/null +++ b/services/powermanager/WorkDuration.cpp @@ -0,0 +1,51 @@ +/** + * Copyright (C) 2023 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. + */ + +#define LOG_TAG "WorkDuration" + +#include <android/WorkDuration.h> +#include <android/performance_hint.h> +#include <binder/Parcel.h> +#include <utils/Log.h> + +namespace android::os { + +WorkDuration::WorkDuration(int64_t startTimestampNanos, int64_t totalDurationNanos, + int64_t cpuDurationNanos, int64_t gpuDurationNanos) + : workPeriodStartTimestampNanos(startTimestampNanos), + actualTotalDurationNanos(totalDurationNanos), + actualCpuDurationNanos(cpuDurationNanos), + actualGpuDurationNanos(gpuDurationNanos) {} + +status_t WorkDuration::writeToParcel(Parcel* parcel) const { + if (parcel == nullptr) { + ALOGE("%s: Null parcel", __func__); + return BAD_VALUE; + } + + parcel->writeInt64(workPeriodStartTimestampNanos); + parcel->writeInt64(actualTotalDurationNanos); + parcel->writeInt64(actualCpuDurationNanos); + parcel->writeInt64(actualGpuDurationNanos); + parcel->writeInt64(timestampNanos); + return OK; +} + +status_t WorkDuration::readFromParcel(const Parcel*) { + return INVALID_OPERATION; +} + +} // namespace android::os diff --git a/services/powermanager/include/android/WorkDuration.h b/services/powermanager/include/android/WorkDuration.h new file mode 100644 index 0000000000..99b5b8b1b4 --- /dev/null +++ b/services/powermanager/include/android/WorkDuration.h @@ -0,0 +1,71 @@ +/** + * Copyright (C) 2023 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 <binder/Parcelable.h> +#include <math.h> + +struct AWorkDuration {}; + +namespace android::os { + +/** + * C++ Parcelable version of {@link PerformanceHintManager.WorkDuration} that can be used in + * binder calls. + * This file needs to be kept in sync with the WorkDuration in + * frameworks/base/core/java/android/os/WorkDuration.java + */ +struct WorkDuration : AWorkDuration, android::Parcelable { + WorkDuration() = default; + ~WorkDuration() = default; + + WorkDuration(int64_t workPeriodStartTimestampNanos, int64_t actualTotalDurationNanos, + int64_t actualCpuDurationNanos, int64_t actualGpuDurationNanos); + status_t writeToParcel(Parcel* parcel) const override; + status_t readFromParcel(const Parcel* parcel) override; + + inline bool equalsWithoutTimestamp(const WorkDuration& other) const { + return workPeriodStartTimestampNanos == other.workPeriodStartTimestampNanos && + actualTotalDurationNanos == other.actualTotalDurationNanos && + actualCpuDurationNanos == other.actualCpuDurationNanos && + actualGpuDurationNanos == other.actualGpuDurationNanos; + } + + bool operator==(const WorkDuration& other) const { + return timestampNanos == other.timestampNanos && equalsWithoutTimestamp(other); + } + + bool operator!=(const WorkDuration& other) const { return !(*this == other); } + + friend std::ostream& operator<<(std::ostream& os, const WorkDuration& workDuration) { + os << "{" + << "workPeriodStartTimestampNanos: " << workDuration.workPeriodStartTimestampNanos + << ", actualTotalDurationNanos: " << workDuration.actualTotalDurationNanos + << ", actualCpuDurationNanos: " << workDuration.actualCpuDurationNanos + << ", actualGpuDurationNanos: " << workDuration.actualGpuDurationNanos + << ", timestampNanos: " << workDuration.timestampNanos << "}"; + return os; + } + + int64_t workPeriodStartTimestampNanos; + int64_t actualTotalDurationNanos; + int64_t actualCpuDurationNanos; + int64_t actualGpuDurationNanos; + int64_t timestampNanos; +}; + +} // namespace android::os diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index 17fa7bedf7..0989863b7d 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -93,6 +93,7 @@ cc_defaults { "libscheduler", "libserviceutils", "libshaders", + "libsurfaceflinger_common", "libtimestats", "libtonemap", "libsurfaceflingerflags", @@ -175,7 +176,6 @@ filegroup { "FrontEnd/LayerLifecycleManager.cpp", "FrontEnd/RequestedLayerState.cpp", "FrontEnd/TransactionHandler.cpp", - "FlagManager.cpp", "FpsReporter.cpp", "FrameTracer/FrameTracer.cpp", "FrameTracker.cpp", diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp index 370e4b66e8..2740a979f3 100644 --- a/services/surfaceflinger/CompositionEngine/Android.bp +++ b/services/surfaceflinger/CompositionEngine/Android.bp @@ -63,7 +63,10 @@ cc_defaults { cc_library { name: "libcompositionengine", defaults: ["libcompositionengine_defaults"], - static_libs: ["libsurfaceflingerflags"], + static_libs: [ + "libsurfaceflinger_common", + "libsurfaceflingerflags", + ], srcs: [ "src/planner/CachedSet.cpp", "src/planner/Flattener.cpp", @@ -108,6 +111,7 @@ cc_library { "libgtest", "libgmock", "libcompositionengine", + "libsurfaceflinger_common_test", "libsurfaceflingerflags_test", ], local_include_dirs: ["include"], @@ -143,6 +147,7 @@ cc_test { "librenderengine_mocks", "libgmock", "libgtest", + "libsurfaceflinger_common_test", "libsurfaceflingerflags_test", ], // For some reason, libvulkan isn't picked up from librenderengine diff --git a/services/surfaceflinger/CompositionEngine/AndroidTest.xml b/services/surfaceflinger/CompositionEngine/AndroidTest.xml new file mode 100644 index 0000000000..94e92f0143 --- /dev/null +++ b/services/surfaceflinger/CompositionEngine/AndroidTest.xml @@ -0,0 +1,33 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright 2023 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. +--> +<configuration description="Config for libcompositionengine_test"> + <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer"> + <option name="cleanup" value="true" /> + <option name="push" + value="libcompositionengine_test->/data/local/tmp/libcompositionengine_test" /> + </target_preparer> + + <!-- + Disable SELinux so that crashes in the test suite produces symbolized stack traces. + --> + <target_preparer class="com.android.tradefed.targetprep.DisableSELinuxTargetPreparer" /> + + <option name="test-suite-tag" value="apct" /> + <test class="com.android.tradefed.testtype.GTest" > + <option name="native-test-device-path" value="/data/local/tmp" /> + <option name="module-name" value="libcompositionengine_test" /> + </test> +</configuration> diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 5b6591aeac..2ffe92b028 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -63,13 +63,14 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) mDisplayToken(args.displayToken), mSequenceId(args.sequenceId), mCompositionDisplay{args.compositionDisplay}, - mActiveModeFPSTrace("ActiveModeFPS -" + to_string(getId())), - mActiveModeFPSHwcTrace("ActiveModeFPS_HWC -" + to_string(getId())), - mRenderFrameRateFPSTrace("RenderRateFPS -" + to_string(getId())), + mActiveModeFPSTrace(concatId("ActiveModeFPS")), + mActiveModeFPSHwcTrace(concatId("ActiveModeFPS_HWC")), + mRenderFrameRateFPSTrace(concatId("RenderRateFPS")), mPhysicalOrientation(args.physicalOrientation), mIsPrimary(args.isPrimary), mRequestedRefreshRate(args.requestedRefreshRate), - mRefreshRateSelector(std::move(args.refreshRateSelector)) { + mRefreshRateSelector(std::move(args.refreshRateSelector)), + mDesiredActiveModeChanged(concatId("DesiredActiveModeChanged"), false) { mCompositionDisplay->editState().isSecure = args.isSecure; mCompositionDisplay->createRenderSurface( compositionengine::RenderSurfaceCreationArgsBuilder() diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index a044534964..a40f310711 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -269,6 +269,11 @@ public: void dump(utils::Dumper&) const; private: + template <size_t N> + inline std::string concatId(const char (&str)[N]) const { + return std::string(ftl::Concat(str, ' ', getId().value).str()); + } + const sp<SurfaceFlinger> mFlinger; HWComposer& mHwComposer; const wp<IBinder> mDisplayToken; @@ -316,8 +321,7 @@ private: mutable std::mutex mActiveModeLock; ActiveModeInfo mDesiredActiveMode GUARDED_BY(mActiveModeLock); - TracedOrdinal<bool> mDesiredActiveModeChanged GUARDED_BY(mActiveModeLock) = - {ftl::Concat("DesiredActiveModeChanged-", getId().value).c_str(), false}; + TracedOrdinal<bool> mDesiredActiveModeChanged GUARDED_BY(mActiveModeLock); ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext); bool mIsModeSetPending GUARDED_BY(kMainThreadContext) = false; diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp index 2a6443a98a..2d957e6334 100644 --- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp +++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp @@ -24,6 +24,7 @@ #include <android-base/file.h> #include <android/binder_ibinder_platform.h> #include <android/binder_manager.h> +#include <common/FlagManager.h> #include <gui/TraceUtils.h> #include <log/log.h> #include <utils/Trace.h> @@ -281,7 +282,7 @@ bool AidlComposer::isSupported(OptionalFeature feature) const { } bool AidlComposer::getDisplayConfigurationsSupported() const { - return mComposerInterfaceVersion >= 3; + return mComposerInterfaceVersion >= 3 && FlagManager::getInstance().vrr_config(); } std::vector<Capability> AidlComposer::getCapabilities() { diff --git a/services/surfaceflinger/DisplayHardware/DisplayMode.h b/services/surfaceflinger/DisplayHardware/DisplayMode.h index f32fb3a5c7..ba0825c5af 100644 --- a/services/surfaceflinger/DisplayHardware/DisplayMode.h +++ b/services/surfaceflinger/DisplayHardware/DisplayMode.h @@ -29,8 +29,8 @@ #include <scheduler/Fps.h> +#include <common/FlagManager.h> #include "DisplayHardware/Hal.h" -#include "FlagManager.h" #include "Scheduler/StrongTyping.h" namespace android { diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp index f00ef671ad..e005ad3e03 100644 --- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp +++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp @@ -251,7 +251,12 @@ void PowerAdvisor::reportActualWorkDuration() { actualDuration = std::make_optional(*actualDuration + sTargetSafetyMargin); mActualDuration = actualDuration; WorkDuration duration; + duration.workPeriodStartTimestampNanos = mCommitStartTimes[0].ns(); + // TODO(b/284324521): Correctly calculate total duration. duration.durationNanos = actualDuration->ns(); + duration.cpuDurationNanos = actualDuration->ns(); + // TODO(b/284324521): Calculate RenderEngine GPU time. + duration.gpuDurationNanos = 0; duration.timeStampNanos = TimePoint::now().ns(); mHintSessionQueue.push_back(duration); diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index 2a0857d4dd..9476ff4932 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -739,6 +739,7 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a !snapshot.changes.test(RequestedLayerState::Changes::Created)) { if (forceUpdate || snapshot.changes.any(RequestedLayerState::Changes::Geometry | + RequestedLayerState::Changes::BufferSize | RequestedLayerState::Changes::Input)) { updateInput(snapshot, requested, parentSnapshot, path, args); } @@ -1095,6 +1096,8 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot, snapshot.inputInfo.inputConfig |= gui::WindowInfo::InputConfig::TRUSTED_OVERLAY; } + snapshot.inputInfo.contentSize = snapshot.croppedBufferSize.getSize(); + // If the layer is a clone, we need to crop the input region to cloned root to prevent // touches from going outside the cloned area. if (path.isClone()) { diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 66ea15ca2d..0c0639ee83 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -136,6 +136,7 @@ using frontend::RoundedCornerState; using gui::GameMode; using gui::LayerMetadata; using gui::WindowInfo; +using ui::Size; using PresentState = frametimeline::SurfaceFrame::PresentState; @@ -165,6 +166,7 @@ Layer::Layer(const surfaceflinger::LayerCreationArgs& args) mDrawingState.sequence = 0; mDrawingState.transform.set(0, 0); mDrawingState.frameNumber = 0; + mDrawingState.previousFrameNumber = 0; mDrawingState.barrierFrameNumber = 0; mDrawingState.producerId = 0; mDrawingState.barrierProducerId = 0; @@ -2591,6 +2593,9 @@ WindowInfo Layer::fillInputInfo(const InputDisplayArgs& displayArgs) { } } + Rect bufferSize = getBufferSize(getDrawingState()); + info.contentSize = Size(bufferSize.width(), bufferSize.height()); + return info; } @@ -2931,7 +2936,6 @@ void Layer::onLayerDisplayed(ftl::SharedFuture<FenceResult> futureFenceResult, break; } } - if (ch != nullptr) { ch->previousReleaseCallbackId = mPreviousReleaseCallbackId; ch->previousReleaseFences.emplace_back(std::move(futureFenceResult)); @@ -2940,6 +2944,10 @@ void Layer::onLayerDisplayed(ftl::SharedFuture<FenceResult> futureFenceResult, if (mBufferInfo.mBuffer) { mPreviouslyPresentedLayerStacks.push_back(layerStack); } + + if (mDrawingState.frameNumber > 0) { + mDrawingState.previousFrameNumber = mDrawingState.frameNumber; + } } void Layer::onSurfaceFrameCreated( @@ -3144,6 +3152,7 @@ void Layer::releasePreviousBuffer() { void Layer::resetDrawingStateBufferInfo() { mDrawingState.producerId = 0; mDrawingState.frameNumber = 0; + mDrawingState.previousFrameNumber = 0; mDrawingState.releaseBufferListener = nullptr; mDrawingState.buffer = nullptr; mDrawingState.acquireFence = sp<Fence>::make(-1); @@ -3420,6 +3429,7 @@ bool Layer::setTransactionCompletedListeners(const std::vector<sp<CallbackHandle // If this transaction set an acquire fence on this layer, set its acquire time handle->acquireTimeOrFence = mCallbackHandleAcquireTimeOrFence; handle->frameNumber = mDrawingState.frameNumber; + handle->previousFrameNumber = mDrawingState.previousFrameNumber; // Store so latched time and release fence can be set mDrawingState.callbackHandles.push_back(handle); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index f71591044d..28168c3f65 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -138,6 +138,7 @@ public: ui::Dataspace dataspace; uint64_t frameNumber; + uint64_t previousFrameNumber; // high watermark framenumber to use to check for barriers to protect ourselves // from out of order transactions uint64_t barrierFrameNumber; diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 6752a0bafa..b960e33682 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -16,8 +16,8 @@ #include <algorithm> +#include <common/FlagManager.h> #include "Client.h" -#include "FlagManager.h" #include "Layer.h" #include "RefreshRateOverlay.h" diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 7f627f829d..693a357de5 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -43,9 +43,9 @@ #include <utils/Errors.h> #include <utils/Trace.h> +#include <common/FlagManager.h> #include <scheduler/VsyncConfig.h> #include "DisplayHardware/DisplayMode.h" -#include "FlagManager.h" #include "FrameTimeline.h" #include "VSyncDispatch.h" #include "VSyncTracker.h" diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp index 450ba1d841..d309adccf8 100644 --- a/services/surfaceflinger/Scheduler/LayerHistory.cpp +++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp @@ -31,9 +31,9 @@ #include <string> #include <utility> +#include <common/FlagManager.h> #include "../Layer.h" #include "EventThread.h" -#include "FlagManager.h" #include "LayerInfo.h" namespace android::scheduler { diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp index 5892b2b44c..47c8ef9f16 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp +++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp @@ -1487,7 +1487,7 @@ FpsRange RefreshRateSelector::getFrameRateCategoryRange(FrameRateCategory catego case FrameRateCategory::Normal: return FpsRange{60_Hz, 90_Hz}; case FrameRateCategory::Low: - return FpsRange{30_Hz, 60_Hz}; + return FpsRange{30_Hz, 30_Hz}; case FrameRateCategory::NoPreference: case FrameRateCategory::Default: LOG_ALWAYS_FATAL("Should not get fps range for frame rate category: %s", diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index f41243cb32..b54f33451b 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -45,9 +45,9 @@ #include <memory> #include <numeric> +#include <common/FlagManager.h> #include "../Layer.h" #include "EventThread.h" -#include "FlagManager.h" #include "FrameRateOverrideMappings.h" #include "FrontEnd/LayerHandle.h" #include "OneShotTimer.h" diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp index 3e7ec492fa..ef30887037 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp +++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp @@ -25,7 +25,7 @@ #include <scheduler/TimeKeeper.h> -#include "FlagManager.h" +#include <common/FlagManager.h> #include "VSyncDispatchTimerQueue.h" #include "VSyncTracker.h" diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp index 57aa010740..f5f93ce2f1 100644 --- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp +++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp @@ -29,13 +29,13 @@ #include <android-base/logging.h> #include <android-base/stringprintf.h> +#include <common/FlagManager.h> #include <cutils/compiler.h> #include <cutils/properties.h> #include <ftl/concat.h> #include <gui/TraceUtils.h> #include <utils/Log.h> -#include "FlagManager.h" #include "RefreshRateSelector.h" #include "VSyncPredictor.h" diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 2dd035b1c5..644b6ef30e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -113,6 +113,7 @@ #include <unordered_map> #include <vector> +#include <common/FlagManager.h> #include <gui/LayerStatePermissions.h> #include <gui/SchedulingPolicy.h> #include <ui/DisplayIdentification.h> @@ -129,7 +130,6 @@ #include "DisplayHardware/VirtualDisplaySurface.h" #include "DisplayRenderArea.h" #include "Effects/Daltonizer.h" -#include "FlagManager.h" #include "FpsReporter.h" #include "FrameTimeline/FrameTimeline.h" #include "FrameTracer/FrameTracer.h" @@ -875,7 +875,7 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { mRenderEnginePrimeCacheFuture = getRenderEngine().primeCache(shouldPrimeUltraHDR); if (setSchedFifo(true) != NO_ERROR) { - ALOGW("Can't set SCHED_OTHER for primeCache"); + ALOGW("Can't set SCHED_FIFO after primeCache"); } } @@ -2922,7 +2922,6 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, const scheduler::FrameTargeters& frameTargeters, nsecs_t presentStartTime) { ATRACE_CALL(); - ALOGV(__func__); ui::PhysicalDisplayMap<PhysicalDisplayId, std::shared_ptr<FenceTime>> presentFences; ui::PhysicalDisplayMap<PhysicalDisplayId, const sp<Fence>> gpuCompositionDoneFences; @@ -3810,7 +3809,6 @@ void SurfaceFlinger::commitTransactionsLocked(uint32_t transactionFlags) { // first frame before the display is available, we rely // on WMS and DMS to provide the right information // so the client can calculate the hint. - ALOGV("Skipping reporting transform hint update for %s", layer->getDebugName()); layer->skipReportingTransformHint(); } else { layer->updateTransformHint(hintDisplay->getTransformHint()); @@ -6771,8 +6769,7 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r case 1007: // Unused. return NAME_NOT_FOUND; case 1008: // Toggle forced GPU composition. - mDebugDisableHWC = data.readInt32() != 0; - scheduleRepaint(); + sfdo_forceClientComposition(data.readInt32() != 0); return NO_ERROR; case 1009: // Toggle use of transform hint. mDebugDisableTransformHint = data.readInt32() != 0; @@ -9074,6 +9071,11 @@ void SurfaceFlinger::sfdo_scheduleCommit() { setTransactionFlags(eTransactionNeeded | eDisplayTransactionNeeded | eTraversalNeeded); } +void SurfaceFlinger::sfdo_forceClientComposition(bool enabled) { + mDebugDisableHWC = enabled; + scheduleRepaint(); +} + // gui::ISurfaceComposer binder::Status SurfaceComposerAIDL::bootFinished() { @@ -9793,6 +9795,11 @@ binder::Status SurfaceComposerAIDL::scheduleCommit() { return binder::Status::ok(); } +binder::Status SurfaceComposerAIDL::forceClientComposition(bool enabled) { + mFlinger->sfdo_forceClientComposition(enabled); + return binder::Status::ok(); +} + binder::Status SurfaceComposerAIDL::updateSmallAreaDetection(const std::vector<int32_t>& appIds, const std::vector<float>& thresholds) { status_t status; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 9177a5bbcd..9e6da3f80b 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -63,13 +63,13 @@ #include <scheduler/interface/ICompositor.h> #include <ui/FenceResult.h> +#include <common/FlagManager.h> #include "Display/PhysicalDisplay.h" #include "DisplayDevice.h" #include "DisplayHardware/HWC2.h" #include "DisplayHardware/PowerAdvisor.h" #include "DisplayIdGenerator.h" #include "Effects/Daltonizer.h" -#include "FlagManager.h" #include "FrontEnd/DisplayInfo.h" #include "FrontEnd/LayerCreationArgs.h" #include "FrontEnd/LayerLifecycleManager.h" @@ -1462,6 +1462,7 @@ private: void sfdo_setDebugFlash(int delay); void sfdo_scheduleComposite(); void sfdo_scheduleCommit(); + void sfdo_forceClientComposition(bool enabled); }; class SurfaceComposerAIDL : public gui::BnSurfaceComposer { @@ -1573,6 +1574,7 @@ public: binder::Status setDebugFlash(int delay) override; binder::Status scheduleComposite() override; binder::Status scheduleCommit() override; + binder::Status forceClientComposition(bool enabled) override; binder::Status updateSmallAreaDetection(const std::vector<int32_t>& appIds, const std::vector<float>& thresholds) override; binder::Status setSmallAreaDetectionThreshold(int32_t appId, float threshold) override; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 3587a726cd..6a155c17df 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -158,7 +158,7 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle->previousReleaseFence = prevFence; handle->previousReleaseFences.clear(); - FrameEventHistoryStats eventStats(handle->frameNumber, + FrameEventHistoryStats eventStats(handle->frameNumber, handle->previousFrameNumber, handle->gpuCompositionDoneFence->getSnapshot().fence, handle->compositorTiming, handle->refreshStartTime, handle->dequeueReadyTime); diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 3074795f62..245398f2f4 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -56,6 +56,7 @@ public: nsecs_t refreshStartTime = 0; nsecs_t dequeueReadyTime = 0; uint64_t frameNumber = 0; + uint64_t previousFrameNumber = 0; ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; }; diff --git a/services/surfaceflinger/common/Android.bp b/services/surfaceflinger/common/Android.bp new file mode 100644 index 0000000000..5ef22b590b --- /dev/null +++ b/services/surfaceflinger/common/Android.bp @@ -0,0 +1,48 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_native_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_native_license"], +} + +cc_defaults { + name: "libsurfaceflinger_common_defaults", + defaults: [ + "android.hardware.graphics.composer3-ndk_shared", + "surfaceflinger_defaults", + ], + shared_libs: [ + "libSurfaceFlingerProp", + "server_configurable_flags", + ], + static_libs: [ + "librenderengine", + ], + srcs: [ + "FlagManager.cpp", + ], + local_include_dirs: ["include"], + export_include_dirs: ["include"], +} + +cc_library_static { + name: "libsurfaceflinger_common", + defaults: [ + "libsurfaceflinger_common_defaults", + ], + static_libs: [ + "libsurfaceflingerflags", + ], +} + +cc_library_static { + name: "libsurfaceflinger_common_test", + defaults: [ + "libsurfaceflinger_common_defaults", + ], + static_libs: [ + "libsurfaceflingerflags_test", + ], +} diff --git a/services/surfaceflinger/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 13b8a6c0cf..8da7d8e92f 100644 --- a/services/surfaceflinger/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "FlagManager.h" +#include <common/FlagManager.h> #include <SurfaceFlingerProperties.sysprop.h> #include <android-base/parsebool.h> diff --git a/services/surfaceflinger/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index e3e4f80905..e3e4f80905 100644 --- a/services/surfaceflinger/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h diff --git a/services/surfaceflinger/fuzzer/Android.bp b/services/surfaceflinger/fuzzer/Android.bp index 243b8e04df..ab3b3528dd 100644 --- a/services/surfaceflinger/fuzzer/Android.bp +++ b/services/surfaceflinger/fuzzer/Android.bp @@ -39,6 +39,7 @@ cc_defaults { "libgtest_ndk_c++", "libgmock_main_ndk", "librenderengine_mocks", + "libsurfaceflinger_common", "perfetto_trace_protos", "libcompositionengine_mocks", "perfetto_trace_protos", diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_frametracer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_frametracer_fuzzer.cpp index 8978971539..ce8d47e71c 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_frametracer_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_frametracer_fuzzer.cpp @@ -30,12 +30,6 @@ constexpr int32_t kMinRange = 0; constexpr int32_t kConfigDuration = 500; constexpr int32_t kBufferSize = 1024; constexpr int32_t kTimeOffset = 100000; -constexpr perfetto::BackendType backendTypes[] = { - perfetto::kUnspecifiedBackend, - perfetto::kInProcessBackend, - perfetto::kSystemBackend, - perfetto::kCustomBackend, -}; class FrameTracerFuzzer { public: @@ -71,8 +65,7 @@ std::unique_ptr<perfetto::TracingSession> FrameTracerFuzzer::getTracingSessionFo auto* dsCfg = cfg.add_data_sources()->mutable_config(); dsCfg->set_name(android::FrameTracer::kFrameTracerDataSource); - auto tracingSession = - perfetto::Tracing::NewTrace(mFdp.PickValueInArray<perfetto::BackendType>(backendTypes)); + auto tracingSession = perfetto::Tracing::NewTrace(perfetto::kInProcessBackend); tracingSession->Setup(cfg); return tracingSession; } @@ -115,11 +108,15 @@ void FrameTracerFuzzer::process() { std::vector<int32_t> layerIds = generateLayerIds(mFdp.ConsumeIntegralInRange<size_t>(kMinLayerIds, kMaxLayerIds)); + std::unique_ptr<perfetto::TracingSession> tracingSession; while (mFdp.remaining_bytes()) { auto invokeFrametracerAPI = mFdp.PickValueInArray<const std::function<void()>>({ [&]() { mFrameTracer->registerDataSource(); }, [&]() { - auto tracingSession = getTracingSessionForTest(); + if (tracingSession) { + tracingSession->StopBlocking(); + } + tracingSession = getTracingSessionForTest(); tracingSession->StartBlocking(); }, [&]() { traceTimestamp(layerIds, layerIds.size()); }, diff --git a/services/surfaceflinger/main_surfaceflinger.cpp b/services/surfaceflinger/main_surfaceflinger.cpp index 9889cb9c5d..6c8972f1fb 100644 --- a/services/surfaceflinger/main_surfaceflinger.cpp +++ b/services/surfaceflinger/main_surfaceflinger.cpp @@ -29,12 +29,12 @@ #include <binder/IPCThreadState.h> #include <binder/IServiceManager.h> #include <binder/ProcessState.h> +#include <common/FlagManager.h> #include <configstore/Utils.h> #include <displayservice/DisplayService.h> #include <errno.h> #include <hidl/LegacySupport.h> #include <processgroup/sched_policy.h> -#include "FlagManager.h" #include "SurfaceFlinger.h" #include "SurfaceFlingerFactory.h" #include "SurfaceFlingerProperties.h" diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 5a3bca115b..dea019431b 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -45,7 +45,7 @@ filegroup { cc_aconfig_library { name: "libsurfaceflingerflags_test", aconfig_declarations: "surfaceflinger_flags", - test: true, + mode: "test", } cc_test { @@ -170,6 +170,7 @@ cc_defaults { "librenderengine_mocks", "libscheduler", "libserviceutils", + "libsurfaceflinger_common_test", "libtimestats", "libtimestats_atoms_proto", "libtimestats_proto", diff --git a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp index aa37754300..c040f29fec 100644 --- a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp +++ b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp @@ -17,7 +17,7 @@ #undef LOG_TAG #define LOG_TAG "FlagManagerTest" -#include "FlagManager.h" +#include <common/FlagManager.h> #include "FlagUtils.h" #include <gmock/gmock.h> diff --git a/services/surfaceflinger/tests/unittests/FlagUtils.h b/services/surfaceflinger/tests/unittests/FlagUtils.h index 333e4e7e05..550c70d98f 100644 --- a/services/surfaceflinger/tests/unittests/FlagUtils.h +++ b/services/surfaceflinger/tests/unittests/FlagUtils.h @@ -16,7 +16,7 @@ #pragma once -#include "FlagManager.h" +#include <common/FlagManager.h> #define SET_FLAG_FOR_TEST(name, value) TestFlagSetter _testflag_((name), (name), (value)) diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp index 58d7a40fc8..f1e841b88f 100644 --- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp +++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp @@ -30,6 +30,7 @@ #include <gmock/gmock.h> #pragma clang diagnostic pop +#include <common/FlagManager.h> #include <gui/LayerMetadata.h> #include <log/log.h> #include <chrono> @@ -38,9 +39,12 @@ #include "DisplayHardware/HWComposer.h" #include "DisplayHardware/Hal.h" #include "DisplayIdentificationTestHelpers.h" +#include "FlagUtils.h" #include "mock/DisplayHardware/MockComposer.h" #include "mock/DisplayHardware/MockHWC2.h" +#include <com_android_graphics_surfaceflinger_flags.h> + // TODO(b/129481165): remove the #pragma below and fix conversion issues #pragma clang diagnostic pop // ignored "-Wconversion" @@ -57,7 +61,6 @@ using ::aidl::android::hardware::graphics::composer3::RefreshRateChangedDebugDat using hal::IComposerClient; using ::testing::_; using ::testing::DoAll; -using ::testing::ElementsAreArray; using ::testing::Return; using ::testing::SetArgPointee; using ::testing::StrictMock; @@ -217,7 +220,102 @@ TEST_F(HWComposerTest, getModesWithLegacyDisplayConfigs) { } } -TEST_F(HWComposerTest, getModesWithDisplayConfigurations) { +TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_OFF) { + // if vrr_config is off, getDisplayConfigurationsSupported() is off as well + // then getModesWithLegacyDisplayConfigs should be called instead + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::vrr_config, false); + ASSERT_FALSE(FlagManager::getInstance().vrr_config()); + + constexpr hal::HWDisplayId kHwcDisplayId = 2; + constexpr hal::HWConfigId kConfigId = 42; + constexpr int32_t kMaxFrameIntervalNs = 50000000; // 20Fps + + expectHotplugConnect(kHwcDisplayId); + const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED); + ASSERT_TRUE(info); + + EXPECT_CALL(*mHal, getDisplayConfigurationsSupported()).WillRepeatedly(Return(false)); + + { + EXPECT_CALL(*mHal, getDisplayConfigs(kHwcDisplayId, _)) + .WillOnce(Return(HalError::BAD_DISPLAY)); + EXPECT_TRUE(mHwc.getModes(info->id, kMaxFrameIntervalNs).empty()); + } + { + constexpr int32_t kWidth = 480; + constexpr int32_t kHeight = 720; + constexpr int32_t kConfigGroup = 1; + constexpr int32_t kVsyncPeriod = 16666667; + + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::WIDTH, + _)) + .WillRepeatedly(DoAll(SetArgPointee<3>(kWidth), Return(HalError::NONE))); + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, + IComposerClient::Attribute::HEIGHT, _)) + .WillRepeatedly(DoAll(SetArgPointee<3>(kHeight), Return(HalError::NONE))); + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, + IComposerClient::Attribute::CONFIG_GROUP, _)) + .WillRepeatedly(DoAll(SetArgPointee<3>(kConfigGroup), Return(HalError::NONE))); + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, + IComposerClient::Attribute::VSYNC_PERIOD, _)) + .WillRepeatedly(DoAll(SetArgPointee<3>(kVsyncPeriod), Return(HalError::NONE))); + + // Optional Parameters UNSUPPORTED + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::DPI_X, + _)) + .WillOnce(Return(HalError::UNSUPPORTED)); + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::DPI_Y, + _)) + .WillOnce(Return(HalError::UNSUPPORTED)); + + EXPECT_CALL(*mHal, getDisplayConfigs(kHwcDisplayId, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(std::vector<hal::HWConfigId>{kConfigId}), + Return(HalError::NONE))); + + auto modes = mHwc.getModes(info->id, kMaxFrameIntervalNs); + EXPECT_EQ(modes.size(), size_t{1}); + EXPECT_EQ(modes.front().hwcId, kConfigId); + EXPECT_EQ(modes.front().width, kWidth); + EXPECT_EQ(modes.front().height, kHeight); + EXPECT_EQ(modes.front().configGroup, kConfigGroup); + EXPECT_EQ(modes.front().vsyncPeriod, kVsyncPeriod); + EXPECT_EQ(modes.front().dpiX, -1); + EXPECT_EQ(modes.front().dpiY, -1); + + // Optional parameters are supported + constexpr int32_t kDpi = 320; + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::DPI_X, + _)) + .WillOnce(DoAll(SetArgPointee<3>(kDpi), Return(HalError::NONE))); + EXPECT_CALL(*mHal, + getDisplayAttribute(kHwcDisplayId, kConfigId, IComposerClient::Attribute::DPI_Y, + _)) + .WillOnce(DoAll(SetArgPointee<3>(kDpi), Return(HalError::NONE))); + + modes = mHwc.getModes(info->id, kMaxFrameIntervalNs); + EXPECT_EQ(modes.size(), size_t{1}); + EXPECT_EQ(modes.front().hwcId, kConfigId); + EXPECT_EQ(modes.front().width, kWidth); + EXPECT_EQ(modes.front().height, kHeight); + EXPECT_EQ(modes.front().configGroup, kConfigGroup); + EXPECT_EQ(modes.front().vsyncPeriod, kVsyncPeriod); + // DPI values are scaled by 1000 in the legacy implementation. + EXPECT_EQ(modes.front().dpiX, kDpi / 1000.f); + EXPECT_EQ(modes.front().dpiY, kDpi / 1000.f); + } +} + +TEST_F(HWComposerTest, getModesWithDisplayConfigurations_VRR_ON) { + SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::vrr_config, true); + ASSERT_TRUE(FlagManager::getInstance().vrr_config()); + constexpr hal::HWDisplayId kHwcDisplayId = 2; constexpr hal::HWConfigId kConfigId = 42; constexpr int32_t kMaxFrameIntervalNs = 50000000; // 20Fps diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp index faa12a1032..a00e8864dd 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp @@ -1568,7 +1568,7 @@ TEST_P(RefreshRateSelectorTest, // These layers cannot change mode due to smoothSwitchOnly, and will definitely use // active mode (120Hz). {FrameRateCategory::NoPreference, true, 120_Hz, kModeId120}, - {FrameRateCategory::Low, true, 40_Hz, kModeId120}, + {FrameRateCategory::Low, true, 120_Hz, kModeId120}, {FrameRateCategory::Normal, true, 40_Hz, kModeId120}, {FrameRateCategory::High, true, 120_Hz, kModeId120}, }; |