Bug 1140558 - Part 3 - Pass the old environment data to event listeners on environment changes. r=vladan,a=lmandel

This commit is contained in:
Georg Fritzsche 2015-04-02 21:33:46 +02:00
Родитель deec1f89f5
Коммит 1607712baf
6 изменённых файлов: 71 добавлений и 59 удалений

Просмотреть файл

@ -18,6 +18,7 @@ Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/PromiseUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/ObjectUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "ctypes",
"resource://gre/modules/ctypes.jsm");
@ -366,58 +367,46 @@ EnvironmentAddonBuilder.prototype = {
// AddonListener
onEnabled: function() {
this._onChange();
this._onAddonChange();
},
onDisabled: function() {
this._onChange();
this._onAddonChange();
},
onInstalled: function() {
this._onChange();
this._onAddonChange();
},
onUninstalling: function() {
this._onChange();
this._onAddonChange();
},
_onChange: function() {
if (this._pendingTask) {
this._environment._log.trace("_onChange - task already pending");
return;
}
this._environment._log.trace("_onChange");
this._pendingTask = this._updateAddons().then(
(changed) => {
this._pendingTask = null;
if (changed) {
this._environment._onEnvironmentChange("addons-changed");
}
},
(err) => {
this._pendingTask = null;
this._environment._log.error("Error collecting addons", err);
});
_onAddonChange: function() {
this._environment._log.trace("_onAddonChange");
this._checkForChanges("addons-changed");
},
// nsIObserver
observe: function (aSubject, aTopic, aData) {
this._environment._log.trace("observe - Topic " + aTopic);
this._checkForChanges("experiment-changed");
},
if (aTopic == EXPERIMENTS_CHANGED_TOPIC) {
if (this._pendingTask) {
return;
}
this._pendingTask = this._updateAddons().then(
(changed) => {
this._pendingTask = null;
if (changed) {
this._environment._onEnvironmentChange("experiment-changed");
}
},
(err) => {
this._pendingTask = null;
this._environment._log.error("observe: Error collecting addons", err);
});
_checkForChanges: function(changeReason) {
if (this._pendingTask) {
this._environment._log.trace("_checkForChanges - task already pending, dropping change with reason " + changeReason);
return;
}
this._pendingTask = this._updateAddons().then(
(result) => {
this._pendingTask = null;
if (result.changed) {
this._environment._onEnvironmentChange(changeReason, result.oldEnvironment);
}
},
(err) => {
this._pendingTask = null;
this._environment._log.error("_checkForChanges: Error collecting addons", err);
});
},
_shutdownBlocker: function() {
@ -434,7 +423,9 @@ EnvironmentAddonBuilder.prototype = {
* This should only be called from _pendingTask; otherwise we risk
* running this during addon manager shutdown.
*
* @returns Promise<bool> whether the environment changed.
* @returns Promise<Object> This returns a Promise resolved with a status object with the following members:
* changed - Whether the environment changed.
* oldEnvironment - Only set if a change occured, contains the environment data before the change.
*/
_updateAddons: Task.async(function* () {
this._environment._log.trace("_updateAddons");
@ -455,14 +446,17 @@ EnvironmentAddonBuilder.prototype = {
persona: personaId,
};
if (JSON.stringify(addons) !=
JSON.stringify(this._environment._currentEnvironment.addons)) {
let result = {
changed: !ObjectUtils.deepEqual(addons, this._environment._currentEnvironment.addons),
};
if (result.changed) {
this._environment._log.trace("_updateAddons: addons differ");
result.oldEnvironment = Cu.cloneInto(this._environment._currentEnvironment, myScope);
this._environment._currentEnvironment.addons = addons;
return true;
}
this._environment._log.trace("_updateAddons: no changes found");
return false;
return result;
}),
/**
@ -695,7 +689,8 @@ EnvironmentCache.prototype = {
* Register a listener for environment changes.
* @param name The name of the listener. If a new listener is registered
* with the same name, the old listener will be replaced.
* @param listener function(reason, environment)
* @param listener function(reason, oldEnvironment) - Will receive a reason for
the change and the environment data before the change.
*/
registerChangeListener: function (name, listener) {
this._log.trace("registerChangeListener for " + name);
@ -772,8 +767,9 @@ EnvironmentCache.prototype = {
_onPrefChanged: function() {
this._log.trace("_onPrefChanged");
let oldEnvironment = Cu.cloneInto(this._currentEnvironment, myScope);
this._updateSettings();
this._onEnvironmentChange("pref-changed");
this._onEnvironmentChange("pref-changed", oldEnvironment);
},
/**
@ -1059,17 +1055,20 @@ EnvironmentCache.prototype = {
};
},
_onEnvironmentChange: function (what) {
_onEnvironmentChange: function (what, oldEnvironment) {
this._log.trace("_onEnvironmentChange for " + what);
if (this._shutdown) {
this._log.trace("_onEnvironmentChange - Already shut down.");
return;
}
// We are already skipping change events in _checkChanges if there is a pending change task running.
// Further throttling is coming in bug 1143714.
for (let [name, listener] of this._changeListeners) {
try {
this._log.debug("_onEnvironmentChange - calling " + name);
listener(what, this.currentEnvironment);
listener(what, oldEnvironment);
} catch (e) {
this._log.error("_onEnvironmentChange - listener " + name + " caught error", e);
}

Просмотреть файл

@ -176,6 +176,7 @@ this.TelemetryPing = Object.freeze({
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
* @returns {Promise} A promise that resolves when the ping is sent.
*/
send: function(aType, aPayload, aOptions = {}) {
@ -199,6 +200,7 @@ this.TelemetryPing = Object.freeze({
* id, false otherwise.
* @param {Boolean} [aOptions.addEnvironment=false] true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
* @returns {Promise} A promise that resolves when the pings are saved.
*/
savePendingPings: function(aType, aPayload, aOptions = {}) {
@ -224,6 +226,7 @@ this.TelemetryPing = Object.freeze({
* environment data.
* @param {Boolean} [aOptions.overwrite=false] true overwrites a ping with the same name,
* if found.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
* @param {String} [aOptions.filePath] The path to save the ping to. Will save to default
* ping location if not provided.
*
@ -323,6 +326,7 @@ let Impl = {
* id, false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
*
* @returns Promise<Object> A promise that resolves when the ping is completely assembled.
*/
@ -350,7 +354,7 @@ let Impl = {
}
if (aOptions.addEnvironment) {
pingData.environment = TelemetryEnvironment.currentEnvironment;
pingData.environment = aOptions.overrideEnvironment || TelemetryEnvironment.currentEnvironment;
}
return pingData;
@ -414,6 +418,7 @@ let Impl = {
* false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Object} aOptions.overrideEnvironment set to override the environment data.
*
* @returns {Promise} A promise that resolves when the ping is sent.
*/
@ -465,6 +470,7 @@ let Impl = {
* false otherwise.
* @param {Boolean} aOptions.addEnvironment true if the ping should contain the
* environment data.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
*
* @returns {Promise} A promise that resolves when all the pings are saved to disk.
*/
@ -491,6 +497,7 @@ let Impl = {
* @param {Boolean} aOptions.overwrite true overwrites a ping with the same name, if found.
* @param {String} [aOptions.filePath] The path to save the ping to. Will save to default
* ping location if not provided.
* @param {Object} [aOptions.overrideEnvironment=null] set to override the environment data.
*
* @returns {Promise} A promise that resolves with the ping id when the ping is saved to
* disk.

Просмотреть файл

@ -1983,7 +1983,7 @@ let Impl = {
}
}),
_onEnvironmentChange: function(reason, data) {
_onEnvironmentChange: function(reason, oldEnvironment) {
this._log.trace("_onEnvironmentChange", reason);
let payload = this.getSessionPayload(REASON_ENVIRONMENT_CHANGE, true);
@ -1994,6 +1994,7 @@ let Impl = {
retentionDays: RETENTION_DAYS,
addClientId: true,
addEnvironment: true,
overrideEnvironment: oldEnvironment,
};
TelemetryPing.send(getPingType(payload), payload, options);
},

Просмотреть файл

@ -9,6 +9,13 @@ The environment data may also be submitted by other ping types.
*Note:* This is not submitted with all ping types due to privacy concerns. This and other data is inspected under the `data collection policy <https://wiki.mozilla.org/Firefox/Data_Collection>`_.
Some parts of the environment must be fetched asynchronously at startup. We don't want other Telemetry components to block on waiting for the environment, so some items may be missing from it until the async fetching finished.
This currently affects the following sections:
- profile
- addons
Structure::
{
@ -182,8 +189,3 @@ Structure::
persona: <string>, // id of the current persona, null on GONK
},
}
Some parts of the environment must be fetched asynchronously at startup. If a session is very short or terminates early, the following items may be missing from the environment:
- profile
- addons

Просмотреть файл

@ -642,17 +642,20 @@ add_task(function* test_prefWatchPolicies() {
Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST_1], undefined);
Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST_4], expectedValue);
TelemetryEnvironment.registerChangeListener("testWatchPrefs", deferred.resolve);
TelemetryEnvironment.registerChangeListener("testWatchPrefs",
(reason, data) => deferred.resolve(data));
let oldEnvironmentData = TelemetryEnvironment.currentEnvironment;
// Trigger a change in the watched preferences.
Preferences.set(PREF_TEST_1, expectedValue);
Preferences.set(PREF_TEST_2, false);
yield deferred.promise;
let eventEnvironmentData = yield deferred.promise;
// Unregister the listener.
TelemetryEnvironment.unregisterChangeListener("testWatchPrefs");
// Check environment contains the correct data.
Assert.deepEqual(oldEnvironmentData, eventEnvironmentData);
let userPrefs = TelemetryEnvironment.currentEnvironment.settings.userPrefs;
Assert.equal(userPrefs[PREF_TEST_1], expectedValue,

Просмотреть файл

@ -1150,7 +1150,7 @@ add_task(function* test_environmentChange() {
let ping = decodeRequestPayload(request);
Assert.equal(ping.type, PING_TYPE_MAIN);
Assert.equal(ping.environment.settings.userPrefs[PREF_TEST], 1);
Assert.equal(ping.environment.settings.userPrefs[PREF_TEST], undefined);
Assert.equal(ping.payload.info.reason, REASON_ENVIRONMENT_CHANGE);
let subsessionStartDate = new Date(ping.payload.info.subsessionStartDate);
Assert.equal(subsessionStartDate.toISOString(), nowDay.toISOString());
@ -1165,7 +1165,7 @@ add_task(function* test_environmentChange() {
ping = decodeRequestPayload(request);
Assert.equal(ping.type, PING_TYPE_MAIN);
Assert.equal(ping.environment.settings.userPrefs[PREF_TEST], 2);
Assert.equal(ping.environment.settings.userPrefs[PREF_TEST], 1);
Assert.equal(ping.payload.info.reason, REASON_ENVIRONMENT_CHANGE);
subsessionStartDate = new Date(ping.payload.info.subsessionStartDate);
Assert.equal(subsessionStartDate.toISOString(), nowDay.toISOString());
@ -1234,7 +1234,7 @@ add_task(function* test_savedSessionData() {
// We expect one new subsession when starting TelemetrySession and one after triggering
// an environment change.
const expectedSubsessions = sessionState.profileSubsessionCounter + 3;
const expectedSubsessions = sessionState.profileSubsessionCounter + 2;
const expectedUUID = "009fd1ad-b85e-4817-b3e5-000000003785";
fakeGenerateUUID(generateUUID, () => expectedUUID);