Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a0d32afb authored by Egor Pasko's avatar Egor Pasko
Browse files

Expose InputConsumer::probablyHasInput 

Introduce InputConsumer::probablyHasInput() to check for availability of
input events that have not been consumed yet. It performs a poll on the
underlying file descriptor.

The return value is precise (modulo rare syscall interruptions), but
clients should handle it as a performance hint. This hint would allow to
prioritize input-related work, but only when input events are *likely*
pending.

Usage constraints for a future/possible API in the Android Framework:

0. This is experimental. The usefulness of the hint has not been
   broadly proven, and the functionality can be removed soon if there is
   no robust data to support the usefulness.

1. The high-level API visible to apps would be effectively opt-in: does
   not create any state if not used.

2. An app could use this hint to yield from longer tasks on the UI
   thread (which it is not supposed to create in the first place). This
   is a very niche workaround for Chrome's complexity.

3. Using the Input Hint in WebView should probably be disabled: WebView
   has less control over the embedding app than Chrome, and less
   knowledge when to prioritize for input handling.

4. There is a good question whether probablyHasInput() should guarantee
   to return true only when input is queued. While it is good to return
   precise results, clients should only use this API as an optimization
   hint without sacrificing correctness. Due to the growing complexity
   of input handling on Android, it is not too hard to imagine that in
   some not-too-distant future input would get filtered by the Framework
   on the application side, potentially skipping the contents from the
   ready-to-read file descriptor or an accumulated InputConsumer batch.

Bug: b/314973388
Test: atest 'InputChannelTest#ProbablyHasInput'
Test: atest InputPublisherAndConsumerTest
Change-Id: I9681a9ddd0249e3bfd5285c6570dee7a9f5de951
parent 8e185db6

include/input/InputTransport.h

+14 −0
Original line number Diff line number Diff line
@@ -283,6 +283,13 @@ public: 
     */

    status_t receiveMessage(InputMessage* msg);



    /* Tells whether there is a message in the channel available to be received.

     *

     * This is only a performance hint and may return false negative results. Clients should not

     * rely on availability of the message based on the return value.

     */

    bool probablyHasInput() const;



    /* Return a new object that has a duplicate of this channel's fd. */

    std::unique_ptr<InputChannel> dup() const;
@@ -518,6 +525,13 @@ public: 
     */

    int32_t getPendingBatchSource() const;



    /* Returns true when there is *likely* a pending batch or a pending event in the channel.

     *

     * This is only a performance hint and may return false negative results. Clients should not

     * rely on availability of the message based on the return value.

     */

    bool probablyHasInput() const;



    std::string dump() const;



private:

libs/input/InputTransport.cpp

+21 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ 
#include <fcntl.h>

#include <inttypes.h>

#include <math.h>

#include <poll.h>

#include <sys/socket.h>

#include <sys/types.h>

#include <unistd.h>
@@ -517,6 +518,22 @@ status_t InputChannel::receiveMessage(InputMessage* msg) { 
    return OK;

}



bool InputChannel::probablyHasInput() const {

    struct pollfd pfds = {.fd = mFd, .events = POLLIN};

    if (::poll(&pfds, /*nfds=*/1, /*timeout=*/0) <= 0) {

        // This can be a false negative because EAGAIN and ENOMEM are not handled. The latter should

        // be extremely rare. The EAGAIN is also unlikely because it happens only when the signal

        // arrives while the syscall is executed, and the syscall is quick. Hitting EAGAIN too often

        // would be a sign of having too many signals, which is a bigger performance problem. A

        // common tradition is to repeat the syscall on each EAGAIN, but it is not necessary here.

        // In other words, the missing one liner is replaced by a multiline explanation.

        return false;

    }

    // From poll(2): The bits returned in |revents| can include any of those specified in |events|,

    // or one of the values POLLERR, POLLHUP, or POLLNVAL.

    return (pfds.revents & POLLIN) != 0;

}



