Move everything KCrash related from Application to ApplicationX11

Summary:
This change ensures that kwin_wayland does not pull in KCrash. We
don't want and need KCrash in the Wayland case. If KWin crashes the
session goes down - restarting doesn't make any sense, we need to
relogin.

Similar drkonqi just doesn't work as it doesn't have a windowing
system to connect to. After all the windowing system just crashed.

Also the AlternativeWM dialog doesn't make any sense on Wayland.
Similar thought: there is no windowing system to show this nice dialog.

Overall it's better to have system default behavior
(e.g. systemd-coredump) than using KCrash in the very special case of
kwin_wayland.

Reviewers: #plasma

Subscribers: plasma-devel

Projects: #plasma

Differential Revision: https://phabricator.kde.org/D1550

CMakeLists.txt

@@ -485,7 +485,6 @@ set(kwin_KDE_LIBS
     KF5::ConfigCore
     KF5::CoreAddons
     KF5::ConfigWidgets
-    KF5::Crash
     KF5::GlobalAccel
     KF5::GlobalAccelPrivate
     KF5::I18n
@@ -570,7 +569,7 @@ target_link_libraries(kwin ${DL_LIBRARY})
 endif()
 
 kf5_add_kdeinit_executable(kwin_x11 main_x11.cpp)
-target_link_libraries(kdeinit_kwin_x11 kwin)
+target_link_libraries(kdeinit_kwin_x11 kwin KF5::Crash)
 
 install(TARGETS kwin             ${INSTALL_TARGETS_DEFAULT_ARGS} LIBRARY NAMELINK_SKIP )
 install(TARGETS kdeinit_kwin_x11 ${INSTALL_TARGETS_DEFAULT_ARGS} )

main.cpp

@@ -36,23 +36,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 // KDE
 #include <KAboutData>
-#include <KConfig>
-#include <KConfigGroup>
-#include <KCrash>
 #include <KLocalizedString>
 #include <KPluginMetaData>
 #include <KSharedConfig>
 // Qt
 #include <qplatformdefs.h>
-#include <QComboBox>
 #include <qcommandlineparser.h>
-#include <QDialog>
-#include <QDialogButtonBox>
-#include <QLabel>
-#include <QPushButton>
 #include <QQuickWindow>
 #include <QStandardPaths>
-#include <QVBoxLayout>
 #include <QtDBus/QtDBus>
 
 // system
@@ -80,52 +71,6 @@ Atoms* atoms;
 int screen_number = -1;
 bool is_multihead = false;
 
-class AlternativeWMDialog : public QDialog
-{
-public:
-    AlternativeWMDialog()
-        : QDialog() {
-        QWidget* mainWidget = new QWidget(this);
-        QVBoxLayout* layout = new QVBoxLayout(mainWidget);
-        QString text = i18n(
-                           "KWin is unstable.\n"
-                           "It seems to have crashed several times in a row.\n"
-                           "You can select another window manager to run:");
-        QLabel* textLabel = new QLabel(text, mainWidget);
-        layout->addWidget(textLabel);
-        wmList = new QComboBox(mainWidget);
-        wmList->setEditable(true);
-        layout->addWidget(wmList);
-
-        addWM(QStringLiteral("metacity"));
-        addWM(QStringLiteral("openbox"));
-        addWM(QStringLiteral("fvwm2"));
-        addWM(QStringLiteral(KWIN_INTERNAL_NAME_X11));
-
-        QVBoxLayout *mainLayout = new QVBoxLayout(this);
-        mainLayout->addWidget(mainWidget);
-        QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this);
-        buttons->button(QDialogButtonBox::Ok)->setDefault(true);
-        connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept);
-        connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject);
-        mainLayout->addWidget(buttons);
-
-        raise();
-    }
-
-    void addWM(const QString& wm) {
-        // TODO: Check if WM is installed
-        if (!QStandardPaths::findExecutable(wm).isEmpty())
-            wmList->addItem(wm);
-    }
-    QString selectedWM() const {
-        return wmList->currentText();
-    }
-
-private:
-    QComboBox* wmList;
-};
-
 int Application::crashes = 0;
 
 bool Application::isX11MultiHead()
@@ -196,8 +141,6 @@ void Application::start()
         m_config->reparseConfiguration();
     }
 
-    crashChecking();
-
     performStartup();
 }
 
@@ -213,55 +156,6 @@ void Application::destroyAtoms()
     atoms = nullptr;
 }
 
