Use weak pointers for registered QgsOptionsWidgetFactorys

Avoids QGIS crashing if a plugin crashes without deregistering its QgsOptionsWidgetFactory.
qgis · Apr 11, 2017 · a45e570 · a45e570
1 parent 7a6efa2
commit a45e570
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 17 deletions.
diff --git 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
@@ -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 <qgsoptionswidgetfactory.h>
+%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 <qgsoptionswidgetfactory.h>
+%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
@@ -9335,7 +9335,14 @@ void QgisApp::showOptionsDialog( QWidget *parent, const QString &currentPage )
 
   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
@@ -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<QgsMapLayerConfigWidgetFactory *> mMapLayerPanelFactories;
-    QList<QgsOptionsWidgetFactory *> mOptionsWidgetFactories;
+    QList<QPointer<QgsOptionsWidgetFactory>> mOptionsWidgetFactories;
 
     QList<QgsCustomDropHandler *> mCustomDropHandlers;
 

diff --git a/src/gui/qgsoptionswidgetfactory.h b/src/gui/qgsoptionswidgetfactory.h
@@ -18,6 +18,7 @@
 
 #include <QListWidgetItem>
 #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;