1011643 - [geolocation] NetworkGeolocationProvider.js makes unnecessary MLS requests

Status: RESOLVED DUPLICATE of bug 1009592

(Core :: DOM: Geolocation, defect)

Description

[David Flanagan [:djf]]

The Tarako device does not have a GPS and determines position with wifi and location services.

I think I've noticed two things about geotagging on Tarako:

1) it can take quite a while for the app to get its initial position, so if you launch the camera and take a picture, there may not be a location yet.

2) from the logcats, it looks like gecko is sending out a location request every 10 seconds or so, which seems like a real waste of battery power and bandwidth.

I'm not positive about either of these things, but I think we should investigate both.

Hopefully, just adding passing an options object as the third argument to watchPosition() is enough to fix this, if we set maximumAge 300000 then we're willing to accept locations as much as 5 minutes old.  That might get us a faster initial position and cause less frequent position update requests.

Hopefully we can fix the camera in gaia. But we might also want to get the geolocation gecko people involved to check on the defaults for Tarako because such frequent position update requests don't seem appropriate on a low-powered device like Tarako.

Note that this might be an urgent issue for Tarako, but the general issues of tuning our geolocation requests probably applies to all versions of the camera.

Needinfo Justin to see if he can investigate this.

Needinfo Fabrice to ask who works on geolocation in gecko for Tarako

Comment 1

[[:fabrice] Fabrice Desré]

You can ask dougt about the geolocation details. We should be easily be able to tweak the location refresh timer - especially for mls, where it makes no sense to ping it again if we're still connected to the same wifi ap or cell tower.

Comment 2

[[:fabrice] Fabrice Desré]

Garvan, can you take a look?

Comment 3

[David Flanagan [:djf]]

Okay, I've investigated this.  The camera app calls watchPosition().  On Tarako, according to the logcat at least, we are pinging mozilla location services every 10 seconds.  Even if I set maximumAge in the options object passed to watchPosition() we still ask for a new location every 10 seconds. And this is happening while the phone is sitting still. None of the wifi data should have changed in any appreciable way, so we should know that we haven't moved without making a request to find that out.

This seems like a serious battery drain and it also seems like it could consume a lot of users data plans.  We really should not ship it this way.

Comment 4

[David Flanagan [:djf]]

I've looked at dom/system/NetworkGeolocationProvider.js (on m-c, not the 1.3t version) and have confirmed that it is sending a location request every time the timer fires, without checking to see if the wifi or celltower data has changed since the last time it sent a request.  This seems badly broken.

If we ship Tarako like this we will DDOS our own location service!

Editing the summary line to reflect what is going on here.

Comment 5

[David Flanagan [:djf]]

NetworkGeolocationProvider.js also tries to make location results based on cell tower results. But the code is completely broken because it never assigns any value to gCellResults.  So we can't get geolocation based on cell towers, even if we turn on the pref for that.

Is that supposed to be working?  It looks like these lines:

    if (gCellScanningEnabled) {
      this.updateMobileInfo();
    }

Should be changed to:

    if (gCellScanningEnabled) {
      gCellResults = this.updateMobileInfo();
    }

Is this code supposed to be working?

Comment 6

[David Flanagan [:djf]]

Attachment: wip patch for feedback

Doug, Garvan: would you take a look at this patch? I haven't even tested it yet, but it seems to me that we should be doing something like this to avoid making unnecessary requests to the location service. This seems to me like something that we urgently need to fix for Tarako to avoid overwhelming our location service servers.

Garvan: please feel free to take my patch and run with it.

Do either of you know how the location service actually works?  The definition here of what a significant wifi change should perhaps be tuned by someone who knows how the location is actually computed from the wifi signal.

Comment 8

[:garvan]

See also bug 1009592.

Thanks for looking at this! You are correct it takes too long for initial geolocation, and the code send tons of unnecessary requests in the watch case. The latter will happen in the watch state, which might have valid reasons to check every 5 seconds, but largely these are wasted requests and wasted bandwidth (both for the user and the MLS servers). My understanding is that is wasn't critical enough to jump on for 1.3t, only major breaks (like MLS not working at all) were going for that. The estimated amount of users using watch requests at any given time is within the servers' capabilities AFAIK.

This is a priority bug for for 1.4 IMHO. I am looking forward to getting on it, and I'll cc you, and hopefully I can use your patch to get started.

For comment 5, that code is changed in this patch for 1.3t: bug 1007953. 

I added Hanno to this since he has the most experience with how this data actually looks. 

And I just got a popup that Doug responded on this bug so I'll stop :).

Comment 10

[:garvan]

And my apologies, I pounded over Doug's comment by submitting my comment, there is a bug in Bugzilla.

Comment 11

[:garvan]

Good news I think, priority is bumped up we will try get to get this in for 1.3t deadline.

Comment 12

[[:fabrice] Fabrice Desré]

(In reply to Garvan Keeley [:garvank] from comment #11)
> Good news I think, priority is bumped up we will try get to get this in for
> 1.3t deadline.

Which is in 2 days...