Johan Lorenzo [:jlorenzo] Reporter
	Description • 9 years ago

Pre-requisites
* Have a freshly flashed device
* Be in a place where the geolocation logs shows an accuracy of 0. The Paris office is perfect to constantly reproduce the issue.
* Be connected to a Wi-Fi network

Steps to reproduce
1. Open maps.google.com
2. Allow location sharing
3. Tap the "locate me" button

Expected result
When A-GPS can't get an accurate position, we should switch to another mean of locating, like Mozilla Location Service.

Actual result
An invalid value is returned (in my case 47.000000, 4.000000, in the middle of a forest in France[1], whereas I'm supposed to be in the Paris office) with an accuracy of 0 (the best). So as an end-user who's supposed to be in the middle of the city, Google/Yahoo/Tomtom maps shows your location at 320 km (200 miles) away from your actual location.

Nothing happens if you go outside, until you reboot the phone. After that, you're correctly located at around 5 meters from where you are.

Repro frequency: 5/5

Build info
Build ID               20150619161141
Gaia Revision          31126f9903ce20947a2673cd90efa789a0ba0a2c
Gaia Date              2015-06-19 13:02:56
Gecko Revision         bbb0424b3fe962da6e202ac5200feaaea0001a2e
Gecko Version          41.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150619.015544
Firmware Date          Fri Jun 19 01:55:53 UTC 2015
Bootloader             s1

[1] https://encrypted.google.com/maps/place/47%C2%B000'00.0%22N+4%C2%B000'00.0%22E/@47,4,17z/data=!3m1!4b1!4m2!3m1!1s0x0:0x0

	Johan Lorenzo [:jlorenzo] Reporter
	Comment 1 • 9 years ago

:gerard-majax tried to add some logs to know where this strange value (47.0, 4.0) came from, but we couldn't get a single clue. Kan-Ru, Jamin, could you some places to look for?

	Johan Lorenzo [:jlorenzo] Reporter
	Comment 2 • 9 years ago

[Blocking Requested - why for this release]: Depending on where the GPS is first started (inside or outside), you can end up with a an absolutely wrong position. This is hurting the first time experience, especially on a device like the Z3C.

	Hubert Figuiere [:hub]
	Comment 3 • 9 years ago

Reminds me of Keon where the GPS always thought I was in the middle of Manitoba.... (thousands of km from here) because it didn't have A-GPS working. After a proper GPS fix it was alright.

	Hanno Schlichting
	Comment 4 • 9 years ago

What chipset is the device based on? Qualcomm/Mediatek/Spreadtrum? And if it's Qualcomm does it have Qualcomm's binary blob on it which replaces our GPS/Geo stack? What kind of A-GPS support we get depends on the chipset and the configuration for it is different for each chipset provider. Generally that configuration is done by the OEM/ODM and is outside our control.

	Ravi Dandu
	Comment 5 • 9 years ago

If the number 47.000000, 4.000000) is exactly what was returned, then it must be a estimation of country geographic center. This will need to come from GPS chipset, or WiFi/Cell location provider. 

Is the accuracy of 0, an estimate by visually inspecting the circular uncertainity on maps.google.com, or is it exactly what what was returned by the getCurrentPosition() ?

	:gerard-majax
	Comment 6 • 9 years ago

(In reply to rdandu from comment #5)
> If the number 47.000000, 4.000000) is exactly what was returned, then it
> must be a estimation of country geographic center. This will need to come
> from GPS chipset, or WiFi/Cell location provider. 

That's coming from libloc_eng.

> 
> Is the accuracy of 0, an estimate by visually inspecting the circular
> uncertainity on maps.google.com, or is it exactly what what was returned by
> the getCurrentPosition() ?

It is exactly 0 and we checked that this was the value reported by libloc_eng to Gecko. In fact, this value gets altered a little bit in the upper layers and content gets something like 46.99,3.99

	Johan Lorenzo [:jlorenzo] Reporter
	Comment 7 • 9 years ago

:gerard-majax gave the details.

	Hanno Schlichting
	Comment 8 • 9 years ago

I checked the MLS side. The value doesn't come from MLS, as it would report (lat=48.8600, lon=2.3500, accuracy=690000) for a "center of France" match. So the value does indeed come from the GPS chip.

