Decouple quitOnLastWindowClosed from quitLockEnabled

In a512e210ac5b032c5fc2edf1ddf72e5a414485fda512e21 quitOnLastWindowClosed
was changed to be implemented in terms of quitLockEnabled, but without
any documentation to that end.

Although the two features are similar (automatic quit under certain
conditions), and interact, it doesn't make sense to overlap them until
we actually expose them as a single property (automaticQuit e.g.)

The logic for determining whether we can can quit automatically has
been refactored to take both properties into account, on both a Core
and Gui level. The call sites still need to check the individual
properties to determine whether to activate automatic quit for
that particular code path.

Change-Id: I38c3e8cb30db373ea73dd45f150e5048c0db2f4d
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/corelib/kernel/qcoreapplication.cpp

@@ -400,7 +400,7 @@ struct QCoreApplicationData {
 Q_GLOBAL_STATIC(QCoreApplicationData, coreappdata)
 
 #ifndef QT_NO_QOBJECT
-static bool quitLockRefEnabled = true;
+static bool quitLockEnabled = true;
 #endif
 
 #if defined(Q_OS_WIN)
@@ -1020,14 +1020,14 @@ bool QCoreApplication::testAttribute(Qt::ApplicationAttribute attribute)
 
 bool QCoreApplication::isQuitLockEnabled()
 {
-    return quitLockRefEnabled;
+    return quitLockEnabled;
 }
 
 static bool doNotify(QObject *, QEvent *);
 
 void QCoreApplication::setQuitLockEnabled(bool enabled)
 {
-    quitLockRefEnabled = enabled;
+    quitLockEnabled = enabled;
 }
 
 /*!
@@ -1982,14 +1982,34 @@ void QCoreApplicationPrivate::ref()
 
 void QCoreApplicationPrivate::deref()
 {
-    if (!quitLockRef.deref())
+    quitLockRef.deref();
-        maybeQuit();
+
+    if (quitLockEnabled && canQuitAutomatically())
+        quitAutomatically();
 }
 
-void QCoreApplicationPrivate::maybeQuit()
+bool QCoreApplicationPrivate::canQuitAutomatically()
 {
-    if (quitLockRef.loadRelaxed() == 0 && in_exec && quitLockRefEnabled && shouldQuit())
+    if (!in_exec)
-        QCoreApplication::postEvent(QCoreApplication::instance(), new QEvent(QEvent::Quit));
+        return false;
+
+    if (quitLockEnabled && quitLockRef.loadRelaxed())
+        return false;
+
+    return true;
+}
+
+void QCoreApplicationPrivate::quitAutomatically()
+{
+    Q_Q(QCoreApplication);
+
+    // Explicit requests by the user to quit() is plumbed via the platform
+    // if possible, and delivers the quit event synchronously. For automatic
+    // quits we implicitly support cancelling the quit by showing another
+    // window, which currently relies on removing any posted quit events
+    // from the event queue. As a result, we can't use the normal quit()
+    // code path, and need to post manually.
+    QCoreApplication::postEvent(q, new QEvent(QEvent::Quit));
 }
 
 /*!

src/corelib/kernel/qcoreapplication_p.h

@@ -136,12 +136,9 @@ public:
     QAtomicInt quitLockRef;
     void ref();
     void deref();
-    virtual bool shouldQuit() {
+    virtual bool canQuitAutomatically();
-      return true;
+    void quitAutomatically();
-    }
-
     virtual void quit();
-    void maybeQuit();
 
     static QBasicAtomicPointer<QThread> theMainThread;
     static QThread *mainThread();

src/gui/kernel/qguiapplication.cpp

@@ -210,6 +210,8 @@ qreal QGuiApplicationPrivate::m_maxDevicePixelRatio = 0.0;
 
 static qreal fontSmoothingGamma = 1.7;
 
+bool QGuiApplicationPrivate::quitOnLastWindowClosed = true;
+
 extern void qRegisterGuiVariant();
 #if QT_CONFIG(animation)
 extern void qRegisterGuiGetInterpolator();
@@ -3538,12 +3540,12 @@ void QGuiApplicationPrivate::notifyWindowIconChanged()
 
 void QGuiApplication::setQuitOnLastWindowClosed(bool quit)
 {
-    QCoreApplication::setQuitLockEnabled(quit);
+    QGuiApplicationPrivate::quitOnLastWindowClosed = quit;
 }
 
 bool QGuiApplication::quitOnLastWindowClosed()
 {
-    return QCoreApplication::isQuitLockEnabled();
+    return QGuiApplicationPrivate::quitOnLastWindowClosed;
 }
 
 void QGuiApplicationPrivate::maybeLastWindowClosed(QWindow *closedWindow)
@@ -3561,8 +3563,8 @@ void QGuiApplicationPrivate::maybeLastWindowClosed(QWindow *closedWindow)
     if (in_exec)
         emit q_func()->lastWindowClosed();
 
-    if (QGuiApplication::quitOnLastWindowClosed())
+    if (quitOnLastWindowClosed && canQuitAutomatically())
-        maybeQuit();
+        quitAutomatically();
 }
 
 /*!
@@ -3591,9 +3593,12 @@ bool QGuiApplicationPrivate::lastWindowClosed() const
      return true;
 }
 
-bool QGuiApplicationPrivate::shouldQuit()
+bool QGuiApplicationPrivate::canQuitAutomatically()
 {
-    return lastWindowClosed();
+    if (quitOnLastWindowClosed && !lastWindowClosed())
+        return false;
+
+    return QCoreApplicationPrivate::canQuitAutomatically();
 }
 
 void QGuiApplicationPrivate::quit()

src/gui/kernel/qguiapplication_p.h

@@ -105,11 +105,12 @@ public:
 #if QT_CONFIG(commandlineparser)
     void addQtOptions(QList<QCommandLineOption> *options) override;
 #endif
-    virtual bool shouldQuit() override;
+    bool canQuitAutomatically() override;
     void quit() override;
 
     void maybeLastWindowClosed(QWindow *closedWindow);
     bool lastWindowClosed() const;
+    static bool quitOnLastWindowClosed;
 
     static void captureGlobalModifierState(QEvent *e);
     static Qt::KeyboardModifiers modifier_buttons;

tests/auto/gui/kernel/qguiapplication/tst_qguiapplication.cpp

@@ -77,6 +77,7 @@ private slots:
     void quitOnLastWindowClosed();
     void quitOnLastWindowClosedMulti();
     void dontQuitOnLastWindowClosed();
+    void quitOnLastWindowClosedWithEventLoopLocker();
     void genericPluginsAndWindowSystemEvents();
     void layoutDirection();
     void globalShareContext();
@@ -969,6 +970,121 @@ void tst_QGuiApplication::dontQuitOnLastWindowClosed()
     QCOMPARE(spyLastWindowClosed.count(), 1); // lastWindowClosed emitted
 }
 
+class QuitSpy : public QObject
+{
+    Q_OBJECT
+public:
+    QuitSpy()
+    {
+        qGuiApp->installEventFilter(this);
+    }
+    bool eventFilter(QObject *o, QEvent *e) override
+    {
+        Q_UNUSED(o);
+        if (e->type() == QEvent::Quit)
+            ++quits;
+
+        return false;
+    }
+
+    int quits = 0;
+};
+
+void tst_QGuiApplication::quitOnLastWindowClosedWithEventLoopLocker()
+{
+    int argc = 0;
+    QGuiApplication app(argc, nullptr);
+
+    QVERIFY(app.quitOnLastWindowClosed());
+    QVERIFY(app.isQuitLockEnabled());
+
+    auto defaultRestorer = qScopeGuard([&]{
+        app.setQuitLockEnabled(true);
+        app.setQuitOnLastWindowClosed(true);
+    });
+
+    {
+        // Disabling QEventLoopLocker support should not affect
+        // quitting when last window is closed.
+        app.setQuitLockEnabled(false);
+
+        QuitSpy quitSpy;
+        QWindow window;
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, &window, &QWindow::close);
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 1);
+    }
+
+    {
+        // Disabling quitOnLastWindowClosed support should not affect
+        // quitting when last QEventLoopLocker goes out of scope.
+        app.setQuitLockEnabled(true);
+        app.setQuitOnLastWindowClosed(false);
+
+        QuitSpy quitSpy;
+        QScopedPointer<QEventLoopLocker> locker(new QEventLoopLocker);
+        QWindow window;
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, [&]{ locker.reset(nullptr); });
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 1);
+    }
+
+    {
+        // With both properties enabled we need to get rid of both
+        // the window and locker to trigger a quit.
+        app.setQuitLockEnabled(true);
+        app.setQuitOnLastWindowClosed(true);
+
+        QuitSpy quitSpy;
+        QScopedPointer<QEventLoopLocker> locker(new QEventLoopLocker);
+        QWindow window;
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, &window, &QWindow::close);
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 0);
+
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, [&]{ locker.reset(nullptr); });
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 0);
+
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, [&]{ locker.reset(nullptr); });
+        QTimer::singleShot(0, &window, &QWindow::close);
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 1);
+    }
+
+    {
+        // With neither properties enabled we don't get automatic quit.
+        app.setQuitLockEnabled(false);
+        app.setQuitOnLastWindowClosed(false);
+
+        QuitSpy quitSpy;
+        QScopedPointer<QEventLoopLocker> locker(new QEventLoopLocker);
+        QWindow window;
+        window.show();
+        QVERIFY(QTest::qWaitForWindowExposed(&window));
+        QTimer::singleShot(0, [&]{ locker.reset(nullptr); });
+        QTimer::singleShot(0, &window, &QWindow::close);
+        QTimer::singleShot(200, &app, []{ QCoreApplication::exit(0); });
+        app.exec();
+        QCOMPARE(quitSpy.quits, 0);
+    }
+}
+
 static Qt::ScreenOrientation testOrientationToSend = Qt::PrimaryOrientation;
 
 class TestPlugin : public QObject