Merge "Use Result<InputPublisher::Finished> instead of callback -- try 2" into sc-dev

5 files changed, 79 insertions, 113 deletions

diff --git a/include/input/InputTransport.h b/include/input/InputTransport.h
index f1b22587ce..3e5674e4fb 100644
--- a/include/input/InputTransport.h
+++ b/include/input/InputTransport.h
@@ -33,6 +33,7 @@
 #include <unordered_map>
 
 #include <android-base/chrono_utils.h>
+#include <android-base/result.h>
 #include <android-base/unique_fd.h>
 
 #include <binder/IBinder.h>
@@ -374,20 +375,24 @@ 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 the message sequence number,
-     * whether the consumer handled the message, and the time the event was first read by the
-     * consumer.
+     * If a signal was received, returns a Finished object.
      *
      * The returned sequence number is never 0 unless the operation failed.
      *
-     * 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.
+     * Returned error codes:
+     *         OK on success.
+     *         WOULD_BLOCK if there is no signal present.
+     *         DEAD_OBJECT if the channel's peer has been closed.
+     *         Other errors probably indicate that the channel is broken.
      */
-    status_t receiveFinishedSignal(
-            const std::function<void(uint32_t seq, bool handled, nsecs_t consumeTime)>& callback);
+    android::base::Result<Finished> receiveFinishedSignal();
 
 private:
     std::shared_ptr<InputChannel> mChannel;
diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp
index 6ef01737f9..c2a3cf1816 100644
--- a/libs/input/InputTransport.cpp
+++ b/libs/input/InputTransport.cpp
@@ -629,24 +629,26 @@ status_t InputPublisher::publishDragEvent(uint32_t seq, int32_t eventId, float x
     return mChannel->sendMessage(&msg);
 }
 
-status_t InputPublisher::receiveFinishedSignal(
-        const std::function<void(uint32_t seq, bool handled, nsecs_t consumeTime)>& callback) {
+android::base::Result<InputPublisher::Finished> InputPublisher::receiveFinishedSignal() {
     if (DEBUG_TRANSPORT_ACTIONS) {
-        ALOGD("channel '%s' publisher ~ receiveFinishedSignal", mChannel->getName().c_str());
+        ALOGD("channel '%s' publisher ~ %s", mChannel->getName().c_str(), __func__);
     }
 
     InputMessage msg;
     status_t result = mChannel->receiveMessage(&msg);
     if (result) {
-        return result;
+        return android::base::Error(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 UNKNOWN_ERROR;
+        return android::base::Error(UNKNOWN_ERROR);
     }
-    callback(msg.header.seq, msg.body.finished.handled, msg.body.finished.consumeTime);
-    return OK;
+    return Finished{
+            .seq = msg.header.seq,
+            .handled = msg.body.finished.handled,
+            .consumeTime = msg.body.finished.consumeTime,
+    };
 }
 
 // --- InputConsumer ---
diff --git a/libs/input/tests/InputPublisherAndConsumer_test.cpp b/libs/input/tests/InputPublisherAndConsumer_test.cpp
index b5ed8d7ba7..fc31715668 100644
--- a/libs/input/tests/InputPublisherAndConsumer_test.cpp
+++ b/libs/input/tests/InputPublisherAndConsumer_test.cpp
@@ -27,6 +27,8 @@
 #include <utils/StopWatch.h>
 #include <utils/Timers.h>
 
+using android::base::Result;
+
 namespace android {
 
 class InputPublisherAndConsumerTest : public testing::Test {
@@ -122,23 +124,13 @@ void InputPublisherAndConsumerTest::PublishAndConsumeKeyEvent() {
     ASSERT_EQ(OK, status)
             << "consumer sendFinishedSignal should return OK";
 
-    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)
+    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)
             << "finished signal's consume time should be greater than publish time";
 }
 
@@ -272,23 +264,13 @@ void InputPublisherAndConsumerTest::PublishAndConsumeMotionEvent() {
     ASSERT_EQ(OK, status)
             << "consumer sendFinishedSignal should return OK";
 
-    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)
+    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)
             << "finished signal's consume time should be greater than publish time";
 }
 
