This is a follow-up to bug 672694, which added the js-compartment-count reporter. From bug 672694 comment 3: > TelemetryPing.js has code that assumes that any UNITS_COUNT reporter > reports a number that always increases, and so reports the change between > pings. This makes sense for the page-fault counters, but not for > js-compartment-count... We might need to distinguish between UNITS_COUNT > and UNITS_COUNT_INCREASING. jlebar, I'll let you figure out the best way to handle this :)

Adding a new unit is the right thing to do, I think.

I purposely left the horizontal alignment alone in a few places, because I thought the whitespace added above "_CUMULATIVE" is ugly.

Comment on attachment 547227 [details] [diff] [review] Part 1 - Add UNITS_COUNT_CUMULATIVE. Review of attachment 547227 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/aboutmemory/content/aboutMemory.js @@ +267,5 @@ > if (a._units != b._units) { > return a._units - b._units; // use the enum order from nsIMemoryReporter > } > + if (a._units == UNITS_COUNT || > + a._units == UNITS_COUNT_CUMULATIVE) { This has bitrotted, but in the new code you won't need to make any changes. @@ +533,5 @@ > { > switch(aReporter._units) { > + case UNITS_BYTES: return formatBytes(aReporter._amount); > + case UNITS_COUNT: return formatInt(aReporter._amount); > + case UNITS_COUNT_CUMULATIVE: return formatInt(aReporter._amount); You can fall through the first case and avoid the code duplication. ::: toolkit/components/aboutmemory/tests/chrome/test_aboutmemory.xul @@ +52,5 @@ > const OTHER = Ci.nsIMemoryReporter.KIND_OTHER; > > const BYTES = Ci.nsIMemoryReporter.UNITS_BYTES; > const COUNT = Ci.nsIMemoryReporter.UNITS_COUNT; > + const COUNT_CUMULATIVE = Ci.nsIMemoryReporter.UNITS_COUNT_CUMULATIVE; Can you use COUNT_CUMULATIVE somewhere in the test? Either add a new fake reporter (which will require updating amExpectedText and amvExpectedText) or, if you're lazy, just change one of the COUNT occurrences to COUNT_CUMULATIVE. ::: xpcom/base/nsIMemoryReporter.idl @@ +150,4 @@ > */ > const PRInt32 UNITS_BYTES = 0; > const PRInt32 UNITS_COUNT = 1; > + const PRInt32 UNITS_COUNT_CUMULATIVE = 2; UNITS_PERCENTAGE snuck in and took the value 2, but I don't see why it can't be bumped to 3, esp. as we haven't shipped it yet. This change'll need dev-doc-needed, BTW.

I'm morphing this bug into adding UNITS_COUNT_CUMULATIVE to nsIMemoryReporter (rather than "Telemeterize js-compartment-count", which requires COUNT_CUMULATIVE as a prerequisite) so I can land COUNT_CUMULATIVE and avoid bitrotting Jez in bug 670084 further.

Inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/9b1a8e3b99aa Bug 673837 is the new telemeterize js-compartment-count bug.

jlebar, the old comment for UNITS_COUNT talks about it being cumulative, which is no longer correct. (trevorh on IRC spotted this.) Can you fix this? A follow-up here seems good enough. Thanks!

Thanks for catching this. The patch I posted fixed this; I just failed at merging it. I'll land what you already reviewed.