From a8d4462216d8e009ee5662660a0621b67c6e727f Mon Sep 17 00:00:00 2001 From: Steve Block Date: Mon, 25 Jan 2010 17:18:32 +0000 Subject: Fixes Geolocation maximumAge implementation to work when requestPermission is synchronous Geolocation::requestPermission may be implemented synchronously or asynchronously. See https://bugs.webkit.org/show_bug.cgi?id=26993 The current implementation of maximumAge on Android requires that requestPermission is asynchronous. This change fixes the code to work with both synchronous and asynchronous implementations of requestPermission. This will allow the maximumAge code to be upstreamed to webkit.org. See https://bugs.webkit.org/show_bug.cgi?id=30676 Change-Id: If7115e5d34ec308c91a1873a6841731dc37c18bd --- WebCore/page/Geolocation.cpp | 94 ++++++++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 33 deletions(-) (limited to 'WebCore/page/Geolocation.cpp') diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp index 0727973..1cde883 100644 --- a/WebCore/page/Geolocation.cpp +++ b/WebCore/page/Geolocation.cpp @@ -101,13 +101,17 @@ bool Geolocation::GeoNotifier::hasZeroTimeout() const return m_options->hasTimeout() && m_options->timeout() == 0; } -void Geolocation::GeoNotifier::setCachedPosition(Geoposition* cachedPosition) +void Geolocation::GeoNotifier::setUseCachedPosition() { - // We do not take owenership from the caller, but add our own ref count. - m_cachedPosition = cachedPosition; + m_useCachedPosition = true; m_timer.startOneShot(0); } +void Geolocation::GeoNotifier::makeSuccessCallback(Geoposition* position) +{ + m_successCallback->handleEvent(position); +} + void Geolocation::GeoNotifier::startTimerIfNeeded() { if (m_options->hasTimeout()) @@ -130,12 +134,11 @@ void Geolocation::GeoNotifier::timerFired(Timer*) return; } - if (m_cachedPosition) { - m_successCallback->handleEvent(m_cachedPosition.get()); - // Clear the cached position in case this is a watch request, which + if (m_useCachedPosition) { + // Clear the cached position flag in case this is a watch request, which // will continue to run. - m_cachedPosition = 0; - m_geolocation->requestReturnedCachedPosition(this); + m_useCachedPosition = false; + m_geolocation->requestUsesCachedPosition(this); return; } @@ -274,14 +277,9 @@ PassRefPtr Geolocation::startRequest(PassRefPtrsetFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage)); else { - if (haveSuitableCachedPosition(notifier->m_options.get())) { - if (isAllowed()) - notifier->setCachedPosition(m_positionCache->cachedPosition()); - else { - m_requestsAwaitingCachedPosition.add(notifier); - requestPermission(); - } - } else { + if (haveSuitableCachedPosition(notifier->m_options.get())) + notifier->setUseCachedPosition(); + else { if (notifier->hasZeroTimeout() || m_service->startUpdating(notifier->m_options.get())) notifier->startTimerIfNeeded(); else @@ -311,20 +309,53 @@ void Geolocation::requestTimedOut(GeoNotifier* notifier) stopUpdating(); } -void Geolocation::requestReturnedCachedPosition(GeoNotifier* notifier) +void Geolocation::requestUsesCachedPosition(GeoNotifier* notifier) { - // If this is a one-shot request, stop it. - m_oneShots.remove(notifier); - if (!hasListeners()) - stopUpdating(); + // This is called asynchronously, so the permissions could have been denied + // since we last checked in startRequest. + if (isDenied()) { + notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage)); + return; + } + + m_requestsAwaitingCachedPosition.add(notifier); + + // If permissions are allowed, make the callback + if (isAllowed()) { + makeCachedPositionCallbacks(); + return; + } + + // Request permissions, which may be synchronous or asynchronous. + requestPermission(); +} + +void Geolocation::makeCachedPositionCallbacks() +{ + // All modifications to m_requestsAwaitingCachedPosition are done + // asynchronously, so we don't need to worry about it being modified from + // the callbacks. + GeoNotifierSet::const_iterator end = m_requestsAwaitingCachedPosition.end(); + for (GeoNotifierSet::const_iterator iter = m_requestsAwaitingCachedPosition.begin(); iter != end; ++iter) { + GeoNotifier* notifier = iter->get(); + notifier->makeSuccessCallback(m_positionCache->cachedPosition()); - // Otherwise, if the watch still exists, start the service to get updates. - if (m_watchers.contains(notifier)) { - if (notifier->hasZeroTimeout() || startUpdating(notifier->m_options.get())) - notifier->startTimerIfNeeded(); - else - notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, "Failed to start Geolocation service")); + // If this is a one-shot request, stop it. Otherwise, if the watch still + // exists, start the service to get updates. + if (m_oneShots.contains(notifier)) + m_oneShots.remove(notifier); + else if (m_watchers.contains(notifier)) { + if (notifier->hasZeroTimeout() || startUpdating(notifier->m_options.get())) + notifier->startTimerIfNeeded(); + else + notifier->setFatalError(PositionError::create(PositionError::POSITION_UNAVAILABLE, "Failed to start Geolocation service")); + } } + + m_requestsAwaitingCachedPosition.clear(); + + if (!hasListeners()) + stopUpdating(); } bool Geolocation::haveSuitableCachedPosition(PositionOptions* options) @@ -373,6 +404,7 @@ void Geolocation::setIsAllowed(bool allowed) RefPtr error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage); error->setIsFatal(true); handleError(error.get()); + m_requestsAwaitingCachedPosition.clear(); return; } @@ -381,12 +413,8 @@ void Geolocation::setIsAllowed(bool allowed) // the position from the service will be at least as fresh. if (lastPosition()) makeSuccessCallbacks(); - else { - GeoNotifierSet::const_iterator end = m_requestsAwaitingCachedPosition.end(); - for (GeoNotifierSet::const_iterator iter = m_requestsAwaitingCachedPosition.begin(); iter != end; ++iter) - (*iter)->setCachedPosition(m_positionCache->cachedPosition()); - } - m_requestsAwaitingCachedPosition.clear(); + else + makeCachedPositionCallbacks(); } void Geolocation::sendError(Vector >& notifiers, PositionError* error) -- cgit v1.1