As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 759193 - Don't display [System Principal] for system compartments
: Don't display [System Principal] for system compartments
Product: Toolkit
Classification: Components
Component: about:memory (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Justin Lebar (not reading bugmail)
: Nicholas Nethercote [:njn]
Depends on: 754771
  Show dependency treegraph
Reported: 2012-05-28 13:59 PDT by Justin Lebar (not reading bugmail)
Modified: 2012-10-22 12:25 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (1.73 KB, patch)
2012-05-29 05:38 PDT, Justin Lebar (not reading bugmail)
n.nethercote: review-
Details | Diff | Splinter Review

Description User image Justin Lebar (not reading bugmail) 2012-05-28 13:59:59 PDT
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]?
Comment 1 User image Justin Lebar (not reading bugmail) 2012-05-28 14:05:24 PDT
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 User image Justin Lebar (not reading bugmail) 2012-05-29 05:38:34 PDT
Created attachment 627923 [details] [diff] [review]
Patch v1
Comment 3 User image Nicholas Nethercote [:njn] 2012-05-29 18:46:30 PDT
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?
Comment 4 User image Andrew McCreight [:mccr8] 2012-05-29 19:32:47 PDT
system-compartment and compartment sounds good to me.
Comment 5 User image Justin Lebar (not reading bugmail) 2012-05-29 21:38:13 PDT
(In reply to Andrew McCreight [:mccr8] from comment #4)
> system-compartment and compartment sounds good to me.

Sounds good to me as well!
Comment 6 User image Nicholas Nethercote [:njn] 2012-05-29 23:56:55 PDT
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.
Comment 7 User image Nicholas Nethercote [:njn] 2012-05-30 02:52:48 PDT
Bonus points if you can separately identify add-on compartments using the instructions in :)
Comment 8 User image Justin Lebar (not reading bugmail) 2012-10-22 12:25:43 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.