Closed Bug 1050423 Opened 6 years ago Closed 6 years ago
Killing an app that has an open permissions dialog leaks the Content
[Blocking Requested - why for this release]: STR: 1. Launch the Camera app. 2. When you get the permission dialog asking to share your location with Camera, kill the Camera app via adb (e.g. adb shell b2g-info, pick out the Camera app, adb shell kill -9 PID) 3. Repeat steps 1 and 2 five times. 4. Take an about memory report with minimization (./tools/get_about_memory.py --minimize) Actual results: If you scroll down to "queued-ipc-messages" in the main process you'll see a number of entries for Camera with pid=-1, closed channel, etc Expected results: No camera entries.
qawanted to test on 1.4 and master. I suspect this is a regression.
Sure - I'll have someone look at this.
The path in question: via mCallback : 0xa76dc4a0 [Function contentEvent] --[fun_callscope]--> 0xa690c3d0 [Call <no private>] --[callback]--> 0xa76dc480 [Function ContentPermissionPrompt.prototype.deleg at] --[fun_callscope]--> 0xa69dbba0 [Call <no private>] --[callback]--> 0xa76dc460 [Function onCallback] --[fun_callscope]--> 0xaa25e510 [Call <no private>] --[frame]--> 0xa761d5e0 [Proxy <no private>] --[private]--> 0xa8e5c760 [Proxy <no private>] --[private]--> 0xaaa5b460 [HTMLIFrameElement <no private>] mCallback is a mozContentEvent listener on the system app's window. It was added from https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/annotate/tip/b2g/components/ContentPermissionPrompt.js#l385. That callback is chained to https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/annotate/tip/b2g/components/ContentPermissionPrompt.js#l332 which holds alive the iframe. This needs to get torn down somehow when the app dies but it's not.
blocking as this is a spin-off from https://bugzilla.mozilla.org/show_bug.cgi?id=1047639 needed by CAF
blocking-b2g: 2.0? → 2.0+
This issue does occur on Flame 2.0, Flame 2.1, Flame 1.4, and Buri 2.0. This indicates that it isn't a regression so I'm removing the tag. Environmental Variables: Device: Flame 2.0 BuildID: 20140807094215 Gaia: bb3d2cf36c10f7186899431d4a21a2489864d9c6 Gecko: 4ad32f9626ad Version: 32.0 (2.0) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Environmental Variables: Device: Flame Master BuildID: 20140807084328 Gaia: 54c3c19d439f7dbafda5c6cc3b4850b545a068ba Gecko: 2f198e81ed98 Version: 34.0a1 (Master) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Environmental Variables: Device: Flame 1.4 BuildID: 20140807110116 Gaia: e9dce1f60f729e228810f751417681b5ff937b6b Gecko: 509ebd2e22f7 Version: 30.0 (1.4) Firmware Version: v123 User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 Environmental Variables: Device: Buri 2.0 BuildID: 20140806140415 Gaia: 47fa0ba8197e71cc7034943ff037642e7f35cdfe Gecko: 641b3758537b Version: 32.0 (2.0) Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Kyle, this patch let us remove the event listeners, but still doesn't fix the queued-ipc-message count.
We had issues both on the gecko side and on the gaia side. In gecko, we were sending requests with an id that was expected to be increasing for each request, but unfortunately many call sites do a do_CreateInstance(NS_CONTENT_PERMISSION_PROMPT_CONTRACTID) so we start again at 0 every time. To make everything worse, gaia just ignored the id it received and maintained its own id to send in responses. That's why we were failing in the second prompt: in the first one we sent 0 and received 0, but in the next ones we were still sending 0 while gaia answered 1, 2, etc. Awesome. This patch fixes the gecko side by always sending a new uuid since I'm not sure at all we can consider this composant as a service (the 'historical' users in nsDocument and nsGeoloc are doing do_CreateInstance(NS_CONTENT_PERMISSION_PROMPT_CONTRACTID). Using a uuid here we immune to being a service or a new instance.
Attachment #8469681 - Attachment is obsolete: true
Gaia part: make the content permission manager actually return ids that match requests, instead of doing its own thing. Alive, feel free to redirect the review. Overall, I'm very surprised all this prompt system worked so far...
Attachment #8469836 - Flags: review?(alive)
Comment on attachment 8469836 [details] [diff] [review] gaia patch Review of attachment 8469836 [details] [diff] [review]: ----------------------------------------------------------------- I guess you had to fix some test in permission_manager_test.js
Attachment #8469836 - Flags: review?(alive) → review+
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #9) > Comment on attachment 8469836 [details] [diff] [review] > gaia patch > > Review of attachment 8469836 [details] [diff] [review]: > ----------------------------------------------------------------- > > I guess you had to fix some test in permission_manager_test.js Not yet, but I will.
Attachment #8469834 - Flags: review?(21) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.