Closed Bug 673331 Opened 13 years ago Closed 13 years ago

Add identifying information to system compartments

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla9

People

(Reporter: n.nethercote, Assigned: sandervv)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [MemShrink:P2])

Attachments

(2 files, 4 obsolete files)

Proposed patch for the bugzilla tweaks add-on (jetpack)
1.22 KB, text/plain
Details
Patch to make filename visible of a system compartment v3
15.93 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review 
Bug 672439 added the compartment's address to the path used for system compartment memory reporters.  This means they show up separately in about:memory, but you can't tell anything about each one.

In http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/37c0b12078c400d7# bz said:

> I wonder whether it's worth it to change the sandbox constructor to have a 
> way of attaching information about who's creating the sandbox (as opposed 
> to the principal of the sandbox, which is all we have now) to the sandbox 
> and then expose that info on the compartment somehow.  That seems like the 
> ideal thing from the about:memory pov.

And Dave Townsend added:

> I'm assuming that anything loaded with Components.utils.import ends up in 
> the system compartment? Simply typing those to the URL they were imported
> from would be an easy first step. Presumably something similar would work
> for JS components.
>
> As for plain sandboxes, I suspect as Boris says we may just have to go and 
> manually set those with some new argument to the sandbox constructor or
> something. Would be easy to include the add-on ID that way for restartless
> add-ons.

I'm happy to do the work for this bug, but I'll need some help with identifying the right information to include in the memory reporter's path.
Whiteboard: [MemShrink] → [MemShrink:P2]
I talked to mrbkap about this on IRC.  Here's a plan for sandboxes, which should cover a lot of the system compartments.

In xpccomponents.cpp is the function nsXPCComponents_utils_Sandbox::CallOrConstruct().  It already pulls certain options -- |sandboxPrototype| and |wantXrays| -- out of the optionsObject, which is passed in via argv.  We could add a |sandboxName| option similarly.  It would be stored in the CompartmentPrivate.

Then, at sandbox-creation points where we currently have something like this:

  var sandbox = new Cu.Sandbox(somePrincipal, { sandboxPrototype: someWindow })

we would change it to something like this:

 var sandbox = new Cu.Sandbox(somePrincipal, { sandboxPrototype: someWindow, sandboxName: ("sandbox for " + window.location) })


For Components.utils.import, there isn't a whole lot we could do there, since those don't get their own compartment anyway.
Blocks: 671352
So there are various places where |new Cu.Sandbox(...)| occurs in the code according to mxr.mozilla.org, but I suspect the interesting ones are in Jetpack code somewhere?

If there's a central place in core Jetpack code that's responsible for these sandboxes, that's good, but if every add-on has its own Cu.Sandbox calls that isn't good, because it means every add-on would need to opt into this extra identification scheme for it to be useful.
This patch stores the file location of a system compartment. The file location of each system compartment is now visible in about:memory. As njn said, to gather the origin of all system compartments, it is necessary to modify each add-on which initializes a sandbox (using "Cu.Sandbox(...)").

What is a good idea to make add-on developers aware of the fact that they should add the sandboxName property when creating a new sandbox? How about a simple debug message sent to the javascript error console, when a system compartment is created without a proper sandboxName property? That way, it is unobtrusive for the user / developer and only visible to the "more advanced users" and add-on developers.
Attachment #550560 - Flags: review?(nnethercote)
Comment on attachment 550560 [details] [diff] [review]
Patch to make filename visible of a system compartment

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

