Open Bug 1519394 Opened 5 years ago Updated 2 years ago

Figure out why test_messageChannelWithMessageManager.xul leaks TabGroup when delayConnectedCallback isn't called in browser custom element

Categories

(Toolkit :: UI Widgets, enhancement, P5)

enhancement

Tracking

()

People

(Reporter: bgrins, Unassigned)

References

(Blocks 1 open bug)

Details

See discussion between https://bugzilla.mozilla.org/show_bug.cgi?id=1441935#c20 and https://bugzilla.mozilla.org/show_bug.cgi?id=1441935#c26

If this condition is removed: https://searchfox.org/mozilla-central/rev/b4ebbe90ae4d0468fe6232bb4ce90699738c8125/toolkit/content/widgets/browser-custom-element.js#304-309, then we leak something like this in test_messageChannelWithMessageManager.xul:

0:13.52 INFO |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
0:13.52 INFO | | Per-Inst Leaked| Total Rem|
0:13.52 INFO 0 |TOTAL | 45 1808| 19228 19|
0:13.52 INFO 41 |CondVar | 80 160| 52 2|
0:13.52 INFO 125 |Mutex | 104 208| 300 2|
0:13.52 INFO 215 |SchedulerEventTarget | 48 432| 18 9|
0:13.52 INFO 256 |TabGroup | 392 392| 1 1|
0:13.52 INFO 263 |ThrottledEventQueue | 40 80| 2 2|
0:13.52 INFO 264 |ThrottledEventQueue::Inner | 264 528| 2 2|
0:13.52 INFO 435 |nsTArray_base | 8 8| 4824 1|

Having delayConnectedCallback isn't a problem on its own (it's standard practice in our chrome Custom Elements to use that due to the way XUL documents work), but it seems to be surfacing an existing issue that'd be worth fixing. Also, the <browser> CE is a case where we don't actually need delayConnectedCallback (since nothing in it's connectedCallback touches DOM / layout), so the condition could be removed if we fix the leak.

The command I was using to generate the trace in https://bugzilla.mozilla.org/show_bug.cgi?id=1441935#c23 is:

XPCOM_MEM_LOG_CLASSES=TabGroup ./mach mochitest test_messageChannelWithMessageManager.xul

Priority: -- → P5

I'm seeing an assertion now when trying to remove it Assertion failure: ChildID() == aBC->Canonical()->GetInFlightProcessId() (Attempt to modify a BrowsingContext from a child which doesn't own it): https://bugzilla.mozilla.org/show_bug.cgi?id=1589134#c6.

Blocks: 1590923
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.