From 466006e4a13fd557d7b35c1fe0b7a38384e5ce2e Mon Sep 17 00:00:00 2001 From: Steve Block Date: Wed, 28 Oct 2009 14:40:26 +0000 Subject: [PATCH] Updates Geolocation to use a pair of maps to store watchers. This reflects a recent change submitted to webkit.org (see http://trac.webkit.org/changeset/50159). This came about in the course of upstreaming the logic to make sure the error callback is called when permissions have already been denied (see http://trac.webkit.org/changeset/50190). Also make some minor style and comment changes to reflect other recent submissions to webkit.org. This will avoid future merge conflicts. Change-Id: Iafca02403b781bc2119e6e762cb04df396ca8c11 --- WebCore/page/Geolocation.cpp | 107 ++++++++++++++++++++++++++++++------------- WebCore/page/Geolocation.h | 34 ++++++++++---- 2 files changed, 100 insertions(+), 41 deletions(-) diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp index ba3516dd0..74b3f79ee 100644 --- a/WebCore/page/Geolocation.cpp +++ b/WebCore/page/Geolocation.cpp @@ -41,7 +41,7 @@ namespace WebCore { -static const char* permissionDeniedErrorMessage = "User denied Geolocation"; +static const char permissionDeniedErrorMessage[] = "User denied Geolocation"; Geolocation::GeoNotifier::GeoNotifier(Geolocation* geolocation, PassRefPtr successCallback, PassRefPtr errorCallback, PassRefPtr options) : m_geolocation(geolocation) @@ -49,7 +49,6 @@ Geolocation::GeoNotifier::GeoNotifier(Geolocation* geolocation, PassRefPtr error) { + // This method is called at most once on a given GeoNotifier object. + ASSERT(!m_fatalError); m_fatalError = error; m_timer.startOneShot(0); } +bool Geolocation::GeoNotifier::hasZeroTimeout() const +{ + return m_options->hasTimeout() && m_options->timeout() == 0; +} + void Geolocation::GeoNotifier::setCachedPosition(Geoposition* cachedPosition) { // We do not take owenership from the caller, but add our own ref count. @@ -96,12 +102,52 @@ void Geolocation::GeoNotifier::timerFired(Timer*) } if (m_errorCallback) { - RefPtr error = PositionError::create(PositionError::TIMEOUT, "Timed out"); + RefPtr error = PositionError::create(PositionError::TIMEOUT, "Timeout expired"); m_errorCallback->handleEvent(error.get()); } m_geolocation->requestTimedOut(this); } +void Geolocation::Watchers::set(int id, PassRefPtr notifier) +{ + m_idToNotifierMap.set(id, notifier); + m_notifierToIdMap.set(notifier, id); +} + +void Geolocation::Watchers::remove(int id) +{ + IdToNotifierMap::iterator iter = m_idToNotifierMap.find(id); + if (iter == m_idToNotifierMap.end()) + return; + m_notifierToIdMap.remove(iter->second); + m_idToNotifierMap.remove(iter); +} + +void Geolocation::Watchers::remove(GeoNotifier* notifier) +{ + NotifierToIdMap::iterator iter = m_notifierToIdMap.find(notifier); + if (iter == m_notifierToIdMap.end()) + return; + m_idToNotifierMap.remove(iter->second); + m_notifierToIdMap.remove(iter); +} + +void Geolocation::Watchers::clear() +{ + m_idToNotifierMap.clear(); + m_notifierToIdMap.clear(); +} + +bool Geolocation::Watchers::isEmpty() const +{ + return m_idToNotifierMap.isEmpty(); +} + +void Geolocation::Watchers::getNotifiersVector(Vector >& copy) const +{ + copyValuesToVector(m_idToNotifierMap, copy); +} + static const char* databaseName = "/CachedPosition.db"; class CachedPositionManager { @@ -275,7 +321,7 @@ void Geolocation::disconnectFrame() void Geolocation::getCurrentPosition(PassRefPtr successCallback, PassRefPtr errorCallback, PassRefPtr options) { - RefPtr notifier = makeRequest(successCallback, errorCallback, options); + RefPtr notifier = startRequest(successCallback, errorCallback, options); ASSERT(notifier); m_oneShots.add(notifier); @@ -283,24 +329,23 @@ void Geolocation::getCurrentPosition(PassRefPtr successCallbac int Geolocation::watchPosition(PassRefPtr successCallback, PassRefPtr errorCallback, PassRefPtr options) { - RefPtr notifier = makeRequest(successCallback, errorCallback, options); + RefPtr notifier = startRequest(successCallback, errorCallback, options); ASSERT(notifier); - static int sIdentifier = 0; - m_watchers.set(++sIdentifier, notifier); - - return sIdentifier; + static int nextAvailableWatchId = 1; + m_watchers.set(nextAvailableWatchId, notifier.release()); + return nextAvailableWatchId++; } -PassRefPtr Geolocation::makeRequest(PassRefPtr successCallback, PassRefPtr errorCallback, PassRefPtr options) +PassRefPtr Geolocation::startRequest(PassRefPtr successCallback, PassRefPtr errorCallback, PassRefPtr options) { RefPtr notifier = GeoNotifier::create(this, successCallback, errorCallback, options); // Check whether permissions have already been denied. Note that if this is the case, // the permission state can not change again in the lifetime of this page. - if (isDenied()) { + if (isDenied()) notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage)); - } else { + else { if (haveSuitableCachedPosition(notifier->m_options.get())) { ASSERT(m_cachedPositionManager->cachedPosition()); if (isAllowed()) @@ -310,7 +355,7 @@ PassRefPtr Geolocation::makeRequest(PassRefPtrstartUpdating(notifier->m_options.get())) + if (notifier->hasZeroTimeout() || m_service->startUpdating(notifier->m_options.get())) notifier->startTimerIfNeeded(); else notifier->setFatalError(PositionError::create(PositionError::UNKNOWN_ERROR, "Failed to start Geolocation service")); @@ -324,12 +369,7 @@ void Geolocation::fatalErrorOccurred(Geolocation::GeoNotifier* notifier) { // This request has failed fatally. Remove it from our lists. m_oneShots.remove(notifier); - for (GeoNotifierMap::iterator iter = m_watchers.begin(); iter != m_watchers.end(); ++iter) { - if (iter->second == notifier) { - m_watchers.remove(iter); - break; - } - } + m_watchers.remove(notifier); if (!hasListeners()) m_service->stopUpdating(); @@ -400,7 +440,7 @@ void Geolocation::setIsAllowed(bool allowed) m_allowGeolocation = allowed ? Yes : No; if (!isAllowed()) { - RefPtr error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage); + RefPtr error = PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage); error->setIsFatal(true); handleError(error.get()); return; @@ -462,7 +502,7 @@ void Geolocation::stopTimersForOneShots() void Geolocation::stopTimersForWatchers() { Vector > copy; - copyValuesToVector(m_watchers, copy); + m_watchers.getNotifiersVector(copy); stopTimer(copy); } @@ -476,15 +516,16 @@ void Geolocation::stopTimers() void Geolocation::handleError(PositionError* error) { ASSERT(error); - + Vector > oneShotsCopy; copyToVector(m_oneShots, oneShotsCopy); Vector > watchersCopy; - copyValuesToVector(m_watchers, watchersCopy); + m_watchers.getNotifiersVector(watchersCopy); // Clear the lists before we make the callbacks, to avoid clearing notifiers - // added by calls to Geolocation methods from the callbacks. + // added by calls to Geolocation methods from the callbacks, and to prevent + // further callbacks to these notifiers. m_oneShots.clear(); if (error->isFatal()) m_watchers.clear(); @@ -514,19 +555,20 @@ void Geolocation::requestPermission() page->chrome()->requestGeolocationPermissionForFrame(m_frame, this); } -void Geolocation::geolocationServicePositionChanged(GeolocationService*) +void Geolocation::geolocationServicePositionChanged(GeolocationService* service) { + ASSERT_UNUSED(service, service == m_service); ASSERT(m_service->lastPosition()); m_cachedPositionManager->setCachedPosition(m_service->lastPosition()); // Stop all currently running timers. stopTimers(); - + if (!isAllowed()) { // requestPermission() will ask the chrome for permission. This may be // implemented synchronously or asynchronously. In both cases, - // makeSucessCallbacks() will be called if permission is granted, so + // makeSuccessCallbacks() will be called if permission is granted, so // there's nothing more to do here. requestPermission(); return; @@ -539,15 +581,16 @@ void Geolocation::makeSuccessCallbacks() { ASSERT(m_service->lastPosition()); ASSERT(isAllowed()); - + Vector > oneShotsCopy; copyToVector(m_oneShots, oneShotsCopy); - + Vector > watchersCopy; - copyValuesToVector(m_watchers, watchersCopy); - + m_watchers.getNotifiersVector(watchersCopy); + // Clear the lists before we make the callbacks, to avoid clearing notifiers - // added by calls to Geolocation methods from the callbacks. + // added by calls to Geolocation methods from the callbacks, and to prevent + // further callbacks to these notifiers. m_oneShots.clear(); sendPosition(oneShotsCopy, m_service->lastPosition()); diff --git a/WebCore/page/Geolocation.h b/WebCore/page/Geolocation.h index 401189b6f..d9b23c486 100644 --- a/WebCore/page/Geolocation.h +++ b/WebCore/page/Geolocation.h @@ -82,7 +82,8 @@ private: public: static PassRefPtr create(Geolocation* geolocation, PassRefPtr positionCallback, PassRefPtr positionErrorCallback, PassRefPtr options) { return adoptRef(new GeoNotifier(geolocation, positionCallback, positionErrorCallback, options)); } - void setFatalError(PassRefPtr error); + void setFatalError(PassRefPtr); + bool hasZeroTimeout() const; void setCachedPosition(Geoposition* cachedPosition); void startTimerIfNeeded(); void timerFired(Timer*); @@ -96,14 +97,29 @@ private: RefPtr m_cachedPosition; private: - GeoNotifier(Geolocation* geolocation, PassRefPtr, PassRefPtr, PassRefPtr); + GeoNotifier(Geolocation*, PassRefPtr, PassRefPtr, PassRefPtr); + }; + + class Watchers { + public: + void set(int id, PassRefPtr); + void remove(int id); + void remove(GeoNotifier*); + void clear(); + bool isEmpty() const; + void getNotifiersVector(Vector >&) const; + private: + typedef HashMap > IdToNotifierMap; + typedef HashMap, int> NotifierToIdMap; + IdToNotifierMap m_idToNotifierMap; + NotifierToIdMap m_notifierToIdMap; }; bool hasListeners() const { return !m_oneShots.isEmpty() || !m_watchers.isEmpty(); } void sendError(Vector >&, PositionError*); void sendPosition(Vector >&, Geoposition*); - + static void stopTimer(Vector >&); void stopTimersForOneShots(); void stopTimersForWatchers(); @@ -113,26 +129,26 @@ private: void handleError(PositionError*); void requestPermission(); - PassRefPtr makeRequest(PassRefPtr, PassRefPtr, PassRefPtr); // GeolocationServiceClient virtual void geolocationServicePositionChanged(GeolocationService*); virtual void geolocationServiceErrorOccurred(GeolocationService*); + PassRefPtr startRequest(PassRefPtr, PassRefPtr, PassRefPtr); + // EventListener virtual bool operator==(const EventListener&); virtual void handleEvent(ScriptExecutionContext*, Event*); - void fatalErrorOccurred(GeoNotifier* notifier); - void requestTimedOut(GeoNotifier* notifier); - void requestReturnedCachedPosition(GeoNotifier* notifier); + void fatalErrorOccurred(GeoNotifier*); + void requestTimedOut(GeoNotifier*); + void requestReturnedCachedPosition(GeoNotifier*); bool haveSuitableCachedPosition(PositionOptions*); typedef HashSet > GeoNotifierSet; - typedef HashMap > GeoNotifierMap; GeoNotifierSet m_oneShots; - GeoNotifierMap m_watchers; + Watchers m_watchers; Frame* m_frame; OwnPtr m_service; -- 2.11.0