Closed
Bug 1050423
Opened 10 years ago
Closed 10 years ago
Killing an app that has an open permissions dialog leaks the ContentParent
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:2.0+, firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v1.4 affected, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: khuey, Assigned: fabrice)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
alive
:
review+
|
Details | Diff | Splinter Review |
[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.
Reporter | ||
Comment 1•10 years ago
|
||
qawanted to test on 1.4 and master. I suspect this is a regression.
Flags: needinfo?(jsmith)
Keywords: qawanted,
regression
Reporter | ||
Comment 3•10 years ago
|
||
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.
Comment 4•10 years ago
|
||
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+
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → fabrice
Updated•10 years ago
|
QA Contact: jmercado
Comment 5•10 years ago
|
||
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qaurgent,
qawanted,
regression
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Assignee | ||
Comment 6•10 years ago
|
||
Kyle, this patch let us remove the event listeners, but still doesn't fix the queued-ipc-message count.
Updated•10 years ago
|
Assignee | ||
Comment 7•10 years ago
|
||
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
Assignee | ||
Comment 8•10 years ago
|
||
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 9•10 years ago
|
||
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+
Assignee | ||
Comment 10•10 years ago
|
||
(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.
Assignee | ||
Updated•10 years ago
|
Attachment #8469834 -
Flags: review?(21)
Attachment #8469834 -
Flags: review?(21) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/de28796a8956a48bb98ca67df6a33e0622d642d1
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/b6e9aaed56d0
status-firefox32:
--- → wontfix
status-firefox33:
--- → wontfix
status-firefox34:
--- → fixed
Target Milestone: --- → 2.1 S2 (15aug)
You need to log in
before you can comment on or make changes to this bug.
Description
•