From e948d4b172781acb2a999ae1cd5d64da004aad7a Mon Sep 17 00:00:00 2001 From: Vaibhav Devmurari Date: Thu, 28 Dec 2023 13:41:32 +0000 Subject: Add callbacks and policy to listen to sticky modifier state DD: go/pk_accessibility Test: None Bug: 294546335 Change-Id: I7596fb61d8b8c1b29f51550f94bc0e6302feae67 --- services/inputflinger/InputManager.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'services/inputflinger/InputManager.cpp') diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp index 296f2449ea..4863513eb5 100644 --- a/services/inputflinger/InputManager.cpp +++ b/services/inputflinger/InputManager.cpp @@ -127,7 +127,8 @@ std::shared_ptr createInputFlingerRust() { */ InputManager::InputManager(const sp& readerPolicy, InputDispatcherPolicyInterface& dispatcherPolicy, - PointerChoreographerPolicyInterface& choreographerPolicy) { + PointerChoreographerPolicyInterface& choreographerPolicy, + InputFilterPolicyInterface& inputFilterPolicy) { mInputFlingerRust = createInputFlingerRust(); mDispatcher = createInputDispatcher(dispatcherPolicy); @@ -135,7 +136,8 @@ InputManager::InputManager(const sp& readerPolicy, std::make_unique("InputDispatcher", *mDispatcher)); if (ENABLE_INPUT_FILTER_RUST) { - mInputFilter = std::make_unique(*mTracingStages.back(), *mInputFlingerRust); + mInputFilter = std::make_unique(*mTracingStages.back(), *mInputFlingerRust, + inputFilterPolicy); mTracingStages.emplace_back( std::make_unique("InputFilter", *mInputFilter)); } -- cgit v1.2.3-59-g8ed1b From 8d66013c673bb8e3e8876907f0ad80c91580a443 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Thu, 11 Jan 2024 16:48:44 -0800 Subject: Use aidl-defined InputChannel for parceling Currently, InputChannel is a manually-written parcelable. In this CL, we introduce an aidl-defined InputChannel, which eventually could also be used in Java as well. Now, whenever an InputChannel needs to be written to parcel, we first create the new android.os.InputChannel and then write that object to parcel. Eventually, we can convert the Java side of InputChannel to use this mechanism, as well. Bug: 161009324 Test: adb shell monkey 1000 Test: atest libgui_test Change-Id: Ib44c5ff02b3c77e0425b59a76195ed100e187317 --- include/input/InputTransport.h | 48 +++------- libs/gui/tests/EndToEndNativeInputTest.cpp | 12 ++- libs/input/Android.bp | 1 + libs/input/InputTransport.cpp | 104 +++++++++------------ libs/input/android/InputChannel.aidl | 20 ---- libs/input/android/os/IInputFlinger.aidl | 5 +- libs/input/android/os/InputChannelCore.aidl | 30 ++++++ libs/input/tests/InputChannel_test.cpp | 70 +++++--------- services/inputflinger/InputManager.cpp | 13 ++- services/inputflinger/InputManager.h | 3 +- .../inputflinger/dispatcher/InputDispatcher.cpp | 12 +-- .../inputflinger/tests/InputDispatcher_test.cpp | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 2 +- 13 files changed, 139 insertions(+), 183 deletions(-) delete mode 100644 libs/input/android/InputChannel.aidl create mode 100644 libs/input/android/os/InputChannelCore.aidl (limited to 'services/inputflinger/InputManager.cpp') diff --git a/include/input/InputTransport.h b/include/input/InputTransport.h index 2d64872beb..8b1c2a3ff0 100644 --- a/include/input/InputTransport.h +++ b/include/input/InputTransport.h @@ -35,18 +35,16 @@ #include #include +#include #include -#include #include #include #include #include #include #include -#include #include - namespace android { class Parcel; @@ -231,18 +229,15 @@ struct InputMessage { * input messages across processes. Each channel has a descriptive name for debugging purposes. * * Each endpoint has its own InputChannel object that specifies its file descriptor. + * For parceling, this relies on android::os::InputChannelCore, defined in aidl. * * The input channel is closed when all references to it are released. */ -class InputChannel : public Parcelable { +class InputChannel : private android::os::InputChannelCore { public: - static std::unique_ptr create(const std::string& name, - android::base::unique_fd fd, sp token); - InputChannel() = default; - InputChannel(const InputChannel& other) - : mName(other.mName), mFd(other.dupFd()), mToken(other.mToken){}; - InputChannel(const std::string name, android::base::unique_fd fd, sp token); - ~InputChannel() override; + static std::unique_ptr create(android::os::InputChannelCore&& parceledChannel); + ~InputChannel(); + /** * Create a pair of input channels. * The two returned input channels are equivalent, and are labeled as "server" and "client" @@ -254,9 +249,8 @@ public: std::unique_ptr& outServerChannel, std::unique_ptr& outClientChannel); - inline std::string getName() const { return mName; } - inline const android::base::unique_fd& getFd() const { return mFd; } - inline sp getToken() const { return mToken; } + inline std::string getName() const { return name; } + inline int getFd() const { return fd.get(); } /* Send a message to the other endpoint. * @@ -304,10 +298,7 @@ public: /* Return a new object that has a duplicate of this channel's fd. */ std::unique_ptr dup() const; - void copyTo(InputChannel& outChannel) const; - - status_t readFromParcel(const android::Parcel* parcel) override; - status_t writeToParcel(android::Parcel* parcel) const override; + void copyTo(android::os::InputChannelCore& outChannel) const; /** * The connection token is used to identify the input connection, i.e. @@ -323,26 +314,11 @@ public: */ sp getConnectionToken() const; - bool operator==(const InputChannel& inputChannel) const { - struct stat lhs, rhs; - if (fstat(mFd.get(), &lhs) != 0) { - return false; - } - if (fstat(inputChannel.getFd().get(), &rhs) != 0) { - return false; - } - // If file descriptors are pointing to same inode they are duplicated fds. - return inputChannel.getName() == getName() && inputChannel.getConnectionToken() == mToken && - lhs.st_ino == rhs.st_ino; - } - private: - base::unique_fd dupFd() const; - - std::string mName; - base::unique_fd mFd; + static std::unique_ptr create(const std::string& name, + android::base::unique_fd fd, sp token); - sp mToken; + InputChannel(const std::string name, android::base::unique_fd fd, sp token); }; /* diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index b9ab803101..a9d6e8d3bf 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -63,8 +63,7 @@ namespace android::test { using Transaction = SurfaceComposerClient::Transaction; sp getInputFlinger() { - sp input(defaultServiceManager()->getService( - String16("inputflinger"))); + sp input(defaultServiceManager()->waitForService(String16("inputflinger"))); if (input == nullptr) { ALOGE("Failed to link to input service"); } else { ALOGE("Linked to input"); } @@ -104,8 +103,13 @@ public: if (noInputChannel) { mInputInfo.setInputConfig(WindowInfo::InputConfig::NO_INPUT_CHANNEL, true); } else { - mClientChannel = std::make_shared(); - mInputFlinger->createInputChannel("testchannels", mClientChannel.get()); + android::os::InputChannelCore tempChannel; + android::binder::Status result = + mInputFlinger->createInputChannel("testchannels", &tempChannel); + if (!result.isOk()) { + ADD_FAILURE() << "binder call to createInputChannel failed"; + } + mClientChannel = InputChannel::create(std::move(tempChannel)); mInputInfo.token = mClientChannel->getConnectionToken(); mInputConsumer = new InputConsumer(mClientChannel); } diff --git a/libs/input/Android.bp b/libs/input/Android.bp index 3d3bf1355f..e5fb2c5b7a 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -175,6 +175,7 @@ cc_library { ], srcs: [ "android/os/IInputFlinger.aidl", + "android/os/InputChannelCore.aidl", "AccelerationCurve.cpp", "Input.cpp", "InputDevice.cpp", diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp index 37de00cf35..0e0e80dc79 100644 --- a/libs/input/InputTransport.cpp +++ b/libs/input/InputTransport.cpp @@ -95,6 +95,21 @@ bool debugResampling() { return __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG "Resampling", ANDROID_LOG_INFO); } +android::base::unique_fd dupChannelFd(int fd) { + android::base::unique_fd newFd(::dup(fd)); + if (!newFd.ok()) { + ALOGE("Could not duplicate fd %i : %s", fd, strerror(errno)); + const bool hitFdLimit = errno == EMFILE || errno == ENFILE; + // If this process is out of file descriptors, then throwing that might end up exploding + // on the other side of a binder call, which isn't really helpful. + // Better to just crash here and hope that the FD leak is slow. + // Other failures could be client errors, so we still propagate those back to the caller. + LOG_ALWAYS_FATAL_IF(hitFdLimit, "Too many open files, could not duplicate input channel"); + return {}; + } + return newFd; +} + } // namespace using android::base::Result; @@ -395,15 +410,23 @@ std::unique_ptr InputChannel::create(const std::string& name, return std::unique_ptr(new InputChannel(name, std::move(fd), token)); } -InputChannel::InputChannel(const std::string name, android::base::unique_fd fd, sp token) - : mName(std::move(name)), mFd(std::move(fd)), mToken(std::move(token)) { +std::unique_ptr InputChannel::create( + android::os::InputChannelCore&& parceledChannel) { + return InputChannel::create(parceledChannel.name, parceledChannel.fd.release(), + parceledChannel.token); +} + +InputChannel::InputChannel(const std::string name, android::base::unique_fd fd, sp token) { + this->name = std::move(name); + this->fd.reset(std::move(fd)); + this->token = std::move(token); ALOGD_IF(DEBUG_CHANNEL_LIFECYCLE, "Input channel constructed: name='%s', fd=%d", - getName().c_str(), getFd().get()); + getName().c_str(), getFd()); } InputChannel::~InputChannel() { ALOGD_IF(DEBUG_CHANNEL_LIFECYCLE, "Input channel destroyed: name='%s', fd=%d", - getName().c_str(), getFd().get()); + getName().c_str(), getFd()); } status_t InputChannel::openInputChannelPair(const std::string& name, @@ -441,19 +464,19 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { ATRACE_NAME_IF(ATRACE_ENABLED(), StringPrintf("sendMessage(inputChannel=%s, seq=0x%" PRIx32 ", type=0x%" PRIx32 ")", - mName.c_str(), msg->header.seq, msg->header.type)); + name.c_str(), msg->header.seq, msg->header.type)); const size_t msgLength = msg->size(); InputMessage cleanMsg; msg->getSanitizedCopy(&cleanMsg); ssize_t nWrite; do { - nWrite = ::send(getFd().get(), &cleanMsg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL); + nWrite = ::send(getFd(), &cleanMsg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL); } while (nWrite == -1 && errno == EINTR); if (nWrite < 0) { int error = errno; ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ error sending message of type %s, %s", - mName.c_str(), ftl::enum_string(msg->header.type).c_str(), strerror(error)); + name.c_str(), ftl::enum_string(msg->header.type).c_str(), strerror(error)); if (error == EAGAIN || error == EWOULDBLOCK) { return WOULD_BLOCK; } @@ -465,12 +488,12 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { if (size_t(nWrite) != msgLength) { ALOGD_IF(DEBUG_CHANNEL_MESSAGES, - "channel '%s' ~ error sending message type %s, send was incomplete", mName.c_str(), + "channel '%s' ~ error sending message type %s, send was incomplete", name.c_str(), ftl::enum_string(msg->header.type).c_str()); return DEAD_OBJECT; } - ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ sent message of type %s", mName.c_str(), + ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ sent message of type %s", name.c_str(), ftl::enum_string(msg->header.type).c_str()); return OK; @@ -479,13 +502,13 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { status_t InputChannel::receiveMessage(InputMessage* msg) { ssize_t nRead; do { - nRead = ::recv(getFd().get(), msg, sizeof(InputMessage), MSG_DONTWAIT); + nRead = ::recv(getFd(), msg, sizeof(InputMessage), MSG_DONTWAIT); } while (nRead == -1 && errno == EINTR); if (nRead < 0) { int error = errno; ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ receive message failed, errno=%d", - mName.c_str(), errno); + name.c_str(), errno); if (error == EAGAIN || error == EWOULDBLOCK) { return WOULD_BLOCK; } @@ -497,29 +520,29 @@ status_t InputChannel::receiveMessage(InputMessage* msg) { if (nRead == 0) { // check for EOF ALOGD_IF(DEBUG_CHANNEL_MESSAGES, - "channel '%s' ~ receive message failed because peer was closed", mName.c_str()); + "channel '%s' ~ receive message failed because peer was closed", name.c_str()); return DEAD_OBJECT; } if (!msg->isValid(nRead)) { - ALOGE("channel '%s' ~ received invalid message of size %zd", mName.c_str(), nRead); + ALOGE("channel '%s' ~ received invalid message of size %zd", name.c_str(), nRead); return BAD_VALUE; } - ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ received message of type %s", mName.c_str(), + ALOGD_IF(DEBUG_CHANNEL_MESSAGES, "channel '%s' ~ received message of type %s", name.c_str(), ftl::enum_string(msg->header.type).c_str()); if (ATRACE_ENABLED()) { // Add an additional trace point to include data about the received message. std::string message = StringPrintf("receiveMessage(inputChannel=%s, seq=0x%" PRIx32 ", type=0x%" PRIx32 ")", - mName.c_str(), msg->header.seq, msg->header.type); + name.c_str(), msg->header.seq, msg->header.type); ATRACE_NAME(message.c_str()); } return OK; } bool InputChannel::probablyHasInput() const { - struct pollfd pfds = {.fd = mFd, .events = POLLIN}; + struct pollfd pfds = {.fd = fd.get(), .events = POLLIN}; if (::poll(&pfds, /*nfds=*/1, /*timeout=*/0) <= 0) { // This can be a false negative because EINTR and ENOMEM are not handled. The latter should // be extremely rare. The EINTR is also unlikely because it happens only when the signal @@ -538,7 +561,7 @@ void InputChannel::waitForMessage(std::chrono::milliseconds timeout) const { if (timeout < 0ms) { LOG(FATAL) << "Timeout cannot be negative, received " << timeout.count(); } - struct pollfd pfds = {.fd = mFd, .events = POLLIN}; + struct pollfd pfds = {.fd = fd.get(), .events = POLLIN}; int ret; std::chrono::time_point stopTime = std::chrono::steady_clock::now() + timeout; @@ -551,53 +574,18 @@ void InputChannel::waitForMessage(std::chrono::milliseconds timeout) const { } std::unique_ptr InputChannel::dup() const { - base::unique_fd newFd(dupFd()); + base::unique_fd newFd(dupChannelFd(fd.get())); return InputChannel::create(getName(), std::move(newFd), getConnectionToken()); } -void InputChannel::copyTo(InputChannel& outChannel) const { - outChannel.mName = getName(); - outChannel.mFd = dupFd(); - outChannel.mToken = getConnectionToken(); -} - -status_t InputChannel::writeToParcel(android::Parcel* parcel) const { - if (parcel == nullptr) { - ALOGE("%s: Null parcel", __func__); - return BAD_VALUE; - } - return parcel->writeStrongBinder(mToken) - ?: parcel->writeUtf8AsUtf16(mName) ?: parcel->writeUniqueFileDescriptor(mFd); -} - -status_t InputChannel::readFromParcel(const android::Parcel* parcel) { - if (parcel == nullptr) { - ALOGE("%s: Null parcel", __func__); - return BAD_VALUE; - } - mToken = parcel->readStrongBinder(); - return parcel->readUtf8FromUtf16(&mName) ?: parcel->readUniqueFileDescriptor(&mFd); +void InputChannel::copyTo(android::os::InputChannelCore& outChannel) const { + outChannel.name = getName(); + outChannel.fd.reset(dupChannelFd(fd.get())); + outChannel.token = getConnectionToken(); } sp InputChannel::getConnectionToken() const { - return mToken; -} - -base::unique_fd InputChannel::dupFd() const { - base::unique_fd newFd(::dup(getFd().get())); - if (!newFd.ok()) { - ALOGE("Could not duplicate fd %i for channel %s: %s", getFd().get(), getName().c_str(), - strerror(errno)); - const bool hitFdLimit = errno == EMFILE || errno == ENFILE; - // If this process is out of file descriptors, then throwing that might end up exploding - // on the other side of a binder call, which isn't really helpful. - // Better to just crash here and hope that the FD leak is slow. - // Other failures could be client errors, so we still propagate those back to the caller. - LOG_ALWAYS_FATAL_IF(hitFdLimit, "Too many open files, could not duplicate input channel %s", - getName().c_str()); - return {}; - } - return newFd; + return token; } // --- InputPublisher --- diff --git a/libs/input/android/InputChannel.aidl b/libs/input/android/InputChannel.aidl deleted file mode 100644 index c2d1112dd3..0000000000 --- a/libs/input/android/InputChannel.aidl +++ /dev/null @@ -1,20 +0,0 @@ -/* //device/java/android/android/view/InputChannel.aidl -** -** Copyright 2020, 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. -*/ - -package android; - -parcelable InputChannel cpp_header "input/InputTransport.h"; diff --git a/libs/input/android/os/IInputFlinger.aidl b/libs/input/android/os/IInputFlinger.aidl index 00ebd4d34f..c1aacfbc53 100644 --- a/libs/input/android/os/IInputFlinger.aidl +++ b/libs/input/android/os/IInputFlinger.aidl @@ -16,14 +16,13 @@ package android.os; -import android.InputChannel; +import android.os.InputChannelCore; import android.gui.FocusRequest; -import android.gui.WindowInfo; /** @hide */ interface IInputFlinger { - InputChannel createInputChannel(in @utf8InCpp String name); + InputChannelCore createInputChannel(in @utf8InCpp String name); void removeInputChannel(in IBinder connectionToken); /** * Sets focus to the window identified by the token. This must be called diff --git a/libs/input/android/os/InputChannelCore.aidl b/libs/input/android/os/InputChannelCore.aidl new file mode 100644 index 0000000000..888a553268 --- /dev/null +++ b/libs/input/android/os/InputChannelCore.aidl @@ -0,0 +1,30 @@ +/* //device/java/android/android/view/InputChannel.aidl +** +** Copyright 2020, 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. +*/ + +package android.os; + +import android.os.ParcelFileDescriptor; + +/** + * Input channel struct for sending InputChannel between processes. + * @hide + */ +parcelable InputChannelCore { + @utf8InCpp String name; + ParcelFileDescriptor fd; + IBinder token; +} diff --git a/libs/input/tests/InputChannel_test.cpp b/libs/input/tests/InputChannel_test.cpp index 650c93034c..60feb53dcc 100644 --- a/libs/input/tests/InputChannel_test.cpp +++ b/libs/input/tests/InputChannel_test.cpp @@ -32,37 +32,31 @@ namespace android { +namespace { +bool operator==(const InputChannel& left, const InputChannel& right) { + struct stat lhs, rhs; + if (fstat(left.getFd(), &lhs) != 0) { + return false; + } + if (fstat(right.getFd(), &rhs) != 0) { + return false; + } + // If file descriptors are pointing to same inode they are duplicated fds. + return left.getName() == right.getName() && + left.getConnectionToken() == right.getConnectionToken() && lhs.st_ino == rhs.st_ino; +} +} // namespace + class InputChannelTest : public testing::Test { }; +TEST_F(InputChannelTest, ClientAndServerTokensMatch) { + std::unique_ptr serverChannel, clientChannel; -TEST_F(InputChannelTest, ConstructorAndDestructor_TakesOwnershipOfFileDescriptors) { - // Our purpose here is to verify that the input channel destructor closes the - // file descriptor provided to it. One easy way is to provide it with one end - // of a pipe and to check for EPIPE on the other end after the channel is destroyed. - Pipe pipe; - - android::base::unique_fd sendFd(pipe.sendFd); - - std::unique_ptr inputChannel = - InputChannel::create("channel name", std::move(sendFd), new BBinder()); - - EXPECT_NE(inputChannel, nullptr) << "channel should be successfully created"; - EXPECT_STREQ("channel name", inputChannel->getName().c_str()) - << "channel should have provided name"; - EXPECT_NE(-1, inputChannel->getFd()) << "channel should have valid fd"; - - // InputChannel should be the owner of the file descriptor now - ASSERT_FALSE(sendFd.ok()); -} - -TEST_F(InputChannelTest, SetAndGetToken) { - Pipe pipe; - sp token = new BBinder(); - std::unique_ptr channel = - InputChannel::create("test channel", android::base::unique_fd(pipe.sendFd), token); - - EXPECT_EQ(token, channel->getConnectionToken()); + status_t result = + InputChannel::openInputChannelPair("channel name", serverChannel, clientChannel); + ASSERT_EQ(OK, result) << "should have successfully opened a channel pair"; + EXPECT_EQ(serverChannel->getConnectionToken(), clientChannel->getConnectionToken()); } TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { @@ -71,8 +65,7 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { status_t result = InputChannel::openInputChannelPair("channel name", serverChannel, clientChannel); - ASSERT_EQ(OK, result) - << "should have successfully opened a channel pair"; + ASSERT_EQ(OK, result) << "should have successfully opened a channel pair"; // Name EXPECT_STREQ("channel name (server)", serverChannel->getName().c_str()) @@ -235,25 +228,6 @@ TEST_F(InputChannelTest, SendAndReceive_MotionClassification) { } } -TEST_F(InputChannelTest, InputChannelParcelAndUnparcel) { - std::unique_ptr serverChannel, clientChannel; - - status_t result = - InputChannel::openInputChannelPair("channel parceling", serverChannel, clientChannel); - - ASSERT_EQ(OK, result) << "should have successfully opened a channel pair"; - - InputChannel chan; - Parcel parcel; - ASSERT_EQ(OK, serverChannel->writeToParcel(&parcel)); - parcel.setDataPosition(0); - chan.readFromParcel(&parcel); - - EXPECT_EQ(chan == *serverChannel, true) - << "inputchannel should be equal after parceling and unparceling.\n" - << "name " << chan.getName() << " name " << serverChannel->getName(); -} - TEST_F(InputChannelTest, DuplicateChannelAndAssertEqual) { std::unique_ptr serverChannel, clientChannel; diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp index 4863513eb5..823df67d16 100644 --- a/services/inputflinger/InputManager.cpp +++ b/services/inputflinger/InputManager.cpp @@ -260,13 +260,16 @@ void InputManager::dump(std::string& dump) { } // Used by tests only. -binder::Status InputManager::createInputChannel(const std::string& name, InputChannel* outChannel) { +binder::Status InputManager::createInputChannel(const std::string& name, + android::os::InputChannelCore* outChannel) { IPCThreadState* ipc = IPCThreadState::self(); - const int uid = ipc->getCallingUid(); + const uid_t uid = ipc->getCallingUid(); if (uid != AID_SHELL && uid != AID_ROOT) { - ALOGE("Invalid attempt to register input channel over IPC" - "from non shell/root entity (PID: %d)", ipc->getCallingPid()); - return binder::Status::ok(); + LOG(ERROR) << __func__ << " can only be called by SHELL or ROOT users, " + << "but was called from UID " << uid; + return binder::Status:: + fromExceptionCode(EX_SECURITY, + "This uid is not allowed to call createInputChannel"); } base::Result> channel = mDispatcher->createInputChannel(name); diff --git a/services/inputflinger/InputManager.h b/services/inputflinger/InputManager.h index df944ef807..c479aaf9b6 100644 --- a/services/inputflinger/InputManager.h +++ b/services/inputflinger/InputManager.h @@ -136,7 +136,8 @@ public: void dump(std::string& dump) override; status_t dump(int fd, const Vector& args) override; - binder::Status createInputChannel(const std::string& name, InputChannel* outChannel) override; + binder::Status createInputChannel(const std::string& name, + android::os::InputChannelCore* outChannel) override; binder::Status removeInputChannel(const sp& connectionToken) override; binder::Status setFocusedWindow(const gui::FocusRequest&) override; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index c349a5857f..47560c8aa3 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -5827,7 +5827,7 @@ void InputDispatcher::dumpDispatchStateLocked(std::string& dump) const { for (const auto& [token, connection] : mConnectionsByToken) { dump += StringPrintf(INDENT2 "%i: channelName='%s', windowName='%s', " "status=%s, monitor=%s, responsive=%s\n", - connection->inputChannel->getFd().get(), + connection->inputChannel->getFd(), connection->getInputChannelName().c_str(), connection->getWindowName().c_str(), ftl::enum_string(connection->status).c_str(), @@ -5914,7 +5914,7 @@ Result> InputDispatcher::createInputChannel(const { // acquire lock std::scoped_lock _l(mLock); const sp& token = serverChannel->getConnectionToken(); - auto&& fd = serverChannel->getFd(); + const int fd = serverChannel->getFd(); std::shared_ptr connection = std::make_shared(std::move(serverChannel), /*monitor=*/false, mIdGenerator); @@ -5927,7 +5927,7 @@ Result> InputDispatcher::createInputChannel(const std::function callback = std::bind(&InputDispatcher::handleReceiveCallback, this, std::placeholders::_1, token); - mLooper->addFd(fd.get(), 0, ALOOPER_EVENT_INPUT, sp::make(callback), + mLooper->addFd(fd, 0, ALOOPER_EVENT_INPUT, sp::make(callback), nullptr); } // release lock @@ -5957,7 +5957,7 @@ Result> InputDispatcher::createInputMonitor(int32_ std::shared_ptr connection = std::make_shared(serverChannel, /*monitor=*/true, mIdGenerator); const sp& token = serverChannel->getConnectionToken(); - auto&& fd = serverChannel->getFd(); + const int fd = serverChannel->getFd(); if (mConnectionsByToken.find(token) != mConnectionsByToken.end()) { ALOGE("Created a new connection, but the token %p is already known", token.get()); @@ -5968,7 +5968,7 @@ Result> InputDispatcher::createInputMonitor(int32_ mGlobalMonitorsByDisplay[displayId].emplace_back(serverChannel, pid); - mLooper->addFd(fd.get(), 0, ALOOPER_EVENT_INPUT, sp::make(callback), + mLooper->addFd(fd, 0, ALOOPER_EVENT_INPUT, sp::make(callback), nullptr); } @@ -6007,7 +6007,7 @@ status_t InputDispatcher::removeInputChannelLocked(const sp& connection removeMonitorChannelLocked(connectionToken); } - mLooper->removeFd(connection->inputChannel->getFd().get()); + mLooper->removeFd(connection->inputChannel->getFd()); nsecs_t currentTime = now(); abortBrokenDispatchCycleLocked(currentTime, connection, notify); diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index f1f4a61e73..271e462cec 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -1113,7 +1113,7 @@ public: sp getToken() { return mConsumer.getChannel()->getConnectionToken(); } - int getChannelFd() { return mConsumer.getChannel()->getFd().get(); } + int getChannelFd() { return mConsumer.getChannel()->getFd(); } private: InputConsumer mConsumer; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index aa6be3634d..776eccbed5 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5733,7 +5733,7 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, gui::CreateSurface case ISurfaceComposerClient::eFXSurfaceContainer: case ISurfaceComposerClient::eFXSurfaceBufferState: args.flags |= ISurfaceComposerClient::eNoColorFill; - FMT_FALLTHROUGH; + [[fallthrough]]; case ISurfaceComposerClient::eFXSurfaceEffect: { result = createBufferStateLayer(args, &outResult.handle, &layer); std::atomic* pendingBufferCounter = layer->getPendingBufferCounter(); -- cgit v1.2.3-59-g8ed1b From 7b9f4f531f7ef28d71268d6b9ac475717f04e895 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Fri, 2 Feb 2024 13:07:16 -0800 Subject: Pass unique_ptr of InputChannel to Connection This gets us closer to the goal of storing unique_ptr of InputChannel inside the InputPublisher. In this CL, we are still allowing the "copy" and "dup" functions to exist. Removing those requires significant refactoring on the Java side, including the jni layer and object ownership models. Test: TEST=inputflinger_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Bug: 161009324 Change-Id: If6f44d78c7fc9f3e12729068de1753847abf0ebb --- include/input/InputTransport.h | 11 ++++++++++- libs/input/InputTransport.cpp | 9 ++++++++- .../input/tests/InputPublisherAndConsumer_test.cpp | 13 +++---------- services/inputflinger/InputManager.cpp | 2 +- services/inputflinger/dispatcher/Connection.cpp | 6 +++--- services/inputflinger/dispatcher/Connection.h | 4 ++-- .../inputflinger/dispatcher/InputDispatcher.cpp | 22 +++++++--------------- 7 files changed, 34 insertions(+), 33 deletions(-) (limited to 'services/inputflinger/InputManager.cpp') diff --git a/include/input/InputTransport.h b/include/input/InputTransport.h index d53e8c6cce..42dcd3c394 100644 --- a/include/input/InputTransport.h +++ b/include/input/InputTransport.h @@ -300,6 +300,15 @@ public: void copyTo(android::os::InputChannelCore& outChannel) const; + /** + * Similar to "copyTo", but it takes ownership of the provided InputChannel (and after this is + * called, it destroys it). + * @param from the InputChannel that should be converted to InputChannelCore + * @param outChannel the pre-allocated InputChannelCore to which to transfer the 'from' channel + */ + static void moveChannel(std::unique_ptr from, + android::os::InputChannelCore& outChannel); + /** * The connection token is used to identify the input connection, i.e. * the pair of input channels that were created simultaneously. Input channels @@ -333,7 +342,7 @@ public: ~InputPublisher(); /* Gets the underlying input channel. */ - inline std::shared_ptr getChannel() const { return mChannel; } + inline InputChannel& getChannel() const { return *mChannel; } /* Publishes a key event to the input channel. * diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp index 0e0e80dc79..e49f4eb6f6 100644 --- a/libs/input/InputTransport.cpp +++ b/libs/input/InputTransport.cpp @@ -584,6 +584,13 @@ void InputChannel::copyTo(android::os::InputChannelCore& outChannel) const { outChannel.token = getConnectionToken(); } +void InputChannel::moveChannel(std::unique_ptr from, + android::os::InputChannelCore& outChannel) { + outChannel.name = from->getName(); + outChannel.fd = android::os::ParcelFileDescriptor(std::move(from->fd)); + outChannel.token = from->getConnectionToken(); +} + sp InputChannel::getConnectionToken() const { return token; } @@ -591,7 +598,7 @@ sp InputChannel::getConnectionToken() const { // --- InputPublisher --- InputPublisher::InputPublisher(const std::shared_ptr& channel) - : mChannel(channel), mInputVerifier(channel->getName()) {} + : mChannel(channel), mInputVerifier(mChannel->getName()) {} InputPublisher::~InputPublisher() { } diff --git a/libs/input/tests/InputPublisherAndConsumer_test.cpp b/libs/input/tests/InputPublisherAndConsumer_test.cpp index 2000335521..35430207f9 100644 --- a/libs/input/tests/InputPublisherAndConsumer_test.cpp +++ b/libs/input/tests/InputPublisherAndConsumer_test.cpp @@ -220,7 +220,6 @@ void waitUntilInputAvailable(const InputConsumer& inputConsumer) { class InputPublisherAndConsumerTest : public testing::Test { protected: - std::shared_ptr mServerChannel, mClientChannel; std::unique_ptr mPublisher; std::unique_ptr mConsumer; PreallocatedInputEventFactory mEventFactory; @@ -230,11 +229,9 @@ protected: status_t result = InputChannel::openInputChannelPair("channel name", serverChannel, clientChannel); ASSERT_EQ(OK, result); - mServerChannel = std::move(serverChannel); - mClientChannel = std::move(clientChannel); - mPublisher = std::make_unique(mServerChannel); - mConsumer = std::make_unique(mClientChannel); + mPublisher = std::make_unique(std::move(serverChannel)); + mConsumer = std::make_unique(std::move(clientChannel)); } void publishAndConsumeKeyEvent(); @@ -254,11 +251,7 @@ private: }; TEST_F(InputPublisherAndConsumerTest, GetChannel_ReturnsTheChannel) { - ASSERT_NE(nullptr, mPublisher->getChannel()); - ASSERT_NE(nullptr, mConsumer->getChannel()); - EXPECT_EQ(mServerChannel.get(), mPublisher->getChannel().get()); - EXPECT_EQ(mClientChannel.get(), mConsumer->getChannel().get()); - ASSERT_EQ(mPublisher->getChannel()->getConnectionToken(), + ASSERT_EQ(mPublisher->getChannel().getConnectionToken(), mConsumer->getChannel()->getConnectionToken()); } diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp index 823df67d16..ae066c0f4a 100644 --- a/services/inputflinger/InputManager.cpp +++ b/services/inputflinger/InputManager.cpp @@ -277,7 +277,7 @@ binder::Status InputManager::createInputChannel(const std::string& name, return binder::Status::fromExceptionCode(exceptionCodeFromStatusT(channel.error().code()), channel.error().message().c_str()); } - (*channel)->copyTo(*outChannel); + InputChannel::moveChannel(std::move(*channel), *outChannel); return binder::Status::ok(); } diff --git a/services/inputflinger/dispatcher/Connection.cpp b/services/inputflinger/dispatcher/Connection.cpp index c7963c0512..9dee66f0f8 100644 --- a/services/inputflinger/dispatcher/Connection.cpp +++ b/services/inputflinger/dispatcher/Connection.cpp @@ -20,15 +20,15 @@ namespace android::inputdispatcher { -Connection::Connection(const std::shared_ptr& inputChannel, bool monitor, +Connection::Connection(std::unique_ptr inputChannel, bool monitor, const IdGenerator& idGenerator) : status(Status::NORMAL), monitor(monitor), - inputPublisher(inputChannel), + inputPublisher(std::move(inputChannel)), inputState(idGenerator) {} sp Connection::getToken() const { - return inputPublisher.getChannel()->getConnectionToken(); + return inputPublisher.getChannel().getConnectionToken(); }; } // namespace android::inputdispatcher diff --git a/services/inputflinger/dispatcher/Connection.h b/services/inputflinger/dispatcher/Connection.h index 8d7f182a7d..a834a8cf86 100644 --- a/services/inputflinger/dispatcher/Connection.h +++ b/services/inputflinger/dispatcher/Connection.h @@ -58,11 +58,11 @@ public: // yet received a "finished" response from the application. std::deque> waitQueue; - Connection(const std::shared_ptr& inputChannel, bool monitor, + Connection(std::unique_ptr inputChannel, bool monitor, const IdGenerator& idGenerator); inline const std::string getInputChannelName() const { - return inputPublisher.getChannel()->getName(); + return inputPublisher.getChannel().getName(); } sp getToken() const; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index a62e23a930..af72eb9bcb 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -436,15 +436,6 @@ std::unique_ptr createDispatchEntry(const InputTarget& inputTarge return dispatchEntry; } -status_t openInputChannelPair(const std::string& name, std::shared_ptr& serverChannel, - std::unique_ptr& clientChannel) { - std::unique_ptr uniqueServerChannel; - status_t result = InputChannel::openInputChannelPair(name, uniqueServerChannel, clientChannel); - - serverChannel = std::move(uniqueServerChannel); - return result; -} - template bool sharedPointersEqual(const std::shared_ptr& lhs, const std::shared_ptr& rhs) { if (lhs == nullptr && rhs == nullptr) { @@ -5805,7 +5796,7 @@ void InputDispatcher::dumpDispatchStateLocked(std::string& dump) const { for (const auto& [token, connection] : mConnectionsByToken) { dump += StringPrintf(INDENT2 "%i: channelName='%s', " "status=%s, monitor=%s, responsive=%s\n", - connection->inputPublisher.getChannel()->getFd(), + connection->inputPublisher.getChannel().getFd(), connection->getInputChannelName().c_str(), ftl::enum_string(connection->status).c_str(), toString(connection->monitor), toString(connection->responsive)); @@ -5916,9 +5907,9 @@ Result> InputDispatcher::createInputChannel(const Result> InputDispatcher::createInputMonitor(int32_t displayId, const std::string& name, gui::Pid pid) { - std::shared_ptr serverChannel; + std::unique_ptr serverChannel; std::unique_ptr clientChannel; - status_t result = openInputChannelPair(name, serverChannel, clientChannel); + status_t result = InputChannel::openInputChannelPair(name, serverChannel, clientChannel); if (result) { return base::Error(result) << "Failed to open input channel pair with name " << name; } @@ -5931,10 +5922,11 @@ Result> InputDispatcher::createInputMonitor(int32_ << " without a specified display."; } - std::shared_ptr connection = - std::make_shared(serverChannel, /*monitor=*/true, mIdGenerator); const sp& token = serverChannel->getConnectionToken(); const int fd = serverChannel->getFd(); + std::shared_ptr connection = + std::make_shared(std::move(serverChannel), /*monitor=*/true, + mIdGenerator); auto [_, inserted] = mConnectionsByToken.emplace(token, connection); if (!inserted) { @@ -5985,7 +5977,7 @@ status_t InputDispatcher::removeInputChannelLocked(const sp& connection removeMonitorChannelLocked(connectionToken); } - mLooper->removeFd(connection->inputPublisher.getChannel()->getFd()); + mLooper->removeFd(connection->inputPublisher.getChannel().getFd()); nsecs_t currentTime = now(); abortBrokenDispatchCycleLocked(currentTime, connection, notify); -- cgit v1.2.3-59-g8ed1b