From 90df07157b0564e220ea7ffbe5eb032efa61b74d Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Sat, 15 Oct 2022 22:03:32 -0700 Subject: [PATCH] Fix potential race condition when text input state change and focus change happened at the same time In some cases, stateCommitted may fire after a new surface being focused and in correctly set input method to be inactive. This can be reproduced by switch client between an active text input v3 client and text input v2 client. --- autotests/integration/inputmethod_test.cpp | 80 ++++++++++++++++++++-- src/inputmethod.cpp | 26 +++++-- src/inputmethod.h | 1 + src/wayland/seat_interface.cpp | 3 +- 4 files changed, 98 insertions(+), 12 deletions(-) diff --git a/autotests/integration/inputmethod_test.cpp b/autotests/integration/inputmethod_test.cpp index dabeef263b..504d1a3484 100644 --- a/autotests/integration/inputmethod_test.cpp +++ b/autotests/integration/inputmethod_test.cpp @@ -61,6 +61,7 @@ private Q_SLOTS: void testEnableActive(); void testHidePanel(); void testSwitchFocusedSurfaces(); + void testV2V3SameClient(); void testV3Styling(); void testDisableShowInputPanel(); void testModifierForwarding(); @@ -178,7 +179,7 @@ void InputMethodTest::testEnableDisableV3() QVERIFY(window->isActive()); QCOMPARE(window->frameGeometry().size(), QSize(1280, 1024)); - Test::TextInputV3 *textInputV3 = new Test::TextInputV3(); + auto textInputV3 = std::make_unique(); textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat()))); textInputV3->enable(); @@ -324,6 +325,77 @@ void InputMethodTest::testSwitchFocusedSurfaces() } } +void InputMethodTest::testV2V3SameClient() +{ + touchNow(); + QVERIFY(!kwinApp()->inputMethod()->isActive()); + + QSignalSpy windowAddedSpy(workspace(), &Workspace::windowAdded); + QSignalSpy windowRemovedSpy(workspace(), &Workspace::windowRemoved); + + QSignalSpy activateSpy(kwinApp()->inputMethod(), &InputMethod::activeChanged); + std::unique_ptr textInput(Test::waylandTextInputManager()->createTextInput(Test::waylandSeat())); + + auto textInputV3 = std::make_unique(); + textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat()))); + + std::unique_ptr surface = Test::createSurface(); + std::unique_ptr toplevel(Test::createXdgToplevelSurface(surface.get())); + Window *window = Test::renderAndWaitForShown(surface.get(), QSize(1280, 1024), Qt::red); + QCOMPARE(workspace()->activeWindow(), window); + QCOMPARE(windowAddedSpy.count(), 1); + waylandServer()->seat()->setFocusedTextInputSurface(window->surface()); + QVERIFY(!kwinApp()->inputMethod()->isActive()); + + // Enable and disable v2 + textInput->enable(surface.get()); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(kwinApp()->inputMethod()->isActive()); + + activateSpy.clear(); + textInput->disable(surface.get()); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(!kwinApp()->inputMethod()->isActive()); + + // Enable and disable v3 + activateSpy.clear(); + textInputV3->enable(); + textInputV3->commit(); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(kwinApp()->inputMethod()->isActive()); + + activateSpy.clear(); + textInputV3->disable(); + textInputV3->commit(); + activateSpy.clear(); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(!kwinApp()->inputMethod()->isActive()); + + // Enable v2 and v3 + activateSpy.clear(); + textInputV3->enable(); + textInputV3->commit(); + textInput->enable(surface.get()); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(kwinApp()->inputMethod()->isActive()); + + // Disable v3, should still be active since v2 is active. + activateSpy.clear(); + textInputV3->disable(); + textInputV3->commit(); + activateSpy.wait(200); + QVERIFY(kwinApp()->inputMethod()->isActive()); + + // Disable v2 + activateSpy.clear(); + textInput->disable(surface.get()); + QVERIFY(activateSpy.count() || activateSpy.wait()); + QVERIFY(!kwinApp()->inputMethod()->isActive()); + + toplevel.reset(); + QVERIFY(Test::waitForWindowDestroyed(window)); +} + void InputMethodTest::testV3Styling() { // Create an xdg_toplevel surface and wait for the compositor to catch up. @@ -334,7 +406,7 @@ void InputMethodTest::testV3Styling() QVERIFY(window->isActive()); QCOMPARE(window->frameGeometry().size(), QSize(1280, 1024)); - Test::TextInputV3 *textInputV3 = new Test::TextInputV3(); + auto textInputV3 = std::make_unique(); textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat()))); textInputV3->enable(); @@ -347,7 +419,7 @@ void InputMethodTest::testV3Styling() QVERIFY(kwinApp()->inputMethod()->isActive()); QVERIFY(inputMethodActivateSpy.wait()); auto context = Test::inputMethod()->context(); - QSignalSpy textInputPreeditSpy(textInputV3, &Test::TextInputV3::preeditString); + QSignalSpy textInputPreeditSpy(textInputV3.get(), &Test::TextInputV3::preeditString); zwp_input_method_context_v1_preedit_cursor(context, 0); zwp_input_method_context_v1_preedit_styling(context, 0, 3, 7); zwp_input_method_context_v1_preedit_string(context, 0, "ABCD", "ABCD"); @@ -456,7 +528,7 @@ void InputMethodTest::testModifierForwarding() QVERIFY(window->isActive()); QCOMPARE(window->frameGeometry().size(), QSize(1280, 1024)); - Test::TextInputV3 *textInputV3 = new Test::TextInputV3(); + auto textInputV3 = std::make_unique(); textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat()))); textInputV3->enable(); diff --git a/src/inputmethod.cpp b/src/inputmethod.cpp index 5d2109223b..d32e3732af 100644 --- a/src/inputmethod.cpp +++ b/src/inputmethod.cpp @@ -162,6 +162,23 @@ bool InputMethod::shouldShowOnActive() const || input()->tablet() == input()->lastInputHandler(); } +void InputMethod::refreshActive() +{ + auto seat = waylandServer()->seat(); + auto t2 = seat->textInputV2(); + auto t3 = seat->textInputV3(); + + bool active = false; + if (auto focusedSurface = seat->focusedTextInputSurface()) { + auto client = focusedSurface->client(); + if ((t2->clientSupportsTextInput(client) && t2->isEnabled()) || (t3->clientSupportsTextInput(client) && t3->isEnabled())) { + active = true; + } + } + + setActive(active); +} + void InputMethod::setActive(bool active) { const bool wasActive = waylandServer()->inputMethod()->context(); @@ -246,9 +263,6 @@ void InputMethod::handleFocusedSurfaceChanged() SurfaceInterface *focusedSurface = seat->focusedTextInputSurface(); setTrackedWindow(waylandServer()->findWindow(focusedSurface)); - if (!focusedSurface) { - setActive(false); - } const auto client = focusedSurface ? focusedSurface->client() : nullptr; bool ret = seat->textInputV2()->clientSupportsTextInput(client) @@ -330,8 +344,7 @@ void InputMethod::textInputInterfaceV2EnabledChanged() return; } - auto t = waylandServer()->seat()->textInputV2(); - setActive(t->isEnabled()); + refreshActive(); } void InputMethod::textInputInterfaceV3EnabledChanged() @@ -341,7 +354,7 @@ void InputMethod::textInputInterfaceV3EnabledChanged() } auto t3 = waylandServer()->seat()->textInputV3(); - setActive(t3->isEnabled()); + refreshActive(); if (!t3->isEnabled()) { // reset value of preedit when textinput is disabled resetPendingPreedit(); @@ -366,7 +379,6 @@ void InputMethod::stateCommitted(uint32_t serial) if (auto inputContext = waylandServer()->inputMethod()->context()) { inputContext->sendCommitState(serial); } - setActive(textInputV3->isEnabled()); } void InputMethod::setEnabled(bool enabled) diff --git a/src/inputmethod.h b/src/inputmethod.h index 9ff09c7e13..3f48281abd 100644 --- a/src/inputmethod.h +++ b/src/inputmethod.h @@ -117,6 +117,7 @@ private: bool touchEventTriggered() const; void resetPendingPreedit(); + void refreshActive(); struct { diff --git a/src/wayland/seat_interface.cpp b/src/wayland/seat_interface.cpp index 2aa6a6e504..a57443fe85 100644 --- a/src/wayland/seat_interface.cpp +++ b/src/wayland/seat_interface.cpp @@ -1242,7 +1242,6 @@ void SeatInterface::setFocusedTextInputSurface(SurfaceInterface *surface) d->textInputV3->d->sendLeave(d->focusedTextInputSurface); } d->focusedTextInputSurface = surface; - Q_EMIT focusedTextInputSurfaceChanged(); if (surface) { d->focusedSurfaceDestroyConnection = connect(surface, &SurfaceInterface::aboutToBeDestroyed, this, [this] { @@ -1251,6 +1250,8 @@ void SeatInterface::setFocusedTextInputSurface(SurfaceInterface *surface) d->textInputV2->d->sendEnter(surface, serial); d->textInputV3->d->sendEnter(surface); } + + Q_EMIT focusedTextInputSurfaceChanged(); } SurfaceInterface *SeatInterface::focusedTextInputSurface() const