Fixing the test_timerRestartWatch mochitest for e10s. Background: Web content can pass geolocation a position option called timeout. This is the amount of time the system should try to get a position. See: https://www.w3.org/TR/geolocation-API/#timeout In the non-e10s case, the location provider calls nsIGeolocationProvider::locationUpdatePending which causes the geolocation system to reset it's handing of the above described timeout. This seems to work fine. https://dxr.mozilla.org/mozilla-central/source/xpcom/system/nsIGeolocationProvider.idl#35 In the e10s case, the location provider and the geolocation system live in the parent process. The object that keeps track of the position options (and other things) lives in the child process: https://dxr.mozilla.org/mozilla-central/source/dom/geolocation/nsGeolocation.cpp#77 Currently, we are not passing the 'locationUpdatePending' message from the parent down to the child process. This results in the nsGeolocationRequest not being able to keep track of its timeout state. We added locationUpdatePending for QC to do some magic and override how request timeout works. This is just extra complicated. What I think we should do is: 1) remove locationUpdatePending from nsIGeolocationProvider. 2) update all in-tree geolocation providers. 3) in nsGeolocationRequest, always reset the timer in Notify and SendLocation. This enables Watch() to get multiple timeout callbacks which is the behavior without e10s.

(In reply to Doug Turner (:dougt) from comment #0) > What I think we should do is: > > 1) remove locationUpdatePending from nsIGeolocationProvider. > > 2) update all in-tree geolocation providers. > > 3) in nsGeolocationRequest, always reset the timer in Notify and > SendLocation. This enables Watch() to get multiple timeout callbacks which > is the behavior without e10s. 3) does not exactly follow the spec. The spec says "the first timeout is relative to the moment watchPosition() was called [...] Subsequent timeouts are relative to the moment when [...] the position of the hosting device has changed" That said, I think it's subtle and may not worth to implement.

Comment on attachment 8726071 [details] [diff] [review] remove_locationUpdatePending Review of attachment 8726071 [details] [diff] [review]: ----------------------------------------------------------------- steal review. in NetworkGeolocationProvider.js 495 this.notifyListener("locationUpdatePending"); should be deleted together.

> That said, I think it's subtle and may not worth to implement. I think we've been broken since first implementation.