This looks as I'd expect, but Blake should review this.
Attachment #550560 - Flags: review?(nnethercote) → review?(mrbkap)
(Sander is a Dutch CS student who's just started contributing to Mozilla, BTW.  This is a great effort to get this patch working;  it's not a big patch but it affects several different pieces and so isn't easy for a newcomer.  Nice work!)
For example, given the bugzilla tweak add-on is installed, and the verbose mode of about:memory is enabled, the following compartment information is visible:

│  ├─────541,171 B (01.30%) -- compartment([System Principal], resource://jid0-qbniplfdfa4lpdrjhac6vbqn20q-api-utils-lib/traceback.js, 0xffffffffa6ba9800)
│  │     ├──139,264 B (00.33%) -- gc-heap
│  │     │  ├───75,600 B (00.18%) -- objects
│  │     │  ├───29,880 B (00.07%) -- shapes
│  │     │  ├───28,336 B (00.07%) -- arena-unused
│  │     │  ├────4,448 B (00.01%) -- strings
│  │     │  ├──────544 B (00.00%) -- arena-headers
│  │     │  └──────456 B (00.00%) -- arena-padding
│  │     ├──137,368 B (00.33%) -- tjit-data
│  │     │  ├───74,000 B (00.18%) -- allocators-reserve
│  │     │  └───63,368 B (00.15%) -- allocators-main
│  │     ├──131,072 B (00.31%) -- tjit-code
│  │     ├───65,536 B (00.16%) -- mjit-code
│  │     ├───40,124 B (00.10%) -- string-chars
│  │     ├───12,239 B (00.03%) -- scripts
│  │     ├────9,052 B (00.02%) -- property-tables
│  │     ├────5,344 B (00.01%) -- object-slots
│  │     └────1,172 B (00.00%) -- mjit-data
I added the patch for the bugzilla tweaks add-on (or it's jetpack library) here, so others are also able to reproduce the information from the about:memory view.
The sandboxPrototype property is not necessary (otherwise it's given twice to the sandbox constructor) and therefore removed from the jetpack patch.
Attachment #550567 - Attachment is obsolete: true
Hi Sander, I'm really sorry for the delay here. I'll review your patch tomorrow.
Comment on attachment 550560 [details] [diff] [review]
Patch to make filename visible of a system compartment

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

This looks like a good approach. I have some comments to fix before we can commit this, though.

::: js/src/xpconnect/src/xpccomponents.cpp
@@ +3227,5 @@
>              return NS_ERROR_UNEXPECTED;
>          }
>      }
>  
> +    xpc::CompartmentPrivate *compartmentPrivate = \

No need for the \ here.

@@ +3376,5 @@
>  
>              wantXrays = JSVAL_TO_BOOLEAN(option);
>          }
> +
> +        JS_HasProperty(cx, optionsObject, "sandboxName", &found);

Need to check for failure from the JS_HasProperty here.

@@ +3384,5 @@
> +                !JSVAL_IS_STRING(option)) {
> +                return ThrowAndFail(NS_ERROR_INVALID_ARG, cx, _retval);
> +            }
> +
> +            sandboxName = JS_EncodeString(cx, JSVAL_TO_STRING(option));

This can return null if we're out of memory.

::: js/src/xpconnect/src/xpcjsruntime.cpp
@@ +1382,5 @@
>                      // can be many) can be distinguished.
>                      if (c->isSystemCompartment) {
> +                        if (c->data) {
> +                            char *location = ((xpc::CompartmentPrivate*)c->data)->location,
> +                                 *cur = location;

This modifies the path in the CompartmentPrivate. That's a little surprising to me. I think I'd prefer it if we modified a copy of the location in |name|.

Also note that the existing code around here has changed to use ReplaceChar.

@@ +1391,5 @@
> +                                        *cur = '\\';
> +                                    }
> +                                }
> +                                // Including or excluding the NUL-byte?
> +                                name.Append(nsPrintfCString(strlen(location) + 2, ", %s", location));

This is probably better done as two appends.

name.AppendLiteral(", ");
...

::: js/src/xpconnect/src/xpcprivate.h
@@ +4415,5 @@
>      bool cycleCollectionEnabled;
>      JSObject2JSObjectMap *waiverWrapperMap;
>      // NB: we don't want this map to hold a strong reference to the wrapper.
>      nsDataHashtable<nsPtrHashKey<XPCWrappedNative>, JSObject *> *expandoMap;
> +    char *location;

