Bug 837238 - Record session time in seconds not milliseconds; r=rnewman
authorGregory Szorc <gps@mozilla.com>
Tue, 05 Feb 2013 15:22:33 -0800
changeset 130812 d5390130b80e142196a70412acef6da4b0149c80
parent 130811 d9220b65d6463df25e44c21bef426e3d0eeb0d06
child 130813 42abb7556fd9fbe1de3ac897b62e651863d104f5
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs837238
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 837238 - Record session time in seconds not milliseconds; r=rnewman
services/datareporting/sessions.jsm
services/datareporting/tests/xpcshell/test_session_recorder.js
services/healthreport/providers.jsm
services/healthreport/tests/xpcshell/test_provider_sessions.js
--- a/services/datareporting/sessions.jsm
+++ b/services/datareporting/sessions.jsm
@@ -69,16 +69,17 @@ this.SessionRecorder = function (branch)
     throw new Error("branch argument must end with '.': " + branch);
   }
 
   this._log = Log4Moz.repository.getLogger("Services.DataReporting.SessionRecorder");
 
   this._prefs = new Preferences(branch);
   this._lastActivityWasInactive = false;
   this._activeTicks = 0;
+  this.fineTotalTime = 0;
   this._started = false;
   this._timer = null;
   this._startupFieldTries = 0;
 
   this._os = Cc["@mozilla.org/observer-service;1"]
                .getService(Ci.nsIObserverService);
 
 };
@@ -115,22 +116,30 @@ SessionRecorder.prototype = Object.freez
   get activeTicks() {
     return this._prefs.get("current.activeTicks", 0);
   },
 
   incrementActiveTicks: function () {
     this._prefs.set("current.activeTicks", ++this._activeTicks);
   },
 
