Merge changes from topic "reland-input-tracing-tests" into main

* changes:
  Reland "Increase the test timeout used when waiting for events to be traced"
  InputDispatcher_test: Fix flakiness when verifying traced events
  Reland "InputDispatcher_test: Verify all consumed events are traced"

Author: prabirmsp

services/inputflinger/tests/Android.bp

@@ -45,6 +45,7 @@ cc_test {
         "EventHub_test.cpp",
         "FakeEventHub.cpp",
         "FakeInputReaderPolicy.cpp",
+        "FakeInputTracingBackend.cpp",
         "FakePointerController.cpp",
         "FocusResolver_test.cpp",
         "GestureConverter_test.cpp",

services/inputflinger/tests/FakeInputTracingBackend.cpp

@@ -0,0 +1,111 @@
+/*
+ * 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.
+ */
+
+#include "FakeInputTracingBackend.h"
+
+#include <android-base/logging.h>
+#include <utils/Errors.h>
+
+namespace android::inputdispatcher {
+
+namespace {
+
+// Use a larger timeout while waiting for events to be traced, compared to the timeout used while
+// waiting to receive events through the input channel. Events are traced from a separate thread,
+// which does not have the same high thread priority as the InputDispatcher's thread, so the tracer
+// is expected to lag behind the Dispatcher at times.
+constexpr auto TRACE_TIMEOUT = std::chrono::seconds(5);
+
+base::ResultError<> error(const std::ostringstream& ss) {
+    return base::ResultError(ss.str(), BAD_VALUE);
+}
+
+} // namespace
+
+// --- VerifyingTrace ---
+
+void VerifyingTrace::expectKeyDispatchTraced(const KeyEvent& event) {
+    std::scoped_lock lock(mLock);
+    mExpectedEvents.emplace_back(event);
+}
+
+void VerifyingTrace::expectMotionDispatchTraced(const MotionEvent& event) {
+    std::scoped_lock lock(mLock);
+    mExpectedEvents.emplace_back(event);
+}
+
+void VerifyingTrace::verifyExpectedEventsTraced() {
+    std::unique_lock lock(mLock);
+    base::ScopedLockAssertion assumeLocked(mLock);
+
+    base::Result<void> result;
+    mEventTracedCondition.wait_for(lock, TRACE_TIMEOUT, [&]() REQUIRES(mLock) {
+        for (const auto& expectedEvent : mExpectedEvents) {
+            std::visit([&](const auto& event)
+                               REQUIRES(mLock) { result = verifyEventTraced(event); },
+                       expectedEvent);
+            if (!result.ok()) {
+                return false;
+            }
+        }
+        return true;
+    });
+
+    EXPECT_TRUE(result.ok())
+            << "Timed out waiting for all expected events to be traced successfully: "
+            << result.error().message();
+}
+
+void VerifyingTrace::reset() {
+    std::scoped_lock lock(mLock);
+    mTracedEvents.clear();
+    mExpectedEvents.clear();
+}
+
+template <typename Event>
+base::Result<void> VerifyingTrace::verifyEventTraced(const Event& expectedEvent) const {
+    std::ostringstream msg;
+
+    auto tracedEventsIt = mTracedEvents.find(expectedEvent.getId());
+    if (tracedEventsIt == mTracedEvents.end()) {
+        msg << "Expected event with ID 0x" << std::hex << expectedEvent.getId()
+            << " to be traced, but it was not.\n"
+            << "Expected event: " << expectedEvent;
+        return error(msg);
+    }
+
+    return {};
+}
+
+// --- FakeInputTracingBackend ---
+
+void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) const {
+    {
+        std::scoped_lock lock(mTrace->mLock);
+        mTrace->mTracedEvents.emplace(event.id);
+    }
+    mTrace->mEventTracedCondition.notify_all();
+}
+
+void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& event) const {
+    {
+        std::scoped_lock lock(mTrace->mLock);
+        mTrace->mTracedEvents.emplace(event.id);
+    }
+    mTrace->mEventTracedCondition.notify_all();
+}
+
+} // namespace android::inputdispatcher

