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

Commit 7cce0ad0 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Revert "Use Result<InputPublisher::Finished> instead of callback" 

Revert submission 13780058-receiveFinishedSignal

Reason for revert:
Caused severe delay in back navigation on IME-focusable window.

Reverted Changes:
I301c6e9c3:Use Result<InputPublisher::Finished> instead of ca...
I43a0f2d31:Update the usage of receiveFinishedSignal

Bug: 167947340
Fix: 182514338
Test: Manually verified as follows:
 1. Set up the device as "Set up offline" mode.
 2. adb shell am start -n com.google.android.dialer/.extensions.GoogleDialtactsActivity
 3. On one terminal, run adb logcat -s InputMethodManager:*
 4. On another terminal, run adb shell input keyevent 4
 5. Make sure that the following message is not shown.
  "Timeout waiting for IME to handle input event after 2500 ms"

Change-Id: I1e71010f5f4ae268dfcbc3bde50881c2fa3d51d5
parent 4c92c5f6

include/input/InputTransport.h

+9 −14
Original line number Diff line number Diff line
@@ -33,7 +33,6 @@ 
#include <unordered_map>



#include <android-base/chrono_utils.h>

#include <android-base/result.h>

#include <android-base/unique_fd.h>



#include <binder/IBinder.h>
@@ -375,24 +374,20 @@ public: 
     */

    status_t publishDragEvent(uint32_t seq, int32_t eventId, float x, float y, bool isExiting);



    struct Finished {

        uint32_t seq;

        bool handled;

        nsecs_t consumeTime;

    };



    /* Receives the finished signal from the consumer in reply to the original dispatch signal.

     * If a signal was received, returns a Finished object.

     * If a signal was received, returns the message sequence number,

     * whether the consumer handled the message, and the time the event was first read by the

     * consumer.

     *

     * The returned sequence number is never 0 unless the operation failed.

     *

     * Returned error codes:

     *         OK on success.

     *         WOULD_BLOCK if there is no signal present.

     *         DEAD_OBJECT if the channel's peer has been closed.

     * Returns OK on success.

     * Returns WOULD_BLOCK if there is no signal present.

     * Returns DEAD_OBJECT if the channel's peer has been closed.

     * Other errors probably indicate that the channel is broken.

     */

    android::base::Result<Finished> receiveFinishedSignal();

    status_t receiveFinishedSignal(

            const std::function<void(uint32_t seq, bool handled, nsecs_t consumeTime)>& callback);



private:

    std::shared_ptr<InputChannel> mChannel;

libs/input/InputTransport.cpp

+7 −9
Original line number Diff line number Diff line
@@ -629,26 +629,24 @@ status_t InputPublisher::publishDragEvent(uint32_t seq, int32_t eventId, float x 
    return mChannel->sendMessage(&msg);

}



