startTimer: doesn't call anything from telemetry but starts a timer stopTimer: if a corresponding timer is found, calls either T.getHistogramById().add() or T.getKeyedHistogramById().add() Replace startTimer/stopTimer by start/finish and startKeyed/finishKeyed. At the same time switch the implementation to call the real TelemetryStopwatch methods.

Comment on attachment 8974319 [details] Bug 1458204 - telemetry.js: Rename startTimer/stopTimer https://reviewboard.mozilla.org/r/242642/#review248974 Thanks for the patch Mike, and thanks for adding a detailed JSdoc for the new wrappers! One small issue + one nit. ::: devtools/client/performance/performance-controller.js:179 (Diff revision 3) > DetailsView.off(EVENTS.UI_DETAILS_VIEW_SELECTED, this._onDetailsViewSelected); > > this._prefObserver.off("devtools.theme", this._onThemeChanged); > this._prefObserver.destroy(); > + > + this._telemetry.destroy(); We should probably remove this now that destroy was removed from telemetry.js ::: devtools/client/shared/telemetry.js:332 (Diff revision 3) > + * > + * @returns {Boolean} > + * True if the timer was succesfully stopped and the data was added > + * to the histogram, False otherwise. > */ > - startTimer(histogramId) { > + finishKeyed(histogramId, key, obj, cancelledOkay) { The jsDoc uses canceled with a single l, so does the other finish() method. We should probably use canceledOkay here as well for consistency

Comment on attachment 8974319 [details] Bug 1458204 - telemetry.js: Rename startTimer/stopTimer https://reviewboard.mozilla.org/r/242642/#review248974 > We should probably remove this now that destroy was removed from telemetry.js You would think so but performance tools use a wrapper called `PerformanceTelemetry` around our telemetry wrapper. In this case `this._telemetry.destroy()` is calling `PerformanceTelemetry.destroy`, not `Telemetry.destroy` so we need to keep it. > The jsDoc uses canceled with a single l, so does the other finish() method. We should probably use canceledOkay here as well for consistency Done

Pushed by mratcliffe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eaf43c0e3a71 telemetry.js: Rename startTimer/stopTimer r=jdescottes

I think, since this change, all DEVTOOLS_*_TIME_ACTIVE_SECONDS probes have been reporting active time in milliseconds, not seconds, since TelemetryStopwatch seems to count in ms.