Last Comment Bug 716127 - Always sharing Geolocation fails to determine location on every other reload
: Always sharing Geolocation fails to determine location on every other reload
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Geolocation (show other bugs)
: 9 Branch
: All All
: -- normal (vote)
: mozilla12
Assigned To: Josh Matthews [:jdm]
:
:
Mentors:
: 731444 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-06 16:18 PST by Jason Smith [:jsmith]
Modified: 2012-03-01 06:32 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Avoid shutting down geolocation providers if one-shot updates are pending. (935 bytes, patch)
2012-01-08 20:36 PST, Josh Matthews [:jdm]
doug.turner: review+
Details | Diff | Splinter Review
Test for overlapping geolocation provider shutdown and unfulfilled request. (9.12 KB, patch)
2012-01-08 23:21 PST, Josh Matthews [:jdm]
doug.turner: review+
Details | Diff | Splinter Review
Console Log Wifi Geo Share Location Twice Quickly (2.43 KB, text/plain)
2012-02-28 16:20 PST, Jason Smith [:jsmith]
no flags Details
When the network geolocation provider is told to watch for wifi changes, force a change to occur to avoid indefinite waiting. (1.15 KB, patch)
2012-02-28 20:35 PST, Josh Matthews [:jdm]
doug.turner: review+
Details | Diff | Splinter Review

Description Jason Smith [:jsmith] 2012-01-06 16:18:21 PST
Litmus test: [geolocation] always allow sharing location

Steps:

1. Go to http://mozqa.com/data/firefox/geolocation/position.html
2. When prompted to share location, select "always"
3. Reload the page

Expected:

The latitude and longitude for your current geographic location is shown.

Actual:

undefined is outputted to the web page

Additional notes:

Continuous reloads of the page after output will show the geolocation determined every other time. On other times, undefined is outputted.
Comment 1 Josh Matthews [:jdm] 2012-01-08 20:36:52 PST
Created attachment 586871 [details] [diff] [review]
Avoid shutting down geolocation providers if one-shot updates are pending.
Comment 2 Josh Matthews [:jdm] 2012-01-08 20:39:35 PST
Thank you for filing this bug, Jason! The testcase does not actually show any incorrect behaviour, it turns out - the geolocation request includes a timeout, and the "undefined" appears when the request times out and attempts to display an undefined error message (I don't believe the message property is actually in the spec). However, if you remove the timeout, the behaviour is still seen - it turns out that the geolocation service shuts down after 6 seconds unless there are outstanding requests, but it only considers watchPosition requests when making that decision.
Comment 3 Doug Turner (:dougt) 2012-01-08 21:08:01 PST
Comment on attachment 586871 [details] [diff] [review]
Avoid shutting down geolocation providers if one-shot updates are pending.

Review of attachment 586871 [details] [diff] [review]:
-----------------------------------------------------------------

This makes sense.  bonus points for adding a mochitest.
Comment 4 Josh Matthews [:jdm] 2012-01-08 23:21:28 PST
Created attachment 586894 [details] [diff] [review]
Test for overlapping geolocation provider shutdown and unfulfilled request.

Good call. I did a bit of cleaning up in the geolocation service to make this testable.
Comment 5 Doug Turner (:dougt) 2012-01-09 11:52:29 PST
Comment on attachment 586894 [details] [diff] [review]
Test for overlapping geolocation provider shutdown and unfulfilled request.

Review of attachment 586894 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/src/geolocation/nsGeolocation.cpp
@@ +512,4 @@
>  
>  nsresult nsGeolocationService::Init()
>  {
> +  Preferences::AddIntVarCache(&sProviderTimeout, "geo.timeout", 6000);

use sProviderTimeout instead of 6000.
Comment 8 Jason Smith [:jsmith] 2012-02-28 16:10:59 PST
*** Bug 731444 has been marked as a duplicate of this bug. ***
Comment 9 Jason Smith [:jsmith] 2012-02-28 16:19:21 PST
Currently broken on Firefox 12 and Nightly 13 with the following test case:

Steps:

1. Go to 1. Go to http://mozqa.com/data/firefox/geolocation/position.html
2. When prompted to share location, select "share"
3. Reload the page
4. When prompted to share location, select "share"

Expected:

The coordinates of my position should be shown.

Actual:

undefined is returned.

See attached log.
Comment 10 Jason Smith [:jsmith] 2012-02-28 16:20:28 PST
Created attachment 601446 [details]
Console Log Wifi Geo Share Location Twice Quickly
Comment 11 Josh Matthews [:jdm] 2012-02-28 20:28:20 PST
Oh, harumph. The automated test works because we just hammer at the URL without waiting for access point data. In real life, however, the patch here did not change the status quo (much). The problem is that we now don't shutdown the service, but we also don't trigger any further network requests until new access point data is received. If the timeout is removed from the testcase, the correct location is eventually displayed, but we should find a better solution.
Comment 12 Josh Matthews [:jdm] 2012-02-28 20:35:16 PST
Created attachment 601505 [details] [diff] [review]
When the network geolocation provider is told to watch for wifi changes, force a change to occur to avoid indefinite waiting.

This works for me, and makes sense. If we've previously seen a result, there's no reason to expect another one in the near future.
Comment 14 Marco Bonardo [::mak] 2012-03-01 06:32:41 PST
https://hg.mozilla.org/mozilla-central/rev/e485c2c74c4d

Note You need to log in before you can comment on or make changes to this bug.