android::base::Result<InputPublisher::Finished> InputPublisher::receiveFinishedSignal() {

status_t InputPublisher::receiveFinishedSignal(

        const std::function<void(uint32_t seq, bool handled, nsecs_t consumeTime)>& callback) {

    if (DEBUG_TRANSPORT_ACTIONS) {

        ALOGD("channel '%s' publisher ~ %s", mChannel->getName().c_str(), __func__);

        ALOGD("channel '%s' publisher ~ receiveFinishedSignal", mChannel->getName().c_str());

    }



    InputMessage msg;

    status_t result = mChannel->receiveMessage(&msg);

    if (result) {

        return android::base::Error(result);

        return result;

    }

    if (msg.header.type != InputMessage::Type::FINISHED) {

        ALOGE("channel '%s' publisher ~ Received unexpected %s message from consumer",

              mChannel->getName().c_str(), NamedEnum::string(msg.header.type).c_str());

        return android::base::Error(UNKNOWN_ERROR);

        return UNKNOWN_ERROR;

    }

    return Finished{

            .seq = msg.header.seq,

            .handled = msg.body.finished.handled,

            .consumeTime = msg.body.finished.consumeTime,

    };

    callback(msg.header.seq, msg.body.finished.handled, msg.body.finished.consumeTime);

    return OK;

}



// --- InputConsumer ---

libs/input/tests/InputPublisherAndConsumer_test.cpp

+85 −41
Original line number Diff line number Diff line
@@ -27,8 +27,6 @@ 
#include <utils/StopWatch.h>

#include <utils/Timers.h>



using android::base::Result;



namespace android {



class InputPublisherAndConsumerTest : public testing::Test {
@@ -124,13 +122,23 @@ void InputPublisherAndConsumerTest::PublishAndConsumeKeyEvent() { 
    ASSERT_EQ(OK, status)

            << "consumer sendFinishedSignal should return OK";



    Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();

    ASSERT_TRUE(result.ok()) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, result->seq)

            << "receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(result->handled)

            << "receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(result->consumeTime, publishTime)

    uint32_t finishedSeq = 0;

    bool handled = false;

    nsecs_t consumeTime;

    status = mPublisher->receiveFinishedSignal(

            [&finishedSeq, &handled, &consumeTime](uint32_t inSeq, bool inHandled,

                                                   nsecs_t inConsumeTime) -> void {

                finishedSeq = inSeq;

                handled = inHandled;

                consumeTime = inConsumeTime;

            });

    ASSERT_EQ(OK, status)

            << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, finishedSeq)

            << "publisher receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(handled)

            << "publisher receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(consumeTime, publishTime)

            << "finished signal's consume time should be greater than publish time";

}
@@ -264,13 +272,23 @@ void InputPublisherAndConsumerTest::PublishAndConsumeMotionEvent() { 
    ASSERT_EQ(OK, status)

            << "consumer sendFinishedSignal should return OK";



    Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();

    ASSERT_TRUE(result.ok()) << "receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, result->seq)

            << "receiveFinishedSignal should have returned the original sequence number";

    ASSERT_FALSE(result->handled)

            << "receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(result->consumeTime, publishTime)

    uint32_t finishedSeq = 0;

    bool handled = true;

    nsecs_t consumeTime;

    status = mPublisher->receiveFinishedSignal(

            [&finishedSeq, &handled, &consumeTime](uint32_t inSeq, bool inHandled,

                                                   nsecs_t inConsumeTime) -> void {

                finishedSeq = inSeq;

                handled = inHandled;

                consumeTime = inConsumeTime;

            });

    ASSERT_EQ(OK, status)

            << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, finishedSeq)

            << "publisher receiveFinishedSignal should have returned the original sequence number";

    ASSERT_FALSE(handled)

            << "publisher receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(consumeTime, publishTime)

            << "finished signal's consume time should be greater than publish time";

}
@@ -304,14 +322,22 @@ void InputPublisherAndConsumerTest::PublishAndConsumeFocusEvent() { 
    status = mConsumer->sendFinishedSignal(seq, true);

    ASSERT_EQ(OK, status) << "consumer sendFinishedSignal should return OK";



    Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();



    ASSERT_TRUE(result.ok()) << "receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, result->seq)

            << "receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(result->handled)

            << "receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(result->consumeTime, publishTime)

    uint32_t finishedSeq = 0;

    bool handled = false;

    nsecs_t consumeTime;

    status = mPublisher->receiveFinishedSignal(

            [&finishedSeq, &handled, &consumeTime](uint32_t inSeq, bool inHandled,

                                                   nsecs_t inConsumeTime) -> void {

                finishedSeq = inSeq;

                handled = inHandled;

                consumeTime = inConsumeTime;

            });

    ASSERT_EQ(OK, status) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, finishedSeq)

            << "publisher receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(handled)

            << "publisher receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(consumeTime, publishTime)

            << "finished signal's consume time should be greater than publish time";

}
@@ -343,13 +369,22 @@ void InputPublisherAndConsumerTest::PublishAndConsumeCaptureEvent() { 
    status = mConsumer->sendFinishedSignal(seq, true);

    ASSERT_EQ(OK, status) << "consumer sendFinishedSignal should return OK";



    android::base::Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();

    ASSERT_TRUE(result.ok()) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, result->seq)

            << "receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(result->handled)

            << "receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(result->consumeTime, publishTime)

    uint32_t finishedSeq = 0;

    bool handled = false;

    nsecs_t consumeTime;

    status = mPublisher->receiveFinishedSignal(

            [&finishedSeq, &handled, &consumeTime](uint32_t inSeq, bool inHandled,

                                                   nsecs_t inConsumeTime) -> void {

                finishedSeq = inSeq;

                handled = inHandled;

                consumeTime = inConsumeTime;

            });

    ASSERT_EQ(OK, status) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, finishedSeq)

            << "publisher receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(handled)

            << "publisher receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(consumeTime, publishTime)

            << "finished signal's consume time should be greater than publish time";

}
@@ -375,23 +410,32 @@ void InputPublisherAndConsumerTest::PublishAndConsumeDragEvent() { 
    ASSERT_EQ(AINPUT_EVENT_TYPE_DRAG, event->getType())

            << "consumer should have returned a drag event";



    const DragEvent& dragEvent = static_cast<const DragEvent&>(*event);

    DragEvent* dragEvent = static_cast<DragEvent*>(event);

    EXPECT_EQ(seq, consumeSeq);

    EXPECT_EQ(eventId, dragEvent.getId());

    EXPECT_EQ(isExiting, dragEvent.isExiting());

    EXPECT_EQ(x, dragEvent.getX());

    EXPECT_EQ(y, dragEvent.getY());

    EXPECT_EQ(eventId, dragEvent->getId());

    EXPECT_EQ(isExiting, dragEvent->isExiting());

    EXPECT_EQ(x, dragEvent->getX());

    EXPECT_EQ(y, dragEvent->getY());



    status = mConsumer->sendFinishedSignal(seq, true);

    ASSERT_EQ(OK, status) << "consumer sendFinishedSignal should return OK";



    android::base::Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();

    ASSERT_TRUE(result.ok()) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, result->seq)

    uint32_t finishedSeq = 0;

    bool handled = false;

    nsecs_t consumeTime;

    status = mPublisher->receiveFinishedSignal(

            [&finishedSeq, &handled, &consumeTime](uint32_t inSeq, bool inHandled,

                                                   nsecs_t inConsumeTime) -> void {

                finishedSeq = inSeq;

                handled = inHandled;

                consumeTime = inConsumeTime;

            });

    ASSERT_EQ(OK, status) << "publisher receiveFinishedSignal should return OK";

    ASSERT_EQ(seq, finishedSeq)

            << "publisher receiveFinishedSignal should have returned the original sequence number";

    ASSERT_TRUE(result->handled)

    ASSERT_TRUE(handled)

            << "publisher receiveFinishedSignal should have set handled to consumer's reply";

    ASSERT_GE(result->consumeTime, publishTime)

    ASSERT_GE(consumeTime, publishTime)

            << "finished signal's consume time should be greater than publish time";

}

