From a45e5700da9c3e5bdad0482c7238cb724cdf3f38 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 11 Apr 2017 14:25:50 +1000 Subject: [PATCH] Use weak pointers for registered QgsOptionsWidgetFactorys Avoids QGIS crashing if a plugin crashes without deregistering its QgsOptionsWidgetFactory. --- python/auto_sip.blacklist | 1 - python/gui/qgsoptionswidgetfactory.sip | 77 +++++++++++++++++++++++--- src/app/qgisapp.cpp | 9 ++- src/app/qgisapp.h | 3 +- src/gui/qgsoptionswidgetfactory.h | 20 ++++--- 5 files changed, 93 insertions(+), 17 deletions(-) diff --git a/python/auto_sip.blacklist b/python/auto_sip.blacklist index baac760b348..8f1534dc9e0 100644 --- a/python/auto_sip.blacklist +++ b/python/auto_sip.blacklist @@ -429,7 +429,6 @@ gui/qgsnewnamedialog.sip gui/qgsnewvectorlayerdialog.sip gui/qgsnewgeopackagelayerdialog.sip gui/qgsoptionsdialogbase.sip -gui/qgsoptionswidgetfactory.sip gui/qgsorderbydialog.sip gui/qgsowssourceselect.sip gui/qgspanelwidget.sip diff --git a/python/gui/qgsoptionswidgetfactory.sip b/python/gui/qgsoptionswidgetfactory.sip index 2411ae63095..5c765511560 100644 --- a/python/gui/qgsoptionswidgetfactory.sip +++ b/python/gui/qgsoptionswidgetfactory.sip @@ -1,39 +1,102 @@ +/************************************************************************ + * This file has been generated automatically from * + * * + * src/gui/qgsoptionswidgetfactory.h * + * * + * Do not edit manually ! Edit header and run scripts/sipify.pl again * + ************************************************************************/ + + + class QgsOptionsPageWidget : QWidget { -%TypeHeaderCode -#include +%Docstring + Base class for widgets for pages included in the options dialog. +.. versionadded:: 3.0 %End +%TypeHeaderCode +#include "qgsoptionswidgetfactory.h" +%End public: QgsOptionsPageWidget( QWidget *parent /TransferThis/ = 0 ); +%Docstring + Constructor for QgsOptionsPageWidget. +%End public slots: virtual void apply() = 0; +%Docstring + Called to permanently apply the settings shown in the options page (e.g. save them to + QgsSettings objects). This is usually called when the options dialog is accepted. +%End }; -class QgsOptionsWidgetFactory +class QgsOptionsWidgetFactory : QObject { -%TypeHeaderCode -#include +%Docstring +QGIS crashing when a plugin crashes/exits without deregistering a factory %End +%TypeHeaderCode +#include "qgsoptionswidgetfactory.h" +%End public: QgsOptionsWidgetFactory(); +%Docstring +Constructor +%End QgsOptionsWidgetFactory( const QString &title, const QIcon &icon ); - virtual ~QgsOptionsWidgetFactory(); +%Docstring +Constructor +%End virtual QIcon icon() const; +%Docstring + The icon that will be shown in the UI for the panel. + :return: A QIcon for the panel icon. + \see setIcon() + :rtype: QIcon +%End + void setIcon( const QIcon &icon ); +%Docstring + Set the icon to show in the interface for the factory object. + \see icon() +%End + virtual QString title() const; +%Docstring + The title of the panel. + \see setTitle() + :rtype: str +%End + void setTitle( const QString &title ); +%Docstring + Set the title for the interface. + \see title() +%End virtual QgsOptionsPageWidget *createWidget( QWidget *parent = 0 ) const = 0 /Factory/; - +%Docstring + Factory function to create the widget on demand as needed by the options dialog. + \param parent The parent of the widget. + :return: A new widget to show as a page in the options dialog. + :rtype: QgsOptionsPageWidget +%End }; +/************************************************************************ + * This file has been generated automatically from * + * * + * src/gui/qgsoptionswidgetfactory.h * + * * + * Do not edit manually ! Edit header and run scripts/sipify.pl again * + ************************************************************************/ diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp index 3538072be9c..5bd797c58fb 100644 --- a/src/app/qgisapp.cpp +++ b/src/app/qgisapp.cpp @@ -9335,7 +9335,14 @@ void QgisApp::showOptionsDialog( QWidget *parent, const QString ¤tPage ) bool oldCapitalize = mySettings.value( QStringLiteral( "qgis/capitalizeLayerName" ), QVariant( false ) ).toBool(); - std::unique_ptr< QgsOptions > optionsDialog( new QgsOptions( parent, QgisGui::ModalDialogFlags, mOptionsWidgetFactories ) ); + QList< QgsOptionsWidgetFactory * > factories; + Q_FOREACH ( const QPointer< QgsOptionsWidgetFactory > &f, mOptionsWidgetFactories ) + { + // remove any deleted factories + if ( f ) + factories << f; + } + std::unique_ptr< QgsOptions > optionsDialog( new QgsOptions( parent, QgisGui::ModalDialogFlags, factories ) ); if ( !currentPage.isEmpty() ) { optionsDialog->setCurrentPage( currentPage ); diff --git a/src/app/qgisapp.h b/src/app/qgisapp.h index ad2a1f67553..702a749f71c 100644 --- a/src/app/qgisapp.h +++ b/src/app/qgisapp.h @@ -134,6 +134,7 @@ class QgsDiagramProperties; #include "qgsraster.h" #include "qgsrasterminmaxorigin.h" #include "qgsmaplayeractionregistry.h" +#include "qgsoptionswidgetfactory.h" #include "ui_qgisapp.h" #include "qgis_app.h" @@ -1957,7 +1958,7 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow QgsSnappingUtils *mSnappingUtils = nullptr; QList mMapLayerPanelFactories; - QList mOptionsWidgetFactories; + QList> mOptionsWidgetFactories; QList mCustomDropHandlers; diff --git a/src/gui/qgsoptionswidgetfactory.h b/src/gui/qgsoptionswidgetfactory.h index 12742b5aa12..6c106bdf087 100644 --- a/src/gui/qgsoptionswidgetfactory.h +++ b/src/gui/qgsoptionswidgetfactory.h @@ -18,6 +18,7 @@ #include #include "qgis_gui.h" +#include "qgis.h" /** \ingroup gui * \class QgsOptionsPageWidget @@ -33,7 +34,7 @@ class GUI_EXPORT QgsOptionsPageWidget : public QWidget /** * Constructor for QgsOptionsPageWidget. */ - QgsOptionsPageWidget( QWidget *parent = nullptr ) + QgsOptionsPageWidget( QWidget *parent SIP_TRANSFERTHIS = nullptr ) : QWidget( parent ) {} @@ -52,21 +53,26 @@ class GUI_EXPORT QgsOptionsPageWidget : public QWidget * A factory class for creating custom options pages. * \since QGIS 3.0 */ -class GUI_EXPORT QgsOptionsWidgetFactory +// NOTE - this is a QObject so we can detect its destruction and avoid +// QGIS crashing when a plugin crashes/exits without deregistering a factory +class GUI_EXPORT QgsOptionsWidgetFactory : public QObject { + Q_OBJECT + public: //! Constructor - QgsOptionsWidgetFactory() {} + QgsOptionsWidgetFactory() + : QObject() + {} //! Constructor QgsOptionsWidgetFactory( const QString &title, const QIcon &icon ) - : mTitle( title ) + : QObject() + , mTitle( title ) , mIcon( icon ) {} - virtual ~QgsOptionsWidgetFactory() = default; - /** * \brief The icon that will be shown in the UI for the panel. * \returns A QIcon for the panel icon. @@ -97,7 +103,7 @@ class GUI_EXPORT QgsOptionsWidgetFactory * \param parent The parent of the widget. * \returns A new widget to show as a page in the options dialog. */ - virtual QgsOptionsPageWidget *createWidget( QWidget *parent = nullptr ) const = 0; + virtual QgsOptionsPageWidget *createWidget( QWidget *parent = nullptr ) const = 0 SIP_FACTORY; private: QString mTitle;