Fix Geolocation to correctly handle reentrant calls from callbacks.

This fixes http://b/issue?id=2094823.

2 files changed, 22 insertions, 43 deletions

diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp
index a419b0b..359c125 100644
--- a/WebCore/page/Geolocation.cpp
+++ b/WebCore/page/Geolocation.cpp
@@ -209,22 +209,6 @@ void Geolocation::sendError(Vector<RefPtr<GeoNotifier> >& notifiers, PositionErr
      }
 }
 
-void Geolocation::sendErrorToOneShots(PositionError* error)
-{
-    Vector<RefPtr<GeoNotifier> > copy;
-    copyToVector(m_oneShots, copy);
-
-    sendError(copy, error);
-}
-
-void Geolocation::sendErrorToWatchers(PositionError* error)
-{
-    Vector<RefPtr<GeoNotifier> > copy;
-    copyValuesToVector(m_watchers, copy);
-
-    sendError(copy, error);
-}
-
 void Geolocation::sendPosition(Vector<RefPtr<GeoNotifier> >& notifiers, Geoposition* position)
 {
     Vector<RefPtr<GeoNotifier> >::const_iterator end = notifiers.end();
@@ -237,22 +221,6 @@ void Geolocation::sendPosition(Vector<RefPtr<GeoNotifier> >& notifiers, Geoposit
     }
 }
 
-void Geolocation::sendPositionToOneShots(Geoposition* position)
-{
-    Vector<RefPtr<GeoNotifier> > copy;
-    copyToVector(m_oneShots, copy);
-    
-    sendPosition(copy, position);
-}
-
-void Geolocation::sendPositionToWatchers(Geoposition* position)
-{
-    Vector<RefPtr<GeoNotifier> > copy;
-    copyValuesToVector(m_watchers, copy);
-    
-    sendPosition(copy, position);
-}
-
 void Geolocation::stopTimer(Vector<RefPtr<GeoNotifier> >& notifiers)
 {
     Vector<RefPtr<GeoNotifier> >::const_iterator end = notifiers.end();
@@ -287,14 +255,22 @@ void Geolocation::stopTimers()
 void Geolocation::handleError(PositionError* error)
 {
     ASSERT(error);
-    
-    sendErrorToOneShots(error);    
-    sendErrorToWatchers(error);
 
+    Vector<RefPtr<GeoNotifier> > oneShotsCopy;
+    copyToVector(m_oneShots, oneShotsCopy);
+
+    Vector<RefPtr<GeoNotifier> > watchersCopy;
+    copyValuesToVector(m_watchers, watchersCopy);
+
+    // Clear the lists before we make the callbacks, to avoid clearing notifiers
+    // added by calls to Geolocation methods from the callbacks.
     m_oneShots.clear();
     if (error->isFatal())
         m_watchers.clear();
 
+    sendError(oneShotsCopy, error);
+    sendError(watchersCopy, error);
+
     if (!hasListeners())
         m_service->stopUpdating();
 }
@@ -341,11 +317,19 @@ void Geolocation::makeSuccessCallbacks()
     ASSERT(m_service->lastPosition());
     ASSERT(isAllowed());
 
-    sendPositionToOneShots(m_service->lastPosition());
-    sendPositionToWatchers(m_service->lastPosition());
+    Vector<RefPtr<GeoNotifier> > oneShotsCopy;
+    copyToVector(m_oneShots, oneShotsCopy);
+
+    Vector<RefPtr<GeoNotifier> > watchersCopy;
+    copyValuesToVector(m_watchers, watchersCopy);
 
+    // Clear the lists before we make the callbacks, to avoid clearing notifiers
+    // added by calls to Geolocation methods from the callbacks.
     m_oneShots.clear();
 
+    sendPosition(oneShotsCopy, m_service->lastPosition());
+    sendPosition(watchersCopy, m_service->lastPosition());
+
     if (!hasListeners())
         m_service->stopUpdating();
 }
diff --git a/WebCore/page/Geolocation.h b/WebCore/page/Geolocation.h
index 18c2be3..f74c87b 100644
--- a/WebCore/page/Geolocation.h
+++ b/WebCore/page/Geolocation.h
@@ -95,13 +95,8 @@ private:
     bool hasListeners() const { return !m_oneShots.isEmpty() || !m_watchers.isEmpty(); }
 
     void sendError(Vector<RefPtr<GeoNotifier> >&, PositionError*);
-    void sendErrorToOneShots(PositionError*);
-    void sendErrorToWatchers(PositionError*);
-    
     void sendPosition(Vector<RefPtr<GeoNotifier> >&, Geoposition*);
-    void sendPositionToOneShots(Geoposition*);
-    void sendPositionToWatchers(Geoposition*);
-    
+
     static void stopTimer(Vector<RefPtr<GeoNotifier> >&);
     void stopTimersForOneShots();
     void stopTimersForWatchers();