We never call free on this (as is required by JS_Encode, used earlier). It might be better to use an nsCString here, though and call Adopt on the buffer returned from JS_Encode in order to avoid the manual memory management.
Attachment #550560 - Flags: review?(mrbkap) → review-
(In reply to Sander Mathijs van Veen from comment #8)
> Created attachment 550570 [details]
> Proposed patch for the bugzilla tweaks add-on (jetpack)
> 
> The sandboxPrototype property is not necessary (otherwise it's given twice
> to the sandbox constructor) and therefore removed from the jetpack patch.

You should patch XPIProvider.jsm (http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#3445) to make all sandboxes for restartless add-ons named in this way. I wonder, is it possible to make sandboxes constructed from inside other sandboxes have the same name? That might catch all the sandboxes that the Add-on SDK creates. Alternatively you should file a bug to do this in the Add-ons SDK itself to all add-ons created by the SDK get it automatically.
Applied mrbkap's and Mossop's suggestions.

I still have a question about:
nsresult rv = xpc_CreateSandboxObject(cx, &rval, principal, NULL, false, nsCString());
Passing NULL instead of nsCString() is not possible (in the previous patch, I passed NULL for char *). Should I use something else instead of nsCString() or is this the way xpc_CreateSandboxObject should be called?

I also added the sandboxName property to other scripts which initialise a Sandbox object.
Attachment #550560 - Attachment is obsolete: true
Attachment #553108 - Flags: review?(mrbkap)
Comment on attachment 553108 [details] [diff] [review]
Patch to make filename visible of a system compartment v2

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

::: browser/devtools/scratchpad/scratchpad.js
@@ +155,5 @@
>          this._previousLocation != this.gBrowser.contentWindow.location.href) {
>        let contentWindow = this.gBrowser.selectedBrowser.contentWindow;
>        this._contentSandbox = new Cu.Sandbox(contentWindow,
> +        { sandboxPrototype: contentWindow, wantXrays: false, 
> +          sandboxName: 'contentWindow of scratchpad.js'});

This one you called "contentWindow" but the one below you called "chromeSandbox".  Maybe this should be "contentSandbox of scratchpad.js"?

But then, avoiding spaces in the name might be a good idea because memory reporter paths generally don't have spaces.  And ".js" isn't in other sandbox names.  So perhaps "scratchpad-content" and "scratchpad-chrome" would be better names?

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +3444,5 @@
>  
>      let principal = Cc["@mozilla.org/systemprincipal;1"].
>                      createInstance(Ci.nsIPrincipal);
> +    this.bootstrapScopes[aId] = new Components.utils.Sandbox(principal,
> +                                                             {sandboxName: aFile.path});

Do you see this case occurring in practice with add-ons?  Can you give any examples?

(Examples of the other cases would be welcome too.)
(In reply to Nicholas Nethercote [:njn] from comment #13)
> Comment on attachment 553108 [details] [diff] [review]
> Patch to make filename visible of a system compartment v2
> 
> Review of attachment 553108 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/scratchpad/scratchpad.js
> @@ +155,5 @@
> >          this._previousLocation != this.gBrowser.contentWindow.location.href) {
> >        let contentWindow = this.gBrowser.selectedBrowser.contentWindow;
> >        this._contentSandbox = new Cu.Sandbox(contentWindow,
> > +        { sandboxPrototype: contentWindow, wantXrays: false, 
> > +          sandboxName: 'contentWindow of scratchpad.js'});
> 
> This one you called "contentWindow" but the one below you called
> "chromeSandbox".  Maybe this should be "contentSandbox of scratchpad.js"?
> 
> But then, avoiding spaces in the name might be a good idea because memory
> reporter paths generally don't have spaces.  And ".js" isn't in other
> sandbox names.  So perhaps "scratchpad-content" and "scratchpad-chrome"
> would be better names?

You're right, those names will fit better.

> Do you see this case occurring in practice with add-ons?  Can you give any
> examples?
> 
> (Examples of the other cases would be welcome too.)

Example of toolkit/mozapps/extensions/XPIProvider.jsm:

│  ├───────67,091 B (00.03%) -- compartment([System Principal], /home/sander/.mozilla/firefox/pumlekcd.default/extensions/ping.telemetry@mozilla.com.xpi, 0xffffffffacd33800)
│  │       ├──57,344 B (00.03%) -- gc-heap
│  │       │  ├──37,640 B (00.02%) -- arena-unused
│  │       │  ├──10,928 B (00.01%) -- objects
│  │       │  ├───8,320 B (00.00%) -- shapes
│  │       │  ├─────224 B (00.00%) -- arena-headers
│  │       │  ├─────216 B (00.00%) -- arena-padding
│  │       │  └──────16 B (00.00%) -- strings
│  │       ├───3,549 B (00.00%) -- scripts
│  │       ├───3,200 B (00.00%) -- object-slots
│  │       ├───2,772 B (00.00%) -- property-tables
│  │       └─────226 B (00.00%) -- string-chars

Example of scratchpad:

│  ├───────27,306 B (00.01%) -- compartment([System Principal], contentWindow of scratchpad.js, 0xffffffff99c07000)
│  │       ├──24,576 B (00.01%) -- gc-heap
│  │       │  ├──17,608 B (00.01%) -- arena-unused
│  │       │  ├───4,456 B (00.00%) -- objects
│  │       │  ├───2,320 B (00.00%) -- shapes
│  │       │  ├──────96 B (00.00%) -- arena-headers
│  │       │  └──────96 B (00.00%) -- arena-padding
│  │       ├───2,432 B (00.00%) -- object-slots
│  │       ├─────168 B (00.00%) -- property-tables
│  │       └─────130 B (00.00%) -- scripts
> Example of toolkit/mozapps/extensions/XPIProvider.jsm:
> 
> │  ├───────67,091 B (00.03%) -- compartment([System Principal],
> /home/sander/.mozilla/firefox/pumlekcd.default/extensions/ping.
> telemetry@mozilla.com.xpi, 0xffffffffacd33800)

Nice!  It might be worth stripping off the dirname when it's an extension that ends with .xpi?

Do you have a sense of how many of the sandboxes get identified now?  How does Bugzilla Tweaks look?

This is great stuff, per-add-on memory accounting will be fantastic.
Besides the telemetry add-on, I've adblock, firebug, flashblock, nightly tester tools and vimperator installed. However, those add-ons do not show up in about:memory as individual system compartments. The strange thing is that there is still one large system compartment. This way, it is hard to track which add-on consumes a large amount of resources.

│  ├───46,640,878 B (20.82%) -- compartment([System Principal], 0xffffffffb72e5800)
│  │   ├──24,612,864 B (10.99%) -- gc-heap
│  │   │  ├──12,224,144 B (05.46%) -- objects
│  │   │  ├───5,907,880 B (02.64%) -- shapes
│  │   │  ├───5,177,232 B (02.31%) -- arena-unused
│  │   │  ├───1,095,888 B (00.49%) -- strings
│  │   │  ├─────110,352 B (00.05%) -- arena-padding
│  │   │  ├──────96,144 B (00.04%) -- arena-headers
│  │   │  └───────1,224 B (00.00%) -- xml
│  │   ├───6,430,720 B (02.87%) -- mjit-code
│  │   ├───6,381,802 B (02.85%) -- scripts
│  │   ├───3,080,016 B (01.38%) -- object-slots
│  │   ├───2,411,164 B (01.08%) -- string-chars
│  │   ├───2,294,308 B (01.02%) -- property-tables
│  │   ├───1,181,644 B (00.53%) -- mjit-data
│  │   ├─────131,072 B (00.06%) -- tjit-code
│  │   └─────117,288 B (00.05%) -- tjit-data
│  │         ├───74,000 B (00.03%) -- allocators-reserve
│  │         └───43,288 B (00.02%) -- allocators-main

In total there are 3 system compartments (or 4 with scratchpad):
 - the large one mentioned in this comment.
 - the compartment for the telemetry add-on.
 - and an unknown compartment: 

│   ├──────23,210 B (00.01%) -- compartment(moz-nullprincipal:{9c7a4fdd-3bc2-4491-9ae0-8e74622bc439})
│   │      ├──20,480 B (00.01%) -- gc-heap
│   │      │  ├──14,800 B (00.01%) -- arena-unused
│   │      │  ├───3,352 B (00.00%) -- objects
│   │      │  ├───2,200 B (00.00%) -- shapes
│   │      │  ├──────80 B (00.00%) -- arena-headers
│   │      │  └──────48 B (00.00%) -- arena-padding
│   │      ├───2,432 B (00.00%) -- object-slots
│   │      ├─────168 B (00.00%) -- property-tables
│   │      └─────130 B (00.00%) -- scripts

I was grep'ing and searching with mxr for that GUID, but the GUID is regenerated after a restart of firefox, so I was unable to locate the creation of that system compartment.

> Do you have a sense of how many of the sandboxes get identified now?  How does Bugzilla Tweaks look?

On another browser profile, I've the patched (see above) version of the bugzilla tweaks add-on. Six (5 and 1 moz-nullprincipal system compartment) out of the 40 system compartments are not identified.
(In reply to Sander Mathijs van Veen from comment #16)
> Besides the telemetry add-on, I've adblock, firebug, flashblock, nightly
> tester tools and vimperator installed. However, those add-ons do not show up
> in about:memory as individual system compartments. The strange thing is that
> there is still one large system compartment. This way, it is hard to track
> which add-on consumes a large amount of resources.

I think that's because those are not JetPack-based add-ons, and this change wasn't expected to provide info about them, AIUI.


> │   ├──────23,210 B (00.01%) --
> compartment(moz-nullprincipal:{9c7a4fdd-3bc2-4491-9ae0-8e74622bc439})
> 
> I was grep'ing and searching with mxr for that GUID, but the GUID is
> regenerated after a restart of firefox, so I was unable to locate the
> creation of that system compartment.

There's always a null compartment.

> On another browser profile, I've the patched (see above) version of the
> bugzilla tweaks add-on. Six (5 and 1 moz-nullprincipal system compartment)
> out of the 40 system compartments are not identified.

I'm more interested in whether the non-patched version of Bugzilla Tweaks has any of its system compartments identified.
Comment on attachment 553108 [details] [diff] [review]
Patch to make filename visible of a system compartment v2

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

This looks good overall. One more patch addressing a couple of small comments below and this should be good to go.

::: js/src/xpconnect/src/nsXPConnect.cpp
@@ +2072,5 @@
>  
>      jsval rval = JSVAL_VOID;
>      AUTO_MARK_JSVAL(ccx, &rval);
>  
> +    nsresult rv = xpc_CreateSandboxObject(cx, &rval, principal, NULL, false, nsCString());

EmptyCString() is what you want here.

::: js/src/xpconnect/src/xpccomponents.cpp
@@ +3139,5 @@
>  NS_IMPL_ISUPPORTS0(Identity)
>  
>  nsresult
>  xpc_CreateSandboxObject(JSContext * cx, jsval * vp, nsISupports *prinOrSop, JSObject *proto,
> +                        bool wantXrays, nsCString sandboxName)

Both here and in the declaration in xpcprivate.h this should be declared as |const nsACString &sandboxName|.

::: js/src/xpconnect/src/xpcjsruntime.cpp
@@ +1577,5 @@
>              if(c->isSystemCompartment)
>              {
> +                if (c->data)
> +                {
> +                    nsCString location = ((xpc::CompartmentPrivate*)c->data)->location;

This should be an nsCAutoString to try to avoid a malloc call.

::: js/src/xpconnect/src/xpcprivate.h
@@ +4392,5 @@
>            wantXrays(wantXrays),
>            cycleCollectionEnabled(cycleCollectionEnabled),
>            waiverWrapperMap(nsnull),
> +          expandoMap(nsnull),
> +          location()

There is no real need to explicitly call the default constructor here.

@@ +4404,5 @@
>            wantXrays(wantXrays),
>            cycleCollectionEnabled(cycleCollectionEnabled),
>            waiverWrapperMap(nsnull),
> +          expandoMap(nsnull),
> +          location()

Ditto here.
Attachment #553108 - Flags: review?(mrbkap) → review-
Comment on attachment 553108 [details] [diff] [review]
Patch to make filename visible of a system compartment v2

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

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +3444,5 @@
>  
>      let principal = Cc["@mozilla.org/systemprincipal;1"].
>                      createInstance(Ci.nsIPrincipal);
> +    this.bootstrapScopes[aId] = new Components.utils.Sandbox(principal,
> +                                                             {sandboxName: aFile.path});

This will label sandboxes for restartless add-ons with the path to the add-on install (either an XPI file or a directory. It might be better to instead use the full URI to the bootstrap.js that is getting loaded into this sandbox
(In reply to Dave Townsend (:Mossop) from comment #19)

> > +    this.bootstrapScopes[aId] = new Components.utils.Sandbox(principal,
> > +                                                             {sandboxName: aFile.path});
> 
> This will label sandboxes for restartless add-ons with the path to the
> add-on install (either an XPI file or a directory. It might be better to
> instead use the full URI to the bootstrap.js that is getting loaded into
> this sandbox

How can Sander access the full URI?
(In reply to Nicholas Nethercote [:njn] from comment #20)
> (In reply to Dave Townsend (:Mossop) from comment #19)
> 
> > > +    this.bootstrapScopes[aId] = new Components.utils.Sandbox(principal,
> > > +                                                             {sandboxName: aFile.path});
> > 
> > This will label sandboxes for restartless add-ons with the path to the
> > add-on install (either an XPI file or a directory. It might be better to
> > instead use the full URI to the bootstrap.js that is getting loaded into
> > this sandbox
> 
> How can Sander access the full URI?

It's generated about 15 lines later with the call to getURIForResourceInFile. Just doing that earlier and stashing it in a variable should do the trick, though you might have to fix something for the case when the file doesn't exist (perhaps we don't label the sandbox in that case)
This patch uses the proper string classes and I implemented Mossop's suggestion.

> though you might have to fix something for the case when the file doesn't exist (perhaps we don't label the sandbox in that case)

Currently, when the aFile does not exist, a sandbox is still created. Is this the desired behaviour? I'm not sure what "ERROR" does, but the name of that function implies that there is something wrong and we cannot continue (hence the return statement). My patch checks for the aFile existance first and if it does, it will create the sandbox. Is that a sound solution or should I create two separate aFile checks?
Attachment #553108 - Attachment is obsolete: true
Attachment #553977 - Flags: review?(mrbkap)
(In reply to Sander Mathijs van Veen from comment #22)
> Created attachment 553977 [details] [diff] [review]
> Patch to make filename visible of a system compartment v3
> 
> This patch uses the proper string classes and I implemented Mossop's
> suggestion.
> 
> > though you might have to fix something for the case when the file doesn't exist (perhaps we don't label the sandbox in that case)
> 
> Currently, when the aFile does not exist, a sandbox is still created. Is
> this the desired behaviour? I'm not sure what "ERROR" does, but the name of
> that function implies that there is something wrong and we cannot continue
> (hence the return statement). My patch checks for the aFile existance first
> and if it does, it will create the sandbox. Is that a sound solution or
> should I create two separate aFile checks?

Currently code assumes that after loadBootstrapScope is called a sandbox will exist and things will go wrong if it doesn't. We could either correct that to support the case where it doesn't or leave loadBootstrapScope always creating the sandbox.

I believe the case where aFile doesn't exist is rare, and apparently untested. Given that I'd rather leave loadBootstrapScope's behaviour the same here.
Restored the original behaviour of loadBootstrapScope (calling that function will always create a sandbox).

Example of a system compartment identified by its URI:

│  ├──────66,974 B (00.10%) -- compartment([System Principal], jar:file:///home/sander/.mozilla/firefox/o178pk14.clean/extensions/ping.telemetry@mozilla.com.xpi!/bootstrap.js, 0xffffffffad0b1800)
│  │      ├──57,344 B (00.08%) -- gc-heap
│  │      │  ├──37,640 B (00.05%) -- arena-unused
│  │      │  ├──10,928 B (00.02%) -- objects
│  │      │  ├───8,320 B (00.01%) -- shapes
│  │      │  ├─────224 B (00.00%) -- arena-headers
│  │      │  ├─────216 B (00.00%) -- arena-padding
│  │      │  └──────16 B (00.00%) -- strings
│  │      ├───3,420 B (00.00%) -- scripts
│  │      ├───3,216 B (00.00%) -- object-slots
│  │      ├───2,772 B (00.00%) -- property-tables
│  │      └─────222 B (00.00%) -- string-chars
Attachment #553977 - Attachment is obsolete: true
Attachment #553977 - Flags: review?(mrbkap)
Attachment #553996 - Flags: review?(mrbkap)
Attachment #553996 - Flags: review?(mrbkap) → review+
Sander, I'll submit your patch to the try server for full testing.  If it looks ok, I'll land it for you.
Try run for d7bf72ed6a5b is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=d7bf72ed6a5b
Results (out of 168 total builds):
    exception: 2
    success: 145
    warnings: 21
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nnethercote@mozilla.com-d7bf72ed6a5b
The try run looked good.  I'll land this early next week.
http://hg.mozilla.org/integration/mozilla-inbound/rev/09420e788013
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
Marking this as dev-doc-needed -- https://developer.mozilla.org/en/Components.utils.Sandbox will need updating.  The optional parameter can now have a third property, "sandboxName", which is the name that will be used to identify the sandbox in about:memory (and possibly other places in the future).
Keywords: dev-doc-needed
Eric Shepherd; I have added a small description to https://developer.mozilla.org/en/Components.utils.Sandbox The property description is missing a "since firefox 9.0" tag.
(Sander saw the "dev-doc-needed" tag and thought he had to do the documentation updates himself :)
http://hg.mozilla.org/mozilla-central/rev/09420e788013
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
Target Milestone: --- → mozilla9
Blocks: 681555
Blocks: 681556
Assignee: nnethercote → sandervv
Blocks: 759783
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: