1096718 - display time measured spent in each compartment

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

this is very much a WIP, just to prove that the underlying system. Ideally we could group these compartments by addon and by tab.

Comment 1

[Dave Townsend [:mossop]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #0)
> Created attachment 8520329 [details] [diff] [review]
> compartment_info.patch
> 
> this is very much a WIP, just to prove that the underlying system. Ideally
> we could group these compartments by addon and by tab.

Setting expectations, I'm probably not going to be able to look at this until next week

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8527363 [details] [diff] [review]
compartment_info.patch

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

I think this patch is missing aboutCOMPARTMENTs.xhtml. Also, there are a lot of style issues. It would help a lot if you fix the indentation, curly brace placement, and * placement in the next patch.

::: js/src/jsapi.cpp
@@ +170,5 @@
>  }
>  
> +JS_PUBLIC_API(uint32_t)
> +JS_NumCompartments(JSRuntime *rt) {
> +    return rt->numCompartments - 1;

Why - 1 ?

@@ +174,5 @@
> +    return rt->numCompartments - 1;
> +}
> +
> +JS_PUBLIC_API(void)
> +JS_GetCompartments(JSRuntime *rt, JSCompartment*** compartments) {

Not sure why you're using triple indirection here. Passing a JSCompartment** would work fine.

@@ +193,5 @@
> +    return compartment->addonId;
> +}
> +
> +extern JS_PUBLIC_API(void)
> +JS_GetCompartmentName(JSRuntime *rt, JSCompartment* compartment, char *buf, size_t bufsize) {

There's not really a reason for this function to exist since it's basically just calling back into xpconnect. It would be better to expose the xpconnect function to the rest of gecko.

::: toolkit/components/aboutcompartments/nsCompartmentInfo.cpp
@@ +70,5 @@
> +  JSCompartment** c = new JSCompartment*[num];
> +  nsCOMPtr<nsIMutableArray> compartments = do_CreateInstance(NS_ARRAY_CONTRACTID);
> +  JS_GetCompartments(rt, &c);
> +  char labelBuf[1024];
> +  for (uint32_t pos = 0; pos < num; pos++) {

It seems like this loop could exist inside the JS engine. I think it would be much better to expose a single API function to do this stuff (similar to CollectRuntimeStats in MemoryMetrics.h).

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

This addresses everything but the last comment.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8528686 [details] [diff] [review]
compartment_info.patch

This patch still needs review from a JS peer, but maybe Mossop should look at it first.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

had the conversion from interval to microseconds going the wrong way

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

You can now click on an addon to see the time broken down by compartment

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 8531536 [details] [diff] [review]
compartment_info.patch

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

Some notes here and I'd like to see it again before r+

::: toolkit/components/aboutcompartments/content/aboutCompartments.js
@@ +12,5 @@
> +
> +function go() {
> +    let compartmentInfo = Cc["@mozilla.org/compartment-info;1"]
> +            .getService(Ci.nsICompartmentInfo);
> +    let cpows= compartmentInfo.compartments;

These are compartments, not cpows.

@@ +14,5 @@
> +    let compartmentInfo = Cc["@mozilla.org/compartment-info;1"]
> +            .getService(Ci.nsICompartmentInfo);
> +    let cpows= compartmentInfo.compartments;
> +    let count = cpows.length;
> +    let addons = [];

Looks like you use string keys so this should probably be an object, not an array

@@ +24,5 @@
> +            addons[cpow.addonId].compartments.push({
> +                time: cpow.time,
> +                CPOWTime: cpow.CPOWTime,
> +                label: cpow.label
> +                });

Any reason you're rebuilding the compartment structure here?

@@ +47,5 @@
> +        name.className = "addon";
> +        time.className = "time";
> +        cpow.className = "cpow";
> +        name.textContent = addon;
> +        AddonManager.getAddonByID(addon, function(a) {

No big deal but you could make this more performant by getting all the ids and using AddonManager.getAddonsByIDs to pull them all from the DB in one shot.

::: toolkit/components/aboutcompartments/content/aboutCompartments.xhtml
@@ +5,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<html xmlns="http://www.w3.org/1999/xhtml">
> +  <head>
> +    <title>about:CPOWs</title>

about:compartments

@@ +10,5 @@
> +    <script type="text/javascript;version=1.8" src="chrome://global/content/aboutCompartments.js"></script>
> +    <style>
> +      #debug {
> +      display:none;
> +      }

More strange indenting

@@ +34,5 @@
> +      }
> +    </style>
> +  </head>
> +  <body onload="go()">
> +    <div id="data">

I don't really understand why you're using divs/spans to display tabular data. Why not just use a <table>?

@@ +41,5 @@
> +	<span class="cpow">time in CPOWs</span>
> +	<span class="addon">name</span>
> +      </div>
> +    </div>
> +    <div id="debug"/>

Doesn't look like this is used, except for the style reference that hides it.

::: toolkit/components/aboutcompartments/nsCompartmentInfo.cpp
@@ +20,5 @@
> +    /* readonly attribute wstring label; */
> +    NS_IMETHOD GetLabel(nsAString & aLabel){
> +    aLabel.Assign(mLabel);
> +    return NS_OK;
> +  };

Some strange indentation going on here

::: toolkit/components/aboutcompartments/nsICompartmentInfo.idl
@@ +5,5 @@
> +interface nsICompartment : nsISupports {
> +  readonly attribute AString label;
> +  readonly attribute unsigned long time;
> +  readonly attribute AString addonId;
> +  readonly attribute unsigned long CPOWTime;

Tempted to say that this should include the sandbox metadata where it exists. That would mean say devtools could tag their sandboxes and see their data in here maybe?

@@ +12,5 @@
> +[scriptable, builtinclass, uuid(5795113a-39a1-4087-ba09-98b7d07d025a)]
> +interface nsICompartmentInfo : nsISupports {
> +  //void listCompartments(out unsigned long count, [retval, array, size_is(count)] out nsICompartment compartments);
> +  //readonly attribute AString[] compartments;
> +  readonly attribute nsIArray compartments;

Given the work going on here to get the compartments I think it makes more sense to use the method form. It makes the JS access slightly nicer too.

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8531536 [details] [diff] [review]
compartment_info.patch

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

Looks much better, thanks. I'd like to look at it again after the Vector change.

::: js/src/jsapi.cpp
@@ +169,5 @@
>      return rt->emptyString;
>  }
>  
> +JS_PUBLIC_API(uint32_t)
> +JS_GetCompartmentStats(JSRuntime *rt, CompartmentTimeStats **stats)

Please use a Vector for stats.

@@ +173,5 @@
> +JS_GetCompartmentStats(JSRuntime *rt, CompartmentTimeStats **stats)
> +{
> +    int num = rt->numCompartments;
> +    *stats = new CompartmentTimeStats[num];
> +    int pos = 0;

This should be size_t.

@@ +179,5 @@
> +        CompartmentTimeStats *stat = *stats + pos;
> +        stat->time = c.get()->totalTime;
> +        stat->compartment = c.get();
> +        stat->addonId = c.get()->addonId;
> +        if (rt->compartmentNameCallback) {

You should initialize the label even if compartmentNameCallback is null.

::: js/xpconnect/src/nsXPConnect.cpp
@@ +1132,5 @@
>  }
>  
> +/* [noscript, notxpcom] void unregisterContextCallback(in xpcContextCallback func); */
> +NS_IMETHODIMP_(uint32_t)
> +nsXPConnect::NumCompartments()

Seems to be unused.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8534182 [details] [diff] [review]
compartment_info.patch

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

I'm actually fine with this in order to land something usable and useful. I would, however, suggest that we file a follow-up bug to re-do the front-end a bit.

Specifically, I recommend we try to mimic about:memory's text-based tree style, which is familiar, and has the nice property of being easy to copy and paste. I also think that the tree should be sorted.

However, as a start, I think this is fine.

::: docshell/base/nsAboutRedirector.cpp
@@ +54,3 @@
>      { "memory", "chrome://global/content/aboutMemory.xhtml",
>        nsIAboutModule::ALLOW_SCRIPT },
> +    { "compartments", "chrome://global/content/aboutCOMPARTMENTs.xhtml",

I'm curious about the allcaps casing of COMPARTMENTs... aboutCompartments.xhtml was probably fine.

::: toolkit/components/aboutcompartments/content/aboutCompartments.js
@@ +5,5 @@
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +"use strict";
> +
> +const { classes: Cc, interfaces: Ci, utils: Cu } = Components;

I've always loved this construct.

@@ +19,5 @@
> +    for(let i = 0; i < count; i++) {
> +        let compartment = compartments.queryElementAt(i, Ci.nsICompartment);
> +        if (addons[compartment.addonId]) {
> +            addons[compartment.addonId].time += compartment.time;
> +            addons[compartment.addonId].COMPARTMENTTime += compartment.CPOWTime;

Again, curious about the casing here.

::: toolkit/components/aboutcompartments/nsICompartmentInfo.idl
@@ +1,1 @@
> +#include "nsISupports.idl"

Nit - missing license.

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8534182 [details] [diff] [review]
compartment_info.patch

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

::: browser/installer/package-manifest.in
@@ +166,5 @@
>  
>  ; [Components]
>  @BINPATH@/browser/components/components.manifest
>  @BINPATH@/components/alerts.xpt
> +@BINPATH@/components/compartments.xpt

This doesn't seem to be used.

::: docshell/base/nsAboutRedirector.cpp
@@ +54,3 @@
>      { "memory", "chrome://global/content/aboutMemory.xhtml",
>        nsIAboutModule::ALLOW_SCRIPT },
> +    { "compartments", "chrome://global/content/aboutCOMPARTMENTs.xhtml",

Besides that, the actual filename in the patch is aboutCompartments.xhtml.

::: js/src/jsapi.cpp
@@ +168,5 @@
>      MOZ_ASSERT(rt->hasContexts());
>      return rt->emptyString;
>  }
>  
> +JS_PUBLIC_API(uint32_t)

This can just return void. The length is part of stats.

@@ +169,5 @@
>      return rt->emptyString;
>  }
>  
> +JS_PUBLIC_API(uint32_t)
> +JS_GetCompartmentStats(JSRuntime *rt, CompartmentStatsVector *stats)

Let's pass stats as a CompartmentStatsVector &.

@@ +175,5 @@
> +    size_t num = rt->numCompartments;
> +    stats->initCapacity(num);
> +    size_t pos = 0;
> +    for (CompartmentsIter c(rt, WithAtoms); !c.done(); c.next()) {
> +        (*stats)[pos].time = c.get()->totalTime;

It would be cleaner to have a local variable |CompartmentTimeStats *stat = &stats[pos];| and then just use stat->foo = bar in the rest of the loop.

::: toolkit/components/aboutcompartments/content/aboutCompartments.js
@@ +15,5 @@
> +            .getService(Ci.nsICompartmentInfo);
> +    let compartments = compartmentInfo.getCompartments();
> +    let count = compartments.length;
> +    let addons = {};
> +    for(let i = 0; i < count; i++) {

Space after |for|.

@@ +19,5 @@
> +    for(let i = 0; i < count; i++) {
> +        let compartment = compartments.queryElementAt(i, Ci.nsICompartment);
> +        if (addons[compartment.addonId]) {
> +            addons[compartment.addonId].time += compartment.time;
> +            addons[compartment.addonId].COMPARTMENTTime += compartment.CPOWTime;

I think COMPARTMENTTime should actually be CPOWTime here.

@@ +50,5 @@
> +        el.appendChild(time);
> +        el.appendChild(cpow);
> +        el.appendChild(name);
> +        let div = document.createElement("tr");
> +        for (let compartment in addons[addon].compartments) {

It would be clearer to use |let comp of addons...|. Then you don't need to do the array indexing below.

::: toolkit/components/aboutcompartments/content/aboutCompartments.xhtml
@@ +10,5 @@
> +    <script type="text/javascript;version=1.8" src="chrome://global/content/aboutCompartments.js"></script>
> +    <style>
> +      td.addon {
> +        display: inline-block;
> +        width:400px;

Space after the colon.

::: toolkit/components/aboutcompartments/moz.build
@@ +20,5 @@
> +    'nsCompartmentInfo.h'
> +]
> +
> +LOCAL_INCLUDES += [
> +    '/js/xpconnect/src',

Please remove this section.

::: toolkit/components/aboutcompartments/nsCompartmentInfo.cpp
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#include "nsCompartmentInfo.h"
> +#include "wchar.h"

What's this for?

@@ +11,5 @@
> +#include "nsIJSRuntimeService.h"
> +#include "nsServiceManagerUtils.h"
> +#include "nsIMutableArray.h"
> +#include "nsJSUtils.h"
> +#include "xpcprivate.h"

Rather than including this, please add a function to xpcpublic.h that returns the CPOW time given a compartment. Then you don't need to use CompartmentPrivate here.

@@ +13,5 @@
> +#include "nsIMutableArray.h"
> +#include "nsJSUtils.h"
> +#include "xpcprivate.h"
> +
> +class NS_NO_VTABLE nsCompartment : public nsICompartment {

NS_NO_VTABLE on this class seems wrong since it actually gets instantiated. Please remove.

@@ +14,5 @@
> +#include "nsJSUtils.h"
> +#include "xpcprivate.h"
> +
> +class NS_NO_VTABLE nsCompartment : public nsICompartment {
> + public:

This should not be indented.

@@ +16,5 @@
> +
> +class NS_NO_VTABLE nsCompartment : public nsICompartment {
> + public:
> +
> +  nsCompartment(nsString aLabel, nsString aAddonId, uint32_t aTime, uint32_t aCPOWTime) : mLabel(aLabel), mAddonId(aAddonId), mTime(aTime), mCPOWTime(aCPOWTime) {}

Please split this across multiple lines. Also, uint32_t seems too small a type for some of these times. Shouldn't we be using uint64_t (and convert the idl type to unsigned long long)?

@@ +20,5 @@
> +  nsCompartment(nsString aLabel, nsString aAddonId, uint32_t aTime, uint32_t aCPOWTime) : mLabel(aLabel), mAddonId(aAddonId), mTime(aTime), mCPOWTime(aCPOWTime) {}
> +
> +  NS_DECL_ISUPPORTS
> +
> +    /* readonly attribute wstring label; */

Comment is wrong.

@@ +21,5 @@
> +
> +  NS_DECL_ISUPPORTS
> +
> +    /* readonly attribute wstring label; */
> +    NS_IMETHOD GetLabel(nsAString & aLabel){

Indentation is wrong. Also, the & should go next to nsAString.

@@ +27,5 @@
> +    return NS_OK;
> +  };
> +
> +  /* readonly attribute unsigned long time; */
> +  NS_IMETHOD GetTime(uint32_t *aTime) {

* should go with uint32_t (to be changed to uint64_t).

@@ +31,5 @@
> +  NS_IMETHOD GetTime(uint32_t *aTime) {
> +    *aTime = mTime;
> +    return NS_OK;
> +  }
> +  /* readonly attribute wstring label; */

Comment is wrong.

@@ +32,5 @@
> +    *aTime = mTime;
> +    return NS_OK;
> +  }
> +  /* readonly attribute wstring label; */
> +  NS_IMETHOD GetAddonId(nsAString & aAddonId){

& should go with nsAString.

@@ +38,5 @@
> +    return NS_OK;
> +  };
> +
> +  /* readonly attribute unsigned long time; */
> +  NS_IMETHOD GetCPOWTime(uint32_t *aCPOWTime) {

Same about the *.

@@ +42,5 @@
> +  NS_IMETHOD GetCPOWTime(uint32_t *aCPOWTime) {
> +    *aCPOWTime = mCPOWTime;
> +    return NS_OK;
> +  }
> +  nsString mLabel;

These fields should be private.

@@ +46,5 @@
> +  nsString mLabel;
> +  nsString mAddonId;
> +  uint32_t mTime;
> +  uint32_t mCPOWTime;
> + private:

This line should not be indented.

@@ +51,5 @@
> +  virtual ~nsCompartment() {}
> +};
> +
> +
> +

Too many blank lines here.

@@ +63,5 @@
> +nsCompartmentInfo::~nsCompartmentInfo()
> +{
> +}
> +
> +NS_IMETHODIMP nsCompartmentInfo::GetCompartments(nsIArray* *aCompartments)

NS_IMETHODIMP should go on its own line. Also, both *s should go with nsIArray.

@@ +65,5 @@
> +}
> +
> +NS_IMETHODIMP nsCompartmentInfo::GetCompartments(nsIArray* *aCompartments)
> +{
> +  JSRuntime *rt;

The * should go next to JSRuntime.

@@ +71,5 @@
> +  NS_ENSURE_TRUE(svc, NS_ERROR_FAILURE);
> +  svc->GetRuntime(&rt);
> +  nsCOMPtr<nsIMutableArray> compartments = do_CreateInstance(NS_ARRAY_CONTRACTID);
> +  CompartmentStatsVector stats;
> +  uint32_t num = JS_GetCompartmentStats(rt, &stats);

Please just get the length via stats.length(). And it should be a size_t.

::: toolkit/components/aboutcompartments/nsCompartmentInfo.h
@@ +2,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef COMPARTMENT_INFO

This should be nsCompartmentInfo_h.

Comment 14

[Dave Townsend [:mossop]]

Comment on attachment 8534182 [details] [diff] [review]
compartment_info.patch

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

You've already got plenty of review of the front-end here, all I have to add is a tiny nit

::: toolkit/components/aboutcompartments/content/aboutCompartments.xhtml
@@ +35,5 @@
> +    <table id="data">
> +      <tr class="header">
> +        <td class="time">time</td>
> +        <td class="cpow">time in CPOWs</td>
> +         <td class="addon">name</td>

Nit: Extra spacing

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: compartment_info.patch

Comment 16

[Dave Townsend [:mossop]]

Comment on attachment 8540396 [details] [diff] [review]
compartment_info.patch

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

::: toolkit/components/aboutcompartments/nsICompartmentInfo.idl
@@ +7,5 @@
> +#include "nsISupports.idl"
> +#include "nsIArray.idl"
> +
> +[scriptable, uuid(13dd4c09-ff11-4943-8dc2-d96eb69c963b)]
> +interface nsICompartment : nsISupports {

This IDL is crying out for comments. What is time measured in? What is label?

Comment 17

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8540396 [details] [diff] [review]
compartment_info.patch

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

::: js/src/jsapi.cpp
@@ +166,5 @@
>  
> +JS_PUBLIC_API(void)
> +JS_GetCompartmentStats(JSRuntime *rt, CompartmentStatsVector &stats)
> +{
> +    stats.resizeUninitialized(rt->numCompartments);

You should really check if this returns false here and then return false. Then you need to check the return value in nsCompartmentInfo.cpp and return NS_ERROR_OUT_OF_MEMORY.

::: js/src/jsapi.h
@@ +1042,5 @@
>  extern JS_PUBLIC_API(JSString *)
>  JS_GetEmptyString(JSRuntime *rt);
>  
> +struct CompartmentTimeStats {
> +    char label[1024];

Maybe call this compartmentName?

@@ +1046,5 @@
> +    char label[1024];
> +    JSAddonId *addonId;
> +    JSCompartment *compartment;
> +    uint32_t time;
> +    uint32_t cpowTime;

These two should be uint64_t. Please comment that these are in microseconds.

::: js/xpconnect/src/nsXPConnect.cpp
@@ +351,5 @@
>      if (compartmentPrivate && compartmentPrivate->scope)
>          compartmentPrivate->scope->TraceInside(trc);
>  }
>  
> +uint32_t

This should return a uint64_t. A uint32_t holding microseconds will overflow after about an hour.

@@ +352,5 @@
>          compartmentPrivate->scope->TraceInside(trc);
>  }
>  
> +uint32_t
> +xpc::GetCPOWTimeForCompartment(JSCompartment *compartment) {

Please put the brace on its own line. Also, you might want to move this function below the start of |namespace xpc {| and remove the xpc:: qualifier. Finally, please rename this to GetCompartmentCPOWMicroseconds.

@@ +353,5 @@
>  }
>  
> +uint32_t
> +xpc::GetCPOWTimeForCompartment(JSCompartment *compartment) {
> +    xpc::CompartmentPrivate* compartmentPrivate = xpc::CompartmentPrivate::Get(compartment);

The * should go on the other side. In js/, * goes on the right. Everywhere else it goes on the left.

::: toolkit/components/aboutcompartments/nsCompartmentInfo.cpp
@@ +14,5 @@
> +#include "xpcpublic.h"
> +
> +class nsCompartment : public nsICompartment {
> +public:
> +  nsCompartment(nsString aLabel, nsString aAddonId, uint64_t aTime, uint64_t aCPOWTime) : mLabel(aLabel), mAddonId(aAddonId), mTime(aTime), mCPOWTime(aCPOWTime) {}

It would be better to pass the strings by reference: const nsAString&. Also, please split the line.

@@ +61,5 @@
> +{
> +}
> +
> +NS_IMETHODIMP
> +nsCompartmentInfo::GetCompartments(nsIArray* *aCompartments)

nsIArray** aCompartments.

@@ +71,5 @@
> +  nsCOMPtr<nsIMutableArray> compartments = do_CreateInstance(NS_ARRAY_CONTRACTID);
> +  CompartmentStatsVector stats;
> +  JS_GetCompartmentStats(rt, stats);
> +  size_t num = stats.length();
> +  for (uint32_t pos = 0; pos < num; pos++) {

Should be size_t.

@@ +79,5 @@
> +    } else {
> +      addonId.AssignLiteral("<non-addon>");
> +    }
> +
> +    uint32_t cpowTime = xpc::GetCPOWTimeForCompartment(stats[pos].compartment);

uint64_t.

@@ +83,5 @@
> +    uint32_t cpowTime = xpc::GetCPOWTimeForCompartment(stats[pos].compartment);
> +    nsCString label(stats[pos].label);
> +    compartments->AppendElement(new nsCompartment(NS_ConvertUTF8toUTF16(label), addonId, stats[pos].time, cpowTime), false);
> +  }
> +  NS_ADDREF(*aCompartments = compartments);

compartments.forget(aCompartments);

::: toolkit/components/aboutcompartments/nsICompartmentInfo.idl
@@ +8,5 @@
> +#include "nsIArray.idl"
> +
> +[scriptable, uuid(13dd4c09-ff11-4943-8dc2-d96eb69c963b)]
> +interface nsICompartment : nsISupports {
> +  readonly attribute AString label;

compartmentName as well I guess.

@@ +11,5 @@
> +interface nsICompartment : nsISupports {
> +  readonly attribute AString label;
> +  readonly attribute unsigned long long time;
> +  readonly attribute AString addonId;
> +  readonly attribute unsigned long long CPOWTime;

Yeah, please comment about the units.

Comment 18

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8540396 [details] [diff] [review]
compartment_info.patch

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

Oops. Meant to r+.

Comment 19

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/integration/fx-team/rev/96f157725203

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/96f157725203