Improve the initialization behavior of CategoryModel

CategoryModel was missing an update call in the ::componentComplete
callback, as well as reacting on the name changed signal of the plugin.
An update call was also missing in the setPlugin method.
This patch makes it react on the attached signal instead, and
adds missing update calls.

Fixes: QTBUG-70254
Change-Id: I1a8de0137b4fe4af7c5ffc848799061147febba2
Reviewed-by: Michael Brasser <michael.brasser@live.com>

2 files changed, 27 insertions, 7 deletions

diff --git a/src/location/declarativeplaces/qdeclarativesupportedcategoriesmodel.cpp b/src/location/declarativeplaces/qdeclarativesupportedcategoriesmodel.cpp
index b7c6e319..3bc45c4f 100644
--- a/src/location/declarativeplaces/qdeclarativesupportedcategoriesmodel.cpp
+++ b/src/location/declarativeplaces/qdeclarativesupportedcategoriesmodel.cpp
@@ -39,6 +39,7 @@
 #include "qdeclarativeplaceicon_p.h"
 #include "qgeoserviceprovider.h"
 #include "error_messages_p.h"
+#include <QtCore/private/qobject_p.h>
 
 #include <QCoreApplication>
 #include <QtQml/QQmlInfo>
@@ -121,6 +122,15 @@ QT_BEGIN_NAMESPACE
 */
 
 /*!
+    \qmlmethod void QtLocation::CategoryModel::update()
+    \internal
+
+    Updates the model.
+
+    \note The CategoryModel auto updates automatically when needed. Calling this method explicitly is normally not necessary.
+*/
+
+/*!
     \internal
     \enum QDeclarativeSupportedCategoriesModel::Roles
 */
@@ -143,6 +153,8 @@ QDeclarativeSupportedCategoriesModel::~QDeclarativeSupportedCategoriesModel()
 void QDeclarativeSupportedCategoriesModel::componentComplete()
 {
     m_complete = true;
+    if (m_plugin) // do not try to load or change status when trying to update in componentComplete() if the plugin hasn't been set yet even once.
+        update();
 }
 
 /*!
@@ -255,6 +267,7 @@ void QDeclarativeSupportedCategoriesModel::setPlugin(QDeclarativeGeoServiceProvi
 
     //disconnect the manager of the old plugin if we have one
     if (m_plugin) {
+        disconnect(m_plugin, nullptr, this, nullptr);
         QGeoServiceProvider *serviceProvider = m_plugin->sharedGeoServiceProvider();
         if (serviceProvider) {
             QPlaceManager *placeManager = serviceProvider->placeManager();
@@ -273,14 +286,17 @@ void QDeclarativeSupportedCategoriesModel::setPlugin(QDeclarativeGeoServiceProvi
 
     m_plugin = plugin;
 
-    // handle plugin name changes -> update categories
+    // handle plugin attached changes -> update categories
     if (m_plugin) {
-        connect(m_plugin, SIGNAL(nameChanged(QString)), this, SLOT(connectNotificationSignals()));
-        connect(m_plugin, SIGNAL(nameChanged(QString)), this, SLOT(update()));
+        if (m_plugin->isAttached()) {
+            connectNotificationSignals();
+            update();
+        } else {
+            connect(m_plugin, &QDeclarativeGeoServiceProvider::attached, this, &QDeclarativeSupportedCategoriesModel::update);
+            connect(m_plugin, &QDeclarativeGeoServiceProvider::attached, this, &QDeclarativeSupportedCategoriesModel::connectNotificationSignals);
+        }
     }
 
-    connectNotificationSignals();
-
     if (m_complete)
         emit pluginChanged();
 }
@@ -499,6 +515,9 @@ void QDeclarativeSupportedCategoriesModel::connectNotificationSignals()
 */
 void QDeclarativeSupportedCategoriesModel::update()
 {
+    if (!m_complete)
+        return;
+
     if (m_response)
         return;
 
diff --git a/tests/auto/declarative_core/tst_categorymodel.qml b/tests/auto/declarative_core/tst_categorymodel.qml
index 0b6e50a3..86d0fd4c 100644
--- a/tests/auto/declarative_core/tst_categorymodel.qml
+++ b/tests/auto/declarative_core/tst_categorymodel.qml
@@ -221,7 +221,7 @@ TestCase {
         //iteration.
 
         //try updating with an uninitialized plugin instance.
-        testModel.plugin = uninitializedPlugin;
+        testModel.plugin = uninitializedPlugin; // uninitialized does not trigger update on setPlugin
         testModel.update();
         tryCompare(statusChangedSpy, "count", 2);
         compare(testModel.status, CategoryModel.Error);
@@ -229,8 +229,9 @@ TestCase {
 
         //try searching with plugin a instance
         //that has been provided a non-existent name
+        tryCompare(statusChangedSpy, "count", 0);
         testModel.plugin = nonExistantPlugin;
-        testModel.update();
+//        testModel.update(); //QTBUG-70254
         tryCompare(statusChangedSpy, "count", 2);
         compare(testModel.status, CategoryModel.Error);
     }