Use Result<InputPublisher::Finished> instead of callback -- try 2
When the 'Finished' message is received inside InputDispatcher, we are
currently providing a callback function that gets executed with the
parameters that are matching the InputMessage fields.
This does not scale well for the case where there is more than 1 type of
InputMessage received from the InputConsumer. We would have to provide 2
callbacks, which is not user-friendly.
The calling code inside InputDispatcher is already aware of the
InputMessage struct, but InputMessage is intended to be a protocol of
communication between input channels, and is not meant to be used
elsewhere. To provide the output of 'finished' signal, we introduce a
new 'Finished' struct into InputPublisher. InputPublisher will now try
to read a message, and will provide a response in that struct.
This approach will also now force the caller to check ok(), which will
increase correctness.
Bug: 167947340
Test: atest inputflinger_tests
Test: TBD
Revert submission 13838212-revert-13780058-receiveFinishedSignal-UGCLLLUBPW
Reason for revert: Relanding with fix
Reverted Changes:
Idb3a44b4a:Revert "Update the usage of receiveFinishedSignal"...
I1e71010f5:Revert "Use Result<InputPublisher::Finished> inste...
Change-Id: I9c425bb7249d43648e558214e40fa35aeaa0bb11
diff --git a/libs/input/tests/InputPublisherAndConsumer_test.cpp b/libs/input/tests/InputPublisherAndConsumer_test.cpp
index b5ed8d7..fc31715 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 @@
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 @@
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 @@
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 @@
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 @@
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";
}