Bug 1341349 - Enable sending the crash ping from the crashreporter client only when the overall configuration would allow pings to be sent r=Dexter,gfritzsche

The crashreporter client will send a crash ping autonomously only when it
finds a valid TelemetryServerUrl annotation. This patch makes this annotation
conditional on all the preferences that regulate sending pings (including
official telemetry flags) and prevents it from sending pings before the user
has been notified of the privacy policy.

This is achieved by adding a new _annotateCrashReport() method to the
TelemetrySend obejct which can be called before we've initialized the rest of
the components. It is invoked manually early in the startup process so that
startup crashes are properly annotated, then it's invoked again when the user
is informed of the privacy policy as well as when one of the relevant
preferences is altered. This ensures that the annotations are stripped if the
user disables uploading pings without having to restart Firefox.

MozReview-Commit-ID: 2DKnoWGT1Bp

--HG--
extra : source : b7de7faa9f01ab59004dd13b1a10854edbc70633
This commit is contained in:
Gabriele Svelto 2017-03-08 15:00:26 +01:00
parent 5669ef5c81
commit 133dd0903f
4 changed files with 155 additions and 34 deletions

View File

@ -719,6 +719,9 @@ var Impl = {
// a few observers and initializing the session.
TelemetrySession.earlyInit(this._testMode);
// Annotate crash reports so that we get pings for startup crashes
TelemetrySend.earlyInit();
// For very short session durations, we may never load the client
// id from disk.
// We try to cache it in prefs to avoid this, even though this may
@ -735,11 +738,11 @@ var Impl = {
this._initialized = true;
TelemetryEnvironment.delayedInit();
yield TelemetrySend.setup(this._testMode);
// Load the ClientID.
this._clientID = yield ClientID.getClientID();
yield TelemetrySend.setup(this._testMode);
// Perform TelemetrySession delayed init.
yield TelemetrySession.delayedInit();
// Purge the pings archive by removing outdated pings. We don't wait for

View File

@ -19,6 +19,7 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
Cu.import("resource://gre/modules/Task.jsm", this);
Cu.import("resource://gre/modules/ClientID.jsm");
Cu.import("resource://gre/modules/Log.jsm", this);
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/PromiseUtils.jsm");
@ -45,6 +46,7 @@ const LOGGER_PREFIX = "TelemetrySend::";
const PREF_BRANCH = "toolkit.telemetry.";
const PREF_SERVER = PREF_BRANCH + "server";
const PREF_UNIFIED = PREF_BRANCH + "unified";
const PREF_ENABLED = PREF_BRANCH + "enabled";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_OVERRIDE_OFFICIAL_CHECK = PREF_BRANCH + "send.overrideOfficialCheck";
@ -176,6 +178,14 @@ this.TelemetrySend = {
return TelemetrySendImpl.pendingPingCount;
},
/**
* Partial setup that runs immediately at startup. This currently triggers
* the crash report annotations.
*/
earlyInit() {
TelemetrySendImpl.earlyInit();
},
/**
* Initializes this module.
*
@ -549,14 +559,20 @@ var TelemetrySendImpl = {
// Count of pending pings that were overdue.
_overduePingCount: 0,
// Whether sending pings has been overridden. We have it here instead
// of the top of the file to ease testing.
_overrideOfficialCheck: false,
OBSERVER_TOPICS: [
TOPIC_IDLE_DAILY,
],
OBSERVED_PREFERENCES: [
PREF_ENABLED,
PREF_FHR_UPLOAD_ENABLED,
],
// Whether sending pings has been overridden.
get _overrideOfficialCheck() {
return Preferences.get(PREF_OVERRIDE_OFFICIAL_CHECK, false);
},
get _log() {
if (!this._logger) {
this._logger = Log.repository.getLoggerWithMessagePrefix(LOGGER_NAME, LOGGER_PREFIX);
@ -581,17 +597,27 @@ var TelemetrySendImpl = {
this._testMode = testing;
},
earlyInit() {
this._annotateCrashReport();
},
async setup(testing) {
this._log.trace("setup");
this._testMode = testing;
this._sendingEnabled = true;
this._overrideOfficialCheck = Preferences.get(PREF_OVERRIDE_OFFICIAL_CHECK, false);
Services.obs.addObserver(this, TOPIC_IDLE_DAILY, false);
this._server = Preferences.get(PREF_SERVER, undefined);
// Annotate crash reports so that crash pings are sent correctly and listen
// to pref changes to adjust the annotations accordingly.
for (let pref of this.OBSERVED_PREFERENCES) {
Preferences.observe(pref, this._annotateCrashReport, this);
}
this._annotateCrashReport();
// Check the pending pings on disk now.
try {
await this._checkPendingPings();
@ -607,6 +633,35 @@ var TelemetrySendImpl = {
SendScheduler.triggerSendingPings(true);
},
/**
* Triggers the crash report annotations depending on the current
* configuration. This communicates to the crash reporter if it can send a
* crash ping or not. This method can be called safely before setup() has
* been called.
*/
_annotateCrashReport() {
try {
const cr = Cc["@mozilla.org/toolkit/crash-reporter;1"];
if (cr) {
const crs = cr.getService(Ci.nsICrashReporter);
let clientId = ClientID.getCachedClientID();
let server = this._server || Preferences.get(PREF_SERVER, undefined);
if (!this.sendingEnabled() || !TelemetryReportingPolicy.canUpload()) {
// If we cannot send pings then clear the crash annotations
crs.annotateCrashReport("TelemetryClientId", "");
crs.annotateCrashReport("TelemetryServerURL", "");
} else {
crs.annotateCrashReport("TelemetryClientId", clientId);
crs.annotateCrashReport("TelemetryServerURL", server);
}
}
} catch (e) {
// Ignore errors when crash reporting is disabled
}
},
/**
* Discard old pings from the pending pings and detect overdue ones.
* @return {Boolean} True if we have overdue pings, false otherwise.
@ -638,6 +693,13 @@ var TelemetrySendImpl = {
async shutdown() {
this._shutdown = true;
for (let pref of this.OBSERVED_PREFERENCES) {
// FIXME: When running tests this causes errors to be printed out if
// TelemetrySend.shutdown() is called twice in a row without calling
// TelemetrySend.setup() in-between.
Preferences.ignore(pref, this._annotateCrashReport, this);
}
for (let topic of this.OBSERVER_TOPICS) {
try {
Services.obs.removeObserver(this, topic);
@ -668,7 +730,6 @@ var TelemetrySendImpl = {
this._shutdown = false;
this._currentPings = new Map();
this._overduePingCount = 0;
this._overrideOfficialCheck = Preferences.get(PREF_OVERRIDE_OFFICIAL_CHECK, false);
const histograms = [
"TELEMETRY_SUCCESS",
@ -685,8 +746,11 @@ var TelemetrySendImpl = {
* Notify that we can start submitting data to the servers.
*/
notifyCanUpload() {
// Let the scheduler trigger sending pings if possible.
// Let the scheduler trigger sending pings if possible, also inform the
// crash reporter that it can send crash pings if appropriate.
SendScheduler.triggerSendingPings(true);
this._annotateCrashReport();
return this.promisePendingPingActivity();
},

View File

@ -21,7 +21,6 @@ Cu.import("resource://gre/modules/Timer.jsm");
Cu.import("resource://gre/modules/TelemetrySend.jsm", this);
Cu.import("resource://gre/modules/TelemetryUtils.jsm", this);
Cu.import("resource://gre/modules/AppConstants.jsm");
Cu.import("resource://gre/modules/ClientID.jsm");
const Utils = TelemetryUtils;
@ -66,7 +65,6 @@ const PREF_PREVIOUS_BUILDID = PREF_BRANCH + "previousBuildID";
const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
const PREF_ASYNC_PLUGIN_INIT = "dom.ipc.plugins.asyncInit.enabled";
const PREF_UNIFIED = PREF_BRANCH + "unified";
const PREF_SERVER = PREF_BRANCH + "server";
const MESSAGE_TELEMETRY_PAYLOAD = "Telemetry:Payload";
const MESSAGE_TELEMETRY_THREAD_HANGS = "Telemetry:ChildThreadHangs";
@ -199,25 +197,12 @@ function getPingType(aPayload) {
/**
* Annotate the current session ID with the crash reporter to map potential
* crash pings with the related main ping.
*
* @param sessionId {String} The telemetry session ID
* @param clientId {String} The telemetry client ID
* @param telemetryServer {String} The URL of the telemetry server
*/
function annotateCrashReport(sessionId, clientId, telemetryServer) {
function annotateCrashReport(sessionId) {
try {
const cr = Cc["@mozilla.org/toolkit/crash-reporter;1"];
if (cr) {
const crs = cr.getService(Ci.nsICrashReporter);
crs.setTelemetrySessionId(sessionId);
// We do not annotate the crash if telemetry is disabled to prevent the
// crashreporter client from sending the crash ping.
if (Utils.isTelemetryEnabled) {
crs.annotateCrashReport("TelemetryClientId", clientId);
crs.annotateCrashReport("TelemetryServerURL", telemetryServer);
}
cr.getService(Ci.nsICrashReporter).setTelemetrySessionId(sessionId);
}
} catch (e) {
// Ignore errors when crash reporting is disabled
@ -1488,10 +1473,7 @@ var Impl = {
// the very same value for |_sessionStartDate|.
this._sessionStartDate = this._subsessionStartDate;
// Annotate crash reports using the cached client ID which can be accessed
// synchronously. If it is not available yet, we'll update it later.
annotateCrashReport(this._sessionId, ClientID.getCachedClientID(),
Preferences.get(PREF_SERVER, undefined));
annotateCrashReport(this._sessionId);
// Initialize some probes that are kept in their own modules
this._thirdPartyCookies = new ThirdPartyCookieProbe();
@ -1546,10 +1528,6 @@ var Impl = {
Telemetry.asyncFetchTelemetryData(function() {});
// Update the crash annotation with the proper client ID.
annotateCrashReport(this._sessionId, await ClientID.getClientID(),
Preferences.get(PREF_SERVER, undefined));
if (IS_UNIFIED_TELEMETRY) {
// Check for a previously written aborted session ping.
await TelemetryController.checkAbortedSessionPing();

View File

@ -32,10 +32,12 @@ function run_test() {
// check setting some basic data
do_crash(function() {
// Add various annotations
crashReporter.annotateCrashReport("TestKey", "TestValue");
crashReporter.annotateCrashReport("\u2665", "\u{1F4A9}");
crashReporter.appendAppNotesToCrashReport("Junk");
crashReporter.appendAppNotesToCrashReport("MoreJunk");
// TelemetrySession setup will trigger the session annotation
let scope = {};
Components.utils.import("resource://gre/modules/TelemetryController.jsm", scope);
@ -50,5 +52,79 @@ function run_test() {
"The TelemetrySessionId field is present in the extra file");
Assert.ok(UUID_REGEX.test(extra.TelemetrySessionId),
"The TelemetrySessionId is a UUID");
Assert.ok(!("TelemetryClientId" in extra),
"The TelemetryClientId field is omitted by default");
Assert.ok(!("TelemetryServerURL" in extra),
"The TelemetryServerURL field is omitted by default");
});
do_crash(function() {
// Enable the FHR, official policy bypass (since we're in a test) and
// specify a telemetry server & client ID.
let prefs = Components.classes["@mozilla.org/preferences-service;1"]
.getService(Components.interfaces.nsIPrefBranch);
prefs.setBoolPref("datareporting.policy.dataSubmissionPolicyBypassNotification", true);
prefs.setBoolPref("datareporting.healthreport.uploadEnabled", true);
prefs.setCharPref("toolkit.telemetry.server", "http://a.telemetry.server");
prefs.setCharPref("toolkit.telemetry.cachedClientID",
"f3582dee-22b9-4d73-96d1-79ef5bf2fc24");
// TelemetrySession setup will trigger the session annotation
let scope = {};
Components.utils.import("resource://gre/modules/TelemetryController.jsm", scope);
Components.utils.import("resource://gre/modules/TelemetrySend.jsm", scope);
scope.TelemetrySend.setTestModeEnabled(true);
scope.TelemetryController.testSetup();
}, function(mdump, extra) {
Assert.ok("TelemetryClientId" in extra,
"The TelemetryClientId field is present when the FHR is on");
Assert.equal(extra.TelemetryClientId,
"f3582dee-22b9-4d73-96d1-79ef5bf2fc24",
"The TelemetryClientId matches the expected value");
Assert.ok("TelemetryServerURL" in extra,
"The TelemetryServerURL field is present when the FHR is on");
Assert.equal(extra.TelemetryServerURL, "http://a.telemetry.server",
"The TelemetryServerURL matches the expected value");
});
do_crash(function() {
// Disable the FHR upload, no telemetry annotations should be present.
let prefs = Components.classes["@mozilla.org/preferences-service;1"]
.getService(Components.interfaces.nsIPrefBranch);
prefs.setBoolPref("datareporting.policy.dataSubmissionPolicyBypassNotification", true);
prefs.setBoolPref("datareporting.healthreport.uploadEnabled", false);
// TelemetrySession setup will trigger the session annotation
let scope = {};
Components.utils.import("resource://gre/modules/TelemetryController.jsm", scope);
Components.utils.import("resource://gre/modules/TelemetrySend.jsm", scope);
scope.TelemetrySend.setTestModeEnabled(true);
scope.TelemetryController.testSetup();
}, function(mdump, extra) {
Assert.ok(!("TelemetryClientId" in extra),
"The TelemetryClientId field is omitted when FHR upload is disabled");
Assert.ok(!("TelemetryServerURL" in extra),
"The TelemetryServerURL field is omitted when FHR upload is disabled");
});
do_crash(function() {
// No telemetry annotations should be present if the user has not been
// notified yet
let prefs = Components.classes["@mozilla.org/preferences-service;1"]
.getService(Components.interfaces.nsIPrefBranch);
prefs.setBoolPref("datareporting.policy.dataSubmissionPolicyBypassNotification", false);
prefs.setBoolPref("datareporting.healthreport.uploadEnabled", true);
// TelemetrySession setup will trigger the session annotation
let scope = {};
Components.utils.import("resource://gre/modules/TelemetryController.jsm", scope);
Components.utils.import("resource://gre/modules/TelemetrySend.jsm", scope);
scope.TelemetrySend.setTestModeEnabled(true);
scope.TelemetryController.testSetup();
}, function(mdump, extra) {
Assert.ok(!("TelemetryClientId" in extra),
"The TelemetryClientId field is omitted when FHR upload is disabled");
Assert.ok(!("TelemetryServerURL" in extra),
"The TelemetryServerURL field is omitted when FHR upload is disabled");
});
}