Closed Bug 1364502 Opened 3 years ago Closed 3 years ago
_WINDOWS needs to become "release criteria" quality
59 bytes, text/x-review-board-request
A quantum release criterion is to have no ghost windows. :bsmedberg's proposal is that this is to be measured using GHOST_WINDOWS. To do that we need to at least make GHOST_WINDOWS opt-out. We also need to verify that the MemoryReporterManager on release-channel builds report ghost windows (pretty sure it does, but it pays to be thorough). Note: GHOST_WINDOWS is reported via the same mechanism as other Memory Reporter metrics: occasionally, TelemetrySession decides to grab all of the memory information it can think of to ask. It does this multiple times a session to figure out some picture of typical values within sessions. Probably not relevant for implementation, but might strike people unawares if they haven't run across this before.
Comment on attachment 8869572 [details] bug 1364502 - Allow GHOST_WINDOWS to be recorded opt-out data-r=bsmedberg https://reviewboard.mozilla.org/r/141154/#review144714 Yeah, I'm pretty sure the ghost windows should work on release. I've certainly seen users reporting ghost windows in about:memory on release.
Attachment #8869572 - Flags: review?(continuation) → review+
Comment on attachment 8869572 [details] bug 1364502 - Allow GHOST_WINDOWS to be recorded opt-out data-r=bsmedberg data-review-r?
Comment on attachment 8869572 [details] bug 1364502 - Allow GHOST_WINDOWS to be recorded opt-out data-r=bsmedberg https://reviewboard.mozilla.org/r/141154/#review146214 The only concern is that the code which gathers this telemetry isn't gated somewhere on an opt-in check. So we should test/verify this explicitly. data-r=me
Attachment #8869572 - Flags: review+
Oh Right! _That_ was the reason I was looking into writing a scalar directly into the ghost windows detector. GHOST_WINDOWS _is_ presently opt-in gated within TelemetrySession: http://searchfox.org/mozilla-central/source/toolkit/components/telemetry/TelemetrySession.jsm#1099 So we have a choice: 1) Make GHOST_WINDOWS collect opt-out by moving the canRecordExtended check down a couple-dozen lines and the GHOST_WINDOWS accumulation up a dozen lines or so. 2) Accumulate to an opt-out scalar here: http://searchfox.org/mozilla-central/source/dom/base/nsWindowMemoryReporter.cpp#795 I'm very slightly leaning towards #1, so that's where I'll manoeurvre the patch for now.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/20027d10c52c Allow GHOST_WINDOWS to be recorded opt-out data-r=bsmedberg r=bsmedberg,mccr8
You need to log in before you can comment on or make changes to this bug.