If you can debug it, one place to look at is GonkGPSGeolocationProvider::NetworkLocationUpdate::Update (https://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/GonkGPSGeolocationProvider.cpp#837) line:

provider->mLastGPSPosition->GetTimestamp(&time_ms)

That gets the timestamp of the GPS location and later looks at how old that is. If the GPS timestamp claims to be recent (from the last two minutes), it will currently always be used and the MLS response ignored. That code exists to prevent the position estimate from jumping between a good GPS position and a much less accurate MLS position, just because the GPS chip needed a bit of time to get a new position update. This assumes a GPS chip will only ever return a precise location, which has been true for the GPS chipsets this was tested with.

One thing to look out for is that the threshold is actually 2 minutes 16 seconds, as GPS time doesn't get adjusted for leap seconds, so currently it is 16 seconds ahead of UTC time. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1176569 to suggest adding a comment about this.

In general the combination of QC chipset and GonkGPSGeolocationProvider has never been tested, so there might be any number of other bugs here.

	Johan Lorenzo [:jlorenzo] Reporter
	Comment 9 • 9 years ago

(In reply to Hanno Schlichting [:hannosch] from comment #8)
> I checked the MLS side. The value doesn't come from MLS, as it would report
> (lat=48.8600, lon=2.3500, accuracy=690000) for a "center of France" match.
I confirm. In the logs I had, MLS returned a correct position in Paris.

For the further testing, I'll first try to repro in Canada, during the work week. Then, I'll I try what you propose, once back in Paris.

	Ravi Dandu
	Comment 10 • 9 years ago

The return of (47.0, 4.00000) was from chipset as a estimation of France from country code. It should come from an low accuracy level, i.e. high uncertainity. Adding Qualcomm Engineer Bhavna Sharma.

	Bhavna Sharma
	Comment 11 • 9 years ago

Yes from the code, (47.000000, 4.000000) is based on the country code table.
Please open a CR with our customer engineer to help further.

	:gerard-majax
	Comment 12 • 9 years ago

(In reply to Bhavna Sharma from comment #11)
> Yes from the code, (47.000000, 4.000000) is based on the country code table.
> Please open a CR with our customer engineer to help further.

Thanks! After speaking with a couple of people, I think on Gecko's side we should just make sure we do not trust such a too high accuracy (0.0000000 from a GPS is not something valid). 

Given we are relying on Sony's blobs, I'm not sure what we can do more to fix this.

	Robert Kaiser
	Comment 13 • 9 years ago

(In reply to Hanno Schlichting [:hannosch] from comment #8)
> That gets the timestamp of the GPS location and later looks at how old that
> is. If the GPS timestamp claims to be recent (from the last two minutes), it
> will currently always be used and the MLS response ignored.

Hrm, given A-GPS responses potentially going lower accuracy, would it make sense to use the one with better accuracy (smaller value while treating 0 as Infinite as it is obviously bogus)?

	Ravi Dandu
	Comment 15 • 9 years ago

There are two things to fix here:
1) Accuracy of 0.0: Work with QC/Sony and fix and find this. 
    Workarounds: a) Change accuracy values of 0.0 to something big. Half of circumference of earth ~20,000 Km.
                 b) Ignore locations whose accuracy value of 0.0 
     
2) Algorithm of converging MLS and GPS in Gecko: Currently, fix coming from GPS (HW) is considered always better than MLS without comparing the accuracy value. This should be fixed by making the decision based on Accuracy value, rather than source of location.

	:garvan Assignee
	Comment 16 • 9 years ago

> 2) Algorithm of converging MLS and GPS in Gecko: Currently, fix coming from
> GPS (HW) is considered always better than MLS without comparing the accuracy
> value. This should be fixed by making the decision based on Accuracy value,
> rather than source of location.

This would be adding complexity to the code for an extreme edge case. We have no evidence for the case that MLS is better than GPS. The worst GPS geolocation I have seen was in Manhattan, which was still better than MLS. 
What other platform does not use the GPS when it has a fix?

	Robert Kaiser
	Comment 18 • 9 years ago

