797681 - Facility to disable GonkGPSGeoLocationProvider required

Status: RESOLVED FIXED

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

Description

[Michael Vines [:m1] [:evilmachines]]

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.


[1] https://mxr.mozilla.org/mozilla-central/source/dom/src/geolocation/nsGeolocation.cpp#589

Comment 1

[Andrew Overholt [:overholt]]

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.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

mvines and I need to follow up.  Will try today.

Comment 3

[Doug Turner (:dougt)]

geolocation supports mozsettings.

Comment 4

[Michael Vines [:m1] [:evilmachines]]

(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: https://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/GonkGPSGeolocationProvider.cpp

Comment 5

[Michael Vines [:m1] [:evilmachines]]

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]

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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?

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We've walked a similar path before with RIL.  We can follow this as an example

https://github.com/mozilla/releases-mozilla-central/commit/a7fd215233e6a17236fc40b300102fa1bc2582d7#dom/system/gonk/SystemWorkerManager.cpp

Another approach would be would to have this register as a geolocation provider |category geolocation-provider gonk-location-provider @mozilla.org/gonklocationprovider;1|.

Comment 8

[Michael Vines [:m1] [:evilmachines]]

Cargo culting GPSDGeolocationProvider [1] ought to do the trick nicely!

[1] http://mxr.mozilla.org/mozilla-central/source/dom/system/GPSDGeolocationProvider.manifest

Comment 9

[Kan-Ru Chen [:kanru] (UTC+9)]

Even register it as a geolocation-provider would not prevent it being picked up. Use contractID to resolve the provider would do the trick.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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?

Comment 11

[Doug Turner (:dougt)]

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.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Chris says that the previous comment means that this should be a blocker :)

Comment 14

[Kan-Ru Chen [:kanru] (UTC+9)]

Attachment: Register GonkGPSGeoLocationProvider as a XPCOM service.

so that vender could override the default implementation.

Comment 15

[Doug Turner (:dougt)]

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 16

[Doug Turner (:dougt)]

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.

Comment 17

[Kan-Ru Chen [:kanru] (UTC+9)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/78dd270fab9e

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/78dd270fab9e

Comment 19

[Kan-Ru Chen [:kanru] (UTC+9)]

checkin-needed for aurora. a=blocking-basecamp

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c4a76f8e8d69