Delete try-finally beacuse calling finishInputEvent twice will cause 'Native Crash'

If there is an exception, finishInputEvent method will be called, then NativeInputEventReceiver also send finish signal,will cause a native crash,'Abort message: 'Could not find consume time for seq=xxxx'

[1] https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/android_view_InputEventReceiver.cpp;l=441?q=InputEventRe&ss=android%2Fplatform%2Fsuperproject:frameworks%2F
[2] https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/input/InputTransport.cpp;l=1259?q=InputTRAN&ss=android%2Fplatform%2Fsuperproject:frameworks%2F

Bug: 169866723
Test: atest InputEventSenderAndReceiverTest
Signed-off-by: chenxinyu <chenxinyu7@xiaomi.com>
Change-Id: Ib834e2a960741f7fa33a0661c67f305af0db517a

core/jni/android_view_InputEventReceiver.cpp

@@ -458,10 +458,6 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
                 skipCallbacks = true;
             }
         }
-
-        if (skipCallbacks) {
-            mInputConsumer.sendFinishedSignal(seq, false);
-        }
     }
 }
 

tests/Input/Android.bp

@@ -18,6 +18,7 @@ android_test {
     static_libs: [
         "androidx.test.ext.junit",
         "androidx.test.rules",
+        "services.core.unboosted",
         "truth-prebuilt",
         "ub-uiautomator",
     ],

tests/Input/src/com/android/test/input/InputEventSenderAndReceiverTest.kt

@@ -17,16 +17,11 @@
 package com.android.test.input
 
 import android.os.HandlerThread
-import android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS
 import android.os.Looper
 import android.view.InputChannel
 import android.view.InputEvent
 import android.view.InputEventReceiver
-import android.view.InputEventSender
 import android.view.KeyEvent
-import android.view.MotionEvent
-import java.util.concurrent.LinkedBlockingQueue
-import java.util.concurrent.TimeUnit
 import org.junit.Assert.assertEquals
 import org.junit.After
 import org.junit.Before
@@ -46,54 +41,19 @@ private fun assertKeyEvent(expected: KeyEvent, received: KeyEvent) {
     assertEquals(expected.displayId, received.displayId)
 }
 
-private fun <T> getEvent(queue: LinkedBlockingQueue<T>): T {
+private fun getTestKeyEvent(): KeyEvent {
-    try {
+    return KeyEvent(1 /*downTime*/, 1 /*eventTime*/, KeyEvent.ACTION_DOWN,
-        return queue.poll(DEFAULT_DISPATCHING_TIMEOUT_MILLIS.toLong(), TimeUnit.MILLISECONDS)
+                KeyEvent.KEYCODE_A, 0 /*repeat*/)
-    } catch (e: InterruptedException) {
-        throw RuntimeException("Unexpectedly interrupted while waiting for event")
-    }
 }
 
-class TestInputEventReceiver(channel: InputChannel, looper: Looper) :
+private class CrashingInputEventReceiver(channel: InputChannel, looper: Looper) :
         InputEventReceiver(channel, looper) {
-    private val mInputEvents = LinkedBlockingQueue<InputEvent>()
-
     override fun onInputEvent(event: InputEvent) {
-        when (event) {
+        try {
-            is KeyEvent -> mInputEvents.put(KeyEvent.obtain(event))
+            throw IllegalArgumentException("This receiver crashes when it receives input event")
-            is MotionEvent -> mInputEvents.put(MotionEvent.obtain(event))
+        } finally {
-            else -> throw Exception("Received $event is neither a key nor a motion")
+            finishInputEvent(event, true /*handled*/)
         }
-        finishInputEvent(event, true /*handled*/)
-    }
-
-    fun getInputEvent(): InputEvent {
-        return getEvent(mInputEvents)
-    }
-}
-
-class TestInputEventSender(channel: InputChannel, looper: Looper) :
-        InputEventSender(channel, looper) {
-    data class FinishedSignal(val seq: Int, val handled: Boolean)
-    data class Timeline(val inputEventId: Int, val gpuCompletedTime: Long, val presentTime: Long)
-
-    private val mFinishedSignals = LinkedBlockingQueue<FinishedSignal>()
-    private val mTimelines = LinkedBlockingQueue<Timeline>()
-
-    override fun onInputEventFinished(seq: Int, handled: Boolean) {
-        mFinishedSignals.put(FinishedSignal(seq, handled))
-    }
-
-    override fun onTimelineReported(inputEventId: Int, gpuCompletedTime: Long, presentTime: Long) {
-        mTimelines.put(Timeline(inputEventId, gpuCompletedTime, presentTime))
-    }
-
-    fun getFinishedSignal(): FinishedSignal {
-        return getEvent(mFinishedSignals)
-    }
-
-    fun getTimeline(): Timeline {
-        return getEvent(mTimelines)
     }
 }
 
@@ -102,8 +62,8 @@ class InputEventSenderAndReceiverTest {
         private const val TAG = "InputEventSenderAndReceiverTest"
     }
     private val mHandlerThread = HandlerThread("Process input events")
-    private lateinit var mReceiver: TestInputEventReceiver
+    private lateinit var mReceiver: SpyInputEventReceiver
-    private lateinit var mSender: TestInputEventSender
+    private lateinit var mSender: SpyInputEventSender
 
     @Before
     fun setUp() {
@@ -111,8 +71,8 @@ class InputEventSenderAndReceiverTest {
         mHandlerThread.start()
 
         val looper = mHandlerThread.getLooper()
-        mSender = TestInputEventSender(channels[0], looper)
+        mSender = SpyInputEventSender(channels[0], looper)
-        mReceiver = TestInputEventReceiver(channels[1], looper)
+        mReceiver = SpyInputEventReceiver(channels[1], looper)
     }
 
     @After
@@ -122,8 +82,7 @@ class InputEventSenderAndReceiverTest {
 
     @Test
     fun testSendAndReceiveKey() {
-        val key = KeyEvent(1 /*downTime*/, 1 /*eventTime*/, KeyEvent.ACTION_DOWN,
+        val key = getTestKeyEvent()
-                KeyEvent.KEYCODE_A, 0 /*repeat*/)
         val seq = 10
         mSender.sendInputEvent(seq, key)
         val receivedKey = mReceiver.getInputEvent() as KeyEvent
@@ -133,13 +92,13 @@ class InputEventSenderAndReceiverTest {
         assertKeyEvent(key, receivedKey)
 
         // Check sender
-        assertEquals(TestInputEventSender.FinishedSignal(seq, handled = true), finishedSignal)
+        assertEquals(SpyInputEventSender.FinishedSignal(seq, handled = true), finishedSignal)
     }
 
     // The timeline case is slightly unusual because it goes from InputConsumer to InputPublisher.
     @Test
     fun testSendAndReceiveTimeline() {
-        val sent = TestInputEventSender.Timeline(
+        val sent = SpyInputEventSender.Timeline(
             inputEventId = 1, gpuCompletedTime = 2, presentTime = 3)
         mReceiver.reportTimeline(sent.inputEventId, sent.gpuCompletedTime, sent.presentTime)
         val received = mSender.getTimeline()
@@ -151,7 +110,7 @@ class InputEventSenderAndReceiverTest {
     // event processing.
     @Test
     fun testSendAndReceiveInvalidTimeline() {
-        val sent = TestInputEventSender.Timeline(
+        val sent = SpyInputEventSender.Timeline(
             inputEventId = 1, gpuCompletedTime = 3, presentTime = 2)
         mReceiver.reportTimeline(sent.inputEventId, sent.gpuCompletedTime, sent.presentTime)
         val received = mSender.getTimeline()
@@ -162,4 +121,41 @@ class InputEventSenderAndReceiverTest {
         val receivedSecondTimeline = mSender.getTimeline()
         assertEquals(null, receivedSecondTimeline)
     }
+
+    /**
+     * If a receiver throws an exception during 'onInputEvent' execution, the 'finally' block still
+     * completes, and therefore, finishInputEvent is called. Make sure that there's no crash in the
+     * native layer in these circumstances.
+     * In this test, we are reusing the 'mHandlerThread', but we are creating new sender and
+     * receiver.
+     */
+    @Test
+    fun testCrashingReceiverDoesNotCrash() {
+        val channels = InputChannel.openInputChannelPair("TestChannel2")
+        val sender = SpyInputEventSender(channels[0], mHandlerThread.getLooper())
+
+        // Need a separate thread for the receiver so that the sender can still get the response
+        // after the receiver crashes
+        val receiverThread = HandlerThread("Receive input events")
+        receiverThread.start()
+        val crashingReceiver = CrashingInputEventReceiver(channels[1], receiverThread.getLooper())
+        receiverThread.setUncaughtExceptionHandler { thread, exception ->
+            if (thread == receiverThread && exception is IllegalArgumentException) {
+                // do nothing - this is the exception that we need to ignore
+            } else {
+                throw exception
+            }
+        }
+
+        val key = getTestKeyEvent()
+        val seq = 11
+        sender.sendInputEvent(seq, key)
+        val finishedSignal = sender.getFinishedSignal()
+        assertEquals(SpyInputEventSender.FinishedSignal(seq, handled = true), finishedSignal)
+
+        // Clean up
+        crashingReceiver.dispose()
+        sender.dispose()
+        receiverThread.quitSafely()
+    }
 }

tests/Input/src/com/android/test/input/PointerEventDispatcherTest.kt

@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.test.input
+
+import android.os.HandlerThread
+import android.view.InputChannel
+import android.view.InputDevice
+import android.view.MotionEvent
+import android.view.WindowManagerPolicyConstants.PointerEventListener
+
+import com.android.server.UiThread
+import com.android.server.wm.PointerEventDispatcher
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+
+private class CrashingPointerEventListener : PointerEventListener {
+    override fun onPointerEvent(motionEvent: MotionEvent) {
+        throw IllegalArgumentException("This listener crashes when input event occurs")
+    }
+}
+
+class PointerEventDispatcherTest {
+    companion object {
+        private const val TAG = "PointerEventDispatcherTest"
+    }
+    private val mHandlerThread = HandlerThread("Process input events")
+    private lateinit var mSender: SpyInputEventSender
+    private lateinit var mPointerEventDispatcher: PointerEventDispatcher
+    private val mListener = CrashingPointerEventListener()
+
+    @Before
+    fun setUp() {
+        val channels = InputChannel.openInputChannelPair("TestChannel")
+
+        mHandlerThread.start()
+        val looper = mHandlerThread.getLooper()
+        mSender = SpyInputEventSender(channels[0], looper)
+
+        mPointerEventDispatcher = PointerEventDispatcher(channels[1])
+        mPointerEventDispatcher.registerInputEventListener(mListener)
+    }
+
+    @After
+    fun tearDown() {
+        mHandlerThread.quitSafely()
+    }
+
+    @Test
+    fun testSendMotionToCrashingListenerDoesNotCrash() {
+        // The exception will occur on the UiThread, so we can't catch it here on the test thread
+        UiThread.get().setUncaughtExceptionHandler { thread, exception ->
+            if (thread == UiThread.get() && exception is IllegalArgumentException) {
+                // do nothing - this is the exception that we need to ignore
+            } else {
+                throw exception
+            }
+        }
+
+        // The MotionEvent properties aren't important for this test, as long as the event
+        // is a pointer event, so that it gets processed by CrashingPointerEventListener
+        val downTime = 0L
+        val motionEvent = MotionEvent.obtain(downTime, downTime,
+                MotionEvent.ACTION_DOWN, 0f /* x */, 0f /* y */, 0 /* metaState */)
+        motionEvent.source = InputDevice.SOURCE_TOUCHSCREEN
+        val seq = 10
+        mSender.sendInputEvent(seq, motionEvent)
+        val finishedSignal = mSender.getFinishedSignal()
+
+        // Since the listener raises an exception during the event handling, the event should be
+        // marked as 'not handled'.
+        assertEquals(SpyInputEventSender.FinishedSignal(seq, handled = false), finishedSignal)
+        // Ensure that there aren't double finish calls. This would crash if there's a call
+        // to finish twice.
+        assertNull(mSender.getFinishedSignal())
+    }
+}

tests/Input/src/com/android/test/input/SpyInputEventSenderAndReceiver.kt

@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.test.input
+
+import android.os.InputConstants.DEFAULT_DISPATCHING_TIMEOUT_MILLIS
+import android.os.Looper
+import android.view.InputChannel
+import android.view.InputEvent
+import android.view.InputEventReceiver
+import android.view.InputEventSender
+import android.view.KeyEvent
+import android.view.MotionEvent
+import java.util.concurrent.LinkedBlockingQueue
+import java.util.concurrent.TimeUnit
+
+private fun <T> getEvent(queue: LinkedBlockingQueue<T>): T? {
+    return queue.poll(DEFAULT_DISPATCHING_TIMEOUT_MILLIS.toLong(), TimeUnit.MILLISECONDS)
+}
+
+class SpyInputEventReceiver(channel: InputChannel, looper: Looper) :
+        InputEventReceiver(channel, looper) {
+    private val mInputEvents = LinkedBlockingQueue<InputEvent>()
+
+    override fun onInputEvent(event: InputEvent) {
+        when (event) {
+            is KeyEvent -> mInputEvents.put(KeyEvent.obtain(event))
+            is MotionEvent -> mInputEvents.put(MotionEvent.obtain(event))
+            else -> throw Exception("Received $event is neither a key nor a motion")
+        }
+        finishInputEvent(event, true /*handled*/)
+    }
+
+    fun getInputEvent(): InputEvent? {
+        return getEvent(mInputEvents)
+    }
+}
+
+class SpyInputEventSender(channel: InputChannel, looper: Looper) :
+        InputEventSender(channel, looper) {
+    data class FinishedSignal(val seq: Int, val handled: Boolean)
+    data class Timeline(val inputEventId: Int, val gpuCompletedTime: Long, val presentTime: Long)
+
+    private val mFinishedSignals = LinkedBlockingQueue<FinishedSignal>()
+    private val mTimelines = LinkedBlockingQueue<Timeline>()
+
+    override fun onInputEventFinished(seq: Int, handled: Boolean) {
+        mFinishedSignals.put(FinishedSignal(seq, handled))
+    }
+
+    override fun onTimelineReported(inputEventId: Int, gpuCompletedTime: Long, presentTime: Long) {
+        mTimelines.put(Timeline(inputEventId, gpuCompletedTime, presentTime))
+    }
+
+    fun getFinishedSignal(): FinishedSignal? {
+        return getEvent(mFinishedSignals)
+    }
+
+    fun getTimeline(): Timeline? {
+        return getEvent(mTimelines)
+    }
+}