-void Application::setupCrashHandler()
-{
-    KCrash::setEmergencySaveFunction(Application::crashHandler);
-}
-
-void Application::crashChecking()
-{
-    setupCrashHandler();
-    if (crashes >= 4) {
-        // Something has gone seriously wrong
-        AlternativeWMDialog dialog;
-        QString cmd = QStringLiteral(KWIN_INTERNAL_NAME_X11);
-        if (dialog.exec() == QDialog::Accepted)
-            cmd = dialog.selectedWM();
-        else
-            ::exit(1);
-        if (cmd.length() > 500) {
-            qCDebug(KWIN_CORE) << "Command is too long, truncating";
-            cmd = cmd.left(500);
-        }
-        qCDebug(KWIN_CORE) << "Starting" << cmd << "and exiting";
-        char buf[1024];
-        sprintf(buf, "%s &", cmd.toAscii().data());
-        system(buf);
-        ::exit(1);
-    }
-    if (crashes >= 2) {
-        // Disable compositing if we have had too many crashes
-        qCDebug(KWIN_CORE) << "Too many crashes recently, disabling compositing";
-        KConfigGroup compgroup(KSharedConfig::openConfig(), "Compositing");
-        compgroup.writeEntry("Enabled", false);
-    }
-    // Reset crashes count if we stay up for more that 15 seconds
-    QTimer::singleShot(15 * 1000, this, SLOT(resetCrashesCount()));
-}
-
-void Application::crashHandler(int signal)
-{
-    crashes++;
-
-    fprintf(stderr, "Application::crashHandler() called with signal %d; recent crashes: %d\n", signal, crashes);
-    char cmd[1024];
-    sprintf(cmd, "%s --crashes %d &",
-            QFile::encodeName(QCoreApplication::applicationFilePath()).constData(), crashes);
-
-    sleep(1);
-    system(cmd);
-}
-
 void Application::resetCrashesCount()
 {
     crashes = 0;

main.h

@@ -185,7 +185,6 @@ Q_SIGNALS:
 protected:
     Application(OperationMode mode, int &argc, char **argv);
     virtual void performStartup() = 0;
-    virtual void setupCrashHandler();
 
     void notifyKSplash();
     void createInput();
@@ -213,16 +212,14 @@ protected:
     }
     void destroyAtoms();
 
-    static void crashHandler(int signal);
-
 protected:
     QString m_originalSessionKey;
+    static int crashes;
 
 private Q_SLOTS:
     void resetCrashesCount();
 
 private:
-    void crashChecking();
     QScopedPointer<XcbEventFilter> m_eventFilter;
     bool m_configLock;
     KSharedConfigPtr m_config;
@@ -234,7 +231,6 @@ private:
     bool m_useKActivities = true;
 #endif
     Platform *m_platform = nullptr;
-    static int crashes;
 };
 
 inline static Application *kwinApp()

main_wayland.cpp

@@ -123,12 +123,6 @@ void ApplicationWayland::performStartup()
     createBackend();
 }
 
