diff options
| -rw-r--r-- | WebCore/ChangeLog | 20 | ||||
| -rw-r--r-- | WebCore/page/Geolocation.cpp | 10 | ||||
| -rw-r--r-- | WebCore/page/GeolocationController.cpp | 3 |
3 files changed, 27 insertions, 6 deletions
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 68d2cd1..20bedf2 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,23 @@ +2010-01-18 Steve Falkenburg <sfalken@apple.com> + + Reviewed by Sam Weinig. + + <https://bugs.webkit.org/show_bug.cgi?id=33816> + Crashes in Geolocation code due to refcounting, observer balance issues. + + Hold a ref to the GeoNotifier while dispatching a callback. The code was + copying a data member to avoid accessing a freed this ptr, but was still + using the this ptr. + + Geolocation::removeObserver calls are not always balanced with addObserver. + Instead of asserting and continuing, don't try to remove non-existant + observers. + + * page/Geolocation.cpp: + (WebCore::Geolocation::GeoNotifier::timerFired): Protect notifier. + * page/GeolocationController.cpp: + (WebCore::GeolocationController::removeObserver): Change ASSERT into an if with early return. + 2009-12-14 Sam Weinig <sam@webkit.org> Fix the build. diff --git a/WebCore/page/Geolocation.cpp b/WebCore/page/Geolocation.cpp index 231d57a..cc5cc93 100644 --- a/WebCore/page/Geolocation.cpp +++ b/WebCore/page/Geolocation.cpp @@ -121,15 +121,15 @@ void Geolocation::GeoNotifier::timerFired(Timer<GeoNotifier>*) { m_timer.stop(); - // Cache our pointer to the Geolocation object, as this GeoNotifier object + // Protect this GeoNotifier object, since it // could be deleted by a call to clearWatch in a callback. - Geolocation* geolocation = m_geolocation; + RefPtr<GeoNotifier> protect(this); if (m_fatalError) { if (m_errorCallback) m_errorCallback->handleEvent(m_fatalError.get()); // This will cause this notifier to be deleted. - geolocation->fatalErrorOccurred(this); + m_geolocation->fatalErrorOccurred(this); return; } @@ -138,7 +138,7 @@ void Geolocation::GeoNotifier::timerFired(Timer<GeoNotifier>*) // Clear the cached position in case this is a watch request, which // will continue to run. m_cachedPosition = 0; - geolocation->requestReturnedCachedPosition(this); + m_geolocation->requestReturnedCachedPosition(this); return; } @@ -146,7 +146,7 @@ void Geolocation::GeoNotifier::timerFired(Timer<GeoNotifier>*) RefPtr<PositionError> error = PositionError::create(PositionError::TIMEOUT, "Timeout expired"); m_errorCallback->handleEvent(error.get()); } - geolocation->requestTimedOut(this); + m_geolocation->requestTimedOut(this); } void Geolocation::Watchers::set(int id, PassRefPtr<GeoNotifier> prpNotifier) diff --git a/WebCore/page/GeolocationController.cpp b/WebCore/page/GeolocationController.cpp index 44eba6e..968e854 100644 --- a/WebCore/page/GeolocationController.cpp +++ b/WebCore/page/GeolocationController.cpp @@ -54,7 +54,8 @@ void GeolocationController::addObserver(Geolocation* observer) void GeolocationController::removeObserver(Geolocation* observer) { - ASSERT(m_observers.contains(observer)); + if (!m_observers.contains(observer)) + return; m_observers.remove(observer); if (m_observers.isEmpty()) |