Closed Bug 797681 Opened 7 years ago Closed 7 years ago

Facility to disable GonkGPSGeoLocationProvider required


(Firefox OS Graveyard :: General, defect, P1)

Gonk (Firefox OS)


(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed


(Reporter: m1, Assigned: kanru)



(1 file)

The GonkGPSGeoLocationProvider reference implementation is currently statically linked [1] into nsGeolocation.   This makes it impossible to dynamically swap in an alternative implementation comprehensively without breaking OTA Gecko updates.  Alternative implementations are certainly able to register as a "geolocation-provider" in their manifest, however that does not inhibit GonkGPSGeoLocationProvider from instantation (and subsequently invoking the GPS hal undesirably). 

Some possible solutions:
1) Use a contractID to resolve GonkGPSGeoLocationProvider, so that a dynamic component can override it. 
2) Disable GonkGPSGeoLocationProvider all together for v1.

We discussed this briefly during triage today but were unable to come to a decision as to whether or not we should block on this.  I'm CC'ing some people who'd have a better idea.
mvines and I need to follow up.  Will try today.
geolocation supports mozsettings.
(In reply to Doug Turner (:dougt) from comment #3)
> geolocation supports mozsettings.

That's not really related to the issue here.  The bug is about preventing the code in this file from ever executing within Gecko:
FWIW, I would already have attached a patch to this bug for (1) from the bug description if I wasn't stuck in this Apache2 box.  It's [LOE:S]
Whiteboard: [blocked-on-input Chris Jones and Michael Vines]
We don't want to disable this entirely because testers are using it on other phones.

Doug or Kan-Ru, can one of you guys work with mvines to get feedback on the right patch for approach (1) in comment 0?
Whiteboard: [blocked-on-input Chris Jones and Michael Vines]
We've walked a similar path before with RIL.  We can follow this as an example

Another approach would be would to have this register as a geolocation provider |category geolocation-provider gonk-location-provider;1|.
Cargo culting GPSDGeolocationProvider [1] ought to do the trick nicely!

Even register it as a geolocation-provider would not prevent it being picked up. Use contractID to resolve the provider would do the trick.
Chris, I didn't really understand the answer in comment 6. I don't know enough about what are plans are in this area to know if this blocks or not. Can you make the call?
Whiteboard: [blocked-on-input Chris Jones and Michael Vines]
we are also planning (at least for testing) to use wifi/ip location providers.  when we land that, you'll probably need to do more than just hack out this location provider.
What we want to do here is
 - use GonkGPSGeoLocationProvider by default
 - but allow that implementation to be overridden by a vendor module

My XPCOM-fu isn't strong enough for me to work backwards to a technical solution, but I think Kan-Ru knows what's needed.
Whiteboard: [blocked-on-input Chris Jones and Michael Vines]
Chris says that the previous comment means that this should be a blocker :)
blocking-basecamp: ? → +
Assignee: nobody → kchen
so that vender could override the default implementation.
Attachment #669425 - Flags: review?(doug.turner)
Comment on attachment 669425 [details] [diff] [review]
Register GonkGPSGeoLocationProvider as a XPCOM service.

Review of attachment 669425 [details] [diff] [review]:

Anyone can create their own using the geolocation-provider category.  If you just want to disable the GONK Gps provider, just add a preference.
Comment on attachment 669425 [details] [diff] [review]
Register GonkGPSGeoLocationProvider as a XPCOM service.

Review of attachment 669425 [details] [diff] [review]:

This is fine -- Gonk can override the default geolocation provider.
Attachment #669425 - Flags: review?(doug.turner) → review+
Closed: 7 years ago
Resolution: --- → FIXED
checkin-needed for aurora. a=blocking-basecamp
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.