services/inputflinger/dispatcher/InputDispatcher.cpp

+12 −14
Original line number Diff line number Diff line
@@ -80,7 +80,6 @@ static constexpr bool DEBUG_TOUCH_OCCLUSION = true; 
#define INDENT4 "        "



using android::base::HwTimeoutMultiplier;

using android::base::Result;

using android::base::StringPrintf;

using android::os::BlockUntrustedTouchesMode;

using android::os::IInputConstants;
@@ -3194,17 +3193,17 @@ int InputDispatcher::handleReceiveCallback(int fd, int events, void* data) { 


            nsecs_t currentTime = now();

            bool gotOne = false;

            status_t status = OK;

            status_t status;

            for (;;) {

                Result<InputPublisher::Finished> result =

                        connection->inputPublisher.receiveFinishedSignal();

                if (!result.ok()) {

                    status = result.error().code();

                std::function<void(uint32_t seq, bool handled, nsecs_t consumeTime)> callback =

                        std::bind(&InputDispatcher::finishDispatchCycleLocked, d, currentTime,

                                  connection, std::placeholders::_1, std::placeholders::_2,

                                  std::placeholders::_3);



                status = connection->inputPublisher.receiveFinishedSignal(callback);

                if (status) {

                    break;

                }

                const InputPublisher::Finished& finished = *result;

                d->finishDispatchCycleLocked(currentTime, connection, finished.seq,

                                             finished.handled, finished.consumeTime);

                gotOne = true;

            }

            if (gotOne) {
@@ -4892,7 +4891,8 @@ void InputDispatcher::dumpMonitors(std::string& dump, const std::vector<Monitor> 
    }

}



Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(const std::string& name) {

base::Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(

        const std::string& name) {

#if DEBUG_CHANNEL_CREATION

    ALOGD("channel '%s' ~ createInputChannel", name.c_str());

#endif
@@ -4921,10 +4921,8 @@ Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(const 
    return clientChannel;

}



Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputMonitor(int32_t displayId,

                                                                          bool isGestureMonitor,

                                                                          const std::string& name,

                                                                          int32_t pid) {

base::Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputMonitor(

        int32_t displayId, bool isGestureMonitor, const std::string& name, int32_t pid) {

    std::shared_ptr<InputChannel> serverChannel;

    std::unique_ptr<InputChannel> clientChannel;

    status_t result = openInputChannelPair(name, serverChannel, clientChannel);

services/inputflinger/host/Android.bp

+0 −1
Original line number Diff line number Diff line
@@ -67,7 +67,6 @@ cc_binary { 
    cflags: ["-Wall", "-Werror"],



    shared_libs: [

        "libbase",

        "libbinder",

        "libinputflingerhost",

        "libutils",