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.
This commit is contained in:
Weng Xuetian 2022-10-15 22:03:32 -07:00 committed by Xuetian Weng
parent 495e127d07
commit 90df07157b
4 changed files with 98 additions and 12 deletions

View file

@ -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<Test::TextInputV3>();
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> textInput(Test::waylandTextInputManager()->createTextInput(Test::waylandSeat()));
auto textInputV3 = std::make_unique<Test::TextInputV3>();
textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat())));
std::unique_ptr<KWayland::Client::Surface> surface = Test::createSurface();
std::unique_ptr<Test::XdgToplevel> 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<Test::TextInputV3>();
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<Test::TextInputV3>();
textInputV3->init(Test::waylandTextInputManagerV3()->get_text_input(*(Test::waylandSeat())));
textInputV3->enable();

View file

@ -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)

View file

@ -117,6 +117,7 @@ private:
bool touchEventTriggered() const;
void resetPendingPreedit();
void refreshActive();
struct
{

View file

@ -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