[Webkit-unassigned] [Bug 30676] Geolocation maximumAge property is not applied

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30676





--- Comment #14 from Jeremy Orlow <jorlow at chromium.org>  2010-03-10 10:12:06 PST ---
(From update of attachment 49596)

> Index: WebCore/page/Geolocation.cpp
> ===================================================================
> --- WebCore/page/Geolocation.cpp	(revision 55163)
> +++ WebCore/page/Geolocation.cpp	(working copy)

> +void Geolocation::GeoNotifier::makeSuccessCallback(Geoposition* position)

Make seems like a confusing word.  run, exec, execute all seem more clear.

> +{
> +    m_successCallback->handleEvent(position);
> +}
> +
>  void Geolocation::GeoNotifier::startTimerIfNeeded()
>  {
>      if (m_options->hasTimeout())
> @@ -122,6 +136,14 @@ void Geolocation::GeoNotifier::timerFire
>          return;
>      }
>  
> +    if (m_useCachedPosition) {
> +        // Clear the cached position flag in case this is a watch request, which
> +        // will continue to run.
> +        m_useCachedPosition = false;
> +        m_geolocation->requestUsesCachedPosition(this);
> +        return;
> +    }
> +
>      if (m_errorCallback) {
>          RefPtr<PositionError> error = PositionError::create(PositionError::TIMEOUT, "Timeout expired");
>          m_errorCallback->handleEvent(error.get());
> @@ -155,6 +177,11 @@ void Geolocation::Watchers::remove(GeoNo
>      m_notifierToIdMap.remove(iter);
>  }
>  
> +bool Geolocation::Watchers::contains(GeoNotifier* notifier) const
> +{
> +    return m_notifierToIdMap.contains(notifier);
> +}
> +
>  void Geolocation::Watchers::clear()
>  {
>      m_idToNotifierMap.clear();
> @@ -178,6 +205,7 @@ Geolocation::Geolocation(Frame* frame)
>  #endif
>      , m_allowGeolocation(Unknown)
>      , m_shouldClearCache(false)
> +    , m_positionCache(new GeolocationPositionCache)
>  {
>      if (!m_frame)
>          return;
> @@ -249,14 +277,18 @@ PassRefPtr<Geolocation::GeoNotifier> Geo
>      if (isDenied())
>          notifier->setFatalError(PositionError::create(PositionError::PERMISSION_DENIED, permissionDeniedErrorMessage));
>      else {

Why isn't this just an "else if".  This seems like needless indentation.


> Index: LayoutTests/fast/dom/Geolocation/resources/maximum-age.js
> ===================================================================
> --- LayoutTests/fast/dom/Geolocation/resources/maximum-age.js	(revision 0)
> +++ LayoutTests/fast/dom/Geolocation/resources/maximum-age.js	(revision 0)
> +function testNonZeroMaximumAge() {
> +    // Update the mock service to report an error.
> +    window.layoutTestController.setMockGeolocationError(mockCode, mockMessage);
> +    // The maximumAge is non-zero, so we expect the cached position, not the error from the service.

How does this work, if you set the error here?  Shouldn't this happen after you
get the zero age?

> +    navigator.geolocation.getCurrentPosition(function(p) {
> +        checkPosition(p);
> +        testZeroMaximumAgeError();
> +    }, function(e) {
> +        testFailed('Error callback invoked unexpectedly');
> +        window.layoutTestController.notifyDone();
> +    }, {maximumAge: 1000});
> +}
> +
> +function testZeroMaximumAgeError() {
> +    // The default maximumAge is zero, so we expect the error from the service.
> +    navigator.geolocation.getCurrentPosition(function(p) {
> +        testFailed('Success callback invoked unexpectedly');
> +        window.layoutTestController.notifyDone();
> +    }, function(e) {
> +        checkError(e);
> +        debug('<br /><span class="pass">TEST COMPLETE</span>');
> +        window.layoutTestController.notifyDone();
> +    });
> +}

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.