geolocation.watchposition position_options ignored

RESOLVED FIXED in Firefox 19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: m1, Assigned: jdm)

Tracking

unspecified
mozilla20
ARM
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

Attachments

(1 attachment)

position_options are getting lost when the switch from content to b2g process occurs.

Net effect is that enableHighAccuracy,timeout,maximumAge does not work when running from a content process.   Works fine when OOP is disabled.


Quick Analysis:
1) Content process calls
http://mxr.mozilla.org/mozilla-beta/source/dom/src/geolocation/nsGeolocation.cpp#932
2) On the parent process side we then forge a dummy position_options of JSVAL_VOID (http://mxr.mozilla.org/mozilla-beta/source/dom/ipc/ContentParent.cpp#1903) and pass that void along to the nsIGeolocationProviders
Component: General → Geolocation
Product: Boot2Gecko → Core
josh, you interested in an e10s bug?
blocking-basecamp: ? → -
tracking-b2g18: --- → +
Assignee: nobody → josh
Michael, are you positive that timeout and maximumAge don't work? There's no reason I can see that they should not, since all of the relevant bits of code are executed in the content process.
Yep, pretty sure.   nsIGeolocationProviders run in the system process, and the position_options are getting lost [1] as they exit the content process, so the geolocation providers never get them.

[1] http://mxr.mozilla.org/mozilla-beta/source/dom/ipc/ContentParent.cpp#1903
Indeed, I have a test which demonstrates that timeouts certainly work in content processes in contrived conditions.
Michael, the providers don't need the timeout values. The timeouts are only needed in the content process where the timers run.
Attachment #695517 - Flags: review?(doug.turner)
(In reply to Josh Matthews [:jdm] from comment #5)
> Michael, the providers don't need the timeout values. The timeouts are only
> needed in the content process where the timers run.

Ah, missed that.  Sorry.  I checked with the original reporter for this issue and looks like only real complaint was the lack of the enableHighAccuracy arriving at the provider.  The rest was creative extrapolation.  :)

So attachment 695517 [details] [diff] [review] lgtm!
Comment on attachment 695517 [details] [diff] [review]
Pass enableHighAccuracy geolocation changes to the chrome process.

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

::: dom/tests/unit/test_geolocation_timeout.js
@@ +51,5 @@
> +
> +  if (Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime)
> +        .processType == Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
> +    var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
> +    prefs.setCharPref("geo.wifi.uri", "http://localhost:4444/geo");  

trailing ws

::: dom/tests/unit/test_geolocation_timeout_wrap.js
@@ +2,5 @@
> +const Ci = Components.interfaces;
> +
> +function run_test() {
> +  var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
> +  prefs.setCharPref("geo.wifi.uri", "http://localhost:4444/geo");  

trailing ws
Attachment #695517 - Flags: review?(doug.turner) → review+
Comment on attachment 695517 [details] [diff] [review]
Pass enableHighAccuracy geolocation changes to the chrome process.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Confusion and brokenness if we base b2g builds off of aurora.
Testing completed (on m-c, etc.): manual testing
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Broken high geolocation accuracy for all content.
Testing completed: manual testing
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #695517 - Flags: approval-mozilla-b2g18?
Attachment #695517 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/2606506e12dc
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Attachment #695517 - Flags: approval-mozilla-b2g18?
Attachment #695517 - Flags: approval-mozilla-b2g18+
Attachment #695517 - Flags: approval-mozilla-aurora?
Attachment #695517 - Flags: approval-mozilla-aurora+
Does it means bug 806219 is fixed?
Duplicate of this bug: 806219
You need to log in before you can comment on or make changes to this bug.