910463 - Additional StartUp getting called when single-shot high-precision GPS request is made.

Status: RESOLVED WONTFIX

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

Description

[Dave Huseby [:huseby]]

This is a follow on bug from 898240.  There's a corner case that was discovered that the fix doesn't cover:

(In reply to Stephen Li from comment #33)
> Even though GPS does switch off with this fix, but we see an additional
> "StartUp" getting called just before "ShutDown" if a high accuracy fix is
> requested.
> 
> Steps to reproduce the issue:
> Issue a high accuracy single shot request.
> Let it timeout
> Just before shutdown gets called, i see a Startup getting called in the
> provider.
> 
> Expected Behavior:
> Only Shutdown should get called.

Comment 1

[Stephen Li]

Can we raise the priority of this issue to P1 ?
Randomly I have seen that for high accuracy request, the service does not shutdown at all. But it happens intermittently.

So there are 2 issues:
1. I see a Startup getting called before Shutdown for high accuracy requests.
2. Randomly I see that the service does not shutdown at all. Since it happens randomly, I have not been able to collect any logs. Also see this behavior only for high accuracy requests.

May be if issue 1> is resolved, the second issue might also get fixed.

Comment 2

[Stephen Li]

Can we have an ETA on this issue ?

Comment 3

[Josh Matthews [:jdm]]

I'll take a look at this today.

Comment 4

[Dave Huseby [:huseby]]

It would be helpful if you take a look at this.  I'm focusing on trying to get a profiler patch landed that is higher priority.  I found that there is an interesting interaction when the first requests come in and the GPS device needs to get started up.  There is almost certainly a logic bug around the shutdown flagging.

My patch for Bug 898420 just made sure that when we clear all requests that we also check for pending requests that also need to get cleared.

If you have any more questions, needinfo? me and I'll see if I can help.

Comment 5

[Josh Matthews [:jdm]]

I am not sure that there is a solution for this problem on Gecko's side. Whenever a geolocation request is terminated (whether from timing out, or a watch being cleared, etc), we tell the geolocation service to attempt to switch back to low accuracy in case the request was the last one requiring high accuracy. In single process Gecko, this is not a big deal - we can simply tell the geolocation provider to switch its accuracy setting. In multiprocess Gecko, however, the provider lives in the parent process, so we have to send a message from the content process requesting the accuracy change. The IPC setup for geolocation is weird though, because we have a single request for each process that represents all requests that live in that process. That means that if we have a request that was using high accuracy and receive a message asking for a change, we have to recreate our listener with a low accuracy parameter. This causes Startup to be called, which is what you're seeing here. Removing the side effect (ie. asking for the accuracy change during termination) would be bad in situations where you have a long-running, low-accuracy watch request and a singleshot high-accuracy request, because we would keep running in high-accuracy mode as long as the watch remained alive.

Comment 6

[Stephen Li]

Thanks Josh.
I think i get your point, it seems there has been a design change.

With the earlier design, with one request, if single shot high accuracy request was made, and if that stopped (either due to timeout or getting a location), nsGeolocation would internally change its state to low accuracy. but not inform the provider that the accuracy state has changed. With this design, the provider assumed that if its ShutDown is called, then all high accuracy requests must have exited.

With the new design, the provider is being informed about the accuracy state change before shutting down.
Which I think is a better design.

The scenario that you have described with one high single shot high accuracy and one low accuracy watch request in progress, works fine. But I still see an issue with 2 watch requests running, one high accuracy and one low. If I clearWatch the high accuracy tracking request, I do not get the accuracy state change call into the provider and the low accuracy watch request continues to get high accuracy fixes. This is an issue that needs to be looked into.

Comment 7

[Stephen Li]

When can we receive the fix for the pending issue as stated above:

>>  But I still see an issue with 2 watch requests running, one high accuracy and one low. If I clearWatch the high accuracy tracking request, I do not get the accuracy state change call into the provider and the low accuracy watch request continues to get high accuracy fixes. This is an issue that needs to be looked into.

Comment 8

[Dave Huseby [:huseby]]

Is there any more information on how to reproduce the bug more reliably?

The analysis and potential rework to fix this intermittent bug doesn't match up with the risk.  Do you think this an absolute blocker for koi?  If not, we'll keep it in our backlog and tackle it later.

Comment 9

[Dave Huseby [:huseby]]

Closing this bug due to the intermittent repro nature.  If this is really a problem we should devote more resources to, we can re-open this or file a new bug.

Comment 10

[Dave Huseby [:huseby]]

clearing needinfo