+  /**
+   * Total time of this session in integer seconds.
+   *
+   * See also fineTotalTime for the time in milliseconds.
+   */
   get totalTime() {
     return this._prefs.get("current.totalTime", 0);
   },
 
   updateTotalTime: function () {
-    this._prefs.set("current.totalTime", Date.now() - this.startDate);
+    // We store millisecond precision internally to prevent drift from
+    // repeated rounding.
+    this.fineTotalTime = Date.now() - this.startDate;
+    this._prefs.set("current.totalTime", Math.floor(this.fineTotalTime / 1000));
   },
 
   get main() {
     return this._prefs.get("current.main", -1);
   },
 
   set _main(value) {
     if (!Number.isInteger(value)) {
--- a/services/datareporting/tests/xpcshell/test_session_recorder.js
+++ b/services/datareporting/tests/xpcshell/test_session_recorder.js
@@ -56,24 +56,28 @@ add_test(function test_basic() {
 add_task(function test_current_properties() {
   let now = new Date();
   let recorder = getRecorder("current_properties", now);
   yield sleep(25);
   recorder.onStartup();
 
   do_check_eq(recorder.startDate.getTime(), now.getTime());
   do_check_eq(recorder.activeTicks, 0);
-  do_check_true(recorder.totalTime > 0);
+  do_check_true(recorder.fineTotalTime > 0);
   do_check_eq(recorder.main, 500);
   do_check_eq(recorder.firstPaint, 1000);
   do_check_eq(recorder.sessionRestored, 1500);
 
   recorder.incrementActiveTicks();
   do_check_eq(recorder.activeTicks, 1);
 
+  recorder._startDate = new Date(Date.now() - 1000);
+  recorder.updateTotalTime();
+  do_check_eq(recorder.totalTime, 1);
+
   recorder.onShutdown();
 });
 
 // If startup info isn't present yet, we should install a timer and get
 // it eventually.
 add_task(function test_current_availability() {
   let recorder = new SessionRecorder("testing.current_availability.");
   let now = new Date();
@@ -265,34 +269,34 @@ add_task(function test_record_activity()
   yield sleep(25);
   recorder.onStartup();
   let total = recorder.totalTime;
   yield sleep(25);
 
   for (let i = 0; i < 3; i++) {
     Services.obs.notifyObservers(null, "user-interaction-active", null);
     yield sleep(25);
-    do_check_true(recorder.totalTime > total);
-    total = recorder.totalTime;
+    do_check_true(recorder.fineTotalTime > total);
+    total = recorder.fineTotalTime;
   }
 
   do_check_eq(recorder.activeTicks, 3);
 
   // Now send inactive. We should increment total time but not active.
   Services.obs.notifyObservers(null, "user-interaction-inactive", null);
   do_check_eq(recorder.activeTicks, 3);
-  do_check_true(recorder.totalTime > total);
-  total = recorder.totalTime;
+  do_check_true(recorder.fineTotalTime > total);
+  total = recorder.fineTotalTime;
   yield sleep(25);
 
   // If we send active again, this should be counted as inactive.
   Services.obs.notifyObservers(null, "user-interaction-active", null);
   do_check_eq(recorder.activeTicks, 3);
-  do_check_true(recorder.totalTime > total);
-  total = recorder.totalTime;
+  do_check_true(recorder.fineTotalTime > total);
+  total = recorder.fineTotalTime;
   yield sleep(25);
 
   // If we send active again, this should be counted as active.
   Services.obs.notifyObservers(null, "user-interaction-active", null);
   do_check_eq(recorder.activeTicks, 4);
 
   Services.obs.notifyObservers(null, "user-interaction-active", null);
   do_check_eq(recorder.activeTicks, 5);
--- a/services/healthreport/providers.jsm
+++ b/services/healthreport/providers.jsm
@@ -374,17 +374,17 @@ SysInfoProvider.prototype = Object.freez
 function CurrentSessionMeasurement() {
   Metrics.Measurement.call(this);
 }
 
 CurrentSessionMeasurement.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "current",
-  version: 2,
+  version: 3,
 
   configureStorage: function () {
     return Promise.resolve();
   },
 
   /**
    * All data is stored in prefs, so we have a custom implementation.
    */
@@ -423,17 +423,17 @@ CurrentSessionMeasurement.prototype = Ob
 function PreviousSessionsMeasurement() {
   Metrics.Measurement.call(this);
 }
 
 PreviousSessionsMeasurement.prototype = Object.freeze({
   __proto__: Metrics.Measurement.prototype,
 
   name: "previous",
-  version: 2,
+  version: 3,
 
   DAILY_DISCRETE_NUMERIC_FIELDS: [
     // Milliseconds of sessions that were properly shut down.
     "cleanActiveTicks",
     "cleanTotalTime",
 
     // Milliseconds of sessions that were not properly shut down.
     "abortedActiveTicks",
@@ -483,28 +483,28 @@ SessionsProvider.prototype = Object.free
 
   name: "org.mozilla.appSessions",
 
   measurementTypes: [CurrentSessionMeasurement, PreviousSessionsMeasurement],
 
   constantOnly: true,
 
   collectConstantData: function () {
-    let previous = this.getMeasurement("previous", 2);
+    let previous = this.getMeasurement("previous", 3);
 
     return this.storage.enqueueTransaction(this._recordAndPruneSessions.bind(this));
   },
 
   _recordAndPruneSessions: function () {
     this._log.info("Moving previous sessions from session recorder to storage.");
     let recorder = this.healthReporter.sessionRecorder;
     let sessions = recorder.getPreviousSessions();
     this._log.debug("Found " + Object.keys(sessions).length + " previous sessions.");
 
-    let daily = this.getMeasurement("previous", 2);
+    let daily = this.getMeasurement("previous", 3);
 
     for each (let session in sessions) {
       let type = session.clean ? "clean" : "aborted";
       let date = session.startDate;
       yield daily.addDailyDiscreteNumeric(type + "ActiveTicks", session.activeTicks, date);
       yield daily.addDailyDiscreteNumeric(type + "TotalTime", session.totalTime, date);
 
       for (let field of ["main", "firstPaint", "sessionRestored"]) {
--- a/services/healthreport/tests/xpcshell/test_provider_sessions.js
+++ b/services/healthreport/tests/xpcshell/test_provider_sessions.js
@@ -78,17 +78,17 @@ function getProvider(name, now=new Date(
 
 add_task(function test_current_session() {
   let now = new Date();
   let [provider, storage, recorder] = yield getProvider("current_session", now);
 
   yield sleep(25);
   recorder.onActivity(true);
 
-  let current = provider.getMeasurement("current", 2);
+  let current = provider.getMeasurement("current", 3);
   let values = yield current.getValues();
   let fields = values.singular;
 
   for (let field of ["startDay", "activeTicks", "totalTime", "main", "firstPaint", "sessionRestored"]) {
     do_check_true(fields.has(field));
   }
 
   do_check_eq(fields.get("startDay")[1], Metrics.dateToDays(now));
@@ -119,17 +119,17 @@ add_task(function test_collect() {
 
   recorder = new SessionRecorder("testing.collect.sessions.");
   recorder.onStartup();
 
   yield provider.collectConstantData();
   let sessions = recorder.getPreviousSessions();
   do_check_eq(Object.keys(sessions).length, 0);
 
-  let daily = provider.getMeasurement("previous", 2);
+  let daily = provider.getMeasurement("previous", 3);
   let values = yield daily.getValues();
   do_check_true(values.days.hasDay(now));
   do_check_eq(values.days.size, 1);
 
   let day = values.days.getDay(now);
   do_check_eq(day.size, 5);
 
   for (let field of ["cleanActiveTicks", "cleanTotalTime", "main", "firstPaint", "sessionRestored"]) {
@@ -158,26 +158,30 @@ add_task(function test_collect() {
 
   yield provider.shutdown();
   yield storage.close();
 });
 
 add_task(function test_serialization() {
   let [provider, storage, recorder] = yield getProvider("serialization");
 
-  let current = provider.getMeasurement("current", 2);
+  yield sleep(1025);
+  recorder.onActivity(true);
+
+  let current = provider.getMeasurement("current", 3);
   let data = yield current.getValues();
   do_check_true("singular" in data);
 
   let serializer = current.serializer(current.SERIALIZE_JSON);
   let fields = serializer.singular(data.singular);
 
-  do_check_eq(fields._v, 2);
-  do_check_eq(fields.activeTicks, 0);
+  do_check_eq(fields._v, 3);
+  do_check_eq(fields.activeTicks, 1);
   do_check_eq(fields.startDay, Metrics.dateToDays(recorder.startDate));
   do_check_eq(fields.main, 500);
   do_check_eq(fields.firstPaint, 1000);
   do_check_eq(fields.sessionRestored, 1500);
   do_check_true(fields.totalTime > 0);
 
   yield provider.shutdown();
   yield storage.close();
 });
+