winrt: React on position source status change events

Querying the position source's status is error prone as it is only updated
if position tracking is active. Instead we can react on changes for this
status and get updated information when it is available.

Change-Id: I18d55faf6163568243d2e05d3c63f13e9e45f413
Reviewed-by: Andre de la Rocha <andre.rocha@qt.io>
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Miguel Costa <miguel.costa@qt.io>

2 files changed, 52 insertions, 55 deletions

diff --git a/src/plugins/position/winrt/qgeopositioninfosource_winrt.cpp b/src/plugins/position/winrt/qgeopositioninfosource_winrt.cpp
index b55d2ba3..dcc99e51 100644
--- a/src/plugins/position/winrt/qgeopositioninfosource_winrt.cpp
+++ b/src/plugins/position/winrt/qgeopositioninfosource_winrt.cpp
@@ -57,11 +57,14 @@ using namespace ABI::Windows::Foundation;
 using namespace ABI::Windows::Foundation::Collections;
 
 typedef ITypedEventHandler<Geolocator *, PositionChangedEventArgs *> GeoLocatorPositionHandler;
+typedef ITypedEventHandler<Geolocator *, StatusChangedEventArgs *> GeoLocatorStatusHandler;
 typedef IAsyncOperationCompletedHandler<Geoposition*> PositionHandler;
 typedef IAsyncOperationCompletedHandler<GeolocationAccessStatus> AccessHandler;
 
 Q_DECLARE_LOGGING_CATEGORY(lcPositioningWinRT)
 
+Q_DECLARE_METATYPE(QGeoPositionInfoSource::Error)
+
 QT_BEGIN_NAMESPACE
 
 #ifndef Q_OS_WINRT
@@ -116,33 +119,14 @@ public:
     bool updatesOngoing;
     int minimumUpdateInterval;
 
-    PositionStatus nativeStatus() const;
+    PositionStatus positionStatus = PositionStatus_NotInitialized;
 };
 
-PositionStatus QGeoPositionInfoSourceWinRTPrivate::nativeStatus() const
-{
-#ifdef Q_OS_WINRT
-    qCDebug(lcPositioningWinRT) << __FUNCTION__;
-
-    PositionStatus status;
-    HRESULT hr = QEventDispatcherWinRT::runOnXamlThread([this, &status]() {
-        return locator->get_LocationStatus(&status);
-    });
-    if (FAILED(hr)) {
-        qErrnoWarning(hr, "Could not query status");
-        return PositionStatus_NotAvailable;
-    }
-    return status;
-#else
-    return PositionStatus_Ready;
-#endif
-}
-
-
 QGeoPositionInfoSourceWinRT::QGeoPositionInfoSourceWinRT(QObject *parent)
     : QGeoPositionInfoSource(parent)
     , d_ptr(new QGeoPositionInfoSourceWinRTPrivate)
 {
+    qRegisterMetaType<QGeoPositionInfoSource::Error>();
     qCDebug(lcPositioningWinRT) << __FUNCTION__;
     CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
     Q_D(QGeoPositionInfoSourceWinRT);
@@ -190,6 +174,7 @@ int QGeoPositionInfoSourceWinRT::init()
     }
 
     d->positionToken.value = 0;
+    d->statusToken.value = 0;
 
     d->periodicTimer.setSingleShot(true);
     connect(&d->periodicTimer, &QTimer::timeout, this, &QGeoPositionInfoSourceWinRT::virtualPositionUpdate);
