Closed Bug 1264643 Opened 4 years ago Closed 4 years ago

Active gUM permission menu at shutdown leaks the world

Categories

(Firefox :: Site Permissions, defect)

48 Branch
Unspecified
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox48 --- fix-optional
firefox49 --- verified

People

(Reporter: pehrsons, Assigned: florian)

Details

(Keywords: regression)

Attachments

(1 file)

To reproduce:
* Go to: https://mozilla.github.io/webrtc-landing/gum_test.html
* Select Audio & Video
* Accept the gUM permissions prompt
* Wait until you see video
* Quit Firefox

Expected: Clean Shutdown
Actual: Leaking the world

I see at least an 80% repro rate. I saw some cases of shutdown in a bad order (images alive on gfx shutdown) but I'm not sure if that is masking this issue or if it's the other way around.


I saved CC logs for one of these leaks on shutdown, and then got:
> pehrsons:~/Comoyo/heapgraph $ ./find_roots.py ~/cc_logs/cc-edges.32732-2.log nsGlobalWindow
> Parsing /Users/pehrsons/cc_logs/cc-edges.32732-2.log. Done loading graph.
> 
> 0x130af6230 [FragmentOrElement (XUL) menu id='webRTC-sharingCamera-menu' chrome://browser/content/hiddenWindow.xul]
>     --[Preserved wrapper]--> 0x12005f400 [JS Object (XULElement)]
>     --[global]--> 0x135c8a1a0 [JS Object (Window)]
>     --[UnwrapDOMObject(obj)]--> 0x1202b3000 [nsGlobalWindow # 2 inner chrome://browser/content/hiddenWindow.xul]
> 
>     Root 0x130af6230 is a ref counted object with 4 unknown edge(s).
>     known edges:
>        0x12b509980 [FragmentOrElement (XUL) menupopup chrome://browser/content/hiddenWindow.xul] --[GetParent()]--> 0x130af6230
>        0x12005f400 [JS Object (XULElement)] --[UnwrapDOMObject(obj)]--> 0x130af6230
>        0x1224922c0 [FragmentOrElement (XUL) window id='main-window' chrome://browser/content/hiddenWindow.xul] --[mAttrsAndChildren[i]]--> 0x130af6230

Which, if I'm interpreting this correctly, means that the gUM permission menu is keeping nsGlobalObject alive.
Florian, do you perhaps know how to drive this further?
Flags: needinfo?(florian)
I tried to reproduce about 10 times but couldn't reproduce. Could there be a missing step?

The XUL node in your graph is the menu with a camera icon in the global mac menubar, it's attached to the hidden window. This is not the gUM permission prompt attached to the location bar.
Flags: needinfo?(florian)
Seems like you need non-e10s mode. I couldn't repro in e10s mode just now.
Flags: needinfo?(florian)
After debugging, it turns out that witout e10s we remove the message listeners at shutdown before the global indicator is removed, so it never gets removed. Not sure if it's a regression from bug 973001 or from bug 1109087, but it certainly looks like the fallout of some e10s work we did.

Andreas, any chance you could confirm the patch fixes the problem for you? Thanks!
Attachment #8742912 - Flags: review?(felipc)
Attachment #8742912 - Flags: feedback?(pehrsons)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Flags: needinfo?(florian)
Keywords: regression
Note: I remember someone showed me on IRC a screenshot on Windows with the global indicator popup never being removed at shutdown; I couldn't find any explanation for that behavior, but this looks like a good one.
Comment on attachment 8742912 [details] [diff] [review]
Active gUM permission menu at shutdown leaks the world,

Review of attachment 8742912 [details] [diff] [review]:
-----------------------------------------------------------------

Still leaking the world for me, but I see another menu causing it this time; 'webRTC-sharingMicrophone-menu'.

> pehrsons:~/Comoyo/heapgraph $ ./find_roots.py ~/cc_logs/cc-edges.42711-3.log nsGlobalWindow
> Parsing /Users/pehrsons/cc_logs/cc-edges.42711-3.log. Done loading graph.
> 
> 0x12bee0310 [FragmentOrElement (XUL) menu id='webRTC-sharingMicrophone-menu' chrome://browser/content/hiddenWindow.xul]
>     --[Preserved wrapper]--> 0x12085fcd0 [JS Object (XULElement)]
>     --[global]--> 0x12084d060 [JS Object (Window)]
>     --[Services]--> 0x12084f340 [JS Object (Proxy)]
>     --[private]--> 0x118770420 [JS Object (Object)]
>     --[ppmm]--> 0x1187a30a0 [JS Object (ChromeMessageBroadcaster)]
>     --[js::GetObjectPrivate(obj)]--> 0x11ab8de20 [XPCWrappedNative (ChromeMessageBroadcaster)]
>     --[mIdentity]--> 0x11abb9c10 [nsFrameMessageManager]
>     --[listeners[i] mStrongListener]--> 0x121fa1f90 [nsXPCWrappedJS (nsIMessageListener)]
>     --[mJSObj]--> 0x120b6dac0 [JS Object (Object)]
>     --[_streams]--> 0x1224ea220 [JS Object (Array)]
>     --[objectElements[0]]--> 0x132717df0 [JS Object (Object)]
>     --[browser]--> 0x13103e220 [JS Object (Proxy)]
>     --[private]--> 0x12386fca0 [JS Object (XULElement)]
>     --[global]--> 0x12223e060 [JS Object (Window)]
>     --[UnwrapDOMObject(obj)]--> 0x12251f800 [nsGlobalWindow # 4 inner chrome://browser/content/browser.xul]
>     --[mOuterWindow]--> 0x122520000 [nsGlobalWindow # 3 outer ]
> 
>     Root 0x12bee0310 is a ref counted object with 4 unknown edge(s).
>     known edges:
>        0x12c425e00 [FragmentOrElement (XUL) menupopup chrome://browser/content/hiddenWindow.xul] --[GetParent()]--> 0x12bee0310
>        0x120c8c3e0 [FragmentOrElement (XUL) window id='main-window' chrome://browser/content/hiddenWindow.xul] --[mAttrsAndChildren[i]]--> 0x12bee0310
>        0x12085fcd0 [JS Object (XULElement)] --[UnwrapDOMObject(obj)]--> 0x12bee0310
Attachment #8742912 - Flags: feedback?(pehrsons)
Comment on attachment 8742912 [details] [diff] [review]
Active gUM permission menu at shutdown leaks the world,

My apologies, I hadn't built it properly. 3/3 non-leaking now!
Attachment #8742912 - Flags: feedback+
Attachment #8742912 - Flags: review?(felipc) → review+
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #6)
> > pehrsons:~/Comoyo/heapgraph $ ./find_roots.py ~/cc_logs/cc-edges.42711-3.log nsGlobalWindow
> > Parsing /Users/pehrsons/cc_logs/cc-edges.42711-3.log. Done loading graph.

cool, where do I find this script?
(In reply to :Felipe Gomes (needinfo me!) from comment #8)
> (In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #6)
> > > pehrsons:~/Comoyo/heapgraph $ ./find_roots.py ~/cc_logs/cc-edges.42711-3.log nsGlobalWindow
> > > Parsing /Users/pehrsons/cc_logs/cc-edges.42711-3.log. Done loading graph.
> 
> cool, where do I find this script?

All the info you need:
https://developer.mozilla.org/en-US/docs/Mozilla/Performance/GC_and_CC_logs

I got logs at shutdown and cloned the heapgraph repo to get find_roots.py.
https://hg.mozilla.org/integration/fx-team/rev/690226a9fb27054d8a20fc7625a2ebd7067573ce
Bug 1264643 - Active gUM permission menu at shutdown leaks the world, r=felipe.
https://hg.mozilla.org/mozilla-central/rev/690226a9fb27
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Hi Florin, can we have QA's help to verify this?
Flags: needinfo?(florin.mezei)
Setting verification flag.
Flags: needinfo?(florin.mezei) → qe-verify+
Hi Florian,
Since this patch also affects 48, are you also considering to uplift this patch to 48?
Flags: needinfo?(florian)
(In reply to Gerry Chang [:gchang] from comment #14)
> Hi Florian,
> Since this patch also affects 48, are you also considering to uplift this
> patch to 48?

No, as said in comment 4, it's either a regression from bug 973001 or from bug 1109087, which landed respectively in 35 and 43, so at this point another release with the bug won't make a significant difference.

Also, I don't think this bug has any user effect (except maybe for what I described in comment 5, but it was more a speculation than something we really know), as it's only a leak at shutdown. It's mostly a pain for developers using debug builds.
Flags: needinfo?(florian)
I don't think we need to uplift this, then. Marking fix-optional in case anyone disagrees strongly; but we don't need to see this in platform triage.
I was able to reproduce the case when the global indicator popup was not removed at shutdown using Firefox 40.0 on Windows 10 x64. Tried more than 50 times and only got it once.

I've also tried about 50 times for each tested platform (Windows 10x64, Ubuntu 12.04 x86 and Mac OS X 10.11.6) to reproduce it on Fx 49 beta 10 (build ID 20160902105049) without any success.

Based on my results and on the rate of reproduction I would dare to consider this issue verified.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.