diff options
53 files changed, 433 insertions, 288 deletions
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 826a8dbc2c..6b9a0a0e7e 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -4197,14 +4197,14 @@ int Dumpstate::DumpFile(const std::string& title, const std::string& path) { } int read_file_as_long(const char *path, long int *output) { - int fd = TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_NONBLOCK | O_CLOEXEC)); - if (fd < 0) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_NONBLOCK | O_CLOEXEC))); + if (fd.get() < 0) { int err = errno; MYLOGE("Error opening file descriptor for %s: %s\n", path, strerror(err)); return -1; } char buffer[50]; - ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd, buffer, sizeof(buffer))); + ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd.get(), buffer, sizeof(buffer))); if (bytes_read == -1) { MYLOGE("Error reading file %s: %s\n", path, strerror(errno)); return -2; diff --git a/cmds/evemu-record/main.rs b/cmds/evemu-record/main.rs index c30c00fcd2..db3fd77520 100644 --- a/cmds/evemu-record/main.rs +++ b/cmds/evemu-record/main.rs @@ -120,7 +120,7 @@ fn print_device_description( fn print_in_8_byte_chunks( output: &mut impl Write, prefix: &str, - data: &Vec<u8>, + data: &[u8], ) -> Result<(), io::Error> { for (i, byte) in data.iter().enumerate() { if i % 8 == 0 { diff --git a/include/android/surface_control.h b/include/android/surface_control.h index bf1f2e90ad..082387e63a 100644 --- a/include/android/surface_control.h +++ b/include/android/surface_control.h @@ -346,7 +346,7 @@ void ASurfaceTransaction_setZOrder(ASurfaceTransaction* transaction, */ void ASurfaceTransaction_setBuffer(ASurfaceTransaction* transaction, ASurfaceControl* surface_control, AHardwareBuffer* buffer, - int acquire_fence_fd = -1) __INTRODUCED_IN(29); + int acquire_fence_fd) __INTRODUCED_IN(29); /** * Updates the color for \a surface_control. This will make the background color for the diff --git a/services/inputflinger/BlockingQueue.h b/include/input/BlockingQueue.h index f848c82c42..f848c82c42 100644 --- a/services/inputflinger/BlockingQueue.h +++ b/include/input/BlockingQueue.h diff --git a/include/input/Input.h b/include/input/Input.h index ddc376809e..19f4ab38e8 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -1192,15 +1192,17 @@ public: */ struct PointerCaptureRequest { public: - inline PointerCaptureRequest() : enable(false), seq(0) {} - inline PointerCaptureRequest(bool enable, uint32_t seq) : enable(enable), seq(seq) {} + inline PointerCaptureRequest() : window(), seq(0) {} + inline PointerCaptureRequest(sp<IBinder> window, uint32_t seq) : window(window), seq(seq) {} inline bool operator==(const PointerCaptureRequest& other) const { - return enable == other.enable && seq == other.seq; + return window == other.window && seq == other.seq; } - explicit inline operator bool() const { return enable; } + inline bool isEnable() const { return window != nullptr; } - // True iff this is a request to enable Pointer Capture. - bool enable; + // The requesting window. + // If the request is to enable the capture, this is the input token of the window that requested + // pointer capture. Otherwise, this is nullptr. + sp<IBinder> window; // The sequence number for the request. uint32_t seq; diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 84ff9d74ef..ca9b08f8ad 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -159,6 +159,11 @@ cc_defaults { "UtilsHost.cpp", ], }, + android: { + lto: { + thin: true, + }, + }, }, aidl: { @@ -219,9 +224,6 @@ cc_defaults { "-performance-move-const-arg", // b/273486801 "portability*", ], - lto: { - thin: true, - }, } cc_library_headers { diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 35cea8132d..1ffdad50e3 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2980,6 +2980,7 @@ status_t Parcel::restartWrite(size_t desired) uint8_t* data = reallocZeroFree(mData, mDataCapacity, desired, mDeallocZero); if (!data && desired > mDataCapacity) { + LOG_ALWAYS_FATAL("out of memory"); mError = NO_MEMORY; return NO_MEMORY; } diff --git a/libs/input/Android.bp b/libs/input/Android.bp index 0171d74cc4..6b8bc01c24 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -135,6 +135,29 @@ rust_bindgen { ], } +cc_library_static { + name: "iinputflinger_aidl_lib_static", + host_supported: true, + srcs: [ + "android/os/IInputFlinger.aidl", + "android/os/InputChannelCore.aidl", + ], + shared_libs: [ + "libbinder", + ], + whole_static_libs: [ + "libgui_window_info_static", + ], + aidl: { + export_aidl_headers: true, + local_include_dirs: ["."], + include_dirs: [ + "frameworks/native/libs/gui", + "frameworks/native/libs/input", + ], + }, +} + // Contains methods to help access C++ code from rust cc_library_static { name: "libinput_from_rust_to_cpp", @@ -179,8 +202,6 @@ cc_library { "-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION", ], srcs: [ - "android/os/IInputFlinger.aidl", - "android/os/InputChannelCore.aidl", "AccelerationCurve.cpp", "Input.cpp", "InputDevice.cpp", @@ -243,7 +264,6 @@ cc_library { static_libs: [ "inputconstants-cpp", - "libgui_window_info_static", "libui-types", "libtflite_static", "libkernelconfigs", @@ -252,10 +272,10 @@ cc_library { whole_static_libs: [ "com.android.input.flags-aconfig-cc", "libinput_rust_ffi", + "iinputflinger_aidl_lib_static", ], export_static_lib_headers: [ - "libgui_window_info_static", "libui-types", ], @@ -289,14 +309,6 @@ cc_library { ], }, }, - - aidl: { - local_include_dirs: ["."], - export_aidl_headers: true, - include_dirs: [ - "frameworks/native/libs/gui", - ], - }, } // Use bootstrap version of stats logging library. diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index ff9d9a94c9..61a964ece9 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -1034,7 +1034,8 @@ std::tuple<int32_t, std::vector<PointerProperties>, std::vector<PointerCoords>> (splitPointerProperties.size() * (historySize + 1))); if (CC_UNLIKELY(splitPointerProperties.size() != splitCount)) { - LOG(FATAL) << "Cannot split MotionEvent: Requested splitting " << splitCount + // TODO(b/329107108): Promote this to a fatal check once bugs in the caller are resolved. + LOG(ERROR) << "Cannot split MotionEvent: Requested splitting " << splitCount << " pointers from the original event, but the original event only contained " << splitPointerProperties.size() << " of those pointers."; } diff --git a/libs/input/tests/Android.bp b/libs/input/tests/Android.bp index 0485ff6e1e..93af4c2066 100644 --- a/libs/input/tests/Android.bp +++ b/libs/input/tests/Android.bp @@ -13,6 +13,7 @@ cc_test { cpp_std: "c++20", host_supported: true, srcs: [ + "BlockingQueue_test.cpp", "IdGenerator_test.cpp", "InputChannel_test.cpp", "InputDevice_test.cpp", diff --git a/services/inputflinger/tests/BlockingQueue_test.cpp b/libs/input/tests/BlockingQueue_test.cpp index 754a5c451e..924b937080 100644 --- a/services/inputflinger/tests/BlockingQueue_test.cpp +++ b/libs/input/tests/BlockingQueue_test.cpp @@ -14,8 +14,7 @@ * limitations under the License. */ -#include "../BlockingQueue.h" - +#include <input/BlockingQueue.h> #include <gtest/gtest.h> #include <thread> @@ -109,7 +108,7 @@ TEST(BlockingQueueTest, Queue_AllowsMultipleThreads) { BlockingQueue<int> queue(capacity); // Fill queue from a different thread - std::thread fillQueue([&queue](){ + std::thread fillQueue([&queue]() { for (size_t i = 0; i < capacity; i++) { ASSERT_TRUE(queue.push(static_cast<int>(i))); } @@ -136,7 +135,7 @@ TEST(BlockingQueueTest, Queue_BlocksWhileWaitingForElements) { std::atomic_bool hasReceivedElement = false; // fill queue from a different thread - std::thread waitUntilHasElements([&queue, &hasReceivedElement](){ + std::thread waitUntilHasElements([&queue, &hasReceivedElement]() { queue.pop(); // This should block until an element has been added hasReceivedElement = true; }); diff --git a/services/inputflinger/InputProcessor.h b/services/inputflinger/InputProcessor.h index dcbfebc62f..7a00a2dae8 100644 --- a/services/inputflinger/InputProcessor.h +++ b/services/inputflinger/InputProcessor.h @@ -22,7 +22,7 @@ #include <unordered_map> #include <aidl/android/hardware/input/processor/IInputProcessor.h> -#include "BlockingQueue.h" +#include <input/BlockingQueue.h> #include "InputListener.h" namespace android { diff --git a/services/inputflinger/PointerChoreographer.cpp b/services/inputflinger/PointerChoreographer.cpp index b8d0c41f75..c333814078 100644 --- a/services/inputflinger/PointerChoreographer.cpp +++ b/services/inputflinger/PointerChoreographer.cpp @@ -288,7 +288,7 @@ void PointerChoreographer::processDeviceReset(const NotifyDeviceResetArgs& args) void PointerChoreographer::notifyPointerCaptureChanged( const NotifyPointerCaptureChangedArgs& args) { - if (args.request.enable) { + if (args.request.isEnable()) { std::scoped_lock _l(mLock); for (const auto& [_, mousePointerController] : mMousePointersByDisplay) { mousePointerController->fade(PointerControllerInterface::Transition::IMMEDIATE); diff --git a/services/inputflinger/dispatcher/Entry.cpp b/services/inputflinger/dispatcher/Entry.cpp index 264dc03e70..0246d606ac 100644 --- a/services/inputflinger/dispatcher/Entry.cpp +++ b/services/inputflinger/dispatcher/Entry.cpp @@ -110,7 +110,7 @@ PointerCaptureChangedEntry::PointerCaptureChangedEntry(int32_t id, nsecs_t event std::string PointerCaptureChangedEntry::getDescription() const { return StringPrintf("PointerCaptureChangedEvent(pointerCaptureEnabled=%s)", - pointerCaptureRequest.enable ? "true" : "false"); + pointerCaptureRequest.isEnable() ? "true" : "false"); } // --- DragEntry --- diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index f06caa6a40..dc220fe57e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -1715,7 +1715,7 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; sp<IBinder> token; - if (entry->pointerCaptureRequest.enable) { + if (entry->pointerCaptureRequest.isEnable()) { // Enable Pointer Capture. if (haveWindowWithPointerCapture && (entry->pointerCaptureRequest == mCurrentPointerCaptureRequest)) { @@ -1724,7 +1724,7 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( ALOGI("Skipping dispatch of Pointer Capture being enabled: no state change."); return; } - if (!mCurrentPointerCaptureRequest.enable) { + if (!mCurrentPointerCaptureRequest.isEnable()) { // This can happen if a window requests capture and immediately releases capture. ALOGW("No window requested Pointer Capture."); dropReason = DropReason::NO_POINTER_CAPTURE; @@ -1737,6 +1737,8 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( token = mFocusResolver.getFocusedWindowToken(mFocusedDisplayId); LOG_ALWAYS_FATAL_IF(!token, "Cannot find focused window for Pointer Capture."); + LOG_ALWAYS_FATAL_IF(token != entry->pointerCaptureRequest.window, + "Unexpected requested window for Pointer Capture."); mWindowTokenWithPointerCapture = token; } else { // Disable Pointer Capture. @@ -1756,8 +1758,8 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( } token = mWindowTokenWithPointerCapture; mWindowTokenWithPointerCapture = nullptr; - if (mCurrentPointerCaptureRequest.enable) { - setPointerCaptureLocked(false); + if (mCurrentPointerCaptureRequest.isEnable()) { + setPointerCaptureLocked(nullptr); } } @@ -1765,8 +1767,8 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( if (connection == nullptr) { // Window has gone away, clean up Pointer Capture state. mWindowTokenWithPointerCapture = nullptr; - if (mCurrentPointerCaptureRequest.enable) { - setPointerCaptureLocked(false); + if (mCurrentPointerCaptureRequest.isEnable()) { + setPointerCaptureLocked(nullptr); } return; } @@ -1813,7 +1815,7 @@ bool InputDispatcher::dispatchKeyLocked(nsecs_t currentTime, std::shared_ptr<con DropReason* dropReason, nsecs_t& nextWakeupTime) { // Preprocessing. if (!entry->dispatchInProgress) { - if (entry->repeatCount == 0 && entry->action == AKEY_EVENT_ACTION_DOWN && + if (!entry->syntheticRepeat && entry->action == AKEY_EVENT_ACTION_DOWN && (entry->policyFlags & POLICY_FLAG_TRUSTED) && (!(entry->policyFlags & POLICY_FLAG_DISABLE_KEY_REPEAT))) { if (mKeyRepeatState.lastKeyEntry && @@ -3832,9 +3834,10 @@ void InputDispatcher::startDispatchCycleLocked(nsecs_t currentTime, case EventEntry::Type::POINTER_CAPTURE_CHANGED: { const auto& captureEntry = static_cast<const PointerCaptureChangedEntry&>(eventEntry); - status = connection->inputPublisher - .publishCaptureEvent(dispatchEntry->seq, captureEntry.id, - captureEntry.pointerCaptureRequest.enable); + status = + connection->inputPublisher + .publishCaptureEvent(dispatchEntry->seq, captureEntry.id, + captureEntry.pointerCaptureRequest.isEnable()); break; } @@ -4357,6 +4360,20 @@ std::unique_ptr<MotionEntry> InputDispatcher::splitMotionEvent( MotionEvent::split(originalMotionEntry.action, originalMotionEntry.flags, /*historySize=*/0, originalMotionEntry.pointerProperties, originalMotionEntry.pointerCoords, pointerIds); + if (pointerIds.count() != pointerCoords.size()) { + // TODO(b/329107108): Determine why some IDs in pointerIds were not in originalMotionEntry. + // This is bad. We are missing some of the pointers that we expected to deliver. + // Most likely this indicates that we received an ACTION_MOVE events that has + // different pointer ids than we expected based on the previous ACTION_DOWN + // or ACTION_POINTER_DOWN events that caused us to decide to split the pointers + // in this way. + ALOGW("Dropping split motion event because the pointer count is %d but " + "we expected there to be %zu pointers. This probably means we received " + "a broken sequence of pointer ids from the input device: %s", + pointerCoords.size(), pointerIds.count(), + originalMotionEntry.getDescription().c_str()); + return nullptr; + } // TODO(b/327503168): Move this check inside MotionEvent::split once all callers handle it // correctly. @@ -4713,7 +4730,7 @@ void InputDispatcher::notifyDeviceReset(const NotifyDeviceResetArgs& args) { void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs& args) { if (debugInboundEventDetails()) { ALOGD("notifyPointerCaptureChanged - eventTime=%" PRId64 ", enabled=%s", args.eventTime, - args.request.enable ? "true" : "false"); + args.request.isEnable() ? "true" : "false"); } bool needWake = false; @@ -5784,7 +5801,7 @@ std::string InputDispatcher::dumpPointerCaptureStateLocked() const { std::string dump; dump += StringPrintf(INDENT "Pointer Capture Requested: %s\n", - toString(mCurrentPointerCaptureRequest.enable)); + toString(mCurrentPointerCaptureRequest.isEnable())); std::string windowName = "None"; if (mWindowTokenWithPointerCapture) { @@ -6203,7 +6220,7 @@ void InputDispatcher::requestPointerCapture(const sp<IBinder>& windowToken, bool return; } - if (enabled == mCurrentPointerCaptureRequest.enable) { + if (enabled == mCurrentPointerCaptureRequest.isEnable()) { ALOGW("Ignoring request to %s Pointer Capture: " "window has %s requested pointer capture.", enabled ? "enable" : "disable", enabled ? "already" : "not"); @@ -6219,7 +6236,7 @@ void InputDispatcher::requestPointerCapture(const sp<IBinder>& windowToken, bool } } - setPointerCaptureLocked(enabled); + setPointerCaptureLocked(enabled ? windowToken : nullptr); } // release lock // Wake the thread to process command entries. @@ -6849,14 +6866,14 @@ void InputDispatcher::onFocusChangedLocked( } void InputDispatcher::disablePointerCaptureForcedLocked() { - if (!mCurrentPointerCaptureRequest.enable && !mWindowTokenWithPointerCapture) { + if (!mCurrentPointerCaptureRequest.isEnable() && !mWindowTokenWithPointerCapture) { return; } ALOGD_IF(DEBUG_FOCUS, "Disabling Pointer Capture because the window lost focus."); - if (mCurrentPointerCaptureRequest.enable) { - setPointerCaptureLocked(false); + if (mCurrentPointerCaptureRequest.isEnable()) { + setPointerCaptureLocked(nullptr); } if (!mWindowTokenWithPointerCapture) { @@ -6876,8 +6893,8 @@ void InputDispatcher::disablePointerCaptureForcedLocked() { mInboundQueue.push_front(std::move(entry)); } -void InputDispatcher::setPointerCaptureLocked(bool enable) { - mCurrentPointerCaptureRequest.enable = enable; +void InputDispatcher::setPointerCaptureLocked(const sp<IBinder>& windowToken) { + mCurrentPointerCaptureRequest.window = windowToken; mCurrentPointerCaptureRequest.seq++; auto command = [this, request = mCurrentPointerCaptureRequest]() REQUIRES(mLock) { scoped_unlock unlock(mLock); diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index d6eba64dd0..13571b3b5f 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -421,7 +421,8 @@ private: void disablePointerCaptureForcedLocked() REQUIRES(mLock); // Set the Pointer Capture state in the Policy. - void setPointerCaptureLocked(bool enable) REQUIRES(mLock); + // The window is not nullptr for requests to enable, otherwise it is nullptr. + void setPointerCaptureLocked(const sp<IBinder>& window) REQUIRES(mLock); // Dispatcher state at time of last ANR. std::string mLastAnrState GUARDED_BY(mLock); diff --git a/services/inputflinger/dispatcher/trace/InputTracer.cpp b/services/inputflinger/dispatcher/trace/InputTracer.cpp index 83ed452d6e..49e6e21828 100644 --- a/services/inputflinger/dispatcher/trace/InputTracer.cpp +++ b/services/inputflinger/dispatcher/trace/InputTracer.cpp @@ -183,10 +183,21 @@ void InputTracer::traceEventDispatch(const DispatchEntry& dispatchEntry, const int32_t windowId = dispatchEntry.windowId.value_or(0); const int32_t vsyncId = dispatchEntry.windowId.has_value() ? dispatchEntry.vsyncId : 0; - mBackend->traceWindowDispatch({std::move(traced), dispatchEntry.deliveryTime, - dispatchEntry.resolvedFlags, dispatchEntry.targetUid, vsyncId, - windowId, dispatchEntry.transform, dispatchEntry.rawTransform, - /*hmac=*/{}, resolvedKeyRepeatCount}); + const WindowDispatchArgs windowDispatchArgs{std::move(traced), + dispatchEntry.deliveryTime, + dispatchEntry.resolvedFlags, + dispatchEntry.targetUid, + vsyncId, + windowId, + dispatchEntry.transform, + dispatchEntry.rawTransform, + /*hmac=*/{}, + resolvedKeyRepeatCount}; + if (eventState->isEventProcessingComplete) { + mBackend->traceWindowDispatch(std::move(windowDispatchArgs)); + } else { + eventState->pendingDispatchArgs.emplace_back(std::move(windowDispatchArgs)); + } } std::shared_ptr<InputTracer::EventState>& InputTracer::getState( @@ -205,6 +216,12 @@ void InputTracer::EventState::onEventProcessingComplete() { for (const auto& event : events) { writeEventToBackend(event, *tracer.mBackend); } + // Write all pending dispatch args to the trace. + for (const auto& windowDispatchArgs : pendingDispatchArgs) { + tracer.mBackend->traceWindowDispatch(windowDispatchArgs); + } + pendingDispatchArgs.clear(); + isEventProcessingComplete = true; } diff --git a/services/inputflinger/dispatcher/trace/InputTracer.h b/services/inputflinger/dispatcher/trace/InputTracer.h index 529c0fafed..8da9632d7c 100644 --- a/services/inputflinger/dispatcher/trace/InputTracer.h +++ b/services/inputflinger/dispatcher/trace/InputTracer.h @@ -52,6 +52,8 @@ public: private: std::unique_ptr<InputTracingBackendInterface> mBackend; + using WindowDispatchArgs = InputTracingBackendInterface::WindowDispatchArgs; + // The state of a tracked event, shared across all events derived from the original event. struct EventState { explicit inline EventState(InputTracer& tracer) : tracer(tracer){}; @@ -62,6 +64,8 @@ private: InputTracer& tracer; std::vector<const TracedEvent> events; bool isEventProcessingComplete{false}; + // A queue to hold dispatch args from being traced until event processing is complete. + std::vector<const WindowDispatchArgs> pendingDispatchArgs; // TODO(b/210460522): Add additional args for tracking event sensitivity and // dispatch target UIDs. }; diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp index 06f10e5445..c8cc5dcc83 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp @@ -486,7 +486,7 @@ void CursorInputMapper::configureBasicParams() { } void CursorInputMapper::configureOnPointerCapture(const InputReaderConfiguration& config) { - if (config.pointerCaptureRequest.enable) { + if (config.pointerCaptureRequest.isEnable()) { if (mParameters.mode == Parameters::Mode::POINTER) { mParameters.mode = Parameters::Mode::POINTER_RELATIVE; mSource = AINPUT_SOURCE_MOUSE_RELATIVE; diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 3c26d1d73e..7d27d4a9ce 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -923,7 +923,7 @@ void TouchInputMapper::configureInputDevice(nsecs_t when, bool* outResetNeeded) // Determine device mode. if (mParameters.deviceType == Parameters::DeviceType::POINTER && - mConfig.pointerGesturesEnabled && !mConfig.pointerCaptureRequest.enable) { + mConfig.pointerGesturesEnabled && !mConfig.pointerCaptureRequest.isEnable()) { mSource = AINPUT_SOURCE_MOUSE; mDeviceMode = DeviceMode::POINTER; if (hasStylus()) { @@ -1038,7 +1038,7 @@ void TouchInputMapper::configureInputDevice(nsecs_t when, bool* outResetNeeded) (mDeviceMode == DeviceMode::POINTER) || // - when pointer capture is enabled, to preserve the mouse cursor position; (mParameters.deviceType == Parameters::DeviceType::POINTER && - mConfig.pointerCaptureRequest.enable) || + mConfig.pointerCaptureRequest.isEnable()) || // - when we should be showing touches; (mDeviceMode == DeviceMode::DIRECT && mConfig.showTouches) || // - when we should be showing a pointer icon for direct styluses. @@ -1047,7 +1047,7 @@ void TouchInputMapper::configureInputDevice(nsecs_t when, bool* outResetNeeded) if (mPointerController == nullptr) { mPointerController = getContext()->getPointerController(getDeviceId()); } - if (mConfig.pointerCaptureRequest.enable) { + if (mConfig.pointerCaptureRequest.isEnable()) { mPointerController->fade(PointerControllerInterface::Transition::IMMEDIATE); } } else { diff --git a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp index eacc66eeab..99f9e24169 100644 --- a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp @@ -410,9 +410,9 @@ std::list<NotifyArgs> TouchpadInputMapper::reconfigure(nsecs_t when, .setBoolValues({config.touchpadRightClickZoneEnabled}); } std::list<NotifyArgs> out; - if ((!changes.any() && config.pointerCaptureRequest.enable) || + if ((!changes.any() && config.pointerCaptureRequest.isEnable()) || changes.test(InputReaderConfiguration::Change::POINTER_CAPTURE)) { - mPointerCaptured = config.pointerCaptureRequest.enable; + mPointerCaptured = config.pointerCaptureRequest.isEnable(); // The motion ranges are going to change, so bump the generation to clear the cached ones. bumpGeneration(); if (mPointerCaptured) { diff --git a/services/inputflinger/tests/Android.bp b/services/inputflinger/tests/Android.bp index a26153ec5b..09ae6dd28c 100644 --- a/services/inputflinger/tests/Android.bp +++ b/services/inputflinger/tests/Android.bp @@ -39,7 +39,6 @@ cc_test { ], srcs: [ "AnrTracker_test.cpp", - "BlockingQueue_test.cpp", "CapturedTouchpadEventConverter_test.cpp", "CursorInputMapper_test.cpp", "EventHub_test.cpp", diff --git a/services/inputflinger/tests/CursorInputMapper_test.cpp b/services/inputflinger/tests/CursorInputMapper_test.cpp index de740672fc..c44f88006e 100644 --- a/services/inputflinger/tests/CursorInputMapper_test.cpp +++ b/services/inputflinger/tests/CursorInputMapper_test.cpp @@ -166,7 +166,7 @@ protected: } void setPointerCapture(bool enabled) { - mReaderConfiguration.pointerCaptureRequest.enable = enabled; + mReaderConfiguration.pointerCaptureRequest.window = enabled ? sp<BBinder>::make() : nullptr; mReaderConfiguration.pointerCaptureRequest.seq = 1; int32_t generation = mDevice->getGeneration(); std::list<NotifyArgs> args = diff --git a/services/inputflinger/tests/FakeInputReaderPolicy.cpp b/services/inputflinger/tests/FakeInputReaderPolicy.cpp index 9e9371248e..8f593b553a 100644 --- a/services/inputflinger/tests/FakeInputReaderPolicy.cpp +++ b/services/inputflinger/tests/FakeInputReaderPolicy.cpp @@ -178,8 +178,8 @@ void FakeInputReaderPolicy::setTouchAffineTransformation(const TouchAffineTransf transform = t; } -PointerCaptureRequest FakeInputReaderPolicy::setPointerCapture(bool enabled) { - mConfig.pointerCaptureRequest = {enabled, mNextPointerCaptureSequenceNumber++}; +PointerCaptureRequest FakeInputReaderPolicy::setPointerCapture(const sp<IBinder>& window) { + mConfig.pointerCaptureRequest = {window, mNextPointerCaptureSequenceNumber++}; return mConfig.pointerCaptureRequest; } diff --git a/services/inputflinger/tests/FakeInputReaderPolicy.h b/services/inputflinger/tests/FakeInputReaderPolicy.h index da5085db7c..710bb5496f 100644 --- a/services/inputflinger/tests/FakeInputReaderPolicy.h +++ b/services/inputflinger/tests/FakeInputReaderPolicy.h @@ -68,7 +68,7 @@ public: TouchAffineTransformation getTouchAffineTransformation(const std::string& inputDeviceDescriptor, ui::Rotation surfaceRotation); void setTouchAffineTransformation(const TouchAffineTransformation t); - PointerCaptureRequest setPointerCapture(bool enabled); + PointerCaptureRequest setPointerCapture(const sp<IBinder>& window); void setShowTouches(bool enabled); void setDefaultPointerDisplayId(int32_t pointerDisplayId); void setPointerGestureEnabled(bool enabled); diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index f0f4d93ecd..b3b2ee2c9d 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -15,7 +15,6 @@ */ #include "../dispatcher/InputDispatcher.h" -#include "../BlockingQueue.h" #include "FakeApplicationHandle.h" #include "FakeInputTracingBackend.h" #include "TestEventMatchers.h" @@ -32,6 +31,7 @@ #include <flag_macros.h> #include <gmock/gmock.h> #include <gtest/gtest.h> +#include <input/BlockingQueue.h> #include <input/Input.h> #include <input/PrintTools.h> #include <linux/input.h> @@ -308,17 +308,22 @@ public: "signal"; } - PointerCaptureRequest assertSetPointerCaptureCalled(bool enabled) { + PointerCaptureRequest assertSetPointerCaptureCalled(const sp<WindowInfoHandle>& window, + bool enabled) { std::unique_lock lock(mLock); base::ScopedLockAssertion assumeLocked(mLock); - if (!mPointerCaptureChangedCondition.wait_for(lock, 100ms, - [this, enabled]() REQUIRES(mLock) { - return mPointerCaptureRequest->enable == - enabled; - })) { - ADD_FAILURE() << "Timed out waiting for setPointerCapture(" << enabled - << ") to be called."; + if (!mPointerCaptureChangedCondition + .wait_for(lock, 100ms, [this, enabled, window]() REQUIRES(mLock) { + if (enabled) { + return mPointerCaptureRequest->isEnable() && + mPointerCaptureRequest->window == window->getToken(); + } else { + return !mPointerCaptureRequest->isEnable(); + } + })) { + ADD_FAILURE() << "Timed out waiting for setPointerCapture(" << window->getName() << ", " + << enabled << ") to be called."; return {}; } auto request = *mPointerCaptureRequest; @@ -333,7 +338,7 @@ public: if (mPointerCaptureChangedCondition.wait_for(lock, 100ms) != std::cv_status::timeout) { FAIL() << "Expected setPointerCapture(request) to not be called, but was called. " "enabled = " - << std::to_string(mPointerCaptureRequest->enable); + << std::to_string(mPointerCaptureRequest->isEnable()); } mPointerCaptureRequest.reset(); } @@ -7477,6 +7482,12 @@ protected: mWindow->consumeKeyUp(ADISPLAY_ID_DEFAULT, /*expectedFlags=*/0); } + + void injectKeyRepeat(int32_t repeatCount) { + ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, + injectKey(*mDispatcher, AKEY_EVENT_ACTION_DOWN, repeatCount, ADISPLAY_ID_DEFAULT)) + << "Inject key event should return InputEventInjectionResult::SUCCEEDED"; + } }; TEST_F(InputDispatcherKeyRepeatTest, FocusedWindow_ReceivesKeyRepeat) { @@ -7565,6 +7576,17 @@ TEST_F(InputDispatcherKeyRepeatTest, FocusedWindow_RepeatKeyEventsUseUniqueEvent } } +TEST_F(InputDispatcherKeyRepeatTest, FocusedWindow_CorrectRepeatCountWhenInjectKeyRepeat) { + injectKeyRepeat(0); + mWindow->consumeKeyDown(ADISPLAY_ID_DEFAULT); + for (int32_t repeatCount = 1; repeatCount <= 2; ++repeatCount) { + expectKeyRepeatOnce(repeatCount); + } + injectKeyRepeat(1); + // Expect repeatCount to be 3 instead of 1 + expectKeyRepeatOnce(3); +} + /* Test InputDispatcher for MultiDisplay */ class InputDispatcherFocusOnTwoDisplaysTest : public InputDispatcherTest { public: @@ -8709,9 +8731,10 @@ TEST_F(InputDispatcherSingleWindowAnr, StaleKeyEventDoesNotAnr) { // Define a valid key down event that is stale (too old). event.initialize(InputEvent::nextId(), DEVICE_ID, AINPUT_SOURCE_KEYBOARD, ADISPLAY_ID_NONE, INVALID_HMAC, AKEY_EVENT_ACTION_DOWN, /*flags=*/0, AKEYCODE_A, KEY_A, - AMETA_NONE, /*repeatCount=*/1, eventTime, eventTime); + AMETA_NONE, /*repeatCount=*/0, eventTime, eventTime); - const int32_t policyFlags = POLICY_FLAG_FILTERED | POLICY_FLAG_PASS_TO_USER; + const int32_t policyFlags = + POLICY_FLAG_FILTERED | POLICY_FLAG_PASS_TO_USER | POLICY_FLAG_DISABLE_KEY_REPEAT; InputEventInjectionResult result = mDispatcher->injectInputEvent(&event, /*targetUid=*/{}, @@ -9870,7 +9893,7 @@ protected: PointerCaptureRequest requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, bool enabled) { mDispatcher->requestPointerCapture(window->getToken(), enabled); - auto request = mFakePolicy->assertSetPointerCaptureCalled(enabled); + auto request = mFakePolicy->assertSetPointerCaptureCalled(window, enabled); notifyPointerCaptureChanged(request); window->consumeCaptureEvent(enabled); return request; @@ -9903,7 +9926,7 @@ TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLose mWindow->consumeCaptureEvent(false); mWindow->consumeFocusEvent(false); mSecondWindow->consumeFocusEvent(true); - mFakePolicy->assertSetPointerCaptureCalled(false); + mFakePolicy->assertSetPointerCaptureCalled(mWindow, false); // Ensure that additional state changes from InputReader are not sent to the window. notifyPointerCaptureChanged({}); @@ -9922,7 +9945,7 @@ TEST_F(InputDispatcherPointerCaptureTests, UnexpectedStateChangeDisablesPointerC notifyPointerCaptureChanged(request); // Ensure that Pointer Capture is disabled. - mFakePolicy->assertSetPointerCaptureCalled(false); + mFakePolicy->assertSetPointerCaptureCalled(mWindow, false); mWindow->consumeCaptureEvent(false); mWindow->assertNoEvents(); } @@ -9932,13 +9955,13 @@ TEST_F(InputDispatcherPointerCaptureTests, OutOfOrderRequests) { // The first window loses focus. setFocusedWindow(mSecondWindow); - mFakePolicy->assertSetPointerCaptureCalled(false); + mFakePolicy->assertSetPointerCaptureCalled(mWindow, false); mWindow->consumeCaptureEvent(false); // Request Pointer Capture from the second window before the notification from InputReader // arrives. mDispatcher->requestPointerCapture(mSecondWindow->getToken(), true); - auto request = mFakePolicy->assertSetPointerCaptureCalled(true); + auto request = mFakePolicy->assertSetPointerCaptureCalled(mSecondWindow, true); // InputReader notifies Pointer Capture was disabled (because of the focus change). notifyPointerCaptureChanged({}); @@ -9953,11 +9976,11 @@ TEST_F(InputDispatcherPointerCaptureTests, OutOfOrderRequests) { TEST_F(InputDispatcherPointerCaptureTests, EnableRequestFollowsSequenceNumbers) { // App repeatedly enables and disables capture. mDispatcher->requestPointerCapture(mWindow->getToken(), true); - auto firstRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + auto firstRequest = mFakePolicy->assertSetPointerCaptureCalled(mWindow, true); mDispatcher->requestPointerCapture(mWindow->getToken(), false); - mFakePolicy->assertSetPointerCaptureCalled(false); + mFakePolicy->assertSetPointerCaptureCalled(mWindow, false); mDispatcher->requestPointerCapture(mWindow->getToken(), true); - auto secondRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + auto secondRequest = mFakePolicy->assertSetPointerCaptureCalled(mWindow, true); // InputReader notifies that PointerCapture has been enabled for the first request. Since the // first request is now stale, this should do nothing. @@ -9974,10 +9997,10 @@ TEST_F(InputDispatcherPointerCaptureTests, RapidToggleRequests) { // App toggles pointer capture off and on. mDispatcher->requestPointerCapture(mWindow->getToken(), false); - mFakePolicy->assertSetPointerCaptureCalled(false); + mFakePolicy->assertSetPointerCaptureCalled(mWindow, false); mDispatcher->requestPointerCapture(mWindow->getToken(), true); - auto enableRequest = mFakePolicy->assertSetPointerCaptureCalled(true); + auto enableRequest = mFakePolicy->assertSetPointerCaptureCalled(mWindow, true); // InputReader notifies that the latest "enable" request was processed, while skipping over the // preceding "disable" request. @@ -10029,6 +10052,26 @@ TEST_F(InputDispatcherPointerCaptureTests, MouseHoverAndPointerCapture) { mWindow->assertNoEvents(); } +using InputDispatcherPointerCaptureDeathTest = InputDispatcherPointerCaptureTests; + +TEST_F(InputDispatcherPointerCaptureDeathTest, + NotifyPointerCaptureChangedWithWrongTokenAbortsDispatcher) { + testing::GTEST_FLAG(death_test_style) = "threadsafe"; + ScopedSilentDeath _silentDeath; + + mDispatcher->requestPointerCapture(mWindow->getToken(), true); + auto request = mFakePolicy->assertSetPointerCaptureCalled(mWindow, true); + + // Dispatch a pointer changed event with a wrong token. + request.window = mSecondWindow->getToken(); + ASSERT_DEATH( + { + notifyPointerCaptureChanged(request); + mSecondWindow->consumeCaptureEvent(true); + }, + "Unexpected requested window for Pointer Capture."); +} + class InputDispatcherUntrustedTouchesTest : public InputDispatcherTest { protected: constexpr static const float MAXIMUM_OBSCURING_OPACITY = 0.8; diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 835f8b89c3..1d46c9a1e2 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -1165,18 +1165,18 @@ TEST_F(InputReaderTest, GetKeyCodeState_ForwardsRequestsToSubdeviceMappers) { TEST_F(InputReaderTest, ChangingPointerCaptureNotifiesInputListener) { NotifyPointerCaptureChangedArgs args; - auto request = mFakePolicy->setPointerCapture(true); + auto request = mFakePolicy->setPointerCapture(/*window=*/sp<BBinder>::make()); mReader->requestRefreshConfiguration(InputReaderConfiguration::Change::POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_TRUE(args.request.enable) << "Pointer Capture should be enabled."; + ASSERT_TRUE(args.request.isEnable()) << "Pointer Capture should be enabled."; ASSERT_EQ(args.request, request) << "Pointer Capture sequence number should match."; - mFakePolicy->setPointerCapture(false); + mFakePolicy->setPointerCapture(/*window=*/nullptr); mReader->requestRefreshConfiguration(InputReaderConfiguration::Change::POINTER_CAPTURE); mReader->loopOnce(); mFakeListener->assertNotifyCaptureWasCalled(&args); - ASSERT_FALSE(args.request.enable) << "Pointer Capture should be disabled."; + ASSERT_FALSE(args.request.isEnable()) << "Pointer Capture should be disabled."; // Verify that the Pointer Capture state is not updated when the configuration value // does not change. @@ -9802,7 +9802,7 @@ TEST_F(MultiTouchInputMapperTest, Process_TouchpadCapture) { prepareAxes(POSITION | ID | SLOT); mFakeEventHub->addKey(EVENTHUB_ID, BTN_LEFT, 0, AKEYCODE_UNKNOWN, 0); mFakeEventHub->addKey(EVENTHUB_ID, BTN_TOUCH, 0, AKEYCODE_UNKNOWN, 0); - mFakePolicy->setPointerCapture(true); + mFakePolicy->setPointerCapture(/*window=*/sp<BBinder>::make()); mFakePolicy->setPointerController(fakePointerController); MultiTouchInputMapper& mapper = constructAndAddMapper<MultiTouchInputMapper>(); @@ -9934,7 +9934,7 @@ TEST_F(MultiTouchInputMapperTest, Process_TouchpadCapture) { ASSERT_EQ(AMOTION_EVENT_ACTION_UP, args.action); // non captured touchpad should be a mouse source - mFakePolicy->setPointerCapture(false); + mFakePolicy->setPointerCapture(/*window=*/nullptr); configureDevice(InputReaderConfiguration::Change::POINTER_CAPTURE); ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs)); ASSERT_EQ(AINPUT_SOURCE_MOUSE, mapper.getSources()); @@ -10012,14 +10012,14 @@ TEST_F(MultiTouchInputMapperTest, WhenCapturedAndNotCaptured_GetSources) { prepareAxes(POSITION | ID | SLOT); mFakeEventHub->addKey(EVENTHUB_ID, BTN_LEFT, 0, AKEYCODE_UNKNOWN, 0); mFakePolicy->setPointerController(fakePointerController); - mFakePolicy->setPointerCapture(false); + mFakePolicy->setPointerCapture(/*window=*/nullptr); MultiTouchInputMapper& mapper = constructAndAddMapper<MultiTouchInputMapper>(); // uncaptured touchpad should be a pointer device ASSERT_EQ(AINPUT_SOURCE_MOUSE, mapper.getSources()); // captured touchpad should be a touchpad device - mFakePolicy->setPointerCapture(true); + mFakePolicy->setPointerCapture(/*window=*/sp<BBinder>::make()); configureDevice(InputReaderConfiguration::Change::POINTER_CAPTURE); ASSERT_EQ(AINPUT_SOURCE_TOUCHPAD, mapper.getSources()); } @@ -10090,7 +10090,7 @@ protected: prepareAbsoluteAxisResolution(xAxisResolution, yAxisResolution); // In order to enable swipe and freeform gesture in pointer mode, pointer capture // needs to be disabled, and the pointer gesture needs to be enabled. - mFakePolicy->setPointerCapture(false); + mFakePolicy->setPointerCapture(/*window=*/nullptr); mFakePolicy->setPointerGestureEnabled(true); mFakePolicy->setPointerController(fakePointerController); diff --git a/services/inputflinger/tests/PointerChoreographer_test.cpp b/services/inputflinger/tests/PointerChoreographer_test.cpp index 0b43a3335d..8ddb672cfe 100644 --- a/services/inputflinger/tests/PointerChoreographer_test.cpp +++ b/services/inputflinger/tests/PointerChoreographer_test.cpp @@ -445,7 +445,8 @@ TEST_F(PointerChoreographerTest, DoesNotMovePointerForMouseRelativeSource) { {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_MOUSE_RELATIVE, ADISPLAY_ID_NONE)}}); mChoreographer.notifyPointerCaptureChanged( NotifyPointerCaptureChangedArgs(/*id=*/2, systemTime(SYSTEM_TIME_MONOTONIC), - PointerCaptureRequest(/*enable=*/true, /*seq=*/0))); + PointerCaptureRequest(/*window=*/sp<BBinder>::make(), + /*seq=*/0))); // Notify motion as if pointer capture is enabled. mChoreographer.notifyMotion( @@ -482,7 +483,8 @@ TEST_F(PointerChoreographerTest, WhenPointerCaptureEnabledHidesPointer) { // Enable pointer capture and check if the PointerController hid the pointer. mChoreographer.notifyPointerCaptureChanged( NotifyPointerCaptureChangedArgs(/*id=*/1, systemTime(SYSTEM_TIME_MONOTONIC), - PointerCaptureRequest(/*enable=*/true, /*seq=*/0))); + PointerCaptureRequest(/*window=*/sp<BBinder>::make(), + /*seq=*/0))); ASSERT_FALSE(pc->isPointerShown()); } @@ -1327,7 +1329,8 @@ TEST_F(PointerChoreographerTest, DoesNotMovePointerForTouchpadSource) { // Assume that pointer capture is enabled. mChoreographer.notifyPointerCaptureChanged( NotifyPointerCaptureChangedArgs(/*id=*/1, systemTime(SYSTEM_TIME_MONOTONIC), - PointerCaptureRequest(/*enable=*/true, /*seq=*/0))); + PointerCaptureRequest(/*window=*/sp<BBinder>::make(), + /*seq=*/0))); // Notify motion as if pointer capture is enabled. mChoreographer.notifyMotion(MotionArgsBuilder(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHPAD) @@ -1361,7 +1364,8 @@ TEST_F(PointerChoreographerTest, WhenPointerCaptureEnabledTouchpadHidesPointer) // Enable pointer capture and check if the PointerController hid the pointer. mChoreographer.notifyPointerCaptureChanged( NotifyPointerCaptureChangedArgs(/*id=*/1, systemTime(SYSTEM_TIME_MONOTONIC), - PointerCaptureRequest(/*enable=*/true, /*seq=*/0))); + PointerCaptureRequest(/*window=*/sp<BBinder>::make(), + /*seq=*/0))); ASSERT_FALSE(pc->isPointerShown()); } diff --git a/services/inputflinger/tests/fuzzers/BlockingQueueFuzzer.cpp b/services/inputflinger/tests/fuzzers/BlockingQueueFuzzer.cpp index 219b662ffb..863d0a165e 100644 --- a/services/inputflinger/tests/fuzzers/BlockingQueueFuzzer.cpp +++ b/services/inputflinger/tests/fuzzers/BlockingQueueFuzzer.cpp @@ -15,8 +15,8 @@ */ #include <fuzzer/FuzzedDataProvider.h> +#include <input/BlockingQueue.h> #include <thread> -#include "BlockingQueue.h" // Chosen to be a number large enough for variation in fuzzer runs, but not consume too much memory. static constexpr size_t MAX_CAPACITY = 1024; diff --git a/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp b/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp index c2bf275611..643e8b9f97 100644 --- a/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp +++ b/services/inputflinger/tests/fuzzers/TouchpadInputFuzzer.cpp @@ -125,6 +125,9 @@ void setTouchpadSettings(ThreadSafeFuzzedDataProvider& fdp, InputReaderConfigura config.touchpadTapToClickEnabled = fdp.ConsumeBool(); config.touchpadTapDraggingEnabled = fdp.ConsumeBool(); config.touchpadRightClickZoneEnabled = fdp.ConsumeBool(); + + config.pointerCaptureRequest.window = fdp.ConsumeBool() ? sp<BBinder>::make() : nullptr; + config.pointerCaptureRequest.seq = fdp.ConsumeIntegral<uint32_t>(); } } // namespace @@ -145,7 +148,6 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t* data, size_t size) { // Some settings are fuzzed here, as well as in the main loop, to provide randomized data to the // TouchpadInputMapper constructor. setTouchpadSettings(*fdp, policyConfig); - policyConfig.pointerCaptureRequest.enable = fdp->ConsumeBool(); TouchpadInputMapper& mapper = getMapperForDevice<ThreadSafeFuzzedDataProvider, TouchpadInputMapper>(*fdp, device, policyConfig); @@ -164,7 +166,6 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t* data, size_t size) { [&]() -> void { mapper.getSources(); }, [&]() -> void { setTouchpadSettings(*fdp, policyConfig); - policyConfig.pointerCaptureRequest.enable = fdp->ConsumeBool(); std::list<NotifyArgs> unused = mapper.reconfigure(fdp->ConsumeIntegral<nsecs_t>(), policyConfig, InputReaderConfiguration::Change( diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index d77180362b..dc69b819c8 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -83,11 +83,11 @@ cc_defaults { "libprotobuf-cpp-lite", "libsync", "libui", - "libinput", "libutils", "libSurfaceFlingerProp", ], static_libs: [ + "iinputflinger_aidl_lib_static", "libaidlcommonsupport", "libcompositionengine", "libframetimeline", @@ -210,7 +210,6 @@ filegroup { "Scheduler/VsyncModulator.cpp", "Scheduler/VsyncSchedule.cpp", "ScreenCaptureOutput.cpp", - "StartPropertySetThread.cpp", "SurfaceFlinger.cpp", "SurfaceFlingerDefaultFactory.cpp", "Tracing/LayerDataSource.cpp", diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index 18a96f4de7..843b5c5c82 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -19,6 +19,7 @@ #include <chrono> #include <optional> #include <vector> +#include "utils/Timers.h" #include <compositionengine/Display.h> #include <compositionengine/LayerFE.h> @@ -105,6 +106,9 @@ struct CompositionRefreshArgs { bool hasTrustedPresentationListener = false; ICEPowerCallback* powerCallback = nullptr; + + // System time for when frame refresh starts. Used for stats. + nsecs_t refreshStartTime = 0; }; } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h index a1d61323a8..a499928dd0 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h @@ -58,8 +58,7 @@ public: // Called before composition starts. Should return true if this layer has // pending updates which would require an extra display refresh cycle to // process. - virtual bool onPreComposition(nsecs_t refreshStartTime, - bool updatingOutputGeometryThisFrame) = 0; + virtual bool onPreComposition(bool updatingOutputGeometryThisFrame) = 0; struct ClientCompositionTargetSettings { enum class BlurSetting { diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h index 15e4577ae0..1b8cc2758f 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h @@ -43,7 +43,7 @@ public: MOCK_CONST_METHOD0(getCompositionState, const LayerFECompositionState*()); - MOCK_METHOD2(onPreComposition, bool(nsecs_t, bool)); + MOCK_METHOD1(onPreComposition, bool(bool)); MOCK_CONST_METHOD1(prepareClientComposition, std::optional<compositionengine::LayerFE::LayerSettings>( diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp index d87eae3812..b4702085b6 100644 --- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp +++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp @@ -181,10 +181,10 @@ void CompositionEngine::preComposition(CompositionRefreshArgs& args) { bool needsAnotherUpdate = false; - mRefreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC); + mRefreshStartTime = args.refreshStartTime; for (auto& layer : args.layers) { - if (layer->onPreComposition(mRefreshStartTime, args.updatingOutputGeometryThisFrame)) { + if (layer->onPreComposition(args.updatingOutputGeometryThisFrame)) { needsAnotherUpdate = true; } } diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp index da578e2046..042010edd5 100644 --- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp @@ -214,6 +214,7 @@ struct CompositionTestPreComposition : public CompositionEngineTest { TEST_F(CompositionTestPreComposition, preCompositionSetsFrameTimestamp) { const nsecs_t before = systemTime(SYSTEM_TIME_MONOTONIC); + mRefreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC); mEngine.preComposition(mRefreshArgs); const nsecs_t after = systemTime(SYSTEM_TIME_MONOTONIC); @@ -226,12 +227,9 @@ TEST_F(CompositionTestPreComposition, preCompositionInvokesLayerPreCompositionWi nsecs_t ts1 = 0; nsecs_t ts2 = 0; nsecs_t ts3 = 0; - EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)) - .WillOnce(DoAll(SaveArg<0>(&ts1), Return(false))); - EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)) - .WillOnce(DoAll(SaveArg<0>(&ts2), Return(false))); - EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)) - .WillOnce(DoAll(SaveArg<0>(&ts3), Return(false))); + EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts1), Return(false))); + EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts2), Return(false))); + EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts3), Return(false))); mRefreshArgs.outputs = {mOutput1}; mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE}; @@ -245,9 +243,9 @@ TEST_F(CompositionTestPreComposition, preCompositionInvokesLayerPreCompositionWi } TEST_F(CompositionTestPreComposition, preCompositionDefaultsToNoUpdateNeeded) { - EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(false)); - EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false)); - EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(false)); + EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false)); + EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false)); mEngine.setNeedsAnotherUpdateForTest(true); @@ -262,9 +260,9 @@ TEST_F(CompositionTestPreComposition, preCompositionDefaultsToNoUpdateNeeded) { TEST_F(CompositionTestPreComposition, preCompositionSetsNeedsAnotherUpdateIfAtLeastOneLayerRequestsIt) { - EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false)); - EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false)); + EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(true)); + EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false)); + EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false)); mRefreshArgs.outputs = {mOutput1}; mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE}; diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp index 8dfbeb80cd..faa51972e9 100644 --- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp +++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp @@ -48,6 +48,7 @@ namespace impl { using aidl::android::hardware::power::Boost; using aidl::android::hardware::power::Mode; using aidl::android::hardware::power::SessionHint; +using aidl::android::hardware::power::SessionTag; using aidl::android::hardware::power::WorkDuration; PowerAdvisor::~PowerAdvisor() = default; @@ -204,13 +205,36 @@ bool PowerAdvisor::supportsPowerHintSession() { return *mSupportsHintSession; } +bool PowerAdvisor::shouldCreateSessionWithConfig() { + return mSessionConfigSupported && FlagManager::getInstance().adpf_use_fmq_channel(); +} + bool PowerAdvisor::ensurePowerHintSessionRunning() { if (mHintSession == nullptr && !mHintSessionThreadIds.empty() && usePowerHintSession()) { - auto ret = getPowerHal().createHintSession(getpid(), static_cast<int32_t>(getuid()), - mHintSessionThreadIds, mTargetDuration.ns()); - - if (ret.isOk()) { - mHintSession = ret.value(); + if (shouldCreateSessionWithConfig()) { + auto ret = getPowerHal().createHintSessionWithConfig(getpid(), + static_cast<int32_t>(getuid()), + mHintSessionThreadIds, + mTargetDuration.ns(), + SessionTag::SURFACEFLINGER, + &mSessionConfig); + if (ret.isOk()) { + mHintSession = ret.value(); + } + // If it fails the first time we try, or ever returns unsupported, assume unsupported + else if (mFirstConfigSupportCheck || ret.isUnsupported()) { + ALOGI("Hint session with config is unsupported, falling back to a legacy session"); + mSessionConfigSupported = false; + } + mFirstConfigSupportCheck = false; + } + // Immediately try original method after, in case the first way returned unsupported + if (mHintSession == nullptr && !shouldCreateSessionWithConfig()) { + auto ret = getPowerHal().createHintSession(getpid(), static_cast<int32_t>(getuid()), + mHintSessionThreadIds, mTargetDuration.ns()); + if (ret.isOk()) { + mHintSession = ret.value(); + } } } return mHintSession != nullptr; diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h index 1040048b13..13e1263369 100644 --- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.h +++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.h @@ -230,6 +230,9 @@ private: // this normalizes them together and takes the max of the two Duration combineTimingEstimates(Duration totalDuration, Duration flingerDuration); + // Whether to use the new "createHintSessionWithConfig" method + bool shouldCreateSessionWithConfig() REQUIRES(mHintSessionMutex); + bool ensurePowerHintSessionRunning() REQUIRES(mHintSessionMutex); std::unordered_map<DisplayId, DisplayTimingData> mDisplayTimingData; @@ -278,6 +281,13 @@ private: std::promise<bool> mDelayReportActualMutexAcquisitonPromise; bool mTimingTestingMode = false; + // Hint session configuration data + aidl::android::hardware::power::SessionConfig mSessionConfig; + + // Whether createHintSessionWithConfig is supported, assume true until it fails + bool mSessionConfigSupported = true; + bool mFirstConfigSupportCheck = true; + // Whether we should emit ATRACE_INT data for hint sessions static const bool sTraceHintSessionData; diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index cb0e2a1938..867f3af4b1 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -163,7 +163,9 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta LLOGV(layerId, "requested=%" PRIu64 "flags=%" PRIu64, clientState.what, clientChanges); if (clientState.what & layer_state_t::eFlagsChanged) { - if ((oldFlags ^ flags) & (layer_state_t::eLayerHidden | layer_state_t::eLayerOpaque)) { + if ((oldFlags ^ flags) & + (layer_state_t::eLayerHidden | layer_state_t::eLayerOpaque | + layer_state_t::eLayerSecure)) { changes |= RequestedLayerState::Changes::Visibility | RequestedLayerState::Changes::VisibleRegion; } diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp index 2dbcb841ac..43a4397899 100644 --- a/services/surfaceflinger/LayerFE.cpp +++ b/services/surfaceflinger/LayerFE.cpp @@ -82,8 +82,7 @@ const compositionengine::LayerFECompositionState* LayerFE::getCompositionState() return mSnapshot.get(); } -bool LayerFE::onPreComposition(nsecs_t refreshStartTime, bool) { - mCompositionResult.refreshStartTime = refreshStartTime; +bool LayerFE::onPreComposition(bool) { return mSnapshot->hasReadyFrame; } diff --git a/services/surfaceflinger/LayerFE.h b/services/surfaceflinger/LayerFE.h index d584fb7eab..66cb88b20e 100644 --- a/services/surfaceflinger/LayerFE.h +++ b/services/surfaceflinger/LayerFE.h @@ -26,9 +26,6 @@ namespace android { struct CompositionResult { - // TODO(b/238781169) update CE to no longer pass refreshStartTime to LayerFE::onPreComposition - // and remove this field. - nsecs_t refreshStartTime = 0; std::vector<std::pair<ftl::SharedFuture<FenceResult>, ui::LayerStack>> releaseFences; sp<Fence> lastClientCompositionFence = nullptr; }; @@ -39,7 +36,7 @@ public: // compositionengine::LayerFE overrides const compositionengine::LayerFECompositionState* getCompositionState() const override; - bool onPreComposition(nsecs_t refreshStartTime, bool updatingOutputGeometryThisFrame) override; + bool onPreComposition(bool updatingOutputGeometryThisFrame) override; void onLayerDisplayed(ftl::SharedFuture<FenceResult>, ui::LayerStack) override; const char* getDebugName() const override; int32_t getSequence() const override; diff --git a/services/surfaceflinger/StartPropertySetThread.cpp b/services/surfaceflinger/StartPropertySetThread.cpp deleted file mode 100644 index f42cd53e0a..0000000000 --- a/services/surfaceflinger/StartPropertySetThread.cpp +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (C) 2017 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. - */ - -#include <cutils/properties.h> -#include "StartPropertySetThread.h" - -namespace android { - -StartPropertySetThread::StartPropertySetThread(bool timestampPropertyValue): - Thread(false), mTimestampPropertyValue(timestampPropertyValue) {} - -status_t StartPropertySetThread::Start() { - return run("SurfaceFlinger::StartPropertySetThread", PRIORITY_NORMAL); -} - -bool StartPropertySetThread::threadLoop() { - // Set property service.sf.present_timestamp, consumer need check its readiness - property_set(kTimestampProperty, mTimestampPropertyValue ? "1" : "0"); - // Clear BootAnimation exit flag - property_set("service.bootanim.exit", "0"); - property_set("service.bootanim.progress", "0"); - // Start BootAnimation if not started - property_set("ctl.start", "bootanim"); - // Exit immediately - return false; -} - -} // namespace android diff --git a/services/surfaceflinger/StartPropertySetThread.h b/services/surfaceflinger/StartPropertySetThread.h deleted file mode 100644 index bbdcde2809..0000000000 --- a/services/surfaceflinger/StartPropertySetThread.h +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (C) 2017 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_STARTBOOTANIMTHREAD_H -#define ANDROID_STARTBOOTANIMTHREAD_H - -#include <stddef.h> - -#include <utils/Mutex.h> -#include <utils/Thread.h> - -namespace android { - -class StartPropertySetThread : public Thread { -// Boot animation is triggered via calls to "property_set()" which can block -// if init's executing slow operation such as 'mount_all --late' (currently -// happening 1/10th with fsck) concurrently. Running in a separate thread -// allows to pursue the SurfaceFlinger's init process without blocking. -// see b/34499826. -// Any property_set() will block during init stage so need to be offloaded -// to this thread. see b/63844978. -public: - explicit StartPropertySetThread(bool timestampPropertyValue); - status_t Start(); -private: - virtual bool threadLoop(); - static constexpr const char* kTimestampProperty = "service.sf.present_timestamp"; - const bool mTimestampPropertyValue; -}; - -} - -#endif // ANDROID_STARTBOOTANIMTHREAD_H diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index b3adedd4e6..bf210afe6d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -153,7 +153,6 @@ #include "Scheduler/VsyncConfiguration.h" #include "Scheduler/VsyncModulator.h" #include "ScreenCaptureOutput.h" -#include "StartPropertySetThread.h" #include "SurfaceFlingerProperties.h" #include "TimeStats/TimeStats.h" #include "TunnelModeEnabledReporter.h" @@ -566,7 +565,11 @@ void SurfaceFlinger::binderDied(const wp<IBinder>&) { initializeDisplays(); })); - startBootAnim(); + mInitBootPropsFuture.callOnce([this] { + return std::async(std::launch::async, &SurfaceFlinger::initBootProperties, this); + }); + + mInitBootPropsFuture.wait(); } void SurfaceFlinger::run() { @@ -722,13 +725,10 @@ void SurfaceFlinger::bootFinished() { } mBootFinished = true; FlagManager::getMutableInstance().markBootCompleted(); - if (mStartPropertySetThread->join() != NO_ERROR) { - ALOGE("Join StartPropertySetThread failed!"); - } - if (mRenderEnginePrimeCacheFuture.valid()) { - mRenderEnginePrimeCacheFuture.get(); - } + mInitBootPropsFuture.wait(); + mRenderEnginePrimeCacheFuture.wait(); + const nsecs_t now = systemTime(); const nsecs_t duration = now - mBootTime; ALOGI("Boot is finished (%ld ms)", long(ns2ms(duration)) ); @@ -816,8 +816,6 @@ void chooseRenderEngineType(renderengine::RenderEngineCreationArgs::Builder& bui } } -// Do not call property_set on main thread which will be blocked by init -// Use StartPropertySetThread instead. void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { ATRACE_CALL(); ALOGI( "SurfaceFlinger's main thread ready to run. " @@ -919,27 +917,40 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { ALOGW("Can't set SCHED_OTHER for primeCache"); } - bool shouldPrimeUltraHDR = - base::GetBoolProperty("ro.surface_flinger.prime_shader_cache.ultrahdr"s, false); - mRenderEnginePrimeCacheFuture = getRenderEngine().primeCache(shouldPrimeUltraHDR); + mRenderEnginePrimeCacheFuture.callOnce([this] { + const bool shouldPrimeUltraHDR = + base::GetBoolProperty("ro.surface_flinger.prime_shader_cache.ultrahdr"s, false); + return getRenderEngine().primeCache(shouldPrimeUltraHDR); + }); if (setSchedFifo(true) != NO_ERROR) { ALOGW("Can't set SCHED_FIFO after primeCache"); } } - // Inform native graphics APIs whether the present timestamp is supported: - - mStartPropertySetThread = getFactory().createStartPropertySetThread(mHasReliablePresentFences); - - if (mStartPropertySetThread->Start() != NO_ERROR) { - ALOGE("Run StartPropertySetThread failed!"); - } + // Avoid blocking the main thread on `init` to set properties. + mInitBootPropsFuture.callOnce([this] { + return std::async(std::launch::async, &SurfaceFlinger::initBootProperties, this); + }); initTransactionTraceWriter(); ALOGV("Done initializing"); } +// During boot, offload `initBootProperties` to another thread. `property_set` depends on +// `property_service`, which may be delayed by slow operations like `mount_all --late` in +// the `init` process. See b/34499826 and b/63844978. +void SurfaceFlinger::initBootProperties() { + property_set("service.sf.present_timestamp", mHasReliablePresentFences ? "1" : "0"); + + if (base::GetBoolProperty("debug.sf.boot_animation"s, true)) { + // Reset and (if needed) start BootAnimation. + property_set("service.bootanim.exit", "0"); + property_set("service.bootanim.progress", "0"); + property_set("ctl.start", "bootanim"); + } +} + void SurfaceFlinger::initTransactionTraceWriter() { if (!mTransactionTracing) { return; @@ -979,18 +990,6 @@ void SurfaceFlinger::readPersistentProperties() { static_cast<ui::ColorMode>(base::GetIntProperty("persist.sys.sf.color_mode"s, 0)); } -void SurfaceFlinger::startBootAnim() { - // Start boot animation service by setting a property mailbox - // if property setting thread is already running, Start() will be just a NOP - mStartPropertySetThread->Start(); - // Wait until property was set - if (mStartPropertySetThread->join() != NO_ERROR) { - ALOGE("Join StartPropertySetThread failed!"); - } -} - -// ---------------------------------------------------------------------------- - status_t SurfaceFlinger::getSupportedFrameTimestamps( std::vector<FrameEvent>* outSupported) const { *outSupported = { @@ -2784,12 +2783,16 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( } } + refreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC); + for (auto [layer, layerFE] : layers) { + layer->onPreComposition(refreshArgs.refreshStartTime); + } + mCompositionEngine->present(refreshArgs); moveSnapshotsFromCompositionArgs(refreshArgs, layers); for (auto [layer, layerFE] : layers) { CompositionResult compositionResult{layerFE->stealCompositionResult()}; - layer->onPreComposition(compositionResult.refreshStartTime); for (auto& [releaseFence, layerStack] : compositionResult.releaseFences) { Layer* clonedFrom = layer->getClonedFrom().get(); auto owningLayer = clonedFrom ? clonedFrom : layer; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 005f2e6344..0cc8fbb98a 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -89,6 +89,7 @@ #include "Tracing/TransactionTracing.h" #include "TransactionCallbackInvoker.h" #include "TransactionState.h" +#include "Utils/OnceFuture.h" #include <atomic> #include <cstdint> @@ -550,7 +551,7 @@ private: bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId, const std::vector<uint64_t>& mergedTransactionIds) override; void bootFinished(); - virtual status_t getSupportedFrameTimestamps(std::vector<FrameEvent>* outSupported) const; + status_t getSupportedFrameTimestamps(std::vector<FrameEvent>* outSupported) const; sp<IDisplayEventConnection> createDisplayEventConnection( gui::ISurfaceComposer::VsyncSource vsyncSource = gui::ISurfaceComposer::VsyncSource::eVsyncSourceApp, @@ -871,9 +872,6 @@ private: // Traverse through all the layers and compute and cache its bounds. void computeLayerBounds(); - // Boot animation, on/off animations and screen capture - void startBootAnim(); - bool layersHasProtectedLayer(const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) const; void captureScreenCommon(RenderAreaFuture, GetLayerSnapshotsFunction, ui::Size bufferSize, @@ -1184,11 +1182,17 @@ private: ui::Rotation getPhysicalDisplayOrientation(DisplayId, bool isPrimary) const REQUIRES(mStateLock); void traverseLegacyLayers(const LayerVector::Visitor& visitor) const; + + void initBootProperties(); void initTransactionTraceWriter(); - sp<StartPropertySetThread> mStartPropertySetThread; + surfaceflinger::Factory& mFactory; pid_t mPid; - std::future<void> mRenderEnginePrimeCacheFuture; + + // TODO: b/328459745 - Encapsulate in a SystemProperties object. + utils::OnceFuture mInitBootPropsFuture; + + utils::OnceFuture mRenderEnginePrimeCacheFuture; // mStateLock has conventions related to the current thread, because only // the main thread should modify variables protected by mStateLock. diff --git a/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp b/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp index 7e6894d3f7..50b167d5d7 100644 --- a/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp +++ b/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp @@ -26,7 +26,6 @@ #include "FrameTracer/FrameTracer.h" #include "Layer.h" #include "NativeWindowSurface.h" -#include "StartPropertySetThread.h" #include "SurfaceFlingerDefaultFactory.h" #include "SurfaceFlingerProperties.h" @@ -53,11 +52,6 @@ std::unique_ptr<scheduler::VsyncConfiguration> DefaultFactory::createVsyncConfig } } -sp<StartPropertySetThread> DefaultFactory::createStartPropertySetThread( - bool timestampPropertyValue) { - return sp<StartPropertySetThread>::make(timestampPropertyValue); -} - sp<DisplayDevice> DefaultFactory::createDisplayDevice(DisplayDeviceCreationArgs& creationArgs) { return sp<DisplayDevice>::make(creationArgs); } diff --git a/services/surfaceflinger/SurfaceFlingerDefaultFactory.h b/services/surfaceflinger/SurfaceFlingerDefaultFactory.h index 2c6de0e113..540dec832e 100644 --- a/services/surfaceflinger/SurfaceFlingerDefaultFactory.h +++ b/services/surfaceflinger/SurfaceFlingerDefaultFactory.h @@ -29,7 +29,6 @@ public: std::unique_ptr<HWComposer> createHWComposer(const std::string& serviceName) override; std::unique_ptr<scheduler::VsyncConfiguration> createVsyncConfiguration( Fps currentRefreshRate) override; - sp<StartPropertySetThread> createStartPropertySetThread(bool timestampPropertyValue) override; sp<DisplayDevice> createDisplayDevice(DisplayDeviceCreationArgs&) override; sp<GraphicBuffer> createGraphicBuffer(uint32_t width, uint32_t height, PixelFormat format, uint32_t layerCount, uint64_t usage, diff --git a/services/surfaceflinger/SurfaceFlingerFactory.h b/services/surfaceflinger/SurfaceFlingerFactory.h index f310c4ac53..f1fbf013c7 100644 --- a/services/surfaceflinger/SurfaceFlingerFactory.h +++ b/services/surfaceflinger/SurfaceFlingerFactory.h @@ -39,7 +39,6 @@ class IGraphicBufferConsumer; class IGraphicBufferProducer; class Layer; class LayerFE; -class StartPropertySetThread; class SurfaceFlinger; class TimeStats; @@ -71,8 +70,6 @@ public: virtual std::unique_ptr<scheduler::VsyncConfiguration> createVsyncConfiguration( Fps currentRefreshRate) = 0; - virtual sp<StartPropertySetThread> createStartPropertySetThread( - bool timestampPropertyValue) = 0; virtual sp<DisplayDevice> createDisplayDevice(DisplayDeviceCreationArgs&) = 0; virtual sp<GraphicBuffer> createGraphicBuffer(uint32_t width, uint32_t height, PixelFormat format, uint32_t layerCount, diff --git a/services/surfaceflinger/Utils/OnceFuture.h b/services/surfaceflinger/Utils/OnceFuture.h new file mode 100644 index 0000000000..412038ce10 --- /dev/null +++ b/services/surfaceflinger/Utils/OnceFuture.h @@ -0,0 +1,53 @@ +/* + * Copyright 2024 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 <future> +#include <mutex> + +#include <android-base/thread_annotations.h> + +namespace android::utils { + +// Allows a thread to `wait` for a future produced by a different thread. The future is returned by +// the first call to a function `F` that multiple threads may `callOnce`. If no `callOnce` happens, +// then `wait` does nothing. Otherwise, it blocks on the future, then destroys it, which resets the +// `OnceFuture`. +class OnceFuture { +public: + template <typename F> + void callOnce(F f) { + std::lock_guard lock(mMutex); + if (!mFuture.valid()) { + mFuture = f(); + } + } + + void wait() { + std::lock_guard lock(mMutex); + if (mFuture.valid()) { + mFuture.wait(); + mFuture = {}; + } + } + +private: + std::mutex mMutex; + std::future<void> mFuture GUARDED_BY(mMutex); +}; + +} // namespace android::utils diff --git a/services/surfaceflinger/common/include/common/test/FlagUtils.h b/services/surfaceflinger/common/include/common/test/FlagUtils.h index 550c70d98f..d61fcb5438 100644 --- a/services/surfaceflinger/common/include/common/test/FlagUtils.h +++ b/services/surfaceflinger/common/include/common/test/FlagUtils.h @@ -18,7 +18,10 @@ #include <common/FlagManager.h> -#define SET_FLAG_FOR_TEST(name, value) TestFlagSetter _testflag_((name), (name), (value)) +#define SET_FLAG_FOR_TEST(name, value) \ + TestFlagSetter _testflag_ { \ + (name), (name), (value) \ + } namespace android { class TestFlagSetter { diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp index aecfcba6e6..867ff55632 100644 --- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp @@ -560,4 +560,36 @@ TEST_F(LayerLifecycleManagerTest, alphaChangesAlwaysSetsVisibleRegionFlag) { ftl::Flags<RequestedLayerState::Changes>().string()); } +TEST_F(LayerLifecycleManagerTest, layerSecureChangesSetsVisibilityChangeFlag) { + // add a default buffer and make the layer secure + setFlags(1, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure); + setBuffer(1, + std::make_shared<renderengine::mock:: + FakeExternalTexture>(1U /*width*/, 1U /*height*/, + 1ULL /* bufferId */, + HAL_PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_READ_NEVER /*usage*/)); + + mLifecycleManager.commitChanges(); + + // set new buffer but layer secure doesn't change + setBuffer(1, + std::make_shared<renderengine::mock:: + FakeExternalTexture>(1U /*width*/, 1U /*height*/, + 2ULL /* bufferId */, + HAL_PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_READ_NEVER /*usage*/)); + EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), + ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | + RequestedLayerState::Changes::Content) + .get()); + mLifecycleManager.commitChanges(); + + // change layer flags and confirm visibility flag is set + setFlags(1, layer_state_t::eLayerSecure, 0); + EXPECT_TRUE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); +} + } // namespace android::surfaceflinger::frontend diff --git a/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp b/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp index 1d44a3ef77..d9343c7813 100644 --- a/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp +++ b/services/surfaceflinger/tests/unittests/PowerAdvisorTest.cpp @@ -18,7 +18,9 @@ #define LOG_TAG "PowerAdvisorTest" #include <DisplayHardware/PowerAdvisor.h> +#include <android_os.h> #include <binder/Status.h> +#include <common/test/FlagUtils.h> #include <gmock/gmock.h> #include <gtest/gtest.h> #include <powermanager/PowerHalWrapper.h> @@ -55,6 +57,7 @@ protected: std::unique_ptr<PowerAdvisor> mPowerAdvisor; MockPowerHalController* mMockPowerHalController; std::shared_ptr<MockPowerHintSessionWrapper> mMockPowerHintSession; + SET_FLAG_FOR_TEST(android::os::adpf_use_fmq_channel, true); }; bool PowerAdvisorTest::sessionExists() { @@ -75,13 +78,14 @@ void PowerAdvisorTest::SetUp() { void PowerAdvisorTest::startPowerHintSession(bool returnValidSession) { mMockPowerHintSession = std::make_shared<NiceMock<MockPowerHintSessionWrapper>>(); if (returnValidSession) { - ON_CALL(*mMockPowerHalController, createHintSession) - .WillByDefault([&](int32_t, int32_t, const std::vector<int32_t>&, int64_t) { - return HalResult<std::shared_ptr<PowerHintSessionWrapper>>:: - fromStatus(ndk::ScopedAStatus::ok(), mMockPowerHintSession); - }); + ON_CALL(*mMockPowerHalController, createHintSessionWithConfig) + .WillByDefault(DoAll(SetArgPointee<5>(aidl::android::hardware::power::SessionConfig{ + .id = 12}), + Return(HalResult<std::shared_ptr<PowerHintSessionWrapper>>:: + fromStatus(binder::Status::ok(), + mMockPowerHintSession)))); } else { - ON_CALL(*mMockPowerHalController, createHintSession).WillByDefault([] { + ON_CALL(*mMockPowerHalController, createHintSessionWithConfig).WillByDefault([] { return HalResult< std::shared_ptr<PowerHintSessionWrapper>>::fromStatus(ndk::ScopedAStatus::ok(), nullptr); @@ -287,7 +291,7 @@ TEST_F(PowerAdvisorTest, hintSessionValidWhenNullFromPowerHAL) { } TEST_F(PowerAdvisorTest, hintSessionOnlyCreatedOnce) { - EXPECT_CALL(*mMockPowerHalController, createHintSession(_, _, _, _)).Times(1); + EXPECT_CALL(*mMockPowerHalController, createHintSessionWithConfig(_, _, _, _, _, _)).Times(1); mPowerAdvisor->onBootFinished(); startPowerHintSession(); mPowerAdvisor->startPowerHintSession({1, 2, 3}); @@ -339,7 +343,7 @@ TEST_F(PowerAdvisorTest, hintSessionTestNotifyReportRace) { return HalResult<void>::fromStatus(ndk::ScopedAStatus::fromExceptionCode(-127)); }); - ON_CALL(*mMockPowerHalController, createHintSession).WillByDefault([] { + ON_CALL(*mMockPowerHalController, createHintSessionWithConfig).WillByDefault([] { return HalResult<std::shared_ptr<PowerHintSessionWrapper>>:: fromStatus(ndk::ScopedAStatus::fromExceptionCode(-127), nullptr); }); @@ -374,5 +378,17 @@ TEST_F(PowerAdvisorTest, hintSessionTestNotifyReportRace) { EXPECT_EQ(sessionExists(), false); } +TEST_F(PowerAdvisorTest, legacyHintSessionCreationStillWorks) { + SET_FLAG_FOR_TEST(android::os::adpf_use_fmq_channel, false); + mPowerAdvisor->onBootFinished(); + mMockPowerHintSession = std::make_shared<NiceMock<MockPowerHintSessionWrapper>>(); + EXPECT_CALL(*mMockPowerHalController, createHintSession) + .Times(1) + .WillOnce(Return(HalResult<std::shared_ptr<PowerHintSessionWrapper>>:: + fromStatus(binder::Status::ok(), mMockPowerHintSession))); + mPowerAdvisor->enablePowerHintSession(true); + mPowerAdvisor->startPowerHintSession({1, 2, 3}); +} + } // namespace } // namespace android::Hwc2::impl diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index bce7729d80..82023b092a 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -43,7 +43,6 @@ #include "RenderArea.h" #include "Scheduler/MessageQueue.h" #include "Scheduler/RefreshRateSelector.h" -#include "StartPropertySetThread.h" #include "SurfaceFlinger.h" #include "TestableScheduler.h" #include "mock/DisplayHardware/MockComposer.h" @@ -94,10 +93,6 @@ public: return std::make_unique<scheduler::FakePhaseOffsets>(); } - sp<StartPropertySetThread> createStartPropertySetThread(bool timestampPropertyValue) override { - return sp<StartPropertySetThread>::make(timestampPropertyValue); - } - sp<DisplayDevice> createDisplayDevice(DisplayDeviceCreationArgs& creationArgs) override { return sp<DisplayDevice>::make(creationArgs); } |