@@ -215,10 +200,9 @@ QGeoPositionInfoSource::PositioningMethods QGeoPositionInfoSourceWinRT::supporte
 {
     Q_D(const QGeoPositionInfoSourceWinRT);
 
-    PositionStatus status = d->nativeStatus();
-    qCDebug(lcPositioningWinRT) << __FUNCTION__ << status;
+    qCDebug(lcPositioningWinRT) << __FUNCTION__;
 
-    switch (status) {
+    switch (d->positionStatus) {
     case PositionStatus::PositionStatus_NoData:
     case PositionStatus::PositionStatus_Disabled:
     case PositionStatus::PositionStatus_NotAvailable:
@@ -238,7 +222,7 @@ void QGeoPositionInfoSourceWinRT::setPreferredPositioningMethods(QGeoPositionInf
     if (previousPreferredPositioningMethods == preferredPositioningMethods())
         return;
 
-    const bool needsRestart = d->positionToken.value != 0;
+    const bool needsRestart = d->positionToken.value != 0 || d->statusToken.value != 0;
 
     if (needsRestart)
         stopHandler();
@@ -266,7 +250,7 @@ void QGeoPositionInfoSourceWinRT::setUpdateInterval(int msec)
     if (msec != 0 && msec < minimumUpdateInterval())
         msec = minimumUpdateInterval();
 
-    const bool needsRestart = d->positionToken.value != 0;
+    const bool needsRestart = d->positionToken.value != 0 || d->statusToken.value != 0;
 
     if (needsRestart)
         stopHandler();
@@ -330,7 +314,7 @@ bool QGeoPositionInfoSourceWinRT::startHandler()
         return false;
     }
 
-    if (!requestAccess() || !checkNativeState())
+    if (!requestAccess())
         return false;
 
     HRESULT hr = QEventDispatcherWinRT::runOnXamlThread([this, d]() {
@@ -348,6 +332,10 @@ bool QGeoPositionInfoSourceWinRT::startHandler()
                                              &d->positionToken);
         RETURN_HR_IF_FAILED("Could not add position handler");
 
+        hr = d->locator->add_StatusChanged(Callback<GeoLocatorStatusHandler>(this,
+                                                                                 &QGeoPositionInfoSourceWinRT::onStatusChanged).Get(),
+                                             &d->statusToken);
+        RETURN_HR_IF_FAILED("Could not add status handler");
         return hr;
     });
     if (FAILED(hr)) {
@@ -367,9 +355,11 @@ void QGeoPositionInfoSourceWinRT::stopHandler()
         return;
     QEventDispatcherWinRT::runOnXamlThread([d]() {
         d->locator->remove_PositionChanged(d->positionToken);
+        d->locator->remove_StatusChanged(d->statusToken);
         return S_OK;
     });
     d->positionToken.value = 0;
+    d->statusToken.value = 0;
 }
 
 void QGeoPositionInfoSourceWinRT::requestUpdate(int timeout)
@@ -395,12 +385,6 @@ void QGeoPositionInfoSourceWinRT::virtualPositionUpdate()
     Q_D(QGeoPositionInfoSourceWinRT);
     QMutexLocker locker(&d->mutex);
 
-    // Need to check if services are still running and ok
-    if (!checkNativeState()) {
-        stopUpdates();
-        return;
-    }
-
     // The operating system did not provide information in time
     // Hence we send a virtual position update to keep same behavior
     // between backends.
@@ -466,28 +450,10 @@ void QGeoPositionInfoSourceWinRT::setError(QGeoPositionInfoSource::Error positio
     emit QGeoPositionInfoSource::error(positionError);
 }
 
-bool QGeoPositionInfoSourceWinRT::checkNativeState()
+void QGeoPositionInfoSourceWinRT::reactOnError(QGeoPositionInfoSource::Error positionError)
 {
-    Q_D(QGeoPositionInfoSourceWinRT);
-    qCDebug(lcPositioningWinRT) << __FUNCTION__;
-
-    PositionStatus status = d->nativeStatus();
-
-    bool result = false;
-    switch (status) {
-    case PositionStatus::PositionStatus_NotAvailable:
-        setError(QGeoPositionInfoSource::UnknownSourceError);
-        break;
-    case PositionStatus::PositionStatus_Disabled:
-    case PositionStatus::PositionStatus_NoData:
-        setError(QGeoPositionInfoSource::ClosedError);
-        break;
-    default:
-        setError(QGeoPositionInfoSource::NoError);
-        result = true;
-        break;
-    }
-    return result;
+    setError(positionError);
+    stopUpdates();
 }
 
 HRESULT QGeoPositionInfoSourceWinRT::onPositionChanged(IGeolocator *locator, IPositionChangedEventArgs *args)
@@ -597,6 +563,34 @@ HRESULT QGeoPositionInfoSourceWinRT::onPositionChanged(IGeolocator *locator, IPo
     return S_OK;
 }
 
+HRESULT QGeoPositionInfoSourceWinRT::onStatusChanged(IGeolocator *, IStatusChangedEventArgs *args)
+{
+    Q_D(QGeoPositionInfoSourceWinRT);
+
+    HRESULT hr = args->get_Status(&d->positionStatus);
+    RETURN_HR_IF_FAILED("Could not obtain position status");
+    qCDebug(lcPositioningWinRT) << __FUNCTION__ << d->positionStatus;
+    QGeoPositionInfoSource::Error error = QGeoPositionInfoSource::NoError;
+    switch (d->positionStatus) {
+    case PositionStatus::PositionStatus_NotAvailable:
+        error = QGeoPositionInfoSource::UnknownSourceError;
+        break;
+    case PositionStatus::PositionStatus_Disabled:
+        error = QGeoPositionInfoSource::AccessError;
+        break;
+    case PositionStatus::PositionStatus_NoData:
+        error = QGeoPositionInfoSource::ClosedError;
+        break;
+    }
+    if (error != QGeoPositionInfoSource::NoError) {
+        QMetaObject::invokeMethod(this, "reactOnError", Qt::QueuedConnection,
+                                  Q_ARG(QGeoPositionInfoSource::Error,
+                                        QGeoPositionInfoSource::UnknownSourceError));
+    }
+
+    return S_OK;
+}
+
 bool QGeoPositionInfoSourceWinRT::requestAccess() const
 {
     qCDebug(lcPositioningWinRT) << __FUNCTION__;
diff --git a/src/plugins/position/winrt/qgeopositioninfosource_winrt_p.h b/src/plugins/position/winrt/qgeopositioninfosource_winrt_p.h
index 4319ccae..61d0af95 100644
--- a/src/plugins/position/winrt/qgeopositioninfosource_winrt_p.h
+++ b/src/plugins/position/winrt/qgeopositioninfosource_winrt_p.h
@@ -62,6 +62,7 @@ namespace ABI {
             namespace Geolocation{
                 struct IGeolocator;
                 struct IPositionChangedEventArgs;
+                struct IStatusChangedEventArgs;
             }
         }
     }
@@ -90,6 +91,8 @@ public:
 
     HRESULT onPositionChanged(ABI::Windows::Devices::Geolocation::IGeolocator *locator,
                               ABI::Windows::Devices::Geolocation::IPositionChangedEventArgs *args);
+    HRESULT onStatusChanged(ABI::Windows::Devices::Geolocation::IGeolocator *locator,
+                              ABI::Windows::Devices::Geolocation::IStatusChangedEventArgs *args);
 
     bool requestAccess() const;
 Q_SIGNALS:
@@ -105,12 +108,12 @@ private slots:
     void virtualPositionUpdate();
     void singleUpdateTimeOut();
     void updateSynchronized(const QGeoPositionInfo info);
+    void reactOnError(QGeoPositionInfoSource::Error positionError);
 private:
     bool startHandler();
 
     Q_DISABLE_COPY(QGeoPositionInfoSourceWinRT)
     void setError(QGeoPositionInfoSource::Error positionError);
-    bool checkNativeState();
 
     QScopedPointer<QGeoPositionInfoSourceWinRTPrivate> d_ptr;
     Q_DECLARE_PRIVATE(QGeoPositionInfoSourceWinRT)