842710 - Identify TabChildGlobals in about:memory/compartment

Status: RESOLVED FIXED

Description

[Olli Pettay [:smaug]]

Created attachment 715642 [details] [diff] [review]
patch

https://tbpl.mozilla.org/?tree=Try&rev=7b92c1f20459

Comment 1

[Justin Lebar (not reading bugmail)]

It looks like this names the compartments as

  compartment(inProcessTabChildGlobal?ownedby=http://foobar)

?

We do system principal compartments as

  compartment([System Principal], http://foobar)

so perhaps we should stay in line with that and do something like

  compartment([In-process TabChild], http://foobar)

?  I bet njn has an opinion too.  :)

Comment 2

[Olli Pettay [:smaug]]

TabChildGlobal use system principal, so they should have [System Principal]

Comment 3

[Olli Pettay [:smaug]]

and http://... doesn't make sense. TabChildGlobals aren't really in any URL.
Some page may own the element which then owns the TabChildGlobal (in case of in-process).


But I can change the ID to whatever feels the best :)
I rarely use about:memory/compartment so I'm not really familiar with conventions there.

Comment 4

[Justin Lebar (not reading bugmail)]

So the current patch gives us

  compartment([System Principal], inProcessTabChildGlobal?ownedby=...)

?

Comment 5

[Olli Pettay [:smaug]]

Yes
[System Principal], inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul [3]

Comment 6

[Nicholas Nethercote [:njn]]

More specific naming of compartments is always good...

What name would these compartments have had before?

How likely is the case where there isn't an "ownedBy" part?

Comment 7

[Olli Pettay [:smaug]]

Before the patch out-of-process tabchildglobals are just
[System Principal]

and in-process tabchildglobals are
[System Principal|, chrome://browser/content/browser.xul
which less-nicely mixes them with other system compartments.
I mean, the top level chrome doc has chrome://browser/content/browser.xul as document uri, and the
same is used for tabchildglobals, so they all are reported to be similar things.

Comment 8

[Nicholas Nethercote [:njn]]

> [System Principal], inProcessTabChildGlobal?ownedBy=chrome://browser/content/browser.xul [3]

It's a bit ugly but I can't think of anything obviously better.  Go for it.

Comment 9

[Olli Pettay [:smaug]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c4498f0fd07

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7c4498f0fd07

Comment 11

[Nicholas Nethercote [:njn]]

This change just led to bug 842710 being filed -- another case of "if we measure it, we find problems with it".  Nice work.

Comment 12

[Olli Pettay [:smaug]]

You mean bug 844661