STR: 1. Apply the patch. If it doesn't apply cleanly then make sure that only the following tests are the only ones available in browser.ini: [browser_telemetry_button_paintflashing.js] [browser_telemetry_button_responsive.js] 2. ./mach mochitest-devtools browser/devtools/shared/test/ When the second test starts the browser crashes.

Clearing flag histograms didn't work, because flag histograms aren't "initialized" to all-zeroes, their zero-bucket contains a 1 upon initialization. We need to reproduce this for ::Clear().

Actually, since I showed Mike that his patches still had some test failures, what do folks think the correct behavior of flag histograms should be when clear() is invoked: 1) Revert to false, which is what attachment 8521498 [details] [diff] [review] does; 2) Remain as-is, which I think is what Mike expects, assuming I understand his patches correctly.

(In reply to Nathan Froyd (:froydnj) from comment #2) > Actually, since I showed Mike that his patches still had some test failures, > what do folks think the correct behavior of flag histograms should be when > clear() is invoked: > > 1) Revert to false, which is what attachment 8521498 [details] [diff] [review] > [review] does; > 2) Remain as-is, which I think is what Mike expects, assuming I understand > his patches correctly. I expect you to revert to false. The patch doesn't quite fix it... after calling clear() any new pings are ignored. I have attached an xpcshell test that highlights the problem (with your patch applied): ./mach test toolkit/components/telemetry/tests/unit/test_TelemetryFlagClear.js output: 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "Original value:" 0:00.21 TEST_STATUS: Thread-1 run_test PASS [run_test : 12] "[1,0,0]" == "[1,0,0]" 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "Sending ping" 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "New value:" 0:00.21 TEST_STATUS: Thread-1 run_test PASS [run_test : 19] "[0,1,0]" == "[0,1,0]" 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "Clearing histogram" 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "After clearing:" 0:00.21 TEST_STATUS: Thread-1 run_test PASS [run_test : 25] "[1,0,0]" == "[1,0,0]" 0:00.21 PROCESS_OUTPUT: Thread-1 (pid:86265) "After sending ping:" 0:00.21 TEST_STATUS: Thread-1 run_test FAIL [run_test : 30] "[1,0,0]" == "[0,1,0]"

Comment on attachment 8521498 [details] [diff] [review] correctly re-initialize flag histograms after clearing them Review of attachment 8521498 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/chromium/src/base/histogram.cc @@ +152,5 @@ > SampleSet ss; > ss.Resize(*this); > sample_ = ss; > + > + if (histogram_type() == FLAG_HISTOGRAM) { Can't we just make Histogram::Clear() virtual and override from FlagHistogram? @@ +154,5 @@ > sample_ = ss; > + > + if (histogram_type() == FLAG_HISTOGRAM) { > + size_t zero_index = BucketIndex(0); > + static_cast<FlagHistogram*>(this)->LinearHistogram::Accumulate(0, 1, zero_index); Also need to reset FlagHistogram::mSwitched.

I didn't realize that we can now use equal etc. in xpcshell tests... updated.

Indeed, that would be better. Thanks for the catch on |mSwitched|; that is probably what was causing the test failures.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0c591e12fea https://hg.mozilla.org/integration/mozilla-inbound/rev/44d6420a3cc7

https://hg.mozilla.org/mozilla-central/rev/e0c591e12fea https://hg.mozilla.org/mozilla-central/rev/44d6420a3cc7