Merge "Add InputChannel::waitForMessage and use it in EndToEndNativeInputTest" into main

3 files changed, 51 insertions, 33 deletions

diff --git a/include/input/InputTransport.h b/include/input/InputTransport.h
index 750e170c24..2d64872beb 100644
--- a/include/input/InputTransport.h
+++ b/include/input/InputTransport.h
@@ -290,6 +290,17 @@ public:
      */
     bool probablyHasInput() const;
 
+    /* Wait until there is a message in the channel.
+     *
+     * The |timeout| specifies how long to block waiting for an input event to appear. Negative
+     * values are not allowed.
+     *
+     * In some cases returning before timeout expiration can happen without a message available.
+     * This could happen after the channel was closed on the other side. Another possible reason
+     * is incorrect setup of the channel.
+     */
+    void waitForMessage(std::chrono::milliseconds timeout) const;
+
     /* Return a new object that has a duplicate of this channel's fd. */
     std::unique_ptr<InputChannel> dup() const;
 
diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp
index d4b8dbeeb9..b9ab803101 100644
--- a/libs/gui/tests/EndToEndNativeInputTest.cpp
+++ b/libs/gui/tests/EndToEndNativeInputTest.cpp
@@ -168,8 +168,8 @@ public:
         return std::make_unique<InputSurface>(surfaceControl, width, height);
     }
 
