[PATCH libinput] touchpad: fix dwt disabling while a key is still down

[Hans de Goede]

Hi,

On 08-02-16 03:51, Peter Hutterer wrote:
> If dwt is disabled on the commandline, e.g. by setting an xinput property it
> may be disabled before the release event comes in. This caused the timer to
> refresh indefinitely since the key state mask was still on for that key.
> Always updating the key state mask (even when dwt is disabled) fixes that.
>
> If a key is held down while dwt is disabled, this can still cause a indefinite
> timer refresh, so in the timer func, check if dwt is enabled before refreshing
> the timer.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=94015
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Looks good to me:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


> ---
>   src/evdev-mt-touchpad.c |  9 ++++---
>   test/touchpad.c         | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index f7d5d87..1f654e4 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1196,7 +1196,8 @@ tp_keyboard_timeout(uint64_t now, void *data)
>   {
>   	struct tp_dispatch *tp = data;
>
> -	if (long_any_bit_set(tp->dwt.key_mask,
> +	if (tp->dwt.dwt_enabled &&
> +	    long_any_bit_set(tp->dwt.key_mask,
>   			     ARRAY_LENGTH(tp->dwt.key_mask))) {
>   		libinput_timer_set(&tp->dwt.keyboard_timer,
>   				   now + DEFAULT_KEYBOARD_ACTIVITY_TIMEOUT_2);
> @@ -1251,9 +1252,6 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>   	unsigned int timeout;
>   	unsigned int key;
>
> -	if (!tp->dwt.dwt_enabled)
> -		return;
> -
>   	if (event->type != LIBINPUT_EVENT_KEYBOARD_KEY)
>   		return;
>
> @@ -1267,6 +1265,9 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>   		return;
>   	}
>
> +	if (!tp->dwt.dwt_enabled)
> +		return;
> +
>   	/* modifier keys don't trigger disable-while-typing so things like
>   	 * ctrl+zoom or ctrl+click are possible */
>   	if (tp_key_ignore_for_dwt(key))
> diff --git a/test/touchpad.c b/test/touchpad.c
> index c0ddc53..3574edd 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -3114,6 +3114,72 @@ START_TEST(touchpad_dwt_disable_before_touch)
>   }
>   END_TEST
>
> +START_TEST(touchpad_dwt_disable_during_key_release)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	enable_dwt(touchpad);
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, true);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +	disable_dwt(touchpad);
> +	libinput_dispatch(li);
> +	litest_keyboard_key(keyboard, KEY_A, false);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +	/* touch down during timeout, wait, should generate events */
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	libinput_dispatch(li);
> +	litest_timeout_dwt_long();
> +	litest_touch_move_to(touchpad, 0, 70, 50, 50, 50, 10, 1);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_dwt_disable_during_key_hold)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	enable_dwt(touchpad);
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, true);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +
> +	disable_dwt(touchpad);
> +	libinput_dispatch(li);
> +
> +	/* touch down during timeout, wait, should generate events */
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	libinput_dispatch(li);
> +	litest_timeout_dwt_long();
> +	litest_touch_move_to(touchpad, 0, 70, 50, 50, 50, 10, 1);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
>   START_TEST(touchpad_dwt_enable_during_touch)
>   {
>   	struct litest_device *touchpad = litest_current_device();
> @@ -3925,6 +3991,8 @@ litest_setup_tests(void)
>   	litest_add("touchpad:dwt", touchpad_dwt_disabled, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_disable_during_touch, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_disable_before_touch, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_disable_during_key_release, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_disable_during_key_hold, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_enable_during_touch, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_enable_before_touch, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_enable_during_tap, LITEST_TOUCHPAD, LITEST_ANY);
>