services/inputflinger/tests/FakeInputTracingBackend.h

@@ -0,0 +1,88 @@
+/*
+ * 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 "../dispatcher/trace/InputTracingBackendInterface.h"
+
+#include <android-base/result.h>
+#include <android-base/thread_annotations.h>
+#include <gtest/gtest.h>
+#include <input/Input.h>
+#include <condition_variable>
+#include <memory>
+#include <mutex>
+#include <unordered_set>
+#include <vector>
+
+namespace android::inputdispatcher {
+
+/**
+ * A class representing an input trace, used to make assertions on what was traced by
+ * InputDispatcher in tests. This class is thread-safe.
+ */
+class VerifyingTrace {
+public:
+    VerifyingTrace() = default;
+
+    /** Add an expectation for a key event to be traced. */
+    void expectKeyDispatchTraced(const KeyEvent& event);
+
+    /** Add an expectation for a motion event to be traced. */
+    void expectMotionDispatchTraced(const MotionEvent& event);
+
+    /**
+     * Wait and verify that all expected events are traced.
+     * This is a lenient verifier that does not expect the events to be traced in the order
+     * that the events were expected, and does not fail if there are events that are traced that
+     * were not expected. Verifying does not clear the expectations.
+     */
+    void verifyExpectedEventsTraced();
+
+    /** Reset the trace and clear all expectations. */
+    void reset();
+
+private:
+    std::mutex mLock;
+    std::condition_variable mEventTracedCondition;
+    std::unordered_set<uint32_t /*eventId*/> mTracedEvents GUARDED_BY(mLock);
+    std::vector<std::variant<KeyEvent, MotionEvent>> mExpectedEvents GUARDED_BY(mLock);
+
+    friend class FakeInputTracingBackend;
+
+    // Helper to verify that the given event appears as expected in the trace. If the verification
+    // fails, the error message describes why.
+    template <typename Event>
+    base::Result<void> verifyEventTraced(const Event&) const REQUIRES(mLock);
+};
+
+/**
+ * A backend implementation for input tracing that records events to the provided
+ * VerifyingTrace used for testing.
+ */
+class FakeInputTracingBackend : public trace::InputTracingBackendInterface {
+public:
+    FakeInputTracingBackend(std::shared_ptr<VerifyingTrace> trace) : mTrace(trace) {}
+
+private:
+    std::shared_ptr<VerifyingTrace> mTrace;
+
+    void traceKeyEvent(const trace::TracedKeyEvent& entry) const override;
+    void traceMotionEvent(const trace::TracedMotionEvent& entry) const override;
+    void traceWindowDispatch(const WindowDispatchArgs& entry) const override {}
+};
+
+} // namespace android::inputdispatcher

services/inputflinger/tests/InputDispatcher_test.cpp

@@ -17,6 +17,7 @@
 #include "../dispatcher/InputDispatcher.h"
 #include "../BlockingQueue.h"
 #include "FakeApplicationHandle.h"
+#include "FakeInputTracingBackend.h"
 #include "TestEventMatchers.h"
 
 #include <NotifyArgsBuilders.h>
