Bug 1380256 - Implement the update ping with reason "success". r?chutten,mhowell,rweiss
This patch enables sending the "update" ping with reason "success"
after the browser is restarted when an update is successfully applied.
MozReview-Commit-ID: 8LYxhTTrs7l
--- a/browser/components/nsBrowserContentHandler.js
+++ b/browser/components/nsBrowserContentHandler.js
@@ -11,16 +11,18 @@ XPCOMUtils.defineLazyModuleGetter(this,
XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
"resource://gre/modules/PrivateBrowsingUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
"resource:///modules/RecentWindow.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "ShellService",
"resource:///modules/ShellService.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "WindowsUIUtils",
"@mozilla.org/windows-ui-utils;1", "nsIWindowsUIUtils");
+XPCOMUtils.defineLazyModuleGetter(this, "UpdatePing",
+ "resource://gre/modules/UpdatePing.jsm");
const nsISupports = Components.interfaces.nsISupports;
const nsIBrowserDOMWindow = Components.interfaces.nsIBrowserDOMWindow;
const nsIBrowserHandler = Components.interfaces.nsIBrowserHandler;
const nsIBrowserHistory = Components.interfaces.nsIBrowserHistory;
const nsIChannel = Components.interfaces.nsIChannel;
const nsICommandLine = Components.interfaces.nsICommandLine;
@@ -486,16 +488,17 @@ nsBrowserContentHandler.prototype = {
var willRestoreSession = false;
try {
// Read the old value of homepage_override.mstone before
// needHomepageOverride updates it, so that we can later add it to the
// URL if we do end up showing an overridePage. This makes it possible
// to have the overridePage's content vary depending on the version we're
// upgrading from.
let old_mstone = Services.prefs.getCharPref("browser.startup.homepage_override.mstone", "unknown");
+ let old_buildId = Services.prefs.getCharPref("browser.startup.homepage_override.buildID", "unknown");
override = needHomepageOverride(prefb);
if (override != OVERRIDE_NONE) {
switch (override) {
case OVERRIDE_NEW_PROFILE:
// New profile.
overridePage = Services.urlFormatter.formatURLPref("startup.homepage_welcome_url");
additionalPage = Services.urlFormatter.formatURLPref("startup.homepage_welcome_url.additional");
// Turn on 'later run' pages for new profiles.
@@ -507,21 +510,30 @@ nsBrowserContentHandler.prototype = {
// into account because that requires waiting for the session file
// to be read. If a crash occurs after updating, before restarting,
// we may open the startPage in addition to restoring the session.
var ss = Components.classes["@mozilla.org/browser/sessionstartup;1"]
.getService(Components.interfaces.nsISessionStartup);
willRestoreSession = ss.isAutomaticRestoreEnabled();
overridePage = Services.urlFormatter.formatURLPref("startup.homepage_override_url");
- if (prefb.prefHasUserValue("app.update.postupdate"))
+ if (prefb.prefHasUserValue("app.update.postupdate")) {
overridePage = getPostUpdateOverridePage(overridePage);
+ // Send the update ping to signal that the update was successful.
+ UpdatePing.handleUpdateSuccess(old_mstone, old_buildId);
+ }
overridePage = overridePage.replace("%OLD_VERSION%", old_mstone);
break;
+ case OVERRIDE_NEW_BUILD_ID:
+ if (prefb.prefHasUserValue("app.update.postupdate")) {
+ // Send the update ping to signal that the update was successful.
+ UpdatePing.handleUpdateSuccess(old_mstone, old_buildId);
+ }
+ break;
}
}
} catch (ex) {}
// formatURLPref might return "about:blank" if getting the pref fails
if (overridePage == "about:blank")
overridePage = "";
--- a/toolkit/components/telemetry/UpdatePing.jsm
+++ b/toolkit/components/telemetry/UpdatePing.jsm
@@ -36,16 +36,68 @@ this.UpdatePing = {
if (!this._enabled) {
return;
}
Services.obs.addObserver(this, UPDATE_DOWNLOADED_TOPIC);
},
/**
+ * Get the information about the update we're going to apply/was just applied
+ * from the update manager.
+ *
+ * @return {nsIUpdate} The information about the update, if available, or null.
+ */
+ _getActiveUpdate() {
+ let updateManager =
+ Cc["@mozilla.org/updates/update-manager;1"].getService(Ci.nsIUpdateManager);
+ if (!updateManager || !updateManager.activeUpdate) {
+ return null;
+ }
+
+ return updateManager.activeUpdate;
+ },
+
+ /**
+ * Generate an "update" ping with reason "success" and dispatch it
+ * to the Telemetry system.
+ *
+ * @param {String} aPreviousVersion The browser version we updated from.
+ * @param {String} aPreviousBuildId The browser build id we updated from.
+ */
+ handleUpdateSuccess(aPreviousVersion, aPreviousBuildId) {
+ this._log.trace("handleUpdateSuccess");
+
+ // An update could potentially change the update channel. Moreover,
+ // updates can only be applied if the update's channel matches with the build channel.
+ // There's no way to pass this information from the caller nor the environment as,
+ // in that case, the environment would report the "new" channel. However, the
+ // update manager should still have information about the active update: given the
+ // previous assumptions, we can simply get the channel from the update and assume
+ // it matches with the state previous to the update.
+ let update = this._getActiveUpdate();
+
+ const payload = {
+ reason: "success",
+ previousChannel: update ? update.channel : null,
+ previousVersion: aPreviousVersion,
+ previousBuildId: aPreviousBuildId,
+ };
+
+ const options = {
+ addClientId: true,
+ addEnvironment: true,
+ usePingSender: false,
+ };
+
+ TelemetryController.submitExternalPing(PING_TYPE, payload, options)
+ .catch(e => this._log.error("handleUpdateSuccess - failed to submit update ping", e));
+ },
+
+ /**
* Generate an "update" ping with reason "ready" and dispatch it
* to the Telemetry system.
*
* @param {String} aUpdateState The state of the downloaded patch. See
* nsIUpdateService.idl for a list of possible values.
*/
_handleUpdateReady(aUpdateState) {
const ALLOWED_STATES = [
@@ -53,25 +105,22 @@ this.UpdatePing = {
];
if (!ALLOWED_STATES.includes(aUpdateState)) {
this._log.trace("Unexpected update state: " + aUpdateState);
return;
}
// Get the information about the update we're going to apply from the
// update manager.
- let updateManager =
- Cc["@mozilla.org/updates/update-manager;1"].getService(Ci.nsIUpdateManager);
- if (!updateManager || !updateManager.activeUpdate) {
+ let update = this._getActiveUpdate();
+ if (!update) {
this._log.trace("Cannot get the update manager or no update is currently active.");
return;
}
- let update = updateManager.activeUpdate;
-
const payload = {
reason: "ready",
targetChannel: update.channel,
targetVersion: update.appVersion,
targetBuildId: update.buildID,
};
const options = {
--- a/toolkit/components/telemetry/docs/data/update-ping.rst
+++ b/toolkit/components/telemetry/docs/data/update-ping.rst
@@ -1,50 +1,66 @@
"update" ping
==================
-This opt-out ping is sent from Firefox Desktop when a browser update is ready to be applied. There is a
-plan to send this ping after an update is successfully applied and the work will happen in `bug 1380256 <https://bugzilla.mozilla.org/show_bug.cgi?id=1380256>`_.
+This opt-out ping is sent from Firefox Desktop when a browser update is ready to be applied and after it was correctly applied.
Structure:
.. code-block:: js
{
type: "update",
... common ping data
clientId: <UUID>,
environment: { ... },
payload: {
- reason: <string>, // "ready"
- targetChannel: <string>, // "nightly"
- targetVersion: <string>, // "56.01a"
- targetBuildId: <string>, // "20080811053724"
+ reason: <string>, // "ready", "success"
+ targetChannel: <string>, // "nightly" (only present for reason = "ready")
+ targetVersion: <string>, // "56.01a" (only present for reason = "ready")
+ targetBuildId: <string>, // "20080811053724" (only present for reason = "ready")
+ previousChannel: <string>, // "nightly" or null (only present for reason = "success")
+ previousVersion: <string>, // "55.01a" (only present for reason = "success")
+ previousBuildId: <string>, // "20080810053724" (only present for reason = "success")
}
}
payload.reason
--------------
-This field only supports the value ``ready``, meaning that the ping was generated after an update was downloaded
-and marked as ready to be processed. For *non-staged* updates this happens as soon as the download
-finishes and is verified while for *staged* updates this happens before the staging step is started.
+This field supports the following values:
+
+- ``ready`` meaning that the ping was generated after an update was downloaded and marked as ready to be processed. For *non-staged* updates this happens as soon as the download finishes and is verified while for *staged* updates this happens before the staging step is started.
+- ``ready`` the ping was generated after the browser was restarted and the update correctly applied.
payload.targetChannel
-----------------------
The Firefox channel the update was fetched from (only valid for pings with reason "ready").
payload.targetVersion
-----------------------
The Firefox version the browser is updating to. Follows the same format a application.version (only valid for pings with reason "ready").
payload.targetBuildId
-----------------------
The Firefox build id the browser is updating to. Follows the same format a application.buildId (only valid for pings with reason "ready").
+payload.previousChannel
+-----------------------
+The Firefox channel the profile was on before the update was applied (only valid for pings with reason "success").
+This can be ``null``.
+
+payload.previousVersion
+-----------------------
+The Firefox version the browser is updating from. Follows the same format a application.version (only valid for pings with reason "success").
+
+payload.previousBuildId
+-----------------------
+The Firefox build id the browser is updating from. Follows the same format a application.buildId (only valid for pings with reason "success").
+
Expected behaviours
-------------------
The following is a list of conditions and expected behaviours for the ``update`` ping:
- **The ping is generated once every time an update is downloaded, after it was verified:**
- *for users who saw the privacy policy*, the ``update`` ping is sent immediately;
- *for users who did not see the privacy policy*, the ``update`` ping is saved to disk and after the policy is displayed.
--- a/toolkit/components/telemetry/tests/browser/browser.ini
+++ b/toolkit/components/telemetry/tests/browser/browser.ini
@@ -1,5 +1,6 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
[browser_TelemetryGC.js]
+[browser_UpdatePingSuccess.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/browser/browser_UpdatePingSuccess.js
@@ -0,0 +1,61 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/
+*/
+
+"use strict";
+
+Cu.import("resource://testing-common/TelemetryArchiveTesting.jsm", this);
+
+add_task(async function test_updatePing() {
+ const TEST_VERSION = "37.85";
+ const TEST_BUILDID = "20150711123724";
+
+ // Set the preferences needed for the test: they will be cleared up
+ // after it runs.
+ await SpecialPowers.pushPrefEnv({"set": [
+ ["app.update.postupdate", true],
+ ["browser.startup.homepage_override.mstone", TEST_VERSION],
+ ["browser.startup.homepage_override.buildID", TEST_BUILDID],
+ ["toolkit.telemetry.log.level", "Trace"]
+ ]});
+
+ // Start monitoring the ping archive.
+ let archiveChecker = new TelemetryArchiveTesting.Checker();
+ await archiveChecker.promiseInit();
+
+ // Manually call the BrowserContentHandler: this automatically gets called when
+ // the browser is started and an update was applied successfully in order to
+ // display the "update" info page.
+ Cc["@mozilla.org/browser/clh;1"].getService(Ci.nsIBrowserHandler).defaultArgs;
+
+ // We cannot control when the ping will be generated/archived after we trigger
+ // an update, so let's make sure to have one before moving on with validation.
+ let updatePing;
+ await BrowserTestUtils.waitForCondition(async function() {
+ // Check that the ping made it into the Telemetry archive.
+ // The test data is defined in ../data/sharedUpdateXML.js
+ updatePing = await archiveChecker.promiseFindPing("update", [
+ [["payload", "reason"], "success"],
+ [["payload", "previousBuildId"], TEST_BUILDID],
+ [["payload", "previousVersion"], TEST_VERSION]
+ ]);
+ return !!updatePing;
+ }, "Make sure the ping is generated before trying to validate it.", 500, 100);
+
+ ok(updatePing, "The 'update' ping must be correctly sent.");
+
+ // We have no easy way to simulate a previously applied update from toolkit/telemetry.
+ // Instead of moving this test to mozapps/update as well, just test that the
+ // "previousChannel" field is present and either a string or null.
+ ok("previousChannel" in updatePing.payload,
+ "The payload must contain the 'previousChannel' field");
+ const channelField = updatePing.payload.previousChannel;
+ if (channelField != null) {
+ ok(typeof(channelField) == "string", "'previousChannel' must be a string, if available.");
+ }
+
+ // Also make sure that the ping contains both a client id and an
+ // environment section.
+ ok("clientId" in updatePing, "The update ping must report a client id.");
+ ok("environment" in updatePing, "The update ping must report the environment.");
+});