std::unique_ptr<InputChannel> InputChannel::dup() const {

    base::unique_fd newFd(dupFd());

    return InputChannel::create(getName(), std::move(newFd), getConnectionToken());
@@ -1406,6 +1423,10 @@ int32_t InputConsumer::getPendingBatchSource() const { 
    return head.body.motion.source;

}



bool InputConsumer::probablyHasInput() const {

    return hasPendingBatch() || mChannel->probablyHasInput();

}



ssize_t InputConsumer::findBatch(int32_t deviceId, int32_t source) const {

    for (size_t i = 0; i < mBatches.size(); i++) {

        const Batch& batch = mBatches[i];

libs/input/tests/InputChannel_test.cpp

+44 −4
Original line number Diff line number Diff line
@@ -81,8 +81,7 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { 
            << "client channel should have suffixed name";



    // Server->Client communication

    InputMessage serverMsg;

    memset(&serverMsg, 0, sizeof(InputMessage));

    InputMessage serverMsg = {};

    serverMsg.header.type = InputMessage::Type::KEY;

    serverMsg.body.key.action = AKEY_EVENT_ACTION_DOWN;

    EXPECT_EQ(OK, serverChannel->sendMessage(&serverMsg))
@@ -97,8 +96,7 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { 
            << "client channel should receive the correct message from server channel";



    // Client->Server communication

    InputMessage clientReply;

    memset(&clientReply, 0, sizeof(InputMessage));

    InputMessage clientReply = {};

    clientReply.header.type = InputMessage::Type::FINISHED;

    clientReply.header.seq = 0x11223344;

    clientReply.body.finished.handled = true;
@@ -116,6 +114,48 @@ TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) { 
            << "server channel should receive the correct message from client channel";

}



TEST_F(InputChannelTest, ProbablyHasInput) {

    std::unique_ptr<InputChannel> senderChannel, receiverChannel;



    // Open a pair of channels.

    status_t result =

            InputChannel::openInputChannelPair("channel name", senderChannel, receiverChannel);

    ASSERT_EQ(OK, result) << "should have successfully opened a channel pair";



    ASSERT_FALSE(receiverChannel->probablyHasInput());



    // Send one message.

    InputMessage serverMsg = {};

    serverMsg.header.type = InputMessage::Type::KEY;

    serverMsg.body.key.action = AKEY_EVENT_ACTION_DOWN;

    EXPECT_EQ(OK, senderChannel->sendMessage(&serverMsg))

            << "server channel should be able to send message to client channel";



    // Verify input is available.

    bool hasInput = false;

    do {

        // The probablyHasInput() can return false positive under rare circumstances uncontrollable

        // by the tests. Re-request the availability in this case. Returning |false| for a long

        // time is not intended, and would cause a test timeout.

        hasInput = receiverChannel->probablyHasInput();

    } while (!hasInput);

    EXPECT_TRUE(hasInput)

            << "client channel should observe that message is available before receiving it";



    // Receive (consume) the message.

    InputMessage clientMsg;

    EXPECT_EQ(OK, receiverChannel->receiveMessage(&clientMsg))

            << "client channel should be able to receive message from server channel";

    EXPECT_EQ(serverMsg.header.type, clientMsg.header.type)

            << "client channel should receive the correct message from server channel";

    EXPECT_EQ(serverMsg.body.key.action, clientMsg.body.key.action)

            << "client channel should receive the correct message from server channel";



    // Verify input is not available.

    EXPECT_FALSE(receiverChannel->probablyHasInput())

            << "client should not observe any more messages after receiving the single one";

}



TEST_F(InputChannelTest, ReceiveSignal_WhenNoSignalPresent_ReturnsAnError) {

    std::unique_ptr<InputChannel> serverChannel, clientChannel;

libs/input/tests/InputPublisherAndConsumer_test.cpp

+221 −166

File changed.

Preview size limit exceeded, changes collapsed.