Fix PluginParameter not working with script as property values

This patch introduces the undocumented
QDeclarativeGeoServiceProviderParameter::initialized signal, to signal
to the parent when a parameter becomes fully initialized, so that
the parent QDeclarativeGeoServiceProvider can attach.

Task-number: QTBUG-62075
Change-Id: Ie3615abf31d19f39587c5e54b202f8f2c4a711cc
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>

5 files changed, 94 insertions, 7 deletions

diff --git a/src/location/declarativemaps/qdeclarativegeoserviceprovider.cpp b/src/location/declarativemaps/qdeclarativegeoserviceprovider.cpp
index 4b8b2d7c..9e4fee8a 100644
--- a/src/location/declarativemaps/qdeclarativegeoserviceprovider.cpp
+++ b/src/location/declarativemaps/qdeclarativegeoserviceprovider.cpp
@@ -113,12 +113,42 @@ void QDeclarativeGeoServiceProvider::setName(const QString &name)
         return;
 
     name_ = name;
+
+    if (complete_)
+        tryAttach();
+
+    emit nameChanged(name_);
+}
+
+/*!
+    \internal
+*/
+bool QDeclarativeGeoServiceProvider::parametersReady() {
+    for (const QDeclarativeGeoServiceProviderParameter *p: qAsConst(parameters_)) {
+        if (!p->isInitialized())
+            return false;
+    }
+    return true;
+}
+
+/*!
+    \internal
+*/
+void QDeclarativeGeoServiceProvider::tryAttach()
+{
+    if (!parametersReady())
+        return;
+
     delete sharedProvider_;
+    sharedProvider_ = nullptr;
+
+    if (name_.isEmpty())
+        return;
+
     sharedProvider_ = new QGeoServiceProvider(name_, parameterMap());
     sharedProvider_->setLocale(locales_.at(0));
     sharedProvider_->setAllowExperimental(experimental_);
 
-    emit nameChanged(name_);
     emit attached();
 }
 
