Revert "Reland "Let InputReader handle its own thread""
Reason for revert: flaky tests b/144546498
Bug: 144546498
Change-Id: Id5a4c8dc8e634b23b560dde6843b5a5860305e5f
diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp
index 1043390..e7640dd 100644
--- a/services/inputflinger/InputManager.cpp
+++ b/services/inputflinger/InputManager.cpp
@@ -46,6 +46,7 @@
}
void InputManager::initialize() {
+ mReaderThread = new InputReaderThread(mReader);
mDispatcherThread = new InputDispatcherThread(mDispatcher);
}
@@ -56,9 +57,9 @@
return result;
}
- result = mReader->start();
+ result = mReaderThread->run("InputReader", PRIORITY_URGENT_DISPLAY);
if (result) {
- ALOGE("Could not start InputReader due to error %d.", result);
+ ALOGE("Could not start InputReader thread due to error %d.", result);
mDispatcherThread->requestExit();
return result;
@@ -68,9 +69,9 @@
}
status_t InputManager::stop() {
- status_t result = mReader->stop();
+ status_t result = mReaderThread->requestExitAndWait();
if (result) {
- ALOGW("Could not stop InputReader due to error %d.", result);
+ ALOGW("Could not stop InputReader thread due to error %d.", result);
}
result = mDispatcherThread->requestExitAndWait();
diff --git a/services/inputflinger/InputManager.h b/services/inputflinger/InputManager.h
index 2a7ed0f..40f66d8 100644
--- a/services/inputflinger/InputManager.h
+++ b/services/inputflinger/InputManager.h
@@ -43,15 +43,15 @@
/*
* The input manager is the core of the system event processing.
*
- * The input manager has two components.
+ * The input manager uses two threads.
*
- * 1. The InputReader class starts a thread that reads and preprocesses raw input events, applies
- * policy, and posts messages to a queue managed by the InputDispatcherThread.
+ * 1. The InputReaderThread (called "InputReader") reads and preprocesses raw input events,
+ * applies policy, and posts messages to a queue managed by the DispatcherThread.
* 2. The InputDispatcherThread (called "InputDispatcher") thread waits for new events on the
* queue and asynchronously dispatches them to applications.
*
- * By design, the InputReader class and InputDispatcherThread class do not share any
- * internal state. Moreover, all communication is done one way from the InputReader
+ * By design, the InputReaderThread class and InputDispatcherThread class do not share any
+ * internal state. Moreover, all communication is done one way from the InputReaderThread
* into the InputDispatcherThread and never the reverse. Both classes may interact with the
* InputDispatchPolicy, however.
*
@@ -102,6 +102,7 @@
private:
sp<InputReaderInterface> mReader;
+ sp<InputReaderThread> mReaderThread;
sp<InputClassifierInterface> mClassifier;
diff --git a/services/inputflinger/InputReaderBase.cpp b/services/inputflinger/InputReaderBase.cpp
index 2d6f2c1..0422d83 100644
--- a/services/inputflinger/InputReaderBase.cpp
+++ b/services/inputflinger/InputReaderBase.cpp
@@ -33,6 +33,20 @@
namespace android {
+// --- InputReaderThread ---
+
+InputReaderThread::InputReaderThread(const sp<InputReaderInterface>& reader) :
+ Thread(/*canCallJava*/ true), mReader(reader) {
+}
+
+InputReaderThread::~InputReaderThread() {
+}
+
+bool InputReaderThread::threadLoop() {
+ mReader->loopOnce();
+ return true;
+}
+
// --- InputReaderConfiguration ---
std::string InputReaderConfiguration::changesToString(uint32_t changes) {
diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h
index 56c0a73..5d576b9 100644
--- a/services/inputflinger/include/InputReaderBase.h
+++ b/services/inputflinger/include/InputReaderBase.h
@@ -19,12 +19,12 @@
#include "PointerControllerInterface.h"
-#include <input/DisplayViewport.h>
#include <input/Input.h>
#include <input/InputDevice.h>
+#include <input/DisplayViewport.h>
#include <input/VelocityControl.h>
#include <input/VelocityTracker.h>
-#include <utils/Errors.h>
+#include <utils/Thread.h>
#include <utils/RefBase.h>
#include <stddef.h>
@@ -44,16 +44,7 @@
namespace android {
-// --- InputReaderInterface ---
-
-/* The interface for the InputReader shared library.
- *
- * Manages one or more threads that process raw input events and sends cooked event data to an
- * input listener.
- *
- * The implementation must guarantee thread safety for this interface. However, since the input
- * listener is NOT thread safe, all calls to the listener must happen from the same thread.
- */
+/* Processes raw input events and sends cooked event data to an input listener. */
class InputReaderInterface : public virtual RefBase {
protected:
InputReaderInterface() { }
@@ -65,17 +56,18 @@
* This method may be called on any thread (usually by the input manager). */
virtual void dump(std::string& dump) = 0;
- /* Called by the heartbeat to ensures that the reader has not deadlocked. */
+ /* Called by the heatbeat to ensures that the reader has not deadlocked. */
virtual void monitor() = 0;
/* Returns true if the input device is enabled. */
virtual bool isInputDeviceEnabled(int32_t deviceId) = 0;
- /* Makes the reader start processing events from the kernel. */
- virtual status_t start() = 0;
-
- /* Makes the reader stop processing any more events. */
- virtual status_t stop() = 0;
+ /* Runs a single iteration of the processing loop.
+ * Nominally reads and processes one incoming message from the EventHub.
+ *
+ * This method should be called on the input reader thread.
+ */
+ virtual void loopOnce() = 0;
/* Gets information about all input devices.
*
@@ -112,7 +104,17 @@
virtual bool canDispatchToDisplay(int32_t deviceId, int32_t displayId) = 0;
};
-// --- InputReaderConfiguration ---
+/* Reads raw events from the event hub and processes them, endlessly. */
+class InputReaderThread : public Thread {
+public:
+ explicit InputReaderThread(const sp<InputReaderInterface>& reader);
+ virtual ~InputReaderThread();
+
+private:
+ sp<InputReaderInterface> mReader;
+
+ virtual bool threadLoop();
+};
/*
* Input reader configuration.
@@ -283,8 +285,6 @@
std::vector<DisplayViewport> mDisplays;
};
-// --- TouchAffineTransformation ---
-
struct TouchAffineTransformation {
float x_scale;
float x_ymix;
@@ -307,8 +307,6 @@
void applyTo(float& x, float& y) const;
};
-// --- InputReaderPolicyInterface ---
-
/*
* Input reader policy interface.
*
@@ -318,8 +316,8 @@
* The actual implementation is partially supported by callbacks into the DVM
* via JNI. This interface is also mocked in the unit tests.
*
- * These methods will NOT re-enter the input reader interface, so they may be called from
- * any method in the input reader interface.
+ * These methods must NOT re-enter the input reader since they may be called while
+ * holding the input reader lock.
*/
class InputReaderPolicyInterface : public virtual RefBase {
protected:
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 5d52bbc..e57604c 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -38,38 +38,16 @@
#include <unistd.h>
#include <log/log.h>
-#include <utils/Errors.h>
#include <android-base/stringprintf.h>
#include <input/Keyboard.h>
#include <input/VirtualKeyMap.h>
-#include <utils/Thread.h>
+
using android::base::StringPrintf;
namespace android {
-// --- InputReader::InputReaderThread ---
-
-/* Thread that reads raw events from the event hub and processes them, endlessly. */
-class InputReader::InputReaderThread : public Thread {
-public:
- explicit InputReaderThread(InputReader* reader)
- : Thread(/* canCallJava */ true), mReader(reader) {}
-
- ~InputReaderThread() {}
-
-private:
- InputReader* mReader;
-
- bool threadLoop() override {
- mReader->loopOnce();
- return true;
- }
-};
-
-// --- InputReader ---
-
InputReader::InputReader(std::shared_ptr<EventHubInterface> eventHub,
const sp<InputReaderPolicyInterface>& policy,
const sp<InputListenerInterface>& listener)
@@ -83,7 +61,6 @@
mNextTimeout(LLONG_MAX),
mConfigurationChangesToRefresh(0) {
mQueuedListener = new QueuedInputListener(listener);
- mThread = new InputReaderThread(this);
{ // acquire lock
AutoMutex _l(mLock);
@@ -99,28 +76,6 @@
}
}
-status_t InputReader::start() {
- if (mThread->isRunning()) {
- return ALREADY_EXISTS;
- }
- return mThread->run("InputReader", PRIORITY_URGENT_DISPLAY);
-}
-
-status_t InputReader::stop() {
- if (!mThread->isRunning()) {
- return OK;
- }
- if (gettid() == mThread->getTid()) {
- ALOGE("InputReader can only be stopped from outside of the InputReaderThread!");
- return INVALID_OPERATION;
- }
- // Directly calling requestExitAndWait() causes the thread to not exit
- // if mEventHub is waiting for a long timeout.
- mThread->requestExit();
- mEventHub->wake();
- return mThread->requestExitAndWait();
-}
-
void InputReader::loopOnce() {
int32_t oldGeneration;
int32_t timeoutMillis;
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index 3cf4535..7b4321e 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -38,12 +38,12 @@
* that it sends to the input listener. Some functions of the input reader, such as early
* event filtering in low power states, are controlled by a separate policy object.
*
- * The InputReader owns a collection of InputMappers. InputReader starts its own thread, where
- * most of the work happens, but the InputReader can receive queries from other system
+ * The InputReader owns a collection of InputMappers. Most of the work it does happens
+ * on the input reader thread but the InputReader can receive queries from other system
* components running on arbitrary threads. To keep things manageable, the InputReader
* uses a single Mutex to guard its state. The Mutex may be held while calling into the
* EventHub or the InputReaderPolicy but it is never held while calling into the
- * InputListener. All calls to InputListener must happen from InputReader's thread.
+ * InputListener.
*/
class InputReader : public InputReaderInterface {
public:
@@ -55,8 +55,7 @@
virtual void dump(std::string& dump) override;
virtual void monitor() override;
- virtual status_t start() override;
- virtual status_t stop() override;
+ virtual void loopOnce() override;
virtual void getInputDevices(std::vector<InputDeviceInfo>& outInputDevices) override;
@@ -112,9 +111,6 @@
friend class ContextImpl;
private:
- class InputReaderThread;
- sp<InputReaderThread> mThread;
-
Mutex mLock;
Condition mReaderIsAliveCondition;
@@ -137,10 +133,6 @@
KeyedVector<int32_t, InputDevice*> mDevices;
- // With each iteration of the loop, InputReader reads and processes one incoming message from
- // the EventHub.
- void loopOnce();
-
// low-level input event decoding and device management
void processEventsLocked(const RawEvent* rawEvents, size_t count);
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 8d4ab6a..c1c9122 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -1133,8 +1133,12 @@
protected:
sp<FakeInputReaderPolicy> mFakePolicy;
- virtual void SetUp() override { mFakePolicy = new FakeInputReaderPolicy(); }
- virtual void TearDown() override { mFakePolicy.clear(); }
+ virtual void SetUp() {
+ mFakePolicy = new FakeInputReaderPolicy();
+ }
+ virtual void TearDown() {
+ mFakePolicy.clear();
+ }
};
/**
@@ -1317,20 +1321,18 @@
sp<TestInputListener> mFakeListener;
sp<FakeInputReaderPolicy> mFakePolicy;
std::shared_ptr<FakeEventHub> mFakeEventHub;
- std::unique_ptr<InstrumentedInputReader> mReader;
+ sp<InstrumentedInputReader> mReader;
- virtual void SetUp() override {
+ virtual void SetUp() {
mFakeEventHub = std::make_unique<FakeEventHub>();
mFakePolicy = new FakeInputReaderPolicy();
mFakeListener = new TestInputListener();
- mReader = std::make_unique<InstrumentedInputReader>(mFakeEventHub, mFakePolicy,
- mFakeListener);
- ASSERT_EQ(OK, mReader->start());
+ mReader = new InstrumentedInputReader(mFakeEventHub, mFakePolicy, mFakeListener);
}
- virtual void TearDown() override {
- ASSERT_EQ(OK, mReader->stop());
+ virtual void TearDown() {
+ mReader.clear();
mFakeListener.clear();
mFakePolicy.clear();
@@ -1344,18 +1346,24 @@
mFakeEventHub->addConfigurationMap(deviceId, configuration);
}
mFakeEventHub->finishDeviceScan();
+ mReader->loopOnce();
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakePolicy->assertInputDevicesChanged());
ASSERT_NO_FATAL_FAILURE(mFakeEventHub->assertQueueIsEmpty());
}
void disableDevice(int32_t deviceId, InputDevice* device) {
mFakePolicy->addDisabledDevice(deviceId);
- mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_ENABLED_STATE);
+ configureDevice(InputReaderConfiguration::CHANGE_ENABLED_STATE, device);
}
void enableDevice(int32_t deviceId, InputDevice* device) {
mFakePolicy->removeDisabledDevice(deviceId);
- mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_ENABLED_STATE);
+ configureDevice(InputReaderConfiguration::CHANGE_ENABLED_STATE, device);
+ }
+
+ void configureDevice(uint32_t changes, InputDevice* device) {
+ device->configure(ARBITRARY_TIME, mFakePolicy->getReaderConfiguration(), changes);
}
FakeInputMapper* addDeviceWithFakeInputMapper(int32_t deviceId, int32_t controllerNumber,
@@ -1409,22 +1417,28 @@
NotifyDeviceResetArgs resetArgs;
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
+ ASSERT_EQ(ARBITRARY_TIME, resetArgs.eventTime);
ASSERT_EQ(deviceId, resetArgs.deviceId);
ASSERT_EQ(device->isEnabled(), true);
disableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
+ ASSERT_EQ(ARBITRARY_TIME, resetArgs.eventTime);
ASSERT_EQ(deviceId, resetArgs.deviceId);
ASSERT_EQ(device->isEnabled(), false);
disableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasNotCalled());
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyConfigurationChangedWasNotCalled());
ASSERT_EQ(device->isEnabled(), false);
enableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
+ ASSERT_EQ(ARBITRARY_TIME, resetArgs.eventTime);
ASSERT_EQ(deviceId, resetArgs.deviceId);
ASSERT_EQ(device->isEnabled(), true);
}
@@ -1546,7 +1560,7 @@
ASSERT_TRUE(flags[0] && flags[1] && !flags[2] && !flags[3]);
}
-TEST_F(InputReaderTest, WhenDeviceScanFinished_SendsConfigurationChanged) {
+TEST_F(InputReaderTest, LoopOnce_WhenDeviceScanFinished_SendsConfigurationChanged) {
addDevice(1, "ignored", INPUT_DEVICE_CLASS_KEYBOARD, nullptr);
NotifyConfigurationChangedArgs args;
@@ -1555,12 +1569,13 @@
ASSERT_EQ(ARBITRARY_TIME, args.eventTime);
}
-TEST_F(InputReaderTest, ForwardsRawEventsToMappers) {
+TEST_F(InputReaderTest, LoopOnce_ForwardsRawEventsToMappers) {
FakeInputMapper* mapper = nullptr;
ASSERT_NO_FATAL_FAILURE(mapper = addDeviceWithFakeInputMapper(1, 0, "fake",
INPUT_DEVICE_CLASS_KEYBOARD, AINPUT_SOURCE_KEYBOARD, nullptr));
mFakeEventHub->enqueueEvent(0, 1, EV_KEY, KEY_A, 1);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeEventHub->assertQueueIsEmpty());
RawEvent event;
@@ -1587,16 +1602,19 @@
uint32_t prevSequenceNum = resetArgs.sequenceNum;
disableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
ASSERT_TRUE(prevSequenceNum < resetArgs.sequenceNum);
prevSequenceNum = resetArgs.sequenceNum;
enableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
ASSERT_TRUE(prevSequenceNum < resetArgs.sequenceNum);
prevSequenceNum = resetArgs.sequenceNum;
disableDevice(deviceId, device);
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled(&resetArgs));
ASSERT_TRUE(prevSequenceNum < resetArgs.sequenceNum);
prevSequenceNum = resetArgs.sequenceNum;
@@ -1611,6 +1629,7 @@
FakeInputMapper* mapper = new FakeInputMapper(device, AINPUT_SOURCE_TOUCHSCREEN);
device->addMapper(mapper);
mReader->setNextDevice(device);
+ addDevice(deviceId, "fake", deviceClass, nullptr);
const uint8_t hdmi1 = 1;
@@ -1618,20 +1637,13 @@
mFakePolicy->addInputPortAssociation(DEVICE_LOCATION, hdmi1);
// Add default and second display.
- mFakePolicy->clearViewports();
mFakePolicy->addDisplayViewport(DISPLAY_ID, DISPLAY_WIDTH, DISPLAY_HEIGHT,
DISPLAY_ORIENTATION_0, "local:0", NO_PORT, ViewportType::VIEWPORT_INTERNAL);
mFakePolicy->addDisplayViewport(SECONDARY_DISPLAY_ID, DISPLAY_WIDTH, DISPLAY_HEIGHT,
DISPLAY_ORIENTATION_0, "local:1", hdmi1, ViewportType::VIEWPORT_EXTERNAL);
mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_DISPLAY_INFO);
-
- // Add the device, and make sure all of the callbacks are triggered.
- // The device is added after the input port associations are processed since
- // we do not yet support dynamic device-to-display associations.
- ASSERT_NO_FATAL_FAILURE(addDevice(deviceId, "fake", deviceClass, nullptr));
+ mReader->loopOnce();
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyConfigurationChangedWasCalled());
- ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled());
- ASSERT_NO_FATAL_FAILURE(mapper->assertConfigureWasCalled());
// Device should only dispatch to the specified display.
ASSERT_EQ(deviceId, device->getId());
@@ -1640,8 +1652,6 @@
// Can't dispatch event from a disabled device.
disableDevice(deviceId, device);
- ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled());
- ASSERT_NO_FATAL_FAILURE(mapper->assertConfigureWasCalled());
ASSERT_FALSE(mReader->canDispatchToDisplay(deviceId, SECONDARY_DISPLAY_ID));
}
@@ -1664,7 +1674,7 @@
InputDevice* mDevice;
- virtual void SetUp() override {
+ virtual void SetUp() {
mFakeEventHub = std::make_unique<FakeEventHub>();
mFakePolicy = new FakeInputReaderPolicy();
mFakeListener = new TestInputListener();
@@ -1678,7 +1688,7 @@
DEVICE_CONTROLLER_NUMBER, identifier, DEVICE_CLASSES);
}
- virtual void TearDown() override {
+ virtual void TearDown() {
delete mDevice;
delete mFakeContext;
@@ -1902,7 +1912,7 @@
FakeInputReaderContext* mFakeContext;
InputDevice* mDevice;
- virtual void SetUp() override {
+ virtual void SetUp() {
mFakeEventHub = std::make_unique<FakeEventHub>();
mFakePolicy = new FakeInputReaderPolicy();
mFakeListener = new TestInputListener();
@@ -1916,7 +1926,7 @@
mFakeEventHub->addDevice(mDevice->getId(), DEVICE_NAME, 0);
}
- virtual void TearDown() override {
+ virtual void TearDown() {
delete mDevice;
delete mFakeContext;
mFakeListener.clear();
@@ -2579,7 +2589,7 @@
sp<FakePointerController> mFakePointerController;
- virtual void SetUp() override {
+ virtual void SetUp() {
InputMapperTest::SetUp();
mFakePointerController = new FakePointerController();