Don't display [System Principal] for system compartments




5 years ago
5 years ago


(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))



Firefox Tracking Flags

(Not tracked)



(1 attachment)



5 years ago
Here's what I see in about:memory:

>     1.70 MB (00.44%) ++ compartment([System Principal], chrome://adblockplu
>     1.58 MB (00.41%) ++ compartment([System Principal], resource:///modules
>     0.94 MB (00.24%) ++ compartment([System Principal], chrome://browser/co
>     0.87 MB (00.23%) ++ compartment([System Principal], resource://gre/modu
>     0.79 MB (00.21%) ++ compartment([System Principal], resource://gre/modu
>     0.77 MB (00.20%) ++ compartment([System Principal], about:newtab)
>     0.61 MB (00.16%) ++ compartment([System Principal], jar:file:///home/jl

It would be helpful to be able to see more of the compartment's name in non-verbose mode.

We can increase the width of about:memory in a separate bug, but for here: There's no need to say [System Principal] for these compartments.  All the URIs have special schemes indicating that they're not web content.

Can we just get rid of [System Principal]?


5 years ago
Depends on: 754771

Comment 1

5 years ago
Ouch, this is much worse after we landed bug 757299, because now you can't hover over one of these compartments to get the full compartment URI.

Maybe we should back that out.

Comment 2

5 years ago
Created attachment 627923 [details] [diff] [review]
Patch v1
Attachment #627923 - Flags: review?(n.nethercote)


5 years ago
Assignee: nobody → justin.lebar+bug
Comment on attachment 627923 [details] [diff] [review]
Patch v1

Review of attachment 627923 [details] [diff] [review]:

I like having system compartments easily identifiable.  Literally five minutes ago I was grepping through about:memory?verbose for "System Principal" while looking at bug 756261.

We could do something like replacing "[System Principal]" with "sys" when it's present... that would require exposing SYSTEM_PRINCIPAL_SPEC from caps/src/nsSystemPrincipal.cpp to js/xpconnect/src/XPCJSRuntime.cpp.  Hmm, that's a bit ugly.

How about this instead:  replace "compartment(...)" with "user-compartment(...)" or "system-compartment(...)" as appropriate?  That's a win (in terms of width) for System Principal compartments, but a loss for user compartments... maybe keep "compartment(...)" for user compartments?
Attachment #627923 - Flags: review?(n.nethercote) → review-
system-compartment and compartment sounds good to me.

Comment 5

5 years ago
(In reply to Andrew McCreight [:mccr8] from comment #4)
> system-compartment and compartment sounds good to me.

Sounds good to me as well!
And the nice thing about "system-compartment" is that it won't show up in about:compartments, which is good because about:compartments already segregates system compartments.
Bonus points if you can separately identify add-on compartments using the instructions in :)

Comment 8

5 years ago
I don't think we need to fix this anymore because we recently made non-verbose about:memory expand to the full width of the browser window.
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.