QItemEditorFactory: made sure the ownership is taken on the

QItemEditorCreator

The creators were not deleted i nthe destructor of QItemEditorFactory
and they could not be safely used for more than one type.

Task-number: 228255
Reviewed-by: jasplin

2 files changed, 45 insertions, 9 deletions

diff --git a/src/gui/itemviews/qitemeditorfactory.cpp b/src/gui/itemviews/qitemeditorfactory.cpp
index c576e40..480a472 100644
--- a/src/gui/itemviews/qitemeditorfactory.cpp
+++ b/src/gui/itemviews/qitemeditorfactory.cpp
@@ -158,6 +158,10 @@ QByteArray QItemEditorFactory::valuePropertyName(QVariant::Type type) const
 */
 QItemEditorFactory::~QItemEditorFactory()
 {
+    //we make sure we delete all the QItemEditorCreatorBase
+    //this has to be done only once, hence the QSet
+    QSet<QItemEditorCreatorBase*> set = creatorMap.values().toSet();
+    qDeleteAll(set);
 }
 
 /*!
@@ -170,8 +174,16 @@ QItemEditorFactory::~QItemEditorFactory()
 */
 void QItemEditorFactory::registerEditor(QVariant::Type type, QItemEditorCreatorBase *creator)
 {
-   delete creatorMap.value(type, 0);
-   creatorMap[type] = creator;
+    QHash<QVariant::Type, QItemEditorCreatorBase *>::iterator it = creatorMap.find(type);
+    if (it != creatorMap.end()) {
+        QItemEditorCreatorBase *oldCreator = it.value();
+        Q_ASSERT(oldCreator);
+        creatorMap.erase(it);
+        if (!creatorMap.values().contains(oldCreator))
+            delete oldCreator; // if it is no more in use we can delete it
+    }
+
+    creatorMap[type] = creator;
 }
 
 class QDefaultItemEditorFactory : public QItemEditorFactory
diff --git a/tests/auto/qitemeditorfactory/tst_qitemeditorfactory.cpp b/tests/auto/qitemeditorfactory/tst_qitemeditorfactory.cpp
index 5540b38..d9a7d56 100644
--- a/tests/auto/qitemeditorfactory/tst_qitemeditorfactory.cpp
+++ b/tests/auto/qitemeditorfactory/tst_qitemeditorfactory.cpp
@@ -61,16 +61,40 @@ void tst_QItemEditorFactory::createEditor()
 
 void tst_QItemEditorFactory::createCustomEditor()
 {
-    QItemEditorFactory editorFactory;
+    //we make it inherit from QObject so that we can use QPointer
+    class MyEditor : public QObject, public QStandardItemEditorCreator<QDoubleSpinBox>
+    {
+    };
 
-    QItemEditorCreatorBase *creator = new QStandardItemEditorCreator<QDoubleSpinBox>();
-    editorFactory.registerEditor(QVariant::Rect, creator);
+    QPointer<MyEditor> creator = new MyEditor;
+    QPointer<MyEditor> creator2 = new MyEditor;
 
-    QWidget parent;
+    {
+        QItemEditorFactory editorFactory;
+
+        editorFactory.registerEditor(QVariant::Rect, creator);
+        editorFactory.registerEditor(QVariant::RectF, creator);
+
+        //creator should not be deleted as a result of calling the next line
+        editorFactory.registerEditor(QVariant::Rect, creator2);
+        QVERIFY(creator);
+
+        //this should erase creator2
+        editorFactory.registerEditor(QVariant::Rect, creator);
+        QVERIFY(creator2.isNull());
+
+
+        QWidget parent;
+
+        QWidget *w = editorFactory.createEditor(QVariant::Rect, &parent);
+        QCOMPARE(w->metaObject()->className(), "QDoubleSpinBox");
+        QCOMPARE(w->metaObject()->userProperty().type(), QVariant::Double);
+    }
 
-    QWidget *w = editorFactory.createEditor(QVariant::Rect, &parent);
-    QCOMPARE(w->metaObject()->className(), "QDoubleSpinBox");
-    QCOMPARE(w->metaObject()->userProperty().type(), QVariant::Double);
+    //editorFactory has been deleted, so should be creator
+    //because editorFActory has the ownership
+    QVERIFY(creator.isNull());
+    QVERIFY(creator2.isNull());
 
     delete creator;
 }