Bug 1147522 - Move SessionRecorder usage from DRS to TelemetryPing. r=gfritzsche

This commit is contained in:
Alessio Placitelli 2015-04-03 05:42:00 -04:00
parent 00eef8d8b4
commit be05a94f76
6 changed files with 46 additions and 97 deletions

View File

@ -17,7 +17,6 @@ Cu.import("resource://gre/modules/osfile.jsm");
const ROOT_BRANCH = "datareporting.";
const POLICY_BRANCH = ROOT_BRANCH + "policy.";
const SESSIONS_BRANCH = ROOT_BRANCH + "sessions.";
const HEALTHREPORT_BRANCH = ROOT_BRANCH + "healthreport.";
const HEALTHREPORT_LOGGING_BRANCH = HEALTHREPORT_BRANCH + "logging.";
const DEFAULT_LOAD_DELAY_MSEC = 10 * 1000;
@ -65,10 +64,6 @@ this.DataReportingService = function () {
this._os = Cc["@mozilla.org/observer-service;1"]
.getService(Ci.nsIObserverService);
// Used for testing only, when true results in getSessionRecorder() returning
// undefined. Controlled via simulate* methods.
this._simulateNoSessionRecorder = false;
}
DataReportingService.prototype = Object.freeze({
@ -120,16 +115,6 @@ DataReportingService.prototype = Object.freeze({
try {
this._prefs = new Preferences(HEALTHREPORT_BRANCH);
// We don't initialize the sessions recorder unless Health Report is
// around to provide pruning of data.
//
// FUTURE consider having the SessionsRecorder always enabled and/or
// living in its own XPCOM service.
if (this._prefs.get("service.enabled", true)) {
this.sessionRecorder = new SessionRecorder(SESSIONS_BRANCH);
this.sessionRecorder.onStartup();
}
// We can't interact with prefs until after the profile is present.
let policyPrefs = new Preferences(POLICY_BRANCH);
this.policy = new DataReportingPolicy(policyPrefs, this._prefs, this);
@ -282,9 +267,7 @@ DataReportingService.prototype = Object.freeze({
}
}
this._healthReporter = new ns.HealthReporter(HEALTHREPORT_BRANCH,
this.policy,
this.sessionRecorder);
this._healthReporter = new ns.HealthReporter(HEALTHREPORT_BRANCH, this.policy);
// Wait for initialization to finish so if a shutdown occurs before init
// has finished we don't adversely affect app startup on next run.
@ -312,25 +295,6 @@ DataReportingService.prototype = Object.freeze({
resetClientID: Task.async(function* () {
return ClientID.resetClientID();
}),
/**
* Returns the SessionRecorder instance associated with the data reporting service.
* Returns an actual object only if FHR is enabled and after initialization,
* else returns undefined.
*/
getSessionRecorder: function() {
return this._simulateNoSessionRecorder ? undefined : this.sessionRecorder;
},
// These two simulate* methods below are only used for testings and control
// whether getSessionRecorder() behaves normally or forced to return undefined
simulateNoSessionRecorder() {
this._simulateNoSessionRecorder = true;
},
simulateRestoreSessionRecorder() {
this._simulateNoSessionRecorder = false;
},
});
this.NSGetFactory = XPCOMUtils.generateNSGetFactory([DataReportingService]);
@ -341,6 +305,4 @@ this.NSGetFactory = XPCOMUtils.generateNSGetFactory([DataReportingService]);
;
#include policy.jsm
;
#include ../../toolkit/modules/SessionRecorder.jsm
;

View File

@ -28,6 +28,8 @@ Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://gre/modules/TelemetryStopwatch.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TelemetryPing",
"resource://gre/modules/TelemetryPing.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel",
"resource://gre/modules/UpdateChannel.jsm");
@ -1187,11 +1189,11 @@ AbstractHealthReporter.prototype = Object.freeze({
* @param policy
* (HealthReportPolicy) Policy driving execution of HealthReporter.
*/
this.HealthReporter = function (branch, policy, sessionRecorder, stateLeaf=null) {
this.HealthReporter = function (branch, policy, stateLeaf=null) {
this._stateLeaf = stateLeaf;
this._uploadInProgress = false;
AbstractHealthReporter.call(this, branch, policy, sessionRecorder);
AbstractHealthReporter.call(this, branch, policy, TelemetryPing.getSessionRecorder());
if (!this.serverURI) {
throw new Error("No server URI defined. Did you forget to define the pref?");

View File

@ -130,8 +130,8 @@ this.createFakeCrash = function (submitted=false, date=new Date()) {
*
* The purpose of this type is to aid testing of startup and shutdown.
*/
this.InspectedHealthReporter = function (branch, policy, recorder, stateLeaf) {
HealthReporter.call(this, branch, policy, recorder, stateLeaf);
this.InspectedHealthReporter = function (branch, policy, stateLeaf) {
HealthReporter.call(this, branch, policy, stateLeaf);
this.onStorageCreated = null;
this.onProviderManagerInitialized = null;
@ -212,7 +212,7 @@ this.getHealthReporter = function (name, uri=DUMMY_URI, inspected=false) {
}
let policy = new DataReportingPolicy(policyPrefs, prefs, listener);
let type = inspected ? InspectedHealthReporter : HealthReporter;
reporter = new type(branch + "healthreport.", policy, null,
reporter = new type(branch + "healthreport.", policy,
"state-" + name + ".json");
return reporter;

View File

@ -31,8 +31,10 @@ const PREF_SERVER = PREF_BRANCH + "server";
const PREF_ENABLED = PREF_BRANCH + "enabled";
const PREF_LOG_LEVEL = PREF_BRANCH_LOG + "level";
const PREF_LOG_DUMP = PREF_BRANCH_LOG + "dump";
const PREF_CACHED_CLIENTID = PREF_BRANCH + "cachedClientID"
const PREF_CACHED_CLIENTID = PREF_BRANCH + "cachedClientID";
const PREF_FHR_ENABLED = "datareporting.healthreport.service.enabled";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_SESSIONS_BRANCH = "datareporting.sessions.";
const PING_FORMAT_VERSION = 4;
@ -60,6 +62,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "ThirdPartyCookieProbe",
"resource://gre/modules/ThirdPartyCookieProbe.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "TelemetryEnvironment",
"resource://gre/modules/TelemetryEnvironment.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "SessionRecorder",
"resource://gre/modules/SessionRecorder.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel",
"resource://gre/modules/UpdateChannel.jsm");
@ -260,6 +264,14 @@ this.TelemetryPing = Object.freeze({
get shutdown() {
return Impl._shutdownBarrier.client;
},
/**
* The session recorder instance managed by Telemetry.
* @return {Object} The active SessionRecorder instance or null if not available.
*/
getSessionRecorder: function() {
return Impl._sessionRecorder;
},
});
let Impl = {
@ -275,7 +287,8 @@ let Impl = {
_delayedInitTask: null,
// The deferred promise resolved when the initialization task completes.
_delayedInitTaskDeferred: null,
// The session recorder, shared with FHR and the Data Reporting Service.
_sessionRecorder: null,
// This is a public barrier Telemetry clients can use to add blockers to the shutdown
// of TelemetryPing.
// After this barrier, clients can not submit Telemetry pings anymore.
@ -749,6 +762,14 @@ let Impl = {
return Promise.resolve();
}
// Only initialize the session recorder if FHR is enabled.
// TODO: move this after the |enableTelemetryRecording| block and drop the
// PREF_FHR_ENABLED check after bug 1137252 lands.
if (!this._sessionRecorder && Preferences.get(PREF_FHR_ENABLED, true)) {
this._sessionRecorder = new SessionRecorder(PREF_SESSIONS_BRANCH);
this._sessionRecorder.onStartup();
}
// Initialize some probes that are kept in their own modules
this._thirdPartyCookies = new ThirdPartyCookieProbe();
this._thirdPartyCookies.init();

View File

@ -972,24 +972,18 @@ let Impl = {
}
ret.activeTicks = -1;
if ("@mozilla.org/datareporting/service;1" in Cc) {
let drs = Cc["@mozilla.org/datareporting/service;1"]
.getService(Ci.nsISupports)
.wrappedJSObject;
let sr = drs.getSessionRecorder();
if (sr) {
let activeTicks = sr.activeTicks;
if (isSubsession) {
activeTicks = sr.activeTicks - this._subsessionStartActiveTicks;
}
if (clearSubsession) {
this._subsessionStartActiveTicks = activeTicks;
}
ret.activeTicks = activeTicks;
let sr = TelemetryPing.getSessionRecorder();
if (sr) {
let activeTicks = sr.activeTicks;
if (isSubsession) {
activeTicks = sr.activeTicks - this._subsessionStartActiveTicks;
}
if (clearSubsession) {
this._subsessionStartActiveTicks = activeTicks;
}
ret.activeTicks = activeTicks;
}
ret.pingsOverdue = TelemetryFile.pingsOverdue;

View File

@ -71,9 +71,6 @@ const PREF_SERVER = PREF_BRANCH + "server";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_FHR_SERVICE_ENABLED = "datareporting.healthreport.service.enabled";
const SESSION_RECORDER_EXPECTED = HAS_DATAREPORTINGSERVICE &&
Preferences.get(PREF_FHR_SERVICE_ENABLED, true);
const DATAREPORTING_DIR = "datareporting";
const ABORTED_PING_FILE_NAME = "aborted-session-ping";
const ABORTED_SESSION_UPDATE_INTERVAL_MS = 5 * 60 * 1000;
@ -89,11 +86,6 @@ let gServerStarted = false;
let gRequestIterator = null;
let gClientID = null;
XPCOMUtils.defineLazyGetter(this, "gDatareportingService",
() => Cc["@mozilla.org/datareporting/service;1"]
.getService(Ci.nsISupports)
.wrappedJSObject);
function generateUUID() {
let str = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator).generateUUID().toString();
// strip {}
@ -321,7 +313,7 @@ function checkPayload(payload, reason, successfulPings) {
Assert.ok(payload.simpleMeasurements.maximalNumberOfConcurrentThreads >= gNumberOfThreadsLaunched);
let activeTicks = payload.simpleMeasurements.activeTicks;
Assert.ok(SESSION_RECORDER_EXPECTED ? activeTicks >= 0 : activeTicks == -1);
Assert.ok(activeTicks >= 0);
Assert.equal(payload.simpleMeasurements.failedProfileLockCount,
FAILED_PROFILE_LOCK_ATTEMPTS);
@ -491,13 +483,6 @@ function run_test() {
Services.prefs.setBoolPref(PREF_ENABLED, true);
Services.prefs.setBoolPref(PREF_FHR_UPLOAD_ENABLED, true);
// Send the needed startup notifications to the datareporting service
// to ensure that it has been initialized.
if (HAS_DATAREPORTINGSERVICE) {
gDatareportingService.observe(null, "app-startup", null);
gDatareportingService.observe(null, "profile-after-change", null);
}
// Make it look like we've previously failed to lock a profile a couple times.
write_fake_failedprofilelocks_file();
@ -532,20 +517,6 @@ function run_test() {
add_task(function* asyncSetup() {
yield TelemetrySession.setup();
yield TelemetryPing.setup();
if (HAS_DATAREPORTINGSERVICE) {
// force getSessionRecorder()==undefined to check the payload's activeTicks
gDatareportingService.simulateNoSessionRecorder();
}
// When no DRS or no DRS.getSessionRecorder(), activeTicks should be -1.
do_check_eq(TelemetrySession.getPayload().simpleMeasurements.activeTicks, -1);
if (HAS_DATAREPORTINGSERVICE) {
// Restore normal behavior for getSessionRecorder()
gDatareportingService.simulateRestoreSessionRecorder();
}
// Load the client ID from the client ID provider to check for pings sanity.
gClientID = yield ClientID.getClientID();
});
@ -851,14 +822,13 @@ add_task(function* test_checkSubsessionHistograms() {
});
add_task(function* test_checkSubsessionData() {
if (gIsAndroid || !SESSION_RECORDER_EXPECTED) {
// We don't support subsessions yet on Android. Also bail out if we
// can't use the session recorder.
if (gIsAndroid) {
// We don't support subsessions yet on Android.
return;
}
// Keep track of the active ticks count if the session recorder is available.
let sessionRecorder = gDatareportingService.getSessionRecorder();
let sessionRecorder = TelemetryPing.getSessionRecorder();
let activeTicksAtSubsessionStart = sessionRecorder.activeTicks;
let expectedActiveTicks = activeTicksAtSubsessionStart;