-void ApplicationWayland::setupCrashHandler()
-{
-    // this disables auto-restart of kwin_wayland
-    // do nothing hence allowing OS to create dump and so on
-}
-
 void ApplicationWayland::createBackend()
 {
     connect(platform(), &Platform::screensQueried, this, &ApplicationWayland::continueStartupWithScreens);

main_wayland.h

@@ -56,7 +56,6 @@ public:
 
 protected:
     void performStartup() override;
-    void setupCrashHandler() override;
 
 private:
     void createBackend();

main_x11.cpp

@@ -28,12 +28,21 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "xcbutils.h"
 
 // KDE
+#include <KConfigGroup>
+#include <KCrash>
 #include <KLocalizedString>
 #include <KPluginLoader>
 #include <KPluginMetaData>
 // Qt
 #include <qplatformdefs.h>
+#include <QComboBox>
 #include <QCommandLineParser>
+#include <QDialog>
+#include <QDialogButtonBox>
+#include <QFile>
+#include <QLabel>
+#include <QPushButton>
+#include <QVBoxLayout>
 
 // system
 #ifdef HAVE_UNISTD_H
@@ -41,6 +50,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #endif // HAVE_UNISTD_H
 #include <iostream>
 
+Q_LOGGING_CATEGORY(KWIN_CORE, "kwin_core", QtCriticalMsg)
+
 namespace KWin
 {
 
@@ -49,6 +60,54 @@ static void sighandler(int)
     QApplication::exit();
 }
 
+
+
+class AlternativeWMDialog : public QDialog
+{
+public:
+    AlternativeWMDialog()
+        : QDialog() {
+        QWidget* mainWidget = new QWidget(this);
+        QVBoxLayout* layout = new QVBoxLayout(mainWidget);
+        QString text = i18n(
+                           "KWin is unstable.\n"
+                           "It seems to have crashed several times in a row.\n"
+                           "You can select another window manager to run:");
+        QLabel* textLabel = new QLabel(text, mainWidget);
+        layout->addWidget(textLabel);
+        wmList = new QComboBox(mainWidget);
+        wmList->setEditable(true);
+        layout->addWidget(wmList);
+
+        addWM(QStringLiteral("metacity"));
+        addWM(QStringLiteral("openbox"));
+        addWM(QStringLiteral("fvwm2"));
+        addWM(QStringLiteral(KWIN_INTERNAL_NAME_X11));
+
+        QVBoxLayout *mainLayout = new QVBoxLayout(this);
+        mainLayout->addWidget(mainWidget);
+        QDialogButtonBox *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this);
+        buttons->button(QDialogButtonBox::Ok)->setDefault(true);
+        connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept);
+        connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject);
+        mainLayout->addWidget(buttons);
+
+        raise();
+    }
+
+    void addWM(const QString& wm) {
+        // TODO: Check if WM is installed
+        if (!QStandardPaths::findExecutable(wm).isEmpty())
+            wmList->addItem(wm);
+    }
+    QString selectedWM() const {
+        return wmList->currentText();
+    }
+
+private:
+    QComboBox* wmList;
+};
+
 //************************************
 // KWinSelectionOwner
 //************************************
@@ -149,6 +208,8 @@ void ApplicationX11::lostSelection()
 
 void ApplicationX11::performStartup()
 {
+    crashChecking();
+
     if (Application::x11ScreenNumber() == -1) {
         Application::setX11ScreenNumber(QX11Info::appScreen());
     }
@@ -215,6 +276,55 @@ bool ApplicationX11::notify(QObject* o, QEvent* e)
     return QApplication::notify(o, e);
 }
 
+void ApplicationX11::setupCrashHandler()
+{
+    KCrash::setEmergencySaveFunction(ApplicationX11::crashHandler);
+}
+
+void ApplicationX11::crashChecking()
+{
+    setupCrashHandler();
+    if (crashes >= 4) {
+        // Something has gone seriously wrong
+        AlternativeWMDialog dialog;
+        QString cmd = QStringLiteral(KWIN_INTERNAL_NAME_X11);
+        if (dialog.exec() == QDialog::Accepted)
+            cmd = dialog.selectedWM();
+        else
+            ::exit(1);
+        if (cmd.length() > 500) {
+            qCDebug(KWIN_CORE) << "Command is too long, truncating";
+            cmd = cmd.left(500);
+        }
+        qCDebug(KWIN_CORE) << "Starting" << cmd << "and exiting";
+        char buf[1024];
+        sprintf(buf, "%s &", cmd.toAscii().data());
+        system(buf);
+        ::exit(1);
+    }
+    if (crashes >= 2) {
+        // Disable compositing if we have had too many crashes
+        qCDebug(KWIN_CORE) << "Too many crashes recently, disabling compositing";
+        KConfigGroup compgroup(KSharedConfig::openConfig(), "Compositing");
+        compgroup.writeEntry("Enabled", false);
+    }
+    // Reset crashes count if we stay up for more that 15 seconds
+    QTimer::singleShot(15 * 1000, this, SLOT(resetCrashesCount()));
+}
+
+void ApplicationX11::crashHandler(int signal)
+{
+    crashes++;
+
+    fprintf(stderr, "Application::crashHandler() called with signal %d; recent crashes: %d\n", signal, crashes);
+    char cmd[1024];
+    sprintf(cmd, "%s --crashes %d &",
+            QFile::encodeName(QCoreApplication::applicationFilePath()).constData(), crashes);
+
+    sleep(1);
+    system(cmd);
+}
+
 } // namespace
 
 extern "C"

main_x11.h

@@ -56,6 +56,11 @@ private Q_SLOTS:
     void lostSelection();
 
 private:
+    void crashChecking();
+    void setupCrashHandler();
+
+    static void crashHandler(int signal);
+
     QScopedPointer<KWinSelectionOwner> owner;
     bool m_replace;
 };