@@ -660,21 +661,30 @@ class FakeInputDispatcherPolicy : public InputDispatcherPolicyInterface {
 
 // --- InputDispatcherTest ---
 
+// The trace is a global variable for now, to avoid having to pass it into all of the
+// FakeWindowHandles created throughout the tests.
+// TODO(b/210460522): Update the tests to avoid the need to have the trace be a global variable.
+static std::shared_ptr<VerifyingTrace> gVerifyingTrace = std::make_shared<VerifyingTrace>();
+
 class InputDispatcherTest : public testing::Test {
 protected:
     std::unique_ptr<FakeInputDispatcherPolicy> mFakePolicy;
     std::unique_ptr<InputDispatcher> mDispatcher;
 
     void SetUp() override {
+        gVerifyingTrace->reset();
         mFakePolicy = std::make_unique<FakeInputDispatcherPolicy>();
-        mDispatcher = std::make_unique<InputDispatcher>(*mFakePolicy, nullptr);
+        mDispatcher = std::make_unique<InputDispatcher>(*mFakePolicy,
+                                                        std::make_unique<FakeInputTracingBackend>(
+                                                                gVerifyingTrace));
 
         mDispatcher->setInputDispatchMode(/*enabled=*/true, /*frozen=*/false);
         // Start InputDispatcher thread
         ASSERT_EQ(OK, mDispatcher->start());
     }
 
     void TearDown() override {
+        ASSERT_NO_FATAL_FAILURE(gVerifyingTrace->verifyExpectedEventsTraced());
         ASSERT_EQ(OK, mDispatcher->stop());
         mFakePolicy.reset();
         mDispatcher.reset();
@@ -1397,11 +1407,7 @@ class FakeWindowHandle : public WindowInfoHandle {
     }
 
     std::pair<std::optional<uint32_t>, std::unique_ptr<InputEvent>> receiveEvent() {
-        if (mInputReceiver == nullptr) {
-            ADD_FAILURE() << "Invalid receive event on window with no receiver";
-            return std::make_pair(std::nullopt, nullptr);
-        }
-        return mInputReceiver->receiveEvent(CONSUME_TIMEOUT_EVENT_EXPECTED);
+        return receive();
     }
 
     void finishEvent(uint32_t sequenceNum) {
@@ -1439,6 +1445,7 @@ class FakeWindowHandle : public WindowInfoHandle {
 
     int getChannelFd() { return mInputReceiver->getChannelFd(); }
 
+    // FakeWindowHandle uses this consume method to ensure received events are added to the trace.
     std::unique_ptr<InputEvent> consume(std::chrono::milliseconds timeout, bool handled = true) {
         if (mInputReceiver == nullptr) {
             LOG(FATAL) << "Cannot consume event from a window with no input event receiver";
@@ -1447,6 +1454,7 @@ class FakeWindowHandle : public WindowInfoHandle {
         if (event == nullptr) {
             ADD_FAILURE() << "Consume failed: no event";
         }
+        expectReceivedEventTraced(event);
         return event;
     }
 
@@ -1456,6 +1464,37 @@ class FakeWindowHandle : public WindowInfoHandle {
     std::shared_ptr<FakeInputReceiver> mInputReceiver;
     static std::atomic<int32_t> sId; // each window gets a unique id, like in surfaceflinger
     friend class sp<FakeWindowHandle>;
+
+    // FakeWindowHandle uses this receive method to ensure received events are added to the trace.
+    std::pair<std::optional<uint32_t /*seq*/>, std::unique_ptr<InputEvent>> receive() {
+        if (mInputReceiver == nullptr) {
+            ADD_FAILURE() << "Invalid receive event on window with no receiver";
+            return std::make_pair(std::nullopt, nullptr);
+        }
+        auto out = mInputReceiver->receiveEvent(CONSUME_TIMEOUT_EVENT_EXPECTED);
+        const auto& [_, event] = out;
+        expectReceivedEventTraced(event);
+        return std::move(out);
+    }
+
+    void expectReceivedEventTraced(const std::unique_ptr<InputEvent>& event) {
+        if (!event) {
+            return;
+        }
+
+        switch (event->getType()) {
+            case InputEventType::KEY: {
+                gVerifyingTrace->expectKeyDispatchTraced(static_cast<KeyEvent&>(*event));
+                break;
+            }
+            case InputEventType::MOTION: {
+                gVerifyingTrace->expectMotionDispatchTraced(static_cast<MotionEvent&>(*event));
+                break;
+            }
+            default:
+                break;
+        }
+    }
 };
 
 std::atomic<int32_t> FakeWindowHandle::sId{1};