Read pointer choreographer flag at boot
In native code, we are sometimes reading the
enable_pointer_choreographer flag value at boot, and sometimes later
during runtime. Since DeviceConfig flag values can change at runtime,
the flag values loaded later can sometimes change, resulting in an
unexpected state where some code relies on the boot-time value and
others on the "current" value.
While we should have been using read-only flags in the first place, we
cannot convert existing flags to be read-only. So to increase
consistency within native code, change all reads of the
enable_pointer_choreographer flag to happen at boot.
This CL also updates the test to match the change in flagging approach.
Bug: 324534774
Test: Presubmit
Change-Id: I32cd6e52e2df260afd3d7681760a987ed060321a
diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp
index 45f09ae..06f10e5 100644
--- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp
@@ -40,6 +40,8 @@
// The default velocity control parameters that has no effect.
static const VelocityControlParameters FLAT_VELOCITY_CONTROL_PARAMS{};
+static const bool ENABLE_POINTER_CHOREOGRAPHER = input_flags::enable_pointer_choreographer();
+
// --- CursorMotionAccumulator ---
CursorMotionAccumulator::CursorMotionAccumulator() {
@@ -76,9 +78,14 @@
CursorInputMapper::CursorInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig)
+ : CursorInputMapper(deviceContext, readerConfig, ENABLE_POINTER_CHOREOGRAPHER) {}
+
+CursorInputMapper::CursorInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig,
+ bool enablePointerChoreographer)
: InputMapper(deviceContext, readerConfig),
mLastEventTime(std::numeric_limits<nsecs_t>::min()),
- mEnablePointerChoreographer(input_flags::enable_pointer_choreographer()),
+ mEnablePointerChoreographer(enablePointerChoreographer),
mEnableNewMousePointerBallistics(input_flags::enable_new_mouse_pointer_ballistics()) {}
CursorInputMapper::~CursorInputMapper() {
diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.h b/services/inputflinger/reader/mapper/CursorInputMapper.h
index 1ddf6f2..ca541d9 100644
--- a/services/inputflinger/reader/mapper/CursorInputMapper.h
+++ b/services/inputflinger/reader/mapper/CursorInputMapper.h
@@ -133,6 +133,10 @@
explicit CursorInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig);
+ // Constructor for testing.
+ explicit CursorInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig,
+ bool enablePointerChoreographer);
void dumpParameters(std::string& dump);
void configureBasicParams();
void configureOnPointerCapture(const InputReaderConfiguration& config);
diff --git a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp
index b990dd5..eacc66e 100644
--- a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp
@@ -47,6 +47,8 @@
namespace {
+static const bool ENABLE_POINTER_CHOREOGRAPHER = input_flags::enable_pointer_choreographer();
+
/**
* Log details of each gesture output by the gestures library.
* Enable this via "adb shell setprop log.tag.TouchpadInputMapperGestures DEBUG" (requires
@@ -232,6 +234,11 @@
TouchpadInputMapper::TouchpadInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig)
+ : TouchpadInputMapper(deviceContext, readerConfig, ENABLE_POINTER_CHOREOGRAPHER) {}
+
+TouchpadInputMapper::TouchpadInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig,
+ bool enablePointerChoreographer)
: InputMapper(deviceContext, readerConfig),
mGestureInterpreter(NewGestureInterpreter(), DeleteGestureInterpreter),
mPointerController(getContext()->getPointerController(getDeviceId())),
@@ -240,7 +247,7 @@
mGestureConverter(*getContext(), deviceContext, getDeviceId()),
mCapturedEventConverter(*getContext(), deviceContext, mMotionAccumulator, getDeviceId()),
mMetricsId(metricsIdFromInputDeviceIdentifier(deviceContext.getDeviceIdentifier())),
- mEnablePointerChoreographer(input_flags::enable_pointer_choreographer()) {
+ mEnablePointerChoreographer(enablePointerChoreographer) {
RawAbsoluteAxisInfo slotAxisInfo;
deviceContext.getAbsoluteAxisInfo(ABS_MT_SLOT, &slotAxisInfo);
if (!slotAxisInfo.valid || slotAxisInfo.maxValue <= 0) {
diff --git a/services/inputflinger/reader/mapper/TouchpadInputMapper.h b/services/inputflinger/reader/mapper/TouchpadInputMapper.h
index 897edca..9f272cf 100644
--- a/services/inputflinger/reader/mapper/TouchpadInputMapper.h
+++ b/services/inputflinger/reader/mapper/TouchpadInputMapper.h
@@ -72,6 +72,10 @@
void resetGestureInterpreter(nsecs_t when);
explicit TouchpadInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig);
+ // Constructor for testing.
+ explicit TouchpadInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig,
+ bool enablePointerChoreographer);
void updatePalmDetectionMetrics();
[[nodiscard]] std::list<NotifyArgs> sendHardwareState(nsecs_t when, nsecs_t readTime,
SelfContainedHardwareState schs);
diff --git a/services/inputflinger/tests/CursorInputMapper_test.cpp b/services/inputflinger/tests/CursorInputMapper_test.cpp
index 8c17221..de74067 100644
--- a/services/inputflinger/tests/CursorInputMapper_test.cpp
+++ b/services/inputflinger/tests/CursorInputMapper_test.cpp
@@ -157,9 +157,12 @@
mFakePolicy->addDisplayViewport(createPrimaryViewport(ui::Rotation::Rotation0));
}
+ virtual bool isPointerChoreographerEnabled() { return false; }
+
void createMapper() {
createDevice();
- mMapper = createInputMapper<CursorInputMapper>(*mDeviceContext, mReaderConfiguration);
+ mMapper = createInputMapper<CursorInputMapper>(*mDeviceContext, mReaderConfiguration,
+ isPointerChoreographerEnabled());
}
void setPointerCapture(bool enabled) {
@@ -194,10 +197,11 @@
class CursorInputMapperUnitTest : public CursorInputMapperUnitTestBase {
protected:
void SetUp() override {
- input_flags::enable_pointer_choreographer(false);
input_flags::enable_new_mouse_pointer_ballistics(false);
CursorInputMapperUnitTestBase::SetUp();
}
+
+ bool isPointerChoreographerEnabled() override { return false; }
};
TEST_F(CursorInputMapperUnitTest, GetSourcesReturnsMouseInPointerMode) {
@@ -321,10 +325,10 @@
// Disable pointer capture. Afterwards, events should be generated the usual way.
setPointerCapture(false);
- const auto expectedCoords = input_flags::enable_pointer_choreographer()
+ const auto expectedCoords = CursorInputMapperUnitTest::isPointerChoreographerEnabled()
? WithCoords(0, 0)
: WithCoords(INITIAL_CURSOR_X + 10.0f, INITIAL_CURSOR_Y + 20.0f);
- const auto expectedCursorPosition = input_flags::enable_pointer_choreographer()
+ const auto expectedCursorPosition = CursorInputMapperUnitTest::isPointerChoreographerEnabled()
? WithCursorPosition(INVALID_CURSOR_POSITION, INVALID_CURSOR_POSITION)
: WithCursorPosition(INITIAL_CURSOR_X + 10.0f, INITIAL_CURSOR_Y + 20.0f);
args.clear();
@@ -708,7 +712,9 @@
createDevice();
// Associate the InputDevice with the secondary display.
ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, secondaryViewport);
- mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration);
+ mMapper = createInputMapper<
+ CursorInputMapper>(deviceContext, mReaderConfiguration,
+ CursorInputMapperUnitTest::isPointerChoreographerEnabled());
// Ensure input events are generated for the secondary display.
std::list<NotifyArgs> args;
@@ -731,7 +737,9 @@
createDevice();
// Associate the InputDevice with the secondary display.
ViewportFakingInputDeviceContext deviceContext(*mDevice, EVENTHUB_ID, secondaryViewport);
- mMapper = createInputMapper<CursorInputMapper>(deviceContext, mReaderConfiguration);
+ mMapper = createInputMapper<
+ CursorInputMapper>(deviceContext, mReaderConfiguration,
+ CursorInputMapperUnitTest::isPointerChoreographerEnabled());
// The mapper should not generate any events because it is associated with a display that is
// different from the pointer display.
@@ -837,7 +845,9 @@
: public CursorInputMapperUnitTest,
public testing::WithParamInterface<
std::tuple<int32_t /*evdevCode*/, int32_t /*expectedButtonState*/,
- int32_t /*expectedKeyCode*/>> {};
+ int32_t /*expectedKeyCode*/>> {
+ virtual bool isPointerChoreographerEnabled() override { return false; }
+};
TEST_P(CursorInputMapperButtonKeyTest, ProcessShouldHandleButtonKey) {
auto [evdevCode, expectedButtonState, expectedKeyCode] = GetParam();
@@ -956,10 +966,11 @@
class CursorInputMapperUnitTestWithChoreographer : public CursorInputMapperUnitTestBase {
protected:
void SetUp() override {
- input_flags::enable_pointer_choreographer(true);
input_flags::enable_new_mouse_pointer_ballistics(false);
CursorInputMapperUnitTestBase::SetUp();
}
+
+ bool isPointerChoreographerEnabled() override { return true; }
};
TEST_F(CursorInputMapperUnitTestWithChoreographer, PopulateDeviceInfoReturnsRangeFromPolicy) {
@@ -1288,10 +1299,11 @@
class CursorInputMapperUnitTestWithNewBallistics : public CursorInputMapperUnitTestBase {
protected:
void SetUp() override {
- input_flags::enable_pointer_choreographer(true);
input_flags::enable_new_mouse_pointer_ballistics(true);
CursorInputMapperUnitTestBase::SetUp();
}
+
+ bool isPointerChoreographerEnabled() override { return true; }
};
TEST_F(CursorInputMapperUnitTestWithNewBallistics, PointerCaptureDisablesVelocityProcessing) {
@@ -1413,7 +1425,6 @@
class BluetoothCursorInputMapperUnitTest : public CursorInputMapperUnitTestBase {
protected:
void SetUp() override {
- input_flags::enable_pointer_choreographer(false);
SetUpWithBus(BUS_BLUETOOTH);
mFakePointerController = std::make_shared<FakePointerController>();
@@ -1531,12 +1542,13 @@
class BluetoothCursorInputMapperUnitTestWithChoreographer : public CursorInputMapperUnitTestBase {
protected:
void SetUp() override {
- input_flags::enable_pointer_choreographer(true);
SetUpWithBus(BUS_BLUETOOTH);
mFakePointerController = std::make_shared<FakePointerController>();
mFakePolicy->setPointerController(mFakePointerController);
}
+
+ bool isPointerChoreographerEnabled() override { return true; }
};
TEST_F(BluetoothCursorInputMapperUnitTestWithChoreographer, TimestampSmoothening) {
diff --git a/services/inputflinger/tests/TouchpadInputMapper_test.cpp b/services/inputflinger/tests/TouchpadInputMapper_test.cpp
index fbafbad..a92dce5 100644
--- a/services/inputflinger/tests/TouchpadInputMapper_test.cpp
+++ b/services/inputflinger/tests/TouchpadInputMapper_test.cpp
@@ -117,8 +117,11 @@
return base::ResultError("Axis not supported", NAME_NOT_FOUND);
});
createDevice();
- mMapper = createInputMapper<TouchpadInputMapper>(*mDeviceContext, mReaderConfiguration);
+ mMapper = createInputMapper<TouchpadInputMapper>(*mDeviceContext, mReaderConfiguration,
+ isPointerChoreographerEnabled());
}
+
+ virtual bool isPointerChoreographerEnabled() { return false; }
};
class TouchpadInputMapperTest : public TouchpadInputMapperTestBase {
@@ -182,10 +185,9 @@
class TouchpadInputMapperTestWithChoreographer : public TouchpadInputMapperTestBase {
protected:
- void SetUp() override {
- input_flags::enable_pointer_choreographer(true);
- TouchpadInputMapperTestBase::SetUp();
- }
+ void SetUp() override { TouchpadInputMapperTestBase::SetUp(); }
+
+ bool isPointerChoreographerEnabled() override { return true; }
};
// TODO(b/311416205): De-duplicate the test cases after the refactoring is complete and the flagging