Closed
Bug 816850
Opened 12 years ago
Closed 12 years ago
geolocation timeout after airplane mode toggled
Categories
(Core :: DOM: Geolocation, defect, P1)
Tracking
()
People
(Reporter: m1, Assigned: kanru)
Details
Attachments
(1 file, 2 obsolete files)
Whenever we do Airplane mode ON-> OFF or GPS OFF-> ON, consecutive E2E GPS sessions are showing error Timeout, but fixes are getting triggered and final position is being received by HAL. Steps: 1) Make sure airplane mode is OFF or GPS is ON 2) trigger E2E GPS session using test app, it will report the lat/long properly 3) Now set airplane mode to ON or GPS to OFF, wait 30 sec-ish and undo this change 4) trigger E2E GPS session using test app, it will report the lat/long properly --> Here, App unable to show lat/long, gives Timeout-error
Reporter | ||
Comment 1•12 years ago
|
||
Some analysis: When GPS is turned off, it triggers HandleMozsettingValue with value false, which results in calling mGeoLocators[i]->shutdown(). In nsGeolocation::Shutdown, we remove it from the nsGeolocationService. But when we turn back GPS, there is no call to add the nsGeolocation instance back to the mGeoLocators list. Hence even though we get the position fix from the HAL in nsGeolocationService::Update, since the mGeoLocators list is empty, the call does not get processes further. The issue is the only place where the nsGeolocation instance gets added to the service is in nsGeolocation::Init, which is getting called only once when I open the qcgpstest app for the first time and start the session. If i comment out the following code in nsGeolocation::Shutdown, i am able to solve the issue. /*if (mService) { printf_stderr("nsGeolocation:: Remove self from service"); mService->RemoveLocator(this); }*/
Reporter | ||
Comment 2•12 years ago
|
||
Can't share a patch to nsGeolocation.cpp yet, but I think we may just need to move this [1] to here [2] [1] http://mxr.mozilla.org/mozilla-central/source/dom/src/geolocation/nsGeolocation.cpp#1142 [2] http://mxr.mozilla.org/mozilla-central/source/dom/src/geolocation/nsGeolocation.cpp#1091
Broken feature. Thanks for the feedback :). Things are coming along ;). Kan-Ru, can you take a look or reassign? Thanks!
Assignee: nobody → kchen
blocking-basecamp: ? → +
Priority: -- → P3
Updated•12 years ago
|
Priority: P3 → P1
Comment 4•12 years ago
|
||
i can look Kan-Ru if you are overloaded.
Assignee | ||
Comment 5•12 years ago
|
||
I have free cycles.
Assignee | ||
Comment 6•12 years ago
|
||
Don't call nsGeolocation::Shutdown(), this method is destructive. It does not only remove itself from mService but also remove all registered callbacks.
Attachment #687646 -
Flags: review?(doug.turner)
Comment 7•12 years ago
|
||
Comment on attachment 687646 [details] [diff] [review] Don't call nsGeolocation::Shutdown() when disabled via settings. v1 This makes sense. Just change the comment before you check in: Please change the comment to from: // turn things off to: // Do not shutdown the mGeolocators, but instead just stop the device and do a final update of null. See bug #816850.
Attachment #687646 -
Flags: review?(doug.turner) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3b4311176fa
Component: General → Geolocation
Product: Boot2Gecko → Core
Version: unspecified → Trunk
Comment 9•12 years ago
|
||
Backed out for making test_mozsettingsWatch.html fail frequently on OS X 10.6: https://tbpl.mozilla.org/php/getParsedLog.php?id=17551432&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=17551711&tree=Mozilla-Inbound Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/00be45c9abe2
Assignee | ||
Comment 10•12 years ago
|
||
ah, I forget the remove the first watcher
Comment 11•12 years ago
|
||
Also on Windows: https://tbpl.mozilla.org/php/getParsedLog.php?id=17551956&tree=Mozilla-Inbound
Comment 12•12 years ago
|
||
:kanru can you wrap this up quickly?
Assignee | ||
Comment 13•12 years ago
|
||
(In reply to Faramarz (:faramarz) from comment #12) > :kanru can you wrap this up quickly? Yeah, the try run looks good. I'll have a updated patch shortly.
Assignee | ||
Comment 14•12 years ago
|
||
Clear watch after test. https://tbpl.mozilla.org/?tree=Try&rev=0415371db6a9
Attachment #687646 -
Attachment is obsolete: true
Attachment #688120 -
Flags: review?(doug.turner)
Assignee | ||
Comment 15•12 years ago
|
||
Comment on attachment 688120 [details] [diff] [review] Don't call nsGeolocation::Shutdown() when disabled via settings. v1.1 Failed again. The log looks very strange, some tests are just skipped or not shown.
Attachment #688120 -
Flags: review?(doug.turner)
Comment 16•12 years ago
|
||
Hi Kanru, what's the next step here?
Assignee | ||
Comment 17•12 years ago
|
||
Last try: https://tbpl.mozilla.org/?tree=Try&rev=e9277e141086 It still failed, that doesn't make sense :/
Assignee | ||
Comment 18•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=cd83b544930f
Attachment #688120 -
Attachment is obsolete: true
Assignee | ||
Comment 19•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/88986a16df5b
Comment 20•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/88986a16df5b
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Assignee | ||
Comment 21•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/aaa8c4b2af14 https://hg.mozilla.org/releases/mozilla-beta/rev/a29e2be8f636
You need to log in
before you can comment on or make changes to this bug.
Description
•