> Yes, any code wiling to raise the window in the application uses the > mentioned method that does setWindowState, setVisible, raise and > activateWindow in a row. It's convenient since you don't have to think > which state the window is in to raise it.
Thanks, @Ilya Fedin<mailto:fedin-ilja2...@ya.ru>. One reason why it happens in autotests is: Events have to be processed explicitly in Qt, while the XCB thread is doing its own business. If it the race / activation drop occurs in an application, that needs to be fixed. Do you have a bug report / reprocer to look at? Confidential ________________________________ From: Ilya Fedin <fedin-ilja2...@ya.ru> Sent: Tuesday, 12 August 2025 09:23 To: Axel Spoerl via Development <development@qt-project.org> Cc: Axel Spoerl <axel.spo...@qt.io> Subject: Re: [Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive On Tue, 12 Aug 2025 07:07:57 +0000 Axel Spoerl via Development <development@qt-project.org> wrote: > Good morning and thanks to Mitch for initiating this discussion! > > QWindow::requestActivate() returns early with a warning, if the > window in question doesn't accept focus. In all other cases, it > dispatches to virtual QPlatformWindow::requestActivateWindow(), which > can be overridden in each platform implementation. > > * > The default implementation dispatches to > QWindowSystemInterface::handleFocusWindowChanged(). It causes Qt to > treat the given window as the focus window, without acting on > platform level. * XCB: Sends an NET_ACTIVE_WINDOW if the window is > toplevel, otherwise XCB_INPUT_FOCUS_PARENT, to its parent. QXcb > implementation deferres the activation if the window is unmapped. > That's where data races might occur. They are typically harmless in > an application, where the mouse is busy moving and a "forgotten > event" is hardly noticed. * Windows: The win APIs BringWindowToTop() > and SetActiveWindow() are called on the native handle after some > sanity checks. * MacOS: The native window is made key window and > first responder. * Wayland: The wayland XDG shell sends an activation > request to the shell, the normal wayland shell does a no-op. * To my > knowledge, the eglfs amd off screen implementations also implement > platform specific activation. > > => Whenever a window is already active, requestActivate() neither > hurts nor does it anything useful. It may trigger a race on XCB just > after show(), which causes the activation to be dropped. I have only > seen that in autotests. It would be interesting to examine the bug > that Ilya has seen. If the described scenario causes a show() and > requestActivate() just after each other, we indeed might have an > issue. Yes, any code wiling to raise the window in the application uses the mentioned method that does setWindowState, setVisible, raise and activateWindow in a row. It's convenient since you don't have to think which state the window is in to raise it. > > Our existing autotests implement different use cases with and without > QWindow::requestActivate(). A few examples: > > * > tst_QWindow::positioning() expects activation after > QWindow::showNormal(), without calling requestActivate(). That's what > Mitch and I are referring to. The call isn't needed if the one and > only window around is shown. * tst_QWindow::activateAndClose() calls > showNormal() followed by requestActivate(). It has failed on 35 > integrations this year on Ubuntu. I can reproduce that. The test > always passes on XCB, when requestActivate() is removed. That's the > race condition Mitch and I were discussing. > > That said, IMHO > > * > The usage of requestActivate() following a show() in an autotest > should be explained / commented. Reviewers should ask for such > explanation. * The calls can't be removed en masse and light hearted > from autotests. Removals have to be evaluated case by case. * > Let's remove if it fixes flakiness and doesn't hurt the test > otherwise. > > > Cheers > Axel > > Confidential > ________________________________ > From: Development <development-boun...@qt-project.org> on behalf of > EXT Mitch Curtis via Development <development@qt-project.org> Sent: > Tuesday, 12 August 2025 07:29 To: David C. Partridge > <david.partri...@perdrix.co.uk>; development@qt-project.org > <development@qt-project.org> Subject: Re: [Development] Calling > QWindow::requestActivate after QWindow::show and before > QTest::qWaitForWindowActive > > > First we need to agree on whether or not this is the right thing to > do. If we agree that it is, then we should absolutely do that. That’s > what I was advocating for in my comments on the change, too. > > > > > Confidential > > From: David C. Partridge <david.partri...@perdrix.co.uk> > Date: Tuesday, 12 August 2025 at 12:15 > To: EXT Mitch Curtis <mitch.cur...@qt.io> > Subject: RE: [Development] Calling QWindow::requestActivate after > QWindow::show and before QTest::qWaitForWindowActive > > Shouldn't some words about this be added to the user documentation to > tell us not to do that... > > D. > > -----Original Message----- > From: Development <development-boun...@qt-project.org> On Behalf Of > EXT Mitch Curtis via Development > Sent: 12 August 2025 01:53 > To: Qt Development <development@qt-project.org> > Subject: [Development] Calling QWindow::requestActivate after > QWindow::show and before QTest::qWaitForWindowActive > > Hi, > > There's a discussion on > https://codereview.qt-project.org/c/qt/qtdeclarative/+/666818 about > whether we can standardise (by documenting in some form) that we > shouldn't call requestActivate before qWaitForWindowActive. > Essentially, existing code in tests looks like this: > > window->show(); > window->requestActivate(); > QVERIFY(QTest::qWaitForWindowActive(window.data())); > > And would become this: > > window->show(); > QVERIFY(QTest::qWaitForWindowActive(window.data())); > > As Axel has explained, this causes race conditions on XCB, and > apparently is redundant on all platforms: > > > To begin with, an autotest should be as close as possible to a real > > world > scenario. > > When the only window around is being shown, it is automatically > > activated. > The additional window->requestActivate() redundant and not a real > world scenario. This alone justifies its removal. > > I'd like to get agreement that this is the right thing to do on all > platforms before proceeding (and then removing it en masse). > > Cheers. > > Confidential > -- > Development mailing list > Development@qt-project.org > https://lists.qt-project.org/listinfo/development
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development