@@ -322,22 +304,14 @@ void InputPublisherAndConsumerTest::PublishAndConsumeFocusEvent() {
     status = mConsumer->sendFinishedSignal(seq, true);
     ASSERT_EQ(OK, status) << "consumer sendFinishedSignal should return OK";
 
-    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)
+    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)
             << "finished signal's consume time should be greater than publish time";
 }
 
@@ -369,22 +343,13 @@ void InputPublisherAndConsumerTest::PublishAndConsumeCaptureEvent() {
     status = mConsumer->sendFinishedSignal(seq, true);
     ASSERT_EQ(OK, status) << "consumer sendFinishedSignal should return OK";
 
-    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)
+    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)
             << "finished signal's consume time should be greater than publish time";
 }
 
@@ -410,32 +375,23 @@ void InputPublisherAndConsumerTest::PublishAndConsumeDragEvent() {
     ASSERT_EQ(AINPUT_EVENT_TYPE_DRAG, event->getType())
             << "consumer should have returned a drag event";
 
-    DragEvent* dragEvent = static_cast<DragEvent*>(event);
+    const DragEvent& dragEvent = static_cast<const 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";
 
-    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)
+    android::base::Result<InputPublisher::Finished> result = mPublisher->receiveFinishedSignal();
+    ASSERT_TRUE(result.ok()) << "publisher receiveFinishedSignal should return OK";
+    ASSERT_EQ(seq, result->seq)
             << "publisher receiveFinishedSignal should have returned the original sequence number";
-    ASSERT_TRUE(handled)
+    ASSERT_TRUE(result->handled)
             << "publisher receiveFinishedSignal should have set handled to consumer's reply";
-    ASSERT_GE(consumeTime, publishTime)
+    ASSERT_GE(result->consumeTime, publishTime)
             << "finished signal's consume time should be greater than publish time";
 }
 
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index 3e80bd7d2c..fe46d17493 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -80,6 +80,7 @@ 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;
@@ -3283,17 +3284,17 @@ int InputDispatcher::handleReceiveCallback(int fd, int events, void* data) {
 
             nsecs_t currentTime = now();
             bool gotOne = false;
-            status_t status;
+            status_t status = OK;
             for (;;) {
-                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) {
+                Result<InputPublisher::Finished> result =
+                        connection->inputPublisher.receiveFinishedSignal();
+                if (!result.ok()) {
+                    status = result.error().code();
                     break;
                 }
+                const InputPublisher::Finished& finished = *result;
+                d->finishDispatchCycleLocked(currentTime, connection, finished.seq,
+                                             finished.handled, finished.consumeTime);
                 gotOne = true;
             }
             if (gotOne) {
@@ -4998,8 +4999,7 @@ void InputDispatcher::dumpMonitors(std::string& dump, const std::vector<Monitor>
     }
 }
 
-base::Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(
-        const std::string& name) {
+Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(const std::string& name) {
 #if DEBUG_CHANNEL_CREATION
     ALOGD("channel '%s' ~ createInputChannel", name.c_str());
 #endif
@@ -5028,8 +5028,10 @@ base::Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputChannel(
     return clientChannel;
 }
 
-base::Result<std::unique_ptr<InputChannel>> InputDispatcher::createInputMonitor(
-        int32_t displayId, bool isGestureMonitor, const std::string& name, int32_t pid) {
+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);
diff --git a/services/inputflinger/host/Android.bp b/services/inputflinger/host/Android.bp
index 18d0226818..743587c6f8 100644
--- a/services/inputflinger/host/Android.bp
+++ b/services/inputflinger/host/Android.bp
@@ -67,6 +67,7 @@ cc_binary {
     cflags: ["-Wall", "-Werror"],
 
     shared_libs: [
+        "libbase",
         "libbinder",
         "libinputflingerhost",
         "libutils",