@@ -147,11 +177,17 @@ QStringList QDeclarativeGeoServiceProvider::availableServiceProviders()
 void QDeclarativeGeoServiceProvider::componentComplete()
 {
     complete_ = true;
-    if (!name_.isEmpty()) {
-        return;
+
+    for (QDeclarativeGeoServiceProviderParameter *p: qAsConst(parameters_)) {
+        if (!p->isInitialized()) {
+            connect(p, &QDeclarativeGeoServiceProviderParameter::initialized,
+                    this, &QDeclarativeGeoServiceProvider::tryAttach);
+        }
     }
 
-    if (!prefer_.isEmpty()
+    if (!name_.isEmpty()) {
+        tryAttach();
+    } else if (!prefer_.isEmpty()
             || required_->mappingRequirements() != NoMappingFeatures
             || required_->routingRequirements() != NoRoutingFeatures
             || required_->geocodingRequirements() != NoGeocodingFeatures
@@ -796,15 +832,18 @@ QDeclarativeGeoServiceProviderParameter::~QDeclarativeGeoServiceProviderParamete
     \qmlproperty string PluginParameter::name
 
     This property holds the name of the plugin parameter as a single formatted string.
+    This property is a write-once property.
 */
 void QDeclarativeGeoServiceProviderParameter::setName(const QString &name)
 {
-    if (name_ == name)
+    if (!name_.isEmpty() || name.isEmpty())
         return;
 
     name_ = name;
 
     emit nameChanged(name_);
+    if (value_.isValid())
+        emit initialized();
 }
 
 QString QDeclarativeGeoServiceProviderParameter::name() const
@@ -816,15 +855,18 @@ QString QDeclarativeGeoServiceProviderParameter::name() const
     \qmlproperty QVariant PluginParameter::value
 
     This property holds the value of the plugin parameter which support different types of values (variant).
+    This property is a write-once property.
 */
 void QDeclarativeGeoServiceProviderParameter::setValue(const QVariant &value)
 {
-    if (value_ == value)
+    if (value_.isValid() || !value.isValid() || value.isNull())
         return;
 
     value_ = value;
 
     emit valueChanged(value_);
+    if (!name_.isEmpty())
+        emit initialized();
 }
 
 QVariant QDeclarativeGeoServiceProviderParameter::value() const
@@ -832,6 +874,11 @@ QVariant QDeclarativeGeoServiceProviderParameter::value() const
     return value_;
 }
 
+bool QDeclarativeGeoServiceProviderParameter::isInitialized() const
+{
+    return !name_.isEmpty() && value_.isValid();
+}
+
 /*******************************************************************************
 *******************************************************************************/
 
diff --git a/src/location/declarativemaps/qdeclarativegeoserviceprovider_p.h b/src/location/declarativemaps/qdeclarativegeoserviceprovider_p.h
index 426c6b4d..c1ad4987 100644
--- a/src/location/declarativemaps/qdeclarativegeoserviceprovider_p.h
+++ b/src/location/declarativemaps/qdeclarativegeoserviceprovider_p.h
@@ -78,9 +78,12 @@ public:
     void setValue(const QVariant &value);
     QVariant value() const;
 
+    bool isInitialized() const;
+
 Q_SIGNALS:
     void nameChanged(const QString &name);
     void valueChanged(const QVariant &value);
+    void initialized();
 
 private:
     QString name_;
@@ -210,6 +213,8 @@ Q_SIGNALS:
     void allowExperimentalChanged(bool allow);
 
 private:
+    bool parametersReady();
+    void tryAttach();
     static void parameter_append(QQmlListProperty<QDeclarativeGeoServiceProviderParameter> *prop, QDeclarativeGeoServiceProviderParameter *mapObject);
     static int parameter_count(QQmlListProperty<QDeclarativeGeoServiceProviderParameter> *prop);
     static QDeclarativeGeoServiceProviderParameter *parameter_at(QQmlListProperty<QDeclarativeGeoServiceProviderParameter> *prop, int index);
diff --git a/tests/auto/declarative_core/tst_plugin.qml b/tests/auto/declarative_core/tst_plugin.qml
index 3dabba07..7b880f1d 100644
--- a/tests/auto/declarative_core/tst_plugin.qml
+++ b/tests/auto/declarative_core/tst_plugin.qml
@@ -99,9 +99,14 @@ Item {
             verify(invalidPlugin.supportsRouting())
             verify(invalidPlugin.supportsPlaces())
 
-            invalidPlugin.name = ''
+            invalidPlugin.name = 'here'
             compare(invalidAttachedSpy.count, 2)
+            verify(invalidPlugin.supportsMapping(Plugin.OnlineMappingFeature))
+            verify(invalidPlugin.supportsGeocoding(Plugin.OnlineGeocodingFeature))
+            verify(invalidPlugin.supportsRouting(Plugin.OnlineRoutingFeature))
 
+            invalidPlugin.name = ''
+            compare(invalidAttachedSpy.count, 2)
             verify(!invalidPlugin.supportsMapping())
             verify(!invalidPlugin.supportsGeocoding())
             verify(!invalidPlugin.supportsRouting())
diff --git a/tests/auto/declarative_ui/tst_map.qml b/tests/auto/declarative_ui/tst_map.qml
index 408bf96e..5c9deca2 100644
--- a/tests/auto/declarative_ui/tst_map.qml
+++ b/tests/auto/declarative_ui/tst_map.qml
@@ -50,6 +50,20 @@ Item {
             }
         ]
     }
+    Plugin {
+        id: testPluginLazyParameter;
+        name: "qmlgeo.test.plugin"
+        allowExperimental: true
+        property string extraTypeName : undefined
+        PluginParameter { name: "supported"; value: true}
+        PluginParameter { name: "finishRequestImmediately"; value: true}
+        PluginParameter { name: "validateWellKnownValues"; value: true}
+        PluginParameter { name: "extraMapTypeName"; value: testPluginLazyParameter.extraTypeName}
+
+        Component.onCompleted: {
+            extraTypeName = "SomeString"
+        }
+    }
 
     property variant coordinate1: QtPositioning.coordinate(10, 11)
     property variant coordinate2: QtPositioning.coordinate(12, 13)
@@ -106,6 +120,11 @@ Item {
     Map {id: mapTiltBearingHere; plugin: herePlugin; center: coordinate1;
         width: 1000; height: 1000; zoomLevel: 4; bearing: 45.0; tilt: 25.0 }
 
+    Map {
+        id: mapWithLazyPlugin
+        plugin: testPluginLazyParameter
+    }
+
     MapParameter {
         id: testParameter
         type: "cameraCenter_test"
@@ -129,6 +148,11 @@ Item {
             mapCenterSpy.clear();
         }
 
+        function test_lazy_parameter() {
+            compare(mapWithLazyPlugin.supportedMapTypes.length, 5)
+            compare(mapWithLazyPlugin.supportedMapTypes[4].name, "SomeString")
+        }
+
         function test_map_center() {
             // coordinate is set at map element declaration
             compare(map.center.latitude, 10)
diff --git a/tests/auto/geotestplugin/qgeotiledmappingmanagerengine_test.h b/tests/auto/geotestplugin/qgeotiledmappingmanagerengine_test.h
index 5f6f0116..2fcf654d 100644
--- a/tests/auto/geotestplugin/qgeotiledmappingmanagerengine_test.h
+++ b/tests/auto/geotestplugin/qgeotiledmappingmanagerengine_test.h
@@ -68,6 +68,12 @@ public:
         mapTypes << QGeoMapType(QGeoMapType::SatelliteMapDay, tr("SatelliteMapDay"), tr("SatelliteMapDay"), false, false, 2, pluginName);
         mapTypes << QGeoMapType(QGeoMapType::CycleMap, tr("CycleMap"), tr("CycleMap"), false, false, 3, pluginName);
         mapTypes << QGeoMapType(QGeoMapType::CustomMap, tr("AlternateCameraCapabilities"), tr("AlternateCameraCapabilities"), false, false, 4, pluginName);
+
+        if (parameters.contains(QStringLiteral("extraMapTypeName"))) {
+            QString  extraMapTypeName = parameters.value(QStringLiteral("extraMapTypeName")).toString();
+            mapTypes << QGeoMapType(QGeoMapType::CustomMap, extraMapTypeName, extraMapTypeName, false, false, 5, pluginName);
+        }
+
         setSupportedMapTypes(mapTypes);
 
         QGeoTileFetcherTest *fetcher = new QGeoTileFetcherTest(this);