From c7d0c5294bbef1855c84c8072058813371f1af16 Mon Sep 17 00:00:00 2001 From: Steve Block Date: Wed, 16 Dec 2009 12:57:38 +0000 Subject: Fixes a Geolocation bug with cached positions. In the case where a watch request returns a cached position, we must clear the cached position to prevent it from being used repeatedly in the case where the watch timer later fires. Also cleans up some code and adds some comments. This will be upstreamed to webkit.org in https://bugs.webkit.org/show_bug.cgi?id=30676 Change-Id: I4968731453c3dfd34a8d3466fdaee91d4c4158be --- WebCore/page/Geolocation.cpp | 21 +++++++++++++-------- .../platform/android/GeolocationServiceAndroid.cpp | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp index 8dfe19c..5fbad47 100644 --- a/WebCore/page/Geolocation.cpp +++ b/WebCore/page/Geolocation.cpp @@ -101,6 +101,9 @@ void Geolocation::GeoNotifier::timerFired(Timer*) if (m_cachedPosition) { m_successCallback->handleEvent(m_cachedPosition.get()); + // Clear the cached position in case this is a watch request, which + // will continue to run. + m_cachedPosition = 0; geolocation->requestReturnedCachedPosition(this); return; } @@ -403,16 +406,13 @@ void Geolocation::requestTimedOut(GeoNotifier* notifier) void Geolocation::requestReturnedCachedPosition(GeoNotifier* notifier) { // If this is a one-shot request, stop it. - if (m_oneShots.contains(notifier)) { - m_oneShots.remove(notifier); - if (!hasListeners()) - m_service->stopUpdating(); - return; - } + m_oneShots.remove(notifier); + if (!hasListeners()) + m_service->stopUpdating(); // Otherwise, if the watch still exists, start the service to get updates. if (m_watchers.contains(notifier)) { - if (m_service->startUpdating(notifier->m_options.get())) + if (notifier->hasZeroTimeout() || m_service->startUpdating(notifier->m_options.get())) notifier->startTimerIfNeeded(); else notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, "Failed to start Geolocation service")); @@ -471,8 +471,10 @@ void Geolocation::setIsAllowed(bool allowed) makeSuccessCallbacks(); else { GeoNotifierSet::const_iterator end = m_requestsAwaitingCachedPosition.end(); - for (GeoNotifierSet::const_iterator iter = m_requestsAwaitingCachedPosition.begin(); iter != end; ++iter) + for (GeoNotifierSet::const_iterator iter = m_requestsAwaitingCachedPosition.begin(); iter != end; ++iter) { + ASSERT(m_cachedPositionManager->cachedPosition()); (*iter)->setCachedPosition(m_cachedPositionManager->cachedPosition()); + } } m_requestsAwaitingCachedPosition.clear(); } @@ -622,6 +624,9 @@ void Geolocation::geolocationServiceErrorOccurred(GeolocationService* service) { ASSERT(service->lastError()); + // Note that we do not stop timers here. For one-shots, the request is + // cleared in handleError. For watchers, the spec requires that the timer is + // not cleared. handleError(service->lastError()); } diff --git a/WebCore/platform/android/GeolocationServiceAndroid.cpp b/WebCore/platform/android/GeolocationServiceAndroid.cpp index d44b3f0..43a8f5f 100644 --- a/WebCore/platform/android/GeolocationServiceAndroid.cpp +++ b/WebCore/platform/android/GeolocationServiceAndroid.cpp @@ -92,7 +92,7 @@ bool GeolocationServiceAndroid::startUpdating(PositionOptions* options) void GeolocationServiceAndroid::stopUpdating() { // Called when the Geolocation object has no watches or one shots in - // progress. + // progress. This may be called repeatedly. m_javaBridge.clear(); // Reset last position and error to make sure that we always try to get a // new position from the system service when a request is first made. -- cgit v1.1