-    InputEvent *consumeEvent(int timeoutMs = 3000) {
-        waitForEventAvailable(timeoutMs);
+    InputEvent* consumeEvent(std::chrono::milliseconds timeout = 3000ms) {
+        mClientChannel->waitForMessage(timeout);
 
         InputEvent *ev;
         uint32_t seqId;
@@ -302,15 +302,6 @@ public:
         t.apply(true);
     }
 
-private:
-    void waitForEventAvailable(int timeoutMs) {
-        struct pollfd fd;
-
-        fd.fd = mClientChannel->getFd();
-        fd.events = POLLIN;
-        poll(&fd, 1, timeoutMs);
-    }
-
 public:
     sp<SurfaceControl> mSurfaceControl;
     std::shared_ptr<InputChannel> mClientChannel;
@@ -615,7 +606,7 @@ TEST_F(InputSurfacesTest, touchable_region) {
 
     // A tap within the surface but outside the touchable region should not be sent to the surface.
     injectTap(20, 30);
-    EXPECT_EQ(surface->consumeEvent(200 /*timeoutMs*/), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/200ms), nullptr);
 
     injectTap(31, 52);
     surface->expectTap(20, 30);
@@ -981,12 +972,12 @@ TEST_F(InputSurfacesTest, strict_unobscured_input_obscured_window) {
     obscuringSurface->mInputInfo.ownerUid = gui::Uid{22222};
     obscuringSurface->showAt(100, 100);
     injectTap(101, 101);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus();
     surface->assertFocusChange(true);
     injectKey(AKEYCODE_V);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, strict_unobscured_input_partially_obscured_window) {
@@ -1002,12 +993,12 @@ TEST_F(InputSurfacesTest, strict_unobscured_input_partially_obscured_window) {
 
     injectTap(101, 101);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus();
     surface->assertFocusChange(true);
     injectKey(AKEYCODE_V);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, strict_unobscured_input_alpha_window) {
@@ -1024,12 +1015,12 @@ TEST_F(InputSurfacesTest, strict_unobscured_input_alpha_window) {
 
     injectTap(101, 101);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus();
     surface->assertFocusChange(true);
     injectKey(AKEYCODE_V);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, strict_unobscured_input_cropped_window) {
@@ -1046,12 +1037,12 @@ TEST_F(InputSurfacesTest, strict_unobscured_input_cropped_window) {
 
     injectTap(111, 111);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus();
     surface->assertFocusChange(true);
     injectKey(AKEYCODE_V);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, ignore_touch_region_with_zero_sized_blast) {
@@ -1076,12 +1067,12 @@ TEST_F(InputSurfacesTest, drop_input_policy) {
 
     injectTap(101, 101);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus();
     surface->assertFocusChange(true);
     injectKey(AKEYCODE_V);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, layer_with_valid_crop_can_be_focused) {
@@ -1116,7 +1107,7 @@ TEST_F(InputSurfacesTest, cropped_container_replaces_touchable_region_with_null_
 
     // Does not receive events outside its crop
     injectTap(26, 26);
-    EXPECT_EQ(containerSurface->consumeEvent(100), nullptr);
+    EXPECT_EQ(containerSurface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 /**
@@ -1141,7 +1132,7 @@ TEST_F(InputSurfacesTest, uncropped_container_replaces_touchable_region_with_nul
 
     // Does not receive events outside parent bounds
     injectTap(31, 31);
-    EXPECT_EQ(containerSurface->consumeEvent(100), nullptr);
+    EXPECT_EQ(containerSurface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 /**
@@ -1167,7 +1158,7 @@ TEST_F(InputSurfacesTest, replace_touchable_region_with_crop) {
     // Does not receive events outside crop layer bounds
     injectTap(21, 21);
     injectTap(71, 71);
-    EXPECT_EQ(containerSurface->consumeEvent(100), nullptr);
+    EXPECT_EQ(containerSurface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(InputSurfacesTest, child_container_with_no_input_channel_blocks_parent) {
@@ -1184,7 +1175,7 @@ TEST_F(InputSurfacesTest, child_container_with_no_input_channel_blocks_parent) {
             [&](auto &t, auto &sc) { t.reparent(sc, parent->mSurfaceControl); });
     injectTap(101, 101);
 
-    EXPECT_EQ(parent->consumeEvent(100), nullptr);
+    EXPECT_EQ(parent->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 class MultiDisplayTests : public InputSurfacesTest {
@@ -1233,7 +1224,7 @@ TEST_F(MultiDisplayTests, drop_touch_if_layer_on_invalid_display) {
 
     // Touches should be dropped if the layer is on an invalid display.
     injectTapOnDisplay(101, 101, layerStack.id);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     // However, we still let the window be focused and receive keys.
     surface->requestFocus(layerStack.id);
@@ -1271,12 +1262,12 @@ TEST_F(MultiDisplayTests, drop_input_for_secure_layer_on_nonsecure_display) {
 
     injectTapOnDisplay(101, 101, layerStack.id);
 
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 
     surface->requestFocus(layerStack.id);
     surface->assertFocusChange(true);
     injectKeyOnDisplay(AKEYCODE_V, layerStack.id);
-    EXPECT_EQ(surface->consumeEvent(100), nullptr);
+    EXPECT_EQ(surface->consumeEvent(/*timeout=*/100ms), nullptr);
 }
 
 TEST_F(MultiDisplayTests, dont_drop_input_for_secure_layer_on_secure_display) {
diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp
index fa094df159..37de00cf35 100644
--- a/libs/input/InputTransport.cpp
+++ b/libs/input/InputTransport.cpp
@@ -521,11 +521,11 @@ status_t InputChannel::receiveMessage(InputMessage* msg) {
 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
+        // This can be a false negative because EINTR and ENOMEM are not handled. The latter should
+        // be extremely rare. The EINTR is also unlikely because it happens only when the signal
+        // arrives while the syscall is executed, and the syscall is quick. Hitting EINTR 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.
+        // common tradition is to repeat the syscall on each EINTR, but it is not necessary here.
         // In other words, the missing one liner is replaced by a multiline explanation.
         return false;
     }
@@ -534,6 +534,22 @@ bool InputChannel::probablyHasInput() const {
     return (pfds.revents & POLLIN) != 0;
 }
 
+void InputChannel::waitForMessage(std::chrono::milliseconds timeout) const {
+    if (timeout < 0ms) {
+        LOG(FATAL) << "Timeout cannot be negative, received " << timeout.count();
+    }
+    struct pollfd pfds = {.fd = mFd, .events = POLLIN};
+    int ret;
+    std::chrono::time_point<std::chrono::steady_clock> stopTime =
+            std::chrono::steady_clock::now() + timeout;
+    std::chrono::milliseconds remaining = timeout;
+    do {
+        ret = ::poll(&pfds, /*nfds=*/1, /*timeout=*/remaining.count());
+        remaining = std::chrono::duration_cast<std::chrono::milliseconds>(
+                stopTime - std::chrono::steady_clock::now());
+    } while (ret == -1 && errno == EINTR && remaining > 0ms);
+}
+
 std::unique_ptr<InputChannel> InputChannel::dup() const {
     base::unique_fd newFd(dupFd());
     return InputChannel::create(getName(), std::move(newFd), getConnectionToken());