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(-) 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