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
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 @@
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 @@
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 @@
if (isDenied())
notifier->setFatalError(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 @@
stopUpdating();
}
-void Geolocation::requestReturnedCachedPosition(GeoNotifier* notifier)
+void Geolocation::requestUsesCachedPosition(GeoNotifier* notifier)
{
- // If this is a one-shot request, stop it.
- m_oneShots.remove(notifier);
+ // 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());
+
+ // 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();
-
- // 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"));
- }
}
bool Geolocation::haveSuitableCachedPosition(PositionOptions* options)
@@ -373,6 +404,7 @@
RefPtr<PositionError> error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage);
error->setIsFatal(true);
handleError(error.get());
+ m_requestsAwaitingCachedPosition.clear();
return;
}
@@ -381,12 +413,8 @@
// 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<RefPtr<GeoNotifier> >& notifiers, PositionError* error)
diff --git a/WebCore/page/Geolocation.h b/WebCore/page/Geolocation.h
index 808ee9c..077a695 100644
--- a/WebCore/page/Geolocation.h
+++ b/WebCore/page/Geolocation.h
@@ -96,7 +96,8 @@
void setFatalError(PassRefPtr<PositionError>);
bool hasZeroTimeout() const;
- void setCachedPosition(Geoposition* cachedPosition);
+ void setUseCachedPosition();
+ void makeSuccessCallback(Geoposition*);
void startTimerIfNeeded();
void timerFired(Timer<GeoNotifier>*);
@@ -106,7 +107,7 @@
RefPtr<PositionOptions> m_options;
Timer<GeoNotifier> m_timer;
RefPtr<PositionError> m_fatalError;
- RefPtr<Geoposition> m_cachedPosition;
+ bool m_useCachedPosition;
private:
GeoNotifier(Geolocation*, PassRefPtr<PositionCallback>, PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
@@ -161,8 +162,9 @@
void fatalErrorOccurred(GeoNotifier*);
void requestTimedOut(GeoNotifier*);
- void requestReturnedCachedPosition(GeoNotifier*);
+ void requestUsesCachedPosition(GeoNotifier*);
bool haveSuitableCachedPosition(PositionOptions*);
+ void makeCachedPositionCallbacks();
typedef HashSet<RefPtr<GeoNotifier> > GeoNotifierSet;