(In reply to Garvan Keeley [:garvank] from comment #16)
> This would be adding complexity to the code for an extreme edge case. We
> have no evidence for the case that MLS is better than GPS. The worst GPS
> geolocation I have seen was in Manhattan, which was still better than MLS.

As seen in the example given here, we can apparently have A-GPS mechanisms return some low accuracy guess instead of an actual GPS-derived value. I think on other systems, you might not actually get a value returned without an actual GPS fix. Unfortunately, a web app right now does not even have a clue if the geolocation value came from a proper GPS fix. And an accuracy of 0 is wrong for sure in any case.

	:garvan Assignee
	Comment 19 • 9 years ago

> As seen in the example given here, we can apparently have A-GPS mechanisms
> return some low accuracy guess instead of an actual GPS-derived value. 

I'm not sure this behaviour could be classified under 'AGPS', which is specific to satellite positioning and TTFF. I agree it seems like some part of they GPS AGPS system is triggering this bug, and leaking an internal lookup (likely for AGPS purpose) as a false fix value.
A GPS returning an arbitrary guess, in the cases reported here 3000 km away (Vancouver to Winnipeg), is a bug in this specific GPS.  

> an actual GPS fix. Unfortunately, a web app right now does not even have a
> clue if the geolocation value came from a proper GPS fix. And an accuracy of
> 0 is wrong for sure in any case.

Agreed, so I only patched the case where accuracy is impossibly low (<1 cm) to ignore these locations.

	Johan Lorenzo [:jlorenzo] Reporter
	Comment 21 • 9 years ago

FWIW, (47.000000, 4.000000) is not the center of France and neither Winnipeg in Canada (like described in bug 1177216).

	:garvan Assignee
	Comment 22 • 9 years ago

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #21)
> FWIW, (47.000000, 4.000000) is not the center of France and neither Winnipeg
> in Canada (like described in bug 1177216).

True, Winnipeg is considered an east-west centre of the country, but that could just be a Canadian convention, certainly not centre north-south.

	Hanno Schlichting
	Comment 23 • 9 years ago

These kind of "center" values tend to be very coarse grained estimates. We don't know where this particular chipset is getting this value from. But similarly our GeoIP database tends to give round values with just one position after the dot. And for example for the UK it gives back a location in London, and for France one in Paris. Neither of them are the countries geographic center, but both are population centers in their countries.

We don't have control over these static mappings.

	:garvan Assignee
	Comment 24 • 9 years ago

It would be nice to get a statement from the vendor:
1) what are these values and what are they used for?
2) do they recognize this is a bug and not a feature for us?
3) are they fixing it?

	Robert Kaiser
	Comment 25 • 9 years ago

(In reply to Garvan Keeley [:garvank] from comment #19)
> Agreed, so I only patched the case where accuracy is impossibly low (<1 cm)
> to ignore these locations.

I think that's a decent workaround for the moment but I also think that having some position with an accuracy reported as "Infinite" would be better than getting nothing as some web apps only want a country-level accuracy.

	:garvan Assignee
	Comment 27 • 9 years ago

(In reply to Robert Kaiser (:kairo@mozilla.com) - on vacation or slow to reply until the end of June from comment #25)
> (In reply to Garvan Keeley [:garvank] from comment #19)
> > Agreed, so I only patched the case where accuracy is impossibly low (<1 cm)
> > to ignore these locations.
> 
> I think that's a decent workaround for the moment but I also think that
> having some position with an accuracy reported as "Infinite" would be better
> than getting nothing as some web apps only want a country-level accuracy.

Disagree. Features specific to a chipset would need to be of much higher value than this case before we should consider them. Also this is a feature that would make location apps behave differently on FxOS than on Android and iOS, which certainly don't do this. 
Lastly, this is not a _feature_ of the GPS at all, the GPS should not be responding with a location if it has no fix.

	Jamin Liu [:jaliu][:Jamin]
	Comment 28 • 9 years ago

Comment on attachment 8627191 [details] [diff] [review]
bug1176261.diff

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

Look good to me. Thanks.

P.S. I'm not a peer of Geolocation module. I'm not sure whether I'm the right person to make a review.

::: dom/system/gonk/GonkGPSGeolocationProvider.cpp
@@ +114,5 @@
>    };
>  
>    MOZ_ASSERT(location);
>  
> +  const double kImpossibleAccuracy_m = 0.001;

You might consider to use 'float' here since accuracy of GpsLocation is declared as 'float' in gps.h.
(GpsLocation's accuracy is 'float' and nsGeoPosition's accuracy is 'double'.)

	:garvan Assignee
	Comment 29 • 9 years ago

> Look good to me. Thanks.
> 
> P.S. I'm not a peer of Geolocation module. I'm not sure whether I'm the
> right person to make a review.

Thanks for reviewing, I am module peer, and you are the most knowledgable person I know to ask about code interfacing with gps.h :).
 
> You might consider to use 'float' here since accuracy of GpsLocation is
> declared as 'float' in gps.h.
> (GpsLocation's accuracy is 'float' and nsGeoPosition's accuracy is 'double'.)

Ah ok, will change that.

	Pulsebot
	Comment 30 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/3586a2f36958

	Carsten Book [:Tomcat]
	Comment 31 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/3586a2f36958

	Jamin Liu [:jaliu][:Jamin]
	Comment 32 • 9 years ago

Clear the ni flag since Garvan has provided a fix.

	Ravi Dandu
	Comment 33 • 9 years ago

Garvan's change is a workaround for the accuracy value being impossible 0.0 for approximate location (approx country center returned based on country code from MCC/MNC value).

The main issue is that assistance to GPS (XTRA) is not configured correctly on this particular combination of B2G with Z3C. This configuration is a combination of vendor code and Gecko, has not been commercialized previously.  
Alexander Lissy is investigating with vendor, and can update.

	:gerard-majax
	Comment 34 • 9 years ago

I'm trying to hack XTRA support in bug 1177413. I'm also trying to get SUPL working but it's not the same story :(

	:gerard-majax
	Comment 35 • 9 years ago

I have logs showing the same behavior on Flame ...