From e1de85fca110dfd00b3a581a7e3362abe65e9e64 Mon Sep 17 00:00:00 2001 From: Matt King Date: Wed, 1 Apr 2015 09:49:11 -0700 Subject: [PATCH 01/52] Bug 1140592 - about:passwords header height does not match other about:pages. r=liuche --- mobile/android/themes/core/aboutPasswords.css | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mobile/android/themes/core/aboutPasswords.css b/mobile/android/themes/core/aboutPasswords.css index 57eb41d8740..d64c2b64e96 100644 --- a/mobile/android/themes/core/aboutPasswords.css +++ b/mobile/android/themes/core/aboutPasswords.css @@ -41,10 +41,10 @@ .toolbar-buttons > li { background-position: center; - background-size: 32px 32px; + background-size: 24px 24px; background-repeat: no-repeat; - height: 32px; - width: 32px; + height: 20px; + width: 20px; margin: 0 15px; } From 428374a7a3ef6017110bcdf7455673ce3939575d Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Wed, 1 Apr 2015 19:09:26 +0200 Subject: [PATCH 02/52] Bug 1094888 - part 1: make ensurePlacesDefaultQueries async. r=ttaubert --- browser/components/nsBrowserGlue.js | 14 +++++--- browser/components/nsIBrowserGlue.idl | 7 +--- .../places/tests/unit/head_bookmarks.js | 29 ++++++++++++++++ .../places/tests/unit/test_421483.js | 8 ++--- .../unit/test_browserGlue_smartBookmarks.js | 33 ++++--------------- 5 files changed, 49 insertions(+), 42 deletions(-) diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index bb1348e52bd..f3e5933f32b 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -382,6 +382,11 @@ BrowserGlue.prototype = { else if (data == "force-places-init") { this._initPlaces(false); } + else if (data == "smart-bookmarks-init") { + this.ensurePlacesDefaultQueriesInitialized().then(() => { + Services.obs.notifyObservers(null, "test-smart-bookmarks-done", null); + }); + } break; case "initial-migration-will-import-default-bookmarks": this._migrationImportsDefaultBookmarks = true; @@ -1497,7 +1502,7 @@ BrowserGlue.prototype = { // Now apply distribution customized bookmarks. // This should always run after Places initialization. this._distributionCustomizer.applyBookmarks(); - this.ensurePlacesDefaultQueriesInitialized(); + yield this.ensurePlacesDefaultQueriesInitialized(); } else { // An import operation is about to run. @@ -1532,7 +1537,7 @@ BrowserGlue.prototype = { this._distributionCustomizer.applyBookmarks(); // Ensure that smart bookmarks are created once the operation is // complete. - this.ensurePlacesDefaultQueriesInitialized(); + yield this.ensurePlacesDefaultQueriesInitialized(); } catch (e) { Cu.reportError(e); } @@ -2041,8 +2046,7 @@ BrowserGlue.prototype = { this._sanitizer.sanitize(aParentWindow); }, - ensurePlacesDefaultQueriesInitialized: - function BG_ensurePlacesDefaultQueriesInitialized() { + ensurePlacesDefaultQueriesInitialized: Task.async(function* () { // This is actual version of the smart bookmarks, must be increased every // time smart bookmarks change. // When adding a new smart bookmark below, its newInVersion property must @@ -2215,7 +2219,7 @@ BrowserGlue.prototype = { Services.prefs.setIntPref(SMART_BOOKMARKS_PREF, SMART_BOOKMARKS_VERSION); Services.prefs.savePrefFile(null); } - }, + }), // this returns the most recent non-popup browser window getMostRecentBrowserWindow: function BG_getMostRecentBrowserWindow() { diff --git a/browser/components/nsIBrowserGlue.idl b/browser/components/nsIBrowserGlue.idl index 1bb82a9d23a..211cffc362a 100644 --- a/browser/components/nsIBrowserGlue.idl +++ b/browser/components/nsIBrowserGlue.idl @@ -23,7 +23,7 @@ interface nsIDOMWindow; * */ -[scriptable, uuid(781df699-17dc-4237-b3d7-876ddb7085e3)] +[scriptable, uuid(ea083cb7-6b9d-4695-8b34-2e8f6ce2a860)] interface nsIBrowserGlue : nsISupports { /** @@ -35,11 +35,6 @@ interface nsIBrowserGlue : nsISupports */ void sanitize(in nsIDOMWindow aParentWindow); - /** - * Add Smart Bookmarks special queries to bookmarks menu and toolbar folder. - */ - void ensurePlacesDefaultQueriesInitialized(); - /** * Gets the most recent window that's a browser (but not a popup) */ diff --git a/browser/components/places/tests/unit/head_bookmarks.js b/browser/components/places/tests/unit/head_bookmarks.js index f277d34d7eb..9a1fb1ff510 100644 --- a/browser/components/places/tests/unit/head_bookmarks.js +++ b/browser/components/places/tests/unit/head_bookmarks.js @@ -90,3 +90,32 @@ let createCorruptDB = Task.async(function* () { // Check there's a DB now. Assert.ok((yield OS.File.exists(dbPath)), "should have a DB now"); }); + +/** + * Rebuilds smart bookmarks listening to console output to report any message or + * exception generated. + * + * @return {Promise} + * Resolved when done. + */ +function rebuildSmartBookmarks() { + let consoleListener = { + observe(aMsg) { + do_throw("Got console message: " + aMsg.message); + }, + QueryInterface: XPCOMUtils.generateQI([ Ci.nsIConsoleListener ]), + }; + Services.console.reset(); + Services.console.registerListener(consoleListener); + do_register_cleanup(() => { + try { + Services.console.unregisterListener(consoleListener); + } catch (ex) { /* will likely fail */ } + }); + Cc["@mozilla.org/browser/browserglue;1"] + .getService(Ci.nsIObserver) + .observe(null, "browser-glue-test", "smart-bookmarks-init"); + return promiseTopicObserved("test-smart-bookmarks-done").then(() => { + Services.console.unregisterListener(consoleListener); + }); +} diff --git a/browser/components/places/tests/unit/test_421483.js b/browser/components/places/tests/unit/test_421483.js index d67ac5e4b4d..532b2880c60 100644 --- a/browser/components/places/tests/unit/test_421483.js +++ b/browser/components/places/tests/unit/test_421483.js @@ -19,7 +19,7 @@ function run_test() { add_task(function* smart_bookmarks_disabled() { Services.prefs.setIntPref("browser.places.smartBookmarksVersion", -1); - gluesvc.ensurePlacesDefaultQueriesInitialized(); + yield rebuildSmartBookmarks(); let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); @@ -31,7 +31,7 @@ add_task(function* smart_bookmarks_disabled() { add_task(function* create_smart_bookmarks() { Services.prefs.setIntPref("browser.places.smartBookmarksVersion", 0); - gluesvc.ensurePlacesDefaultQueriesInitialized(); + yield rebuildSmartBookmarks(); let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); @@ -51,7 +51,7 @@ add_task(function* remove_smart_bookmark_and_restore() { yield PlacesUtils.bookmarks.remove(guid); Services.prefs.setIntPref("browser.places.smartBookmarksVersion", 0); - gluesvc.ensurePlacesDefaultQueriesInitialized(); + yield rebuildSmartBookmarks(); smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); Assert.equal(smartBookmarkItemIds.length, smartBookmarksCount); @@ -88,7 +88,7 @@ add_task(function* move_smart_bookmark_rename_and_restore() { // restore Services.prefs.setIntPref("browser.places.smartBookmarksVersion", 0); - gluesvc.ensurePlacesDefaultQueriesInitialized(); + yield rebuildSmartBookmarks(); smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); diff --git a/browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js b/browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js index ca66889f878..17fa8b9f2c6 100644 --- a/browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js +++ b/browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js @@ -35,27 +35,6 @@ function countFolderChildren(aFolderItemId) { return cc; } -/** - * Rebuilds smart bookmarks listening to console output to report any message or - * exception generated when calling ensurePlacesDefaultQueriesInitialized(). - */ -function rebuildSmartBookmarks() { - let consoleListener = { - observe: function(aMsg) { - do_throw("Got console message: " + aMsg.message); - }, - - QueryInterface: XPCOMUtils.generateQI([ - Ci.nsIConsoleListener - ]), - }; - Services.console.reset(); - Services.console.registerListener(consoleListener); - Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIBrowserGlue) - .ensurePlacesDefaultQueriesInitialized(); - Services.console.unregisterListener(consoleListener); -} - add_task(function* setup() { // Initialize browserGlue, but remove it's listener to places-init-complete. let bg = Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIObserver); @@ -89,7 +68,7 @@ add_task(function* test_version_0() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 0); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. Assert.equal(countFolderChildren(PlacesUtils.toolbarFolderId), @@ -126,7 +105,7 @@ add_task(function* test_version_change() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. Assert.equal(countFolderChildren(PlacesUtils.toolbarFolderId), @@ -173,7 +152,7 @@ add_task(function* test_version_change_pos() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. Assert.equal(countFolderChildren(PlacesUtils.toolbarFolderId), @@ -240,7 +219,7 @@ add_task(function* test_version_change_pos_moved() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. Assert.equal(countFolderChildren(PlacesUtils.toolbarFolderId), @@ -294,7 +273,7 @@ add_task(function* test_recreation() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. // We should not have recreated the smart bookmark on toolbar. @@ -320,7 +299,7 @@ add_task(function* test_recreation_version_0() { // Set preferences. Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 0); - rebuildSmartBookmarks(); + yield rebuildSmartBookmarks(); // Count items. // We should not have recreated the smart bookmark on toolbar. From 6157e16fbe5acb6e07a23c40b6970ac17129bf6a Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Wed, 1 Apr 2015 19:09:32 +0200 Subject: [PATCH 03/52] Bug 1094888 - part 2: use Bookmarks.jsm in ensurePlacesDefaultQueries. r=ttaubert --- browser/components/nsBrowserGlue.js | 244 ++++++++++++---------------- 1 file changed, 106 insertions(+), 138 deletions(-) diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index f3e5933f32b..97c2c55b6d5 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -1581,7 +1581,7 @@ BrowserGlue.prototype = { .getHistogramById("PLACES_BACKUPS_DAYSFROMLAST") .add(backupAge); } catch (ex) { - Components.utils.reportError("Unable to report telemetry."); + Cu.reportError("Unable to report telemetry."); } if (backupAge > BOOKMARKS_BACKUP_MAX_INTERVAL_DAYS) @@ -2047,10 +2047,10 @@ BrowserGlue.prototype = { }, ensurePlacesDefaultQueriesInitialized: Task.async(function* () { - // This is actual version of the smart bookmarks, must be increased every - // time smart bookmarks change. + // This is the current smart bookmarks version, it must be increased every + // time they change. // When adding a new smart bookmark below, its newInVersion property must - // be set to the version it has been added in, we will compare its value + // be set to the version it has been added in. We will compare its value // to users' smartBookmarksVersion and add new smart bookmarks without // recreating old deleted ones. const SMART_BOOKMARKS_VERSION = 7; @@ -2066,156 +2066,124 @@ BrowserGlue.prototype = { smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF); } catch(ex) {} - // If version is current or smart bookmarks are disabled, just bail out. + // If version is current, or smart bookmarks are disabled, bail out. if (smartBookmarksCurrentVersion == -1 || smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION) { return; } - let batch = { - runBatched: function BG_EPDQI_runBatched() { - let menuIndex = 0; - let toolbarIndex = 0; - let bundle = Services.strings.createBundle("chrome://browser/locale/places/places.properties"); + try { + let menuIndex = 0; + let toolbarIndex = 0; + let bundle = Services.strings.createBundle("chrome://browser/locale/places/places.properties"); + let queryOptions = Ci.nsINavHistoryQueryOptions; - let smartBookmarks = { - MostVisited: { - title: bundle.GetStringFromName("mostVisitedTitle"), - uri: NetUtil.newURI("place:sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING + - "&maxResults=" + MAX_RESULTS), - parent: PlacesUtils.toolbarFolderId, - get position() { return toolbarIndex++; }, - newInVersion: 1 - }, - RecentlyBookmarked: { - title: bundle.GetStringFromName("recentlyBookmarkedTitle"), - uri: NetUtil.newURI("place:folder=BOOKMARKS_MENU" + - "&folder=UNFILED_BOOKMARKS" + - "&folder=TOOLBAR" + - "&queryType=" + - Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS + - "&sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING + - "&maxResults=" + MAX_RESULTS + - "&excludeQueries=1"), - parent: PlacesUtils.bookmarksMenuFolderId, - get position() { return menuIndex++; }, - newInVersion: 1 - }, - RecentTags: { - title: bundle.GetStringFromName("recentTagsTitle"), - uri: NetUtil.newURI("place:"+ - "type=" + - Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY + - "&sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING + - "&maxResults=" + MAX_RESULTS), - parent: PlacesUtils.bookmarksMenuFolderId, - get position() { return menuIndex++; }, - newInVersion: 1 - }, - }; + let smartBookmarks = { + MostVisited: { + title: bundle.GetStringFromName("mostVisitedTitle"), + url: "place:sort=" + queryOptions.SORT_BY_VISITCOUNT_DESCENDING + + "&maxResults=" + MAX_RESULTS, + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + newInVersion: 1 + }, + RecentlyBookmarked: { + title: bundle.GetStringFromName("recentlyBookmarkedTitle"), + url: "place:folder=BOOKMARKS_MENU" + + "&folder=UNFILED_BOOKMARKS" + + "&folder=TOOLBAR" + + "&queryType=" + queryOptions.QUERY_TYPE_BOOKMARKS + + "&sort=" + queryOptions.SORT_BY_DATEADDED_DESCENDING + + "&maxResults=" + MAX_RESULTS + + "&excludeQueries=1", + parentGuid: PlacesUtils.bookmarks.menuGuid, + newInVersion: 1 + }, + RecentTags: { + title: bundle.GetStringFromName("recentTagsTitle"), + url: "place:type=" + queryOptions.RESULTS_AS_TAG_QUERY + + "&sort=" + queryOptions.SORT_BY_LASTMODIFIED_DESCENDING + + "&maxResults=" + MAX_RESULTS, + parentGuid: PlacesUtils.bookmarks.menuGuid, + newInVersion: 1 + }, + }; - if (Services.metro && Services.metro.supported) { - smartBookmarks.Windows8Touch = { - title: PlacesUtils.getString("windows8TouchTitle"), - get uri() { - let metroBookmarksRoot = PlacesUtils.annotations.getItemsWithAnnotation('metro/bookmarksRoot', {}); - if (metroBookmarksRoot.length > 0) { - return NetUtil.newURI("place:folder=" + - metroBookmarksRoot[0] + - "&queryType=" + - Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS + - "&sort=" + - Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING + - "&maxResults=" + MAX_RESULTS + - "&excludeQueries=1") - } - return null; - }, - parent: PlacesUtils.bookmarksMenuFolderId, - get position() { return menuIndex++; }, - newInVersion: 7 - }; - } - - // Set current itemId, parent and position if Smart Bookmark exists, - // we will use these informations to create the new version at the same - // position. - let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); - smartBookmarkItemIds.forEach(function (itemId) { - let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); - if (queryId in smartBookmarks) { - let smartBookmark = smartBookmarks[queryId]; - if (!smartBookmark.uri) { - PlacesUtils.bookmarks.removeItem(itemId); - return; - } - smartBookmark.itemId = itemId; - smartBookmark.parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId); - smartBookmark.updatedPosition = PlacesUtils.bookmarks.getItemIndex(itemId); - } - else { - // We don't remove old Smart Bookmarks because user could still - // find them useful, or could have personalized them. - // Instead we remove the Smart Bookmark annotation. - PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); - } - }); - - for (let queryId in smartBookmarks) { + // Set current guid, parentGuid and index of existing Smart Bookmarks. + // We will use those to create a new version of the bookmark at the same + // position. + let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO); + for (let itemId of smartBookmarkItemIds) { + let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); + if (queryId in smartBookmarks) { + // Known smart bookmark. let smartBookmark = smartBookmarks[queryId]; + smartBookmark.guid = yield PlacesUtils.promiseItemGuid(itemId); - // We update or create only changed or new smart bookmarks. - // Also we respect user choices, so we won't try to create a smart - // bookmark if it has been removed. - if (smartBookmarksCurrentVersion > 0 && - smartBookmark.newInVersion <= smartBookmarksCurrentVersion && - !smartBookmark.itemId || !smartBookmark.uri) + if (!smartBookmark.url) { + yield PlacesUtils.bookmarks.remove(smartBookmark.guid); continue; - - // Remove old version of the smart bookmark if it exists, since it - // will be replaced in place. - if (smartBookmark.itemId) { - PlacesUtils.bookmarks.removeItem(smartBookmark.itemId); } - // Create the new smart bookmark and store its updated itemId. - smartBookmark.itemId = - PlacesUtils.bookmarks.insertBookmark(smartBookmark.parent, - smartBookmark.uri, - smartBookmark.updatedPosition || smartBookmark.position, - smartBookmark.title); - PlacesUtils.annotations.setItemAnnotation(smartBookmark.itemId, - SMART_BOOKMARKS_ANNO, - queryId, 0, - PlacesUtils.annotations.EXPIRE_NEVER); + let bm = yield PlacesUtils.bookmarks.fetch(smartBookmark.guid); + smartBookmark.parentGuid = bm.parentGuid; + smartBookmark.index = bm.index; } - - // If we are creating all Smart Bookmarks from ground up, add a - // separator below them in the bookmarks menu. - if (smartBookmarksCurrentVersion == 0 && - smartBookmarkItemIds.length == 0) { - let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, - menuIndex); - // Don't add a separator if the menu was empty or there is one already. - if (id != -1 && - PlacesUtils.bookmarks.getItemType(id) != PlacesUtils.bookmarks.TYPE_SEPARATOR) { - PlacesUtils.bookmarks.insertSeparator(PlacesUtils.bookmarksMenuFolderId, - menuIndex); - } + else { + // We don't remove old Smart Bookmarks because user could still + // find them useful, or could have personalized them. + // Instead we remove the Smart Bookmark annotation. + PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO); } } - }; - try { - PlacesUtils.bookmarks.runInBatchMode(batch, null); - } - catch(ex) { - Components.utils.reportError(ex); - } - finally { + for (let queryId of Object.keys(smartBookmarks)) { + let smartBookmark = smartBookmarks[queryId]; + + // We update or create only changed or new smart bookmarks. + // Also we respect user choices, so we won't try to create a smart + // bookmark if it has been removed. + if (smartBookmarksCurrentVersion > 0 && + smartBookmark.newInVersion <= smartBookmarksCurrentVersion && + !smartBookmark.guid || !smartBookmark.url) + continue; + + // Remove old version of the smart bookmark if it exists, since it + // will be replaced in place. + if (smartBookmark.guid) { + yield PlacesUtils.bookmarks.remove(smartBookmark.guid); + } + + // Create the new smart bookmark and store its updated guid. + if (!("index" in smartBookmark)) { + if (smartBookmark.parentGuid == PlacesUtils.bookmarks.toolbarGuid) + smartBookmark.index = toolbarIndex++; + else if (smartBookmark.parentGuid == PlacesUtils.bookmarks.menuGuid) + smartBookmark.index = menuIndex++; + } + smartBookmark = yield PlacesUtils.bookmarks.insert(smartBookmark); + let itemId = yield PlacesUtils.promiseItemId(smartBookmark.guid); + PlacesUtils.annotations.setItemAnnotation(itemId, + SMART_BOOKMARKS_ANNO, + queryId, 0, + PlacesUtils.annotations.EXPIRE_NEVER); + } + + // If we are creating all Smart Bookmarks from ground up, add a + // separator below them in the bookmarks menu. + if (smartBookmarksCurrentVersion == 0 && + smartBookmarkItemIds.length == 0) { + let bm = yield PlacesUtils.bookmarks.fetch({ parentGuid: PlacesUtils.bookmarks.menuGuid, + index: menuIndex }); + // Don't add a separator if the menu was empty or there is one already. + if (bm && bm.type != PlacesUtils.bookmarks.TYPE_SEPARATOR) { + yield PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_SEPARATOR, + parentGuid: PlacesUtils.bookmarks.menuGuid, + index: menuIndex }); + } + } + } catch(ex) { + Cu.reportError(ex); + } finally { Services.prefs.setIntPref(SMART_BOOKMARKS_PREF, SMART_BOOKMARKS_VERSION); Services.prefs.savePrefFile(null); } From 6bf3e378afd4d3baf36e06307b3cac71f75be84e Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 18 Mar 2015 12:35:49 +0100 Subject: [PATCH 04/52] Bug 1139460 - Part 1 - Avoid Telemetry payload data changes after collection. r=vladan --- toolkit/components/telemetry/TelemetryPing.jsm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index d93556efab7..cb891507fc6 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -9,6 +9,7 @@ const Cc = Components.classes; const Ci = Components.interfaces; const Cr = Components.results; const Cu = Components.utils; +const myScope = this; Cu.import("resource://gre/modules/Log.jsm"); Cu.import("resource://gre/modules/debug.js", this); @@ -317,6 +318,11 @@ let Impl = { this._log.trace("assemblePing - Type " + aType + ", Server " + this._server + ", aOptions " + JSON.stringify(aOptions)); + // Clone the payload data so we don't race against unexpected changes in subobjects that are + // still referenced by other code. + // We can't trust all callers to do this properly on their own. + let payload = Cu.cloneInto(aPayload, myScope); + // Fill the common ping fields. let pingData = { type: aType, From 1d396639850b05faaeebee7f9f51cac075f78595 Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Fri, 20 Mar 2015 14:32:35 +0100 Subject: [PATCH 05/52] Bug 1139460 - Part 2 - Overhaul telemetry ping submission code. r=vladan --- .../components/telemetry/TelemetryPing.jsm | 80 ++++++++++++++----- toolkit/components/telemetry/docs/pings.rst | 6 ++ 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index cb891507fc6..6241fd6ae99 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -17,6 +17,7 @@ Cu.import("resource://gre/modules/Services.jsm", this); Cu.import("resource://gre/modules/XPCOMUtils.jsm", this); Cu.import("resource://gre/modules/osfile.jsm", this); Cu.import("resource://gre/modules/Promise.jsm", this); +Cu.import("resource://gre/modules/PromiseUtils.jsm", this); Cu.import("resource://gre/modules/DeferredTask.jsm", this); Cu.import("resource://gre/modules/Preferences.jsm"); @@ -40,6 +41,8 @@ const TELEMETRY_DELAY = 60000; const TELEMETRY_TEST_DELAY = 100; // The number of days to keep pings serialised on the disk in case of failures. const DEFAULT_RETENTION_DAYS = 14; +// Timeout after which we consider a ping submission failed. +const PING_SUBMIT_TIMEOUT_MS = 2 * 60 * 1000; XPCOMUtils.defineLazyServiceGetter(this, "Telemetry", "@mozilla.org/base/telemetry;1", @@ -490,8 +493,8 @@ let Impl = { }, error => this._log.error("savePing - Rejection", error)); }, - finishPingRequest: function finishPingRequest(success, startTime, ping, isPersisted) { - this._log.trace("finishPingRequest - Success " + success + ", Persisted " + isPersisted); + onPingRequestFinished: function(success, startTime, ping, isPersisted) { + this._log.trace("onPingRequestFinished - success: " + success + ", persisted: " + isPersisted); let hping = Telemetry.getHistogramById("TELEMETRY_PING"); let hsuccess = Telemetry.getHistogramById("TELEMETRY_SUCCESS"); @@ -541,21 +544,25 @@ let Impl = { doPing: function doPing(ping, isPersisted) { this._log.trace("doPing - Server " + this._server + ", Persisted " + isPersisted); - let deferred = Promise.defer(); - let isNewPing = isNewPingFormat(ping); - let version = isNewPing ? PING_FORMAT_VERSION : 1; - let url = this._server + this.submissionPath(ping) + "?v=" + version; + + const isNewPing = isNewPingFormat(ping); + const version = isNewPing ? PING_FORMAT_VERSION : 1; + const url = this._server + this.submissionPath(ping) + "?v=" + version; + let request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"] .createInstance(Ci.nsIXMLHttpRequest); request.mozBackgroundRequest = true; + request.timeout = PING_SUBMIT_TIMEOUT_MS; + request.open("POST", url, true); request.overrideMimeType("text/plain"); request.setRequestHeader("Content-Type", "application/json; charset=UTF-8"); let startTime = new Date(); + let deferred = PromiseUtils.defer(); - function handler(success) { - let handleCompletion = event => { + let onRequestFinished = (success, event) => { + let onCompletion = () => { if (success) { deferred.resolve(); } else { @@ -563,18 +570,50 @@ let Impl = { } }; - return function(event) { - this.finishPingRequest(success, startTime, ping, isPersisted) - .then(() => handleCompletion(event), - error => { - this._log.error("doPing - Request Success " + success + ", Error " + - error); - handleCompletion(event); - }); - }; - } - request.addEventListener("error", handler(false).bind(this), false); - request.addEventListener("load", handler(true).bind(this), false); + this.onPingRequestFinished(success, startTime, ping, isPersisted) + .then(() => onCompletion(), + (error) => { + this._log.error("doPing - request success: " + success + ", error" + error); + onCompletion(); + }); + }; + + let errorhandler = (event) => { + this._log.error("doPing - error making request to " + url + ": " + event.type); + onRequestFinished(false, event); + }; + request.onerror = errorhandler; + request.ontimeout = errorhandler; + request.onabort = errorhandler; + + request.onload = (event) => { + let status = request.status; + let statusClass = status - (status % 100); + let success = false; + + if (statusClass === 200) { + // We can treat all 2XX as success. + this._log.info("doPing - successfully loaded, status: " + status); + success = true; + } else if (statusClass === 400) { + // 4XX means that something with the request was broken. + this._log.error("doPing - error submitting to " + url + ", status: " + status + + " - ping request broken?"); + // TODO: we should handle this better, but for now we should avoid resubmitting + // broken requests by pretending success. + success = true; + } else if (statusClass === 500) { + // 5XX means there was a server-side error and we should try again later. + this._log.error("doPing - error submitting to " + url + ", status: " + status + + " - server error, should retry later"); + } else { + // We received an unexpected status codes. + this._log.error("doPing - error submitting to " + url + ", status: " + status + + ", type: " + event.type); + } + + onRequestFinished(success, event); + }; // If that's a legacy ping format, just send its payload. let networkPayload = isNewPing ? ping : ping.payload; @@ -588,6 +627,7 @@ let Impl = { .createInstance(Ci.nsIStringInputStream); payloadStream.data = this.gzipCompressString(utf8Payload); request.send(payloadStream); + return deferred.promise; }, diff --git a/toolkit/components/telemetry/docs/pings.rst b/toolkit/components/telemetry/docs/pings.rst index 9490bae454c..3ada231f604 100644 --- a/toolkit/components/telemetry/docs/pings.rst +++ b/toolkit/components/telemetry/docs/pings.rst @@ -19,6 +19,12 @@ If a ping failed to submit (e.g. because of missing internet connection), Teleme *Note:* the :doc:`main pings ` are kept locally even after successful submission to enable the HealthReport and SelfSupport features. They will be deleted after their retention period of 180 days. +The telemetry server team is working towards `the common services status codes `_, but for now the following logic is sufficient for Telemetry: + +* `2XX` - success, don't resubmit +* `4XX` - there was some problem with the request - the client should not try to resubmit as it would just receive the same response +* `5XX` - there was a server-side error, the client should try to resubmit later + Ping types ========== From d23eaf7578169c0f141e4e36605b40ce4973b0be Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Mon, 23 Mar 2015 12:45:29 +0100 Subject: [PATCH 06/52] Bug 1139460 - Part 3 - Make TelemetryPing shutdown block on pending submissions. r=yoric --- .../components/telemetry/TelemetryPing.jsm | 74 ++++++++++++++----- 1 file changed, 57 insertions(+), 17 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index 6241fd6ae99..671c9a06fd9 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -18,6 +18,7 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm", this); Cu.import("resource://gre/modules/osfile.jsm", this); Cu.import("resource://gre/modules/Promise.jsm", this); Cu.import("resource://gre/modules/PromiseUtils.jsm", this); +Cu.import("resource://gre/modules/Task.jsm", this); Cu.import("resource://gre/modules/DeferredTask.jsm", this); Cu.import("resource://gre/modules/Preferences.jsm"); @@ -270,7 +271,12 @@ let Impl = { // The deferred promise resolved when the initialization task completes. _delayedInitTaskDeferred: 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. _shutdownBarrier: new AsyncShutdown.Barrier("TelemetryPing: Waiting for clients."), + // This is a private barrier blocked by pending async ping activity (sending & saving). + _connectionsBarrier: new AsyncShutdown.Barrier("TelemetryPing: Waiting for pending ping activity"), /** * Get the data for the "application" section of the ping. @@ -373,6 +379,14 @@ let Impl = { this._server = aServer; }, + /** + * Track any pending ping send and save tasks through the promise passed here. + * This is needed to block shutdown on any outstanding ping activity. + */ + _trackPendingPingTask: function (aPromise) { + this._connectionsBarrier.client.addBlocker("Waiting for ping task", aPromise); + }, + /** * Adds a ping to the pending ping list by moving it to the saved pings directory * and adding it to the pending ping list. @@ -410,7 +424,7 @@ let Impl = { this._log.trace("send - Type " + aType + ", Server " + this._server + ", aOptions " + JSON.stringify(aOptions)); - return this.assemblePing(aType, aPayload, aOptions) + let promise = this.assemblePing(aType, aPayload, aOptions) .then(pingData => { // Once ping is assembled, send it along with the persisted ping in the backlog. let p = [ @@ -422,16 +436,27 @@ let Impl = { return Promise.all(p); }, error => this._log.error("send - Rejection", error)); + + this._trackPendingPingTask(promise); + return promise; }, /** * Send the persisted pings to the server. + * + * @return Promise A promise that is resolved when all pings finished sending or failed. */ sendPersistedPings: function sendPersistedPings() { this._log.trace("sendPersistedPings"); + let pingsIterator = Iterator(this.popPayloads()); - let p = [data for (data in pingsIterator)].map(data => this.doPing(data, true)); - return Promise.all(p); + let p = [for (data of pingsIterator) this.doPing(data, true).catch((e) => { + this._log.error("sendPersistedPings - doPing rejected", e); + })]; + + let promise = Promise.all(p); + this._trackPendingPingTask(promise); + return promise; }, /** @@ -788,21 +813,34 @@ let Impl = { return this._delayedInitTaskDeferred.promise; }, + // Do proper shutdown waiting and cleanup. + _cleanupOnShutdown: Task.async(function*() { + if (!this._initialized) { + return; + } + + try { + // First wait for clients processing shutdown. + yield this._shutdownBarrier.wait(); + // Then wait for any outstanding async ping activity. + yield this._connectionsBarrier.wait(); + + // Should down dependent components. + try { + yield TelemetryEnvironment.shutdown(); + } catch (e) { + this._log.error("shutdown - environment shutdown failure", e); + } + } finally { + // Reset state. + this._initialized = false; + this._initStarted = false; + } + }), + shutdown: function() { this._log.trace("shutdown"); - let cleanup = () => { - if (!this._initialized) { - return; - } - let reset = () => { - this._initialized = false; - this._initStarted = false; - }; - return this._shutdownBarrier.wait().then( - () => TelemetryEnvironment.shutdown().then(reset, reset)); - }; - // We can be in one the following states here: // 1) setupTelemetry was never called // or it was called and @@ -818,11 +856,11 @@ let Impl = { // This handles 4). if (!this._delayedInitTask) { // We already ran the delayed initialization. - return cleanup(); + return this._cleanupOnShutdown(); } // This handles 2) and 3). - return this._delayedInitTask.finalize().then(cleanup); + return this._delayedInitTask.finalize().then(() => this._cleanupOnShutdown()); }, /** @@ -867,6 +905,8 @@ let Impl = { initialized: this._initialized, initStarted: this._initStarted, haveDelayedInitTask: !!this._delayedInitTask, + shutdownBarrier: this._shutdownBarrier.state, + connectionsBarrier: this._connectionsBarrier.state, }; }, }; From ebddb88ad1fc098cc625b527a4660a4dc31b2116 Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Thu, 19 Mar 2015 15:50:09 +0100 Subject: [PATCH 07/52] Bug 1139460 - Bonus: Remove unused function from experiments code. rs=yoric --- browser/experiments/Experiments.jsm | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/browser/experiments/Experiments.jsm b/browser/experiments/Experiments.jsm index a8ef7ad26ca..d7995642dea 100644 --- a/browser/experiments/Experiments.jsm +++ b/browser/experiments/Experiments.jsm @@ -139,28 +139,6 @@ function configureLogging() { } } -// Takes an array of promises and returns a promise that is resolved once all of -// them are rejected or resolved. -function allResolvedOrRejected(promises) { - if (!promises.length) { - return Promise.resolve([]); - } - - let countdown = promises.length; - let deferred = Promise.defer(); - - for (let p of promises) { - let helper = () => { - if (--countdown == 0) { - deferred.resolve(); - } - }; - Promise.resolve(p).then(helper, helper); - } - - return deferred.promise; -} - // Loads a JSON file using OS.file. file is a string representing the path // of the file to be read, options contains additional options to pass to // OS.File.read. From b4c1d5b9043d477da107d250ae7441c7268bf561 Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 08/52] Bug 1139460 - Part 5 - Fix race leading to TelemetryPing shutdown timeout by aborting all pending ping requests on shutdown. r=vladan,f=yoric --- .../components/telemetry/TelemetryPing.jsm | 20 ++++- .../components/telemetry/tests/unit/head.js | 2 + .../tests/unit/test_TelemetryPing.js | 2 - .../tests/unit/test_TelemetryPingShutdown.js | 73 +++++++++++++++++++ .../tests/unit/test_TelemetrySession.js | 1 - .../telemetry/tests/unit/xpcshell.ini | 1 + 6 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 toolkit/components/telemetry/tests/unit/test_TelemetryPingShutdown.js diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index 671c9a06fd9..8dbc74b6719 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -278,6 +278,9 @@ let Impl = { // This is a private barrier blocked by pending async ping activity (sending & saving). _connectionsBarrier: new AsyncShutdown.Barrier("TelemetryPing: Waiting for pending ping activity"), + // This tracks all pending ping requests to the server. + _pendingPingRequests: new Map(), + /** * Get the data for the "application" section of the ping. */ @@ -583,6 +586,8 @@ let Impl = { request.overrideMimeType("text/plain"); request.setRequestHeader("Content-Type", "application/json; charset=UTF-8"); + this._pendingPingRequests.set(url, request); + let startTime = new Date(); let deferred = PromiseUtils.defer(); @@ -595,6 +600,7 @@ let Impl = { } }; + this._pendingPingRequests.delete(url); this.onPingRequestFinished(success, startTime, ping, isPersisted) .then(() => onCompletion(), (error) => { @@ -819,6 +825,18 @@ let Impl = { return; } + // Abort any pending ping XHRs. + for (let [url, request] of this._pendingPingRequests) { + this._log.trace("_cleanupOnShutdown - aborting ping request for " + url); + try { + request.abort(); + } catch (e) { + this._log.error("_cleanupOnShutdown - failed to abort request to " + url, e); + } + } + this._pendingPingRequests.clear(); + + // Now do an orderly shutdown. try { // First wait for clients processing shutdown. yield this._shutdownBarrier.wait(); @@ -829,7 +847,7 @@ let Impl = { try { yield TelemetryEnvironment.shutdown(); } catch (e) { - this._log.error("shutdown - environment shutdown failure", e); + this._log.error("_cleanupOnShutdown - environment shutdown failure", e); } } finally { // Reset state. diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js index a16c4a92d56..39fff81df67 100644 --- a/toolkit/components/telemetry/tests/unit/head.js +++ b/toolkit/components/telemetry/tests/unit/head.js @@ -9,6 +9,8 @@ const gIsMac = ("@mozilla.org/xpcom/mac-utils;1" in Components.classes); const gIsAndroid = ("@mozilla.org/android/bridge;1" in Components.classes); const gIsGonk = ("@mozilla.org/cellbroadcast/gonkservice;1" in Components.classes); +const HAS_DATAREPORTINGSERVICE = "@mozilla.org/datareporting/service;1" in Components.classes; + let gOldAppInfo = null; let gGlobalScope = this; diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js index bb2c9ff9ea2..bed8d7452e4 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryPing.js @@ -35,8 +35,6 @@ const PREF_ENABLED = PREF_BRANCH + "enabled"; const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled"; const PREF_FHR_SERVICE_ENABLED = "datareporting.healthreport.service.enabled"; -const HAS_DATAREPORTINGSERVICE = "@mozilla.org/datareporting/service;1" in Cc; - const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); let gHttpServer = new HttpServer(); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryPingShutdown.js b/toolkit/components/telemetry/tests/unit/test_TelemetryPingShutdown.js new file mode 100644 index 00000000000..999d5172016 --- /dev/null +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryPingShutdown.js @@ -0,0 +1,73 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test that TelemetryPing sends close to shutdown don't lead +// to AsyncShutdown timeouts. + +"use strict"; + +const { utils: Cu, interfaces: Ci, classes: Cc } = Components; + +Cu.import("resource://gre/modules/Services.jsm", this); +Cu.import("resource://gre/modules/TelemetryPing.jsm", this); +Cu.import("resource://gre/modules/Timer.jsm", this); +Cu.import("resource://gre/modules/XPCOMUtils.jsm", this); +Cu.import("resource://gre/modules/AsyncShutdown.jsm", this); +Cu.import("resource://testing-common/httpd.js", this); + +const PREF_BRANCH = "toolkit.telemetry."; +const PREF_ENABLED = PREF_BRANCH + "enabled"; +const PREF_FHR_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled"; + +function contentHandler(metadata, response) +{ + dump("contentHandler called for path: " + metadata._path + "\n"); + // We intentionally don't finish writing the response here to let the + // client time out. + response.processAsync(); + response.setHeader("Content-Type", "text/plain"); +} + +function run_test() { + // Addon manager needs a profile directory + do_get_profile(); + loadAddonManager("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2"); + + 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) { + let drs = Cc["@mozilla.org/datareporting/service;1"] + .getService(Ci.nsISupports) + .wrappedJSObject; + drs.observe(null, "app-startup", null); + drs.observe(null, "profile-after-change", null); + } + + run_next_test(); +} + +add_task(function* test_sendTimeout() { + const TIMEOUT = 100; + // Enable testing mode for AsyncShutdown, otherwise some testing-only functionality + // is not available. + Services.prefs.setBoolPref("toolkit.asyncshutdown.testing", true); + Services.prefs.setIntPref("toolkit.asyncshutdown.crash_timeout", TIMEOUT); + + let httpServer = new HttpServer(); + httpServer.registerPrefixHandler("/", contentHandler); + httpServer.start(-1); + + yield TelemetryPing.setup(); + TelemetryPing.setServer("http://localhost:" + httpServer.identity.primaryPort); + TelemetryPing.send("test-ping-type", {}); + + // Trigger the AsyncShutdown phase TelemetryPing hangs off. + AsyncShutdown.profileBeforeChange._trigger(); + AsyncShutdown.sendTelemetry._trigger(); + + // If we get here, we didn't time out in the shutdown routines. + Assert.ok(true, "Didn't time out on shutdown."); +}); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 62cd9228326..b5f0d862377 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -70,7 +70,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 HAS_DATAREPORTINGSERVICE = "@mozilla.org/datareporting/service;1" in Cc; const SESSION_RECORDER_EXPECTED = HAS_DATAREPORTINGSERVICE && Preferences.get(PREF_FHR_SERVICE_ENABLED, true); diff --git a/toolkit/components/telemetry/tests/unit/xpcshell.ini b/toolkit/components/telemetry/tests/unit/xpcshell.ini index 1653350f3b0..d7ee82a4637 100644 --- a/toolkit/components/telemetry/tests/unit/xpcshell.ini +++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini @@ -33,6 +33,7 @@ skip-if = android_version == "18" # Bug 1144395: crash on Android 4.3 skip-if = android_version == "18" [test_TelemetryPing_idle.js] +[test_TelemetryPingShutdown.js] [test_TelemetryStopwatch.js] [test_TelemetryPingBuildID.js] # Bug 1144395: crash on Android 4.3 From 583c2c1f0280f87b628ba031e54ee55438513a30 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Fri, 13 Mar 2015 10:55:06 -0400 Subject: [PATCH 09/52] Bug 1140558 - Part 1 - Switch TelemetryEnvironment to a model which keeps track of the current state constantly and makes the current environment available synchronously. r=vladan/Dexter --- .../telemetry/TelemetryEnvironment.jsm | 961 +++++++++--------- .../components/telemetry/TelemetryPing.jsm | 63 +- .../components/telemetry/TelemetrySession.jsm | 8 +- .../components/telemetry/docs/environment.rst | 12 +- .../tests/unit/test_TelemetryEnvironment.js | 127 +-- .../tests/unit/test_TelemetrySession.js | 4 +- 6 files changed, 559 insertions(+), 616 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index 7b989005f59..3567eee3f84 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -9,6 +9,7 @@ this.EXPORTED_SYMBOLS = [ ]; const {classes: Cc, interfaces: Ci, utils: Cu} = Components; +const myScope = this; Cu.import("resource://gre/modules/AddonManager.jsm"); Cu.import("resource://gre/modules/XPCOMUtils.jsm", this); @@ -29,6 +30,102 @@ XPCOMUtils.defineLazyModuleGetter(this, "ProfileAge", XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel", "resource://gre/modules/UpdateChannel.jsm"); +var gGlobalEnvironment; +function getGlobal() { + if (!gGlobalEnvironment) { + gGlobalEnvironment = new EnvironmentCache(); + } + return gGlobalEnvironment; +} + +const TelemetryEnvironment = { + get currentEnvironment() { + return getGlobal().currentEnvironment; + }, + + onInitialized: function() { + return getGlobal().onInitialized(); + }, + + registerChangeListener: function(name, listener) { + return getGlobal().registerChangeListener(name, listener); + }, + + unregisterChangeListener: function(name) { + return getGlobal().unregisterChangeListener(name); + }, + + // Policy to use when saving preferences. Exported for using them in tests. + RECORD_PREF_STATE: 1, // Don't record the preference value + RECORD_PREF_VALUE: 2, // We only record user-set prefs. + RECORD_PREF_NOTIFY_ONLY: 3, // Record nothing, just notify of changes. + + // Testing method + _watchPreferences: function(prefMap) { + return getGlobal()._watchPreferences(prefMap); + }, +}; + +const DEFAULT_ENVIRONMENT_PREFS = new Map([ + ["app.feedback.baseURL", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.support.baseURL", TelemetryEnvironment.RECORD_PREF_VALUE], + ["accessibility.browsewithcaret", TelemetryEnvironment.RECORD_PREF_VALUE], + ["accessibility.force_disabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.auto", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.interval", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.service.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.silent", TelemetryEnvironment.RECORD_PREF_VALUE], + ["app.update.url", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.cache.disk.enable", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.cache.disk.capacity", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.cache.memory.enable", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.cache.offline.enable", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.formfill.enable", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.newtab.url", TelemetryEnvironment.RECORD_PREF_STATE], + ["browser.newtabpage.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.newtabpage.enhanced", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.polaris.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.shell.checkDefaultBrowser", TelemetryEnvironment.RECORD_PREF_VALUE], + ["browser.startup.homepage", TelemetryEnvironment.RECORD_PREF_STATE], + ["browser.startup.page", TelemetryEnvironment.RECORD_PREF_VALUE], + ["devtools.chrome.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["devtools.debugger.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["devtools.debugger.remote-enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["dom.ipc.plugins.asyncInit", TelemetryEnvironment.RECORD_PREF_VALUE], + ["dom.ipc.plugins.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["experiments.manifest.uri", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.blocklist.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.blocklist.url", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.strictCompatibility", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.update.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.update.url", TelemetryEnvironment.RECORD_PREF_VALUE], + ["extensions.update.background.url", TelemetryEnvironment.RECORD_PREF_VALUE], + ["general.smoothScroll", TelemetryEnvironment.RECORD_PREF_VALUE], + ["gfx.direct2d.disabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["gfx.direct2d.force-enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["gfx.direct2d.use1_1", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.acceleration.disabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.acceleration.force-enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.async-pan-zoom.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.async-video-oop.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.async-video.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.componentalpha.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.d3d11.disable-warp", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.d3d11.force-warp", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.prefer-d3d9", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layers.prefer-opengl", TelemetryEnvironment.RECORD_PREF_VALUE], + ["layout.css.devPixelsPerPx", TelemetryEnvironment.RECORD_PREF_VALUE], + ["network.proxy.autoconfig_url", TelemetryEnvironment.RECORD_PREF_STATE], + ["network.proxy.http", TelemetryEnvironment.RECORD_PREF_STATE], + ["network.proxy.ssl", TelemetryEnvironment.RECORD_PREF_STATE], + ["pdfjs.disabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["places.history.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["privacy.trackingprotection.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["privacy.donottrackheader.enabled", TelemetryEnvironment.RECORD_PREF_VALUE], + ["services.sync.serverURL", TelemetryEnvironment.RECORD_PREF_STATE], +]); + const LOGGER_NAME = "Toolkit.Telemetry"; const PREF_BLOCKLIST_ENABLED = "extensions.blocklist.enabled"; @@ -218,95 +315,392 @@ function getServicePack() { } #endif -this.TelemetryEnvironment = { - _shutdown: true, +/** + * Encapsulates the asynchronous magic interfacing with the addon manager. The builder + * is owned by a parent environment object and is an addon listener. + */ +function EnvironmentAddonBuilder(environment) { + this._environment = environment; - // A map of (sync) listeners that will be called on environment changes. - _changeListeners: new Map(), - // Async task for collecting the environment data. - _collectTask: null, - - // Policy to use when saving preferences. Exported for using them in tests. - RECORD_PREF_STATE: 1, // Don't record the preference value - RECORD_PREF_VALUE: 2, // We only record user-set prefs. - RECORD_PREF_NOTIFY_ONLY: 3, // Record nothing, just notify of changes. - - // A map of watched preferences which trigger an Environment change when modified. - // Every entry contains a recording policy (RECORD_PREF_*). - _watchedPrefs: null, - - // The Addons change listener, init by |_startWatchingAddons| . - _addonsListener: null, - - // AddonManager may shut down before us, in which case we cache the addons here. - // It is always null if the AM didn't shut down before us. - // If cached, it is an object containing the addon information, suitable for use - // in the environment data. - _cachedAddons: null, + // The pending task blocks addon manager shutdown. It can either be the initial load + // or a change load. + this._pendingTask = null; + // Set to true once initial load is complete and we're watching for changes. + this._loaded = false; +} +EnvironmentAddonBuilder.prototype = { /** - * Initialize TelemetryEnvironment. + * Get the initial set of addons. + * @returns Promise when the initial load is complete. */ - init: function () { - if (!this._shutdown) { - this._log.error("init - Already initialized"); - return; + init: function() { + // Some tests don't initialize the addon manager. This accounts for the + // unfortunate reality of life. + try { + AddonManager.shutdown.addBlocker("EnvironmentAddonBuilder", + () => this._shutdownBlocker()); + } catch (err) { + return Promise.reject(err); } - this._configureLog(); - this._log.trace("init"); - this._shutdown = false; - this._startWatchingPrefs(); - this._startWatchingAddons(); + this._pendingTask = this._updateAddons().then( + () => { this._pendingTask = null; }, + (err) => { + this._environment._log.error("init - Exception in _updateAddons", err); + this._pendingTask = null; + } + ); - AddonManager.shutdown.addBlocker("TelemetryEnvironment: caching addons", - () => this._blockAddonManagerShutdown(), - () => this._getState()); + return this._pendingTask; }, /** - * Shutdown TelemetryEnvironment. - * @return Promise<> that is resolved when shutdown is finished. + * Register an addon listener and watch for changes. */ - shutdown: Task.async(function* () { - if (this._shutdown) { - if (this._log) { - this._log.error("shutdown - Already shut down"); - } else { - Cu.reportError("TelemetryEnvironment.shutdown - Already shut down"); - } + watchForChanges: function() { + this._loaded = true; + AddonManager.addAddonListener(this); + Services.obs.addObserver(this, EXPERIMENTS_CHANGED_TOPIC, false); + }, + + // AddonListener + onEnabled: function() { + this._onChange(); + }, + onDisabled: function() { + this._onChange(); + }, + onInstalled: function() { + this._onChange(); + }, + onUninstalling: function() { + this._onChange(); + }, + + _onChange: function() { + if (this._pendingTask) { + this._environment._log.trace("_onChange - task already pending"); return; } - this._log.trace("shutdown"); - this._shutdown = true; - this._stopWatchingPrefs(); - this._stopWatchingAddons(); - this._changeListeners.clear(); - yield this._collectTask; + 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); + }); + }, - this._cachedAddons = null; + // nsIObserver + observe: function (aSubject, aTopic, aData) { + this._environment._log.trace("observe - Topic " + aTopic); + + 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); + }); + } + }, + + _shutdownBlocker: function() { + if (this._loaded) { + AddonManager.removeAddonListener(this); + Services.obs.removeObserver(this, EXPERIMENTS_CHANGED_TOPIC); + } + return this._pendingTask; + }, + + /** + * Collect the addon data for the environment. + * + * This should only be called from _pendingTask; otherwise we risk + * running this during addon manager shutdown. + * + * @returns Promise whether the environment changed. + */ + _updateAddons: Task.async(function* () { + this._environment._log.trace("_updateAddons"); + let personaId = null; +#ifndef MOZ_WIDGET_GONK + let theme = LightweightThemeManager.currentTheme; + if (theme) { + personaId = theme.id; + } +#endif + + let addons = { + activeAddons: yield this._getActiveAddons(), + theme: yield this._getActiveTheme(), + activePlugins: this._getActivePlugins(), + activeGMPlugins: yield this._getActiveGMPlugins(), + activeExperiment: this._getActiveExperiment(), + persona: personaId, + }; + + if (JSON.stringify(addons) != + JSON.stringify(this._environment._currentEnvironment.addons)) { + this._environment._log.trace("_updateAddons: addons differ"); + this._environment._currentEnvironment.addons = addons; + return true; + } + this._environment._log.trace("_updateAddons: no changes found"); + return false; }), - _configureLog: function () { - if (this._log) { - return; + /** + * Get the addon data in object form. + * @return Promise containing the addon data. + */ + _getActiveAddons: Task.async(function* () { + // Request addons, asynchronously. + let allAddons = yield promiseGetAddonsByTypes(["extension", "service"]); + + let activeAddons = {}; + for (let addon of allAddons) { + // Skip addons which are not active. + if (!addon.isActive) { + continue; + } + + // Make sure to have valid dates. + let installDate = new Date(Math.max(0, addon.installDate)); + let updateDate = new Date(Math.max(0, addon.updateDate)); + + activeAddons[addon.id] = { + blocklisted: (addon.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED), + description: addon.description, + name: addon.name, + userDisabled: addon.userDisabled, + appDisabled: addon.appDisabled, + version: addon.version, + scope: addon.scope, + type: addon.type, + foreignInstall: addon.foreignInstall, + hasBinaryComponents: addon.hasBinaryComponents, + installDay: truncateToDays(installDate.getTime()), + updateDay: truncateToDays(updateDate.getTime()), + }; } - this._log = Log.repository.getLoggerWithMessagePrefix( - LOGGER_NAME, "TelemetryEnvironment::"); + + return activeAddons; + }), + + /** + * Get the currently active theme data in object form. + * @return Promise containing the active theme data. + */ + _getActiveTheme: Task.async(function* () { + // Request themes, asynchronously. + let themes = yield promiseGetAddonsByTypes(["theme"]); + + let activeTheme = {}; + // We only store information about the active theme. + let theme = themes.find(theme => theme.isActive); + if (theme) { + // Make sure to have valid dates. + let installDate = new Date(Math.max(0, theme.installDate)); + let updateDate = new Date(Math.max(0, theme.updateDate)); + + activeTheme = { + id: theme.id, + blocklisted: (theme.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED), + description: theme.description, + name: theme.name, + userDisabled: theme.userDisabled, + appDisabled: theme.appDisabled, + version: theme.version, + scope: theme.scope, + foreignInstall: theme.foreignInstall, + hasBinaryComponents: theme.hasBinaryComponents, + installDay: truncateToDays(installDate.getTime()), + updateDay: truncateToDays(updateDate.getTime()), + }; + } + + return activeTheme; + }), + + /** + * Get the plugins data in object form. + * @return Object containing the plugins data. + */ + _getActivePlugins: function () { + let pluginTags = + Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost).getPluginTags({}); + + let activePlugins = []; + for (let tag of pluginTags) { + // Skip plugins which are not active. + if (tag.disabled) { + continue; + } + + // Make sure to have a valid date. + let updateDate = new Date(Math.max(0, tag.lastModifiedTime)); + + activePlugins.push({ + name: tag.name, + version: tag.version, + description: tag.description, + blocklisted: tag.blocklisted, + disabled: tag.disabled, + clicktoplay: tag.clicktoplay, + mimeTypes: tag.getMimeTypes({}), + updateDay: truncateToDays(updateDate.getTime()), + }); + } + + return activePlugins; + }, + + /** + * Get the GMPlugins data in object form. + * @return Object containing the GMPlugins data. + * + * This should only be called from _pendingTask; otherwise we risk + * running this during addon manager shutdown. + */ + _getActiveGMPlugins: Task.async(function* () { + // Request plugins, asynchronously. + let allPlugins = yield promiseGetAddonsByTypes(["plugin"]); + + let activeGMPlugins = {}; + for (let plugin of allPlugins) { + // Only get GM Plugin info. + if (!plugin.isGMPlugin) { + continue; + } + + activeGMPlugins[plugin.id] = { + version: plugin.version, + userDisabled: plugin.userDisabled, + applyBackgroundUpdates: plugin.applyBackgroundUpdates, + }; + } + + return activeGMPlugins; + }), + + /** + * Get the active experiment data in object form. + * @return Object containing the active experiment data. + */ + _getActiveExperiment: function () { + let experimentInfo = {}; + try { + let scope = {}; + Cu.import("resource:///modules/experiments/Experiments.jsm", scope); + let experiments = scope.Experiments.instance(); + let activeExperiment = experiments.getActiveExperimentID(); + if (activeExperiment) { + experimentInfo.id = activeExperiment; + experimentInfo.branch = experiments.getActiveExperimentBranch(); + } + } catch(e) { + // If this is not Firefox, the import will fail. + } + + return experimentInfo; + }, +}; + +function EnvironmentCache() { + this._log = Log.repository.getLoggerWithMessagePrefix( + LOGGER_NAME, "TelemetryEnvironment::"); + this._log.trace("constructor"); + + this._shutdown = false; + + // A map of listeners that will be called on environment changes. + this._changeListeners = new Map(); + + // A map of watched preferences which trigger an Environment change when + // modified. Every entry contains a recording policy (RECORD_PREF_*). + this._watchedPrefs = DEFAULT_ENVIRONMENT_PREFS; + + this._currentEnvironment = { + build: this._getBuild(), + partner: this._getPartner(), + system: this._getSystem(), + }; + + this._updateSettings(); + +#ifndef MOZ_WIDGET_ANDROID + this._currentEnvironment.profile = {}; +#endif + + // Build the remaining asynchronous parts of the environment. Don't register change listeners + // until the initial environment has been built. + + this._addonBuilder = new EnvironmentAddonBuilder(this); + + this._initTask = Promise.all([this._addonBuilder.init(), this._updateProfile()]) + .then( + () => { + this._initTask = null; + this._startWatchingPrefs(); + this._addonBuilder.watchForChanges(); + return this.currentEnvironment; + }, + (err) => { + // log errors but eat them for consumers + this._log.error("error while initializing", err); + this._initTask = null; + this._startWatchingPrefs(); + this._addonBuilder.watchForChanges(); + return this.currentEnvironment; + }); +} +EnvironmentCache.prototype = { + /** + * The current environment data. The returned data is cloned to avoid + * unexpected sharing or mutation. + * @returns object + */ + get currentEnvironment() { + return Cu.cloneInto(this._currentEnvironment, myScope); + }, + + /** + * Wait for the current enviroment to be fully initialized. + * @returns Promise + */ + onInitialized: function() { + if (this._initTask) { + return this._initTask; + } + return Promise.resolve(this.currentEnvironment); }, /** * Register a listener for environment changes. - * It's fine to call this on an unitialized TelemetryEnvironment. - * @param name The name of the listener - good for debugging purposes. - * @param listener A JS callback function. + * @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) */ registerChangeListener: function (name, listener) { - this._configureLog(); this._log.trace("registerChangeListener for " + name); if (this._shutdown) { - this._log.warn("registerChangeListener - already shutdown") + this._log.warn("registerChangeListener - already shutdown"); return; } this._changeListeners.set(name, listener); @@ -318,10 +712,9 @@ this.TelemetryEnvironment = { * @param name The name of the listener to remove. */ unregisterChangeListener: function (name) { - this._configureLog(); this._log.trace("unregisterChangeListener for " + name); if (this._shutdown) { - this._log.warn("registerChangeListener - already shutdown") + this._log.warn("registerChangeListener - already shutdown"); return; } this._changeListeners.delete(name); @@ -332,11 +725,9 @@ this.TelemetryEnvironment = { * @param aPreferences A map of preferences names and their recording policy. */ _watchPreferences: function (aPreferences) { - if (this._watchedPrefs) { - this._stopWatchingPrefs(); - } - + this._stopWatchingPrefs(); this._watchedPrefs = aPreferences; + this._updateSettings(); this._startWatchingPrefs(); }, @@ -347,23 +738,19 @@ this.TelemetryEnvironment = { * @return An object containing the preferences values. */ _getPrefData: function () { - if (!this._watchedPrefs) { - return {}; - } - let prefData = {}; for (let pref in this._watchedPrefs) { // Only record preferences if they are non-default and policy allows recording. if (!Preferences.isSet(pref) || - this._watchedPrefs[pref] == this.RECORD_PREF_NOTIFY_ONLY) { + this._watchedPrefs[pref] == TelemetryEnvironment.RECORD_PREF_NOTIFY_ONLY) { continue; } // Check the policy for the preference and decide if we need to store its value // or whether it changed from the default value. let prefValue = undefined; - if (this._watchedPrefs[pref] == this.RECORD_PREF_STATE) { - prefValue = null; + if (this._watchedPrefs[pref] == TelemetryEnvironment.RECORD_PREF_STATE) { + prefValue = ""; } else { prefValue = Preferences.get(pref, null); } @@ -378,35 +765,26 @@ this.TelemetryEnvironment = { _startWatchingPrefs: function () { this._log.trace("_startWatchingPrefs - " + this._watchedPrefs); - if (!this._watchedPrefs) { - return; - } - for (let pref in this._watchedPrefs) { Preferences.observe(pref, this._onPrefChanged, this); } }, + _onPrefChanged: function() { + this._log.trace("_onPrefChanged"); + this._updateSettings(); + this._onEnvironmentChange("pref-changed"); + }, + /** * Do not receive any more change notifications for the preferences. */ _stopWatchingPrefs: function () { this._log.trace("_stopWatchingPrefs"); - if (!this._watchedPrefs) { - return; - } - for (let pref in this._watchedPrefs) { Preferences.ignore(pref, this._onPrefChanged, this); } - - this._watchedPrefs = null; - }, - - _onPrefChanged: function () { - this._log.trace("_onPrefChanged"); - this._onEnvironmentChange("pref-changed"); }, /** @@ -470,16 +848,15 @@ this.TelemetryEnvironment = { }, /** - * Get the settings data in object form. - * @return Object containing the settings. + * Update the cached settings data. */ - _getSettings: function () { + _updateSettings: function () { let updateChannel = null; try { updateChannel = UpdateChannel.get(); } catch (e) {} - return { + this._currentEnvironment.settings = { blocklistEnabled: Preferences.get(PREF_BLOCKLIST_ENABLED, true), #ifndef MOZ_WIDGET_ANDROID isDefaultBrowser: this._isDefaultBrowser(), @@ -497,23 +874,20 @@ this.TelemetryEnvironment = { }, /** - * Get the profile data in object form. - * @return Object containing the profile data. + * Update the cached profile data. + * @returns Promise<> resolved when the I/O is complete. */ - _getProfile: Task.async(function* () { + _updateProfile: Task.async(function* () { let profileAccessor = new ProfileAge(null, this._log); let creationDate = yield profileAccessor.created; let resetDate = yield profileAccessor.reset; - let profileData = { - creationDate: truncateToDays(creationDate), - }; - + this._currentEnvironment.profile.creationDate = + truncateToDays(creationDate); if (resetDate) { - profileData.resetDate = truncateToDays(resetDate); + this._currentEnvironment.profile.resetDate = truncateToDays(resetDate); } - return profileData; }), /** @@ -531,7 +905,7 @@ this.TelemetryEnvironment = { // Get the PREF_APP_PARTNER_BRANCH branch and append its children to partner data. let partnerBranch = Services.prefs.getBranch(PREF_APP_PARTNER_BRANCH); - partnerData.partnerNames = partnerBranch.getChildList("") + partnerData.partnerNames = partnerBranch.getChildList(""); return partnerData; }, @@ -685,349 +1059,6 @@ this.TelemetryEnvironment = { }; }, - /** - * Get the addon data in object form. - * @return Object containing the addon data. - * - * This should only be called from the environment collection task - * or _blockAddonManagerShutdown, otherwise we risk running this - * during addon manager shutdown. - */ - _getActiveAddons: Task.async(function* () { - - // Request addons, asynchronously. - let allAddons = yield promiseGetAddonsByTypes(["extension", "service"]); - - let activeAddons = {}; - for (let addon of allAddons) { - // Skip addons which are not active. - if (!addon.isActive) { - continue; - } - - // Make sure to have valid dates. - let installDate = new Date(Math.max(0, addon.installDate)); - let updateDate = new Date(Math.max(0, addon.updateDate)); - - activeAddons[addon.id] = { - blocklisted: (addon.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED), - description: addon.description, - name: addon.name, - userDisabled: addon.userDisabled, - appDisabled: addon.appDisabled, - version: addon.version, - scope: addon.scope, - type: addon.type, - foreignInstall: addon.foreignInstall, - hasBinaryComponents: addon.hasBinaryComponents, - installDay: truncateToDays(installDate.getTime()), - updateDay: truncateToDays(updateDate.getTime()), - }; - } - - return activeAddons; - }), - - /** - * Get the currently active theme data in object form. - * @return Object containing the active theme data. - * - * This should only be called from the environment collection task - * or _blockAddonManagerShutdown, otherwise we risk running this - * during addon manager shutdown. - */ - _getActiveTheme: Task.async(function* () { - // Request themes, asynchronously. - let themes = yield promiseGetAddonsByTypes(["theme"]); - - let activeTheme = {}; - // We only store information about the active theme. - let theme = themes.find(theme => theme.isActive); - if (theme) { - // Make sure to have valid dates. - let installDate = new Date(Math.max(0, theme.installDate)); - let updateDate = new Date(Math.max(0, theme.updateDate)); - - activeTheme = { - id: theme.id, - blocklisted: (theme.blocklistState !== Ci.nsIBlocklistService.STATE_NOT_BLOCKED), - description: theme.description, - name: theme.name, - userDisabled: theme.userDisabled, - appDisabled: theme.appDisabled, - version: theme.version, - scope: theme.scope, - foreignInstall: theme.foreignInstall, - hasBinaryComponents: theme.hasBinaryComponents, - installDay: truncateToDays(installDate.getTime()), - updateDay: truncateToDays(updateDate.getTime()), - }; - } - - return activeTheme; - }), - - /** - * Get the plugins data in object form. - * @return Object containing the plugins data. - */ - _getActivePlugins: function () { - let pluginTags = - Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost).getPluginTags({}); - - let activePlugins = []; - for (let tag of pluginTags) { - // Skip plugins which are not active. - if (tag.disabled) { - continue; - } - - // Make sure to have a valid date. - let updateDate = new Date(Math.max(0, tag.lastModifiedTime)); - - activePlugins.push({ - name: tag.name, - version: tag.version, - description: tag.description, - blocklisted: tag.blocklisted, - disabled: tag.disabled, - clicktoplay: tag.clicktoplay, - mimeTypes: tag.getMimeTypes({}), - updateDay: truncateToDays(updateDate.getTime()), - }); - } - - return activePlugins; - }, - - /** - * Get the GMPlugins data in object form. - * @return Object containing the GMPlugins data. - * - * This should only be called from the environment collection task - * or _blockAddonManagerShutdown, otherwise we risk running this - * during addon manager shutdown. - */ - _getActiveGMPlugins: Task.async(function* () { - // Request plugins, asynchronously. - let allPlugins = yield promiseGetAddonsByTypes(["plugin"]); - - let activeGMPlugins = {}; - for (let plugin of allPlugins) { - // Only get GM Plugin info. - if (!plugin.isGMPlugin) { - continue; - } - - activeGMPlugins[plugin.id] = { - version: plugin.version, - userDisabled: plugin.userDisabled, - applyBackgroundUpdates: plugin.applyBackgroundUpdates, - }; - } - - return activeGMPlugins; - }), - - /** - * Get the active experiment data in object form. - * @return Object containing the active experiment data. - */ - _getActiveExperiment: function () { - let experimentInfo = {}; - try { - let scope = {}; - Cu.import("resource:///modules/experiments/Experiments.jsm", scope); - let experiments = scope.Experiments.instance() - let activeExperiment = experiments.getActiveExperimentID(); - if (activeExperiment) { - experimentInfo.id = activeExperiment; - experimentInfo.branch = experiments.getActiveExperimentBranch(); - } - } catch(e) { - // If this is not Firefox, the import will fail. - return experimentInfo; - } - - return experimentInfo; - }, - - /** - * Get the addon data in object form. - * @return Object containing the addon data. - * - * This should only be called from the environment collection task - * or _blockAddonManagerShutdown, otherwise we risk running this - * during addon manager shutdown. - */ - _getAddons: Task.async(function* () { - // AddonManager may have shutdown already, in which case we should have cached addon data. - // It can't shutdown during the collection here because we have a blocker on the AMs - // shutdown barrier that waits for the collect task. - let addons = this._cachedAddons || {}; - if (!this._cachedAddons) { - addons.activeAddons = yield this._getActiveAddons(); - addons.activeTheme = yield this._getActiveTheme(); - addons.activeGMPlugins = yield this._getActiveGMPlugins(); - } - - let personaId = null; -#ifndef MOZ_WIDGET_GONK - let theme = LightweightThemeManager.currentTheme; - if (theme) { - personaId = theme.id; - } -#endif - - let addonData = { - activeAddons: addons.activeAddons, - theme: addons.activeTheme, - activePlugins: this._getActivePlugins(), - activeGMPlugins: addons.activeGMPlugins, - activeExperiment: this._getActiveExperiment(), - persona: personaId, - }; - - return addonData; - }), - - /** - * Start watching the addons for changes. - */ - _startWatchingAddons: function () { - // Define a listener to catch addons changes from the AddonManager. This part is - // tricky, as we only want to detect when the set of active addons changes without - // getting double notifications. - // - // We identified the following cases: - // - // * onEnabled: Gets called when a restartless addon is enabled. Doesn't get called - // if the restartless addon is installed and directly enabled. - // * onDisabled: Gets called when disabling a restartless addon or can get called when - // uninstalling a restartless addon from the UI (see bug 612168). - // * onInstalled: Gets called for all addon installs. - // * onUninstalling: Called the moment before addon uninstall happens. - - this._addonsListener = { - onEnabled: addon => { - this._log.trace("_addonsListener - onEnabled " + addon.id); - this._onActiveAddonsChanged(addon) - }, - onDisabled: addon => { - this._log.trace("_addonsListener - onDisabled " + addon.id); - this._onActiveAddonsChanged(addon); - }, - onInstalled: addon => { - this._log.trace("_addonsListener - onInstalled " + addon.id + - ", isActive: " + addon.isActive); - if (addon.isActive) { - this._onActiveAddonsChanged(addon); - } - }, - onUninstalling: (addon, requiresRestart) => { - this._log.trace("_addonsListener - onUninstalling " + addon.id + - ", isActive: " + addon.isActive + - ", requiresRestart: " + requiresRestart); - if (!addon.isActive || requiresRestart) { - return; - } - this._onActiveAddonsChanged(addon); - }, - }; - - AddonManager.addAddonListener(this._addonsListener); - - // Watch for experiment changes as well. - Services.obs.addObserver(this, EXPERIMENTS_CHANGED_TOPIC, false); - }, - - /** - * Stop watching addons for changes. - */ - _stopWatchingAddons: function () { - if (this._addonsListener) { - AddonManager.removeAddonListener(this._addonsListener); - Services.obs.removeObserver(this, EXPERIMENTS_CHANGED_TOPIC); - } - this._addonsListener = null; - }, - - /** - * Triggered when an addon changes its state. - * @param aAddon The addon which triggered the change. - */ - _onActiveAddonsChanged: function (aAddon) { - const INTERESTING_ADDONS = [ "extension", "plugin", "service", "theme" ]; - - this._log.trace("_onActiveAddonsChanged - id " + aAddon.id + ", type " + aAddon.type); - - if (INTERESTING_ADDONS.find(addon => addon == aAddon.type)) { - this._onEnvironmentChange("addons-changed"); - } - }, - - /** - * Handle experiment change notifications. - */ - observe: function (aSubject, aTopic, aData) { - this._log.trace("observe - Topic " + aTopic); - - if (aTopic == EXPERIMENTS_CHANGED_TOPIC) { - this._onEnvironmentChange("experiment-changed"); - } - }, - - /** - * Get the environment data in object form. - * @return Promise Resolved with the data on success, otherwise rejected. - */ - getEnvironmentData: function() { - if (this._shutdown) { - this._log.error("getEnvironmentData - Already shut down"); - return Promise.reject("Already shutdown"); - } - - this._log.trace("getEnvironmentData"); - if (this._collectTask) { - return this._collectTask; - } - - this._collectTask = this._doGetEnvironmentData(); - let clear = () => this._collectTask = null; - this._collectTask.then(clear, clear); - return this._collectTask; - }, - - _doGetEnvironmentData: Task.async(function* () { - this._log.trace("getEnvironmentData"); - - // Define the data collection function for each section. - let sections = { - "build" : () => this._getBuild(), - "settings": () => this._getSettings(), -#ifndef MOZ_WIDGET_ANDROID - "profile": () => this._getProfile(), -#endif - "partner": () => this._getPartner(), - "system": () => this._getSystem(), - "addons": () => this._getAddons(), - }; - - let data = {}; - // Recover from exceptions in the collection functions and populate the data - // object. We want to recover so that, in the worst-case, we only lose some data - // sections instead of all. - for (let s in sections) { - try { - data[s] = yield sections[s](); - } catch (e) { - this._log.error("_doGetEnvironmentData - There was an exception collecting " + s, e); - } - } - - return data; - }), - _onEnvironmentChange: function (what) { this._log.trace("_onEnvironmentChange for " + what); if (this._shutdown) { @@ -1038,40 +1069,10 @@ this.TelemetryEnvironment = { for (let [name, listener] of this._changeListeners) { try { this._log.debug("_onEnvironmentChange - calling " + name); - listener(); + listener(what, this.currentEnvironment); } catch (e) { - this._log.warning("_onEnvironmentChange - listener " + name + " caught error", e); + this._log.error("_onEnvironmentChange - listener " + name + " caught error", e); } } }, - - /** - * This blocks the AddonManager shutdown barrier, it caches addons we might need later. - * It also lets an active collect task finish first as it may still access the AM. - */ - _blockAddonManagerShutdown: Task.async(function*() { - this._log.trace("_blockAddonManagerShutdown"); - - this._stopWatchingAddons(); - - this._cachedAddons = { - activeAddons: yield this._getActiveAddons(), - activeTheme: yield this._getActiveTheme(), - activeGMPlugins: yield this._getActiveGMPlugins(), - }; - - yield this._collectTask; - }), - - /** - * Get an object describing the current state of this module for AsyncShutdown diagnostics. - */ - _getState: function() { - return { - shutdown: this._shutdown, - hasCollectTask: !!this._collectTask, - hasAddonsListener: !!this._addonsListener, - hasCachedAddons: !!this._cachedAddons, - }; - }, }; diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index 8dbc74b6719..ef5053cdddd 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -350,16 +350,10 @@ let Impl = { } if (aOptions.addEnvironment) { - return TelemetryEnvironment.getEnvironmentData().then(environment => { - pingData.environment = environment; - return pingData; - }, - error => { - this._log.error("assemblePing - Rejection", error); - }); + pingData.environment = TelemetryEnvironment.currentEnvironment; } - return Promise.resolve(pingData); + return pingData; }, popPayloads: function popPayloads() { @@ -427,19 +421,16 @@ let Impl = { this._log.trace("send - Type " + aType + ", Server " + this._server + ", aOptions " + JSON.stringify(aOptions)); - let promise = this.assemblePing(aType, aPayload, aOptions) - .then(pingData => { - // Once ping is assembled, send it along with the persisted ping in the backlog. - let p = [ - // Persist the ping if sending it fails. - this.doPing(pingData, false) - .catch(() => TelemetryFile.savePing(pingData, true)), - this.sendPersistedPings(), - ]; - return Promise.all(p); - }, - error => this._log.error("send - Rejection", error)); + let pingData = this.assemblePing(aType, aPayload, aOptions); + // Once ping is assembled, send it along with the persisted pings in the backlog. + let p = [ + // Persist the ping if sending it fails. + this.doPing(pingData, false) + .catch(() => TelemetryFile.savePing(pingData, true)), + this.sendPersistedPings(), + ]; + let promise = Promise.all(p); this._trackPendingPingTask(promise); return promise; }, @@ -481,9 +472,8 @@ let Impl = { this._log.trace("savePendingPings - Type " + aType + ", Server " + this._server + ", aOptions " + JSON.stringify(aOptions)); - return this.assemblePing(aType, aPayload, aOptions) - .then(pingData => TelemetryFile.savePendingPings(pingData), - error => this._log.error("savePendingPings - Rejection", error)); + let pingData = this.assemblePing(aType, aPayload, aOptions); + return TelemetryFile.savePendingPings(pingData); }, /** @@ -509,16 +499,14 @@ let Impl = { this._log.trace("savePing - Type " + aType + ", Server " + this._server + ", aOptions " + JSON.stringify(aOptions)); - return this.assemblePing(aType, aPayload, aOptions) - .then(pingData => { - if ("filePath" in aOptions) { - return TelemetryFile.savePingToFile(pingData, aOptions.filePath, aOptions.overwrite) - .then(() => { return pingData.id; }); - } else { - return TelemetryFile.savePing(pingData, aOptions.overwrite) - .then(() => { return pingData.id; }); - } - }, error => this._log.error("savePing - Rejection", error)); + let pingData = this.assemblePing(aType, aPayload, aOptions); + if ("filePath" in aOptions) { + return TelemetryFile.savePingToFile(pingData, aOptions.filePath, aOptions.overwrite) + .then(() => { return pingData.id; }); + } else { + return TelemetryFile.savePing(pingData, aOptions.overwrite) + .then(() => { return pingData.id; }); + } }, onPingRequestFinished: function(success, startTime, ping, isPersisted) { @@ -775,8 +763,6 @@ let Impl = { try { this._initialized = true; - yield TelemetryEnvironment.init(); - yield TelemetryFile.loadSavedPings(); // If we have any TelemetryPings lying around, we'll be aggressive // and try to send them all off ASAP. @@ -842,13 +828,6 @@ let Impl = { yield this._shutdownBarrier.wait(); // Then wait for any outstanding async ping activity. yield this._connectionsBarrier.wait(); - - // Should down dependent components. - try { - yield TelemetryEnvironment.shutdown(); - } catch (e) { - this._log.error("_cleanupOnShutdown - environment shutdown failure", e); - } } finally { // Reset state. this._initialized = false; diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 34a91264cfa..ea8f0b397e5 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1517,7 +1517,7 @@ let Impl = { yield this._checkAbortedSessionPing(); TelemetryEnvironment.registerChangeListener(ENVIRONMENT_CHANGE_LISTENER, - () => this._onEnvironmentChange()); + (reason, data) => this._onEnvironmentChange(reason, data)); // Write the first aborted-session ping as early as possible. Just do that // if we are not testing, since calling Telemetry.reset() will make a previous // aborted ping a pending ping. @@ -1531,7 +1531,7 @@ let Impl = { this._delayedInitTaskDeferred.resolve(); } catch (e) { - this._delayedInitTaskDeferred.reject(); + this._delayedInitTaskDeferred.reject(e); } finally { this._delayedInitTask = null; this._delayedInitTaskDeferred = null; @@ -1983,8 +1983,8 @@ let Impl = { } }), - _onEnvironmentChange: function() { - this._log.trace("_onEnvironmentChange"); + _onEnvironmentChange: function(reason, data) { + this._log.trace("_onEnvironmentChange", reason); let payload = this.getSessionPayload(REASON_ENVIRONMENT_CHANGE, true); let clonedPayload = Cu.cloneInto(payload, myScope); diff --git a/toolkit/components/telemetry/docs/environment.rst b/toolkit/components/telemetry/docs/environment.rst index c8e74a0f168..6b15b225061 100644 --- a/toolkit/components/telemetry/docs/environment.rst +++ b/toolkit/components/telemetry/docs/environment.rst @@ -36,8 +36,11 @@ Structure:: autoDownload: , // true on failure }, userPrefs: { - // Two possible behaviours: values of the whitelisted prefs, or for some prefs we - // only record they are present with value being set to null. + // Only prefs which are changed from the default value are listed + // in this block + "pref.name.value": value // some prefs send the value + "pref.name.url": "" // For some privacy-sensitive prefs + // only the fact that the value has been changed is recorded }, }, profile: { // This section is not available on Android. @@ -179,3 +182,8 @@ Structure:: persona: , // 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 diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js index 77e1900c476..7b620ce5e11 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js @@ -601,69 +601,16 @@ add_task(function* asyncSetup() { yield spoofProfileReset(); }); -add_task(function* test_initAndShutdown() { - // Check that init and shutdown work properly. - TelemetryEnvironment.init(); - yield TelemetryEnvironment.shutdown(); - TelemetryEnvironment.init(); - yield TelemetryEnvironment.shutdown(); - - // A double init should be silently handled. - TelemetryEnvironment.init(); - TelemetryEnvironment.init(); - - // getEnvironmentData should return a sane result. - let data = yield TelemetryEnvironment.getEnvironmentData(); - Assert.ok(!!data); - - // The change listener registration should silently fail after shutdown. - yield TelemetryEnvironment.shutdown(); - TelemetryEnvironment.registerChangeListener("foo", () => {}); - TelemetryEnvironment.unregisterChangeListener("foo"); - - // Shutting down again should be ignored. - yield TelemetryEnvironment.shutdown(); - - // Getting the environment data should reject after shutdown. - Assert.ok(yield isRejected(TelemetryEnvironment.getEnvironmentData())); -}); - -add_task(function* test_changeNotify() { - TelemetryEnvironment.init(); - - // Register some listeners - let results = new Array(4).fill(false); - for (let i=0; i results[k] = true); - } - // Trigger environment change notifications. - // TODO: test with proper environment changes, not directly. - TelemetryEnvironment._onEnvironmentChange("foo"); - Assert.ok(results.every(val => val), "All change listeners should have been notified."); - results.fill(false); - TelemetryEnvironment._onEnvironmentChange("bar"); - Assert.ok(results.every(val => val), "All change listeners should have been notified."); - - // Unregister listeners - for (let i=0; i<4; ++i) { - TelemetryEnvironment.unregisterChangeListener("test"+i); - } -}); - add_task(function* test_checkEnvironment() { - yield TelemetryEnvironment.init(); - let environmentData = yield TelemetryEnvironment.getEnvironmentData(); - + let environmentData = yield TelemetryEnvironment.onInitialized(); checkEnvironmentData(environmentData); - - yield TelemetryEnvironment.shutdown(); }); add_task(function* test_prefWatchPolicies() { const PREF_TEST_1 = "toolkit.telemetry.test.pref_new"; const PREF_TEST_2 = "toolkit.telemetry.test.pref1"; const PREF_TEST_3 = "toolkit.telemetry.test.pref2"; + const PREF_TEST_4 = "toolkit.telemetry.test.pref_old"; const expectedValue = "some-test-value"; @@ -671,12 +618,18 @@ add_task(function* test_prefWatchPolicies() { prefsToWatch[PREF_TEST_1] = TelemetryEnvironment.RECORD_PREF_VALUE; prefsToWatch[PREF_TEST_2] = TelemetryEnvironment.RECORD_PREF_STATE; prefsToWatch[PREF_TEST_3] = TelemetryEnvironment.RECORD_PREF_STATE; + prefsToWatch[PREF_TEST_4] = TelemetryEnvironment.RECORD_PREF_VALUE; - yield TelemetryEnvironment.init(); + Preferences.set(PREF_TEST_4, expectedValue); // Set the Environment preferences to watch. TelemetryEnvironment._watchPreferences(prefsToWatch); let deferred = PromiseUtils.defer(); + + // Check that the pref values are missing or present as expected + 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); // Trigger a change in the watched preferences. @@ -688,18 +641,14 @@ add_task(function* test_prefWatchPolicies() { TelemetryEnvironment.unregisterChangeListener("testWatchPrefs"); // Check environment contains the correct data. - let environmentData = yield TelemetryEnvironment.getEnvironmentData(); - - let userPrefs = environmentData.settings.userPrefs; + let userPrefs = TelemetryEnvironment.currentEnvironment.settings.userPrefs; Assert.equal(userPrefs[PREF_TEST_1], expectedValue, "Environment contains the correct preference value."); - Assert.equal(userPrefs[PREF_TEST_2], null, - "Report that the pref was user set and has no value."); + Assert.equal(userPrefs[PREF_TEST_2], "", + "Report that the pref was user set but the value is not shown."); Assert.ok(!(PREF_TEST_3 in userPrefs), "Do not report if preference not user set."); - - yield TelemetryEnvironment.shutdown(); }); add_task(function* test_prefWatch_prefReset() { @@ -710,20 +659,21 @@ add_task(function* test_prefWatch_prefReset() { // Set the preference to a non-default value. Preferences.set(PREF_TEST, false); - yield TelemetryEnvironment.init(); - // Set the Environment preferences to watch. TelemetryEnvironment._watchPreferences(prefsToWatch); let deferred = PromiseUtils.defer(); TelemetryEnvironment.registerChangeListener("testWatchPrefs_reset", deferred.resolve); + Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST], ""); + // Trigger a change in the watched preferences. Preferences.reset(PREF_TEST); yield deferred.promise; + Assert.strictEqual(TelemetryEnvironment.currentEnvironment.settings.userPrefs[PREF_TEST], undefined); + // Unregister the listener. TelemetryEnvironment.unregisterChangeListener("testWatchPrefs_reset"); - yield TelemetryEnvironment.shutdown(); }); add_task(function* test_addonsWatch_InterestingChange() { @@ -732,13 +682,13 @@ add_task(function* test_addonsWatch_InterestingChange() { // We only expect a single notification for each install, uninstall, enable, disable. const EXPECTED_NOTIFICATIONS = 4; - yield TelemetryEnvironment.init(); let deferred = PromiseUtils.defer(); let receivedNotifications = 0; let registerCheckpointPromise = (aExpected) => { return new Promise(resolve => TelemetryEnvironment.registerChangeListener( - "testWatchAddons_Changes" + aExpected, () => { + "testWatchAddons_Changes" + aExpected, (reason, data) => { + Assert.equal(reason, "addons-changed"); receivedNotifications++; resolve(); })); @@ -754,24 +704,26 @@ add_task(function* test_addonsWatch_InterestingChange() { yield AddonTestUtils.installXPIFromURL(ADDON_INSTALL_URL); yield checkpointPromise; assertCheckpoint(1); - + Assert.ok(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons); + checkpointPromise = registerCheckpointPromise(2); let addon = yield AddonTestUtils.getAddonById(ADDON_ID); addon.userDisabled = true; yield checkpointPromise; assertCheckpoint(2); + Assert.ok(!(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons)); checkpointPromise = registerCheckpointPromise(3); addon.userDisabled = false; yield checkpointPromise; assertCheckpoint(3); + Assert.ok(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons); checkpointPromise = registerCheckpointPromise(4); yield AddonTestUtils.uninstallAddonByID(ADDON_ID); yield checkpointPromise; assertCheckpoint(4); - - yield TelemetryEnvironment.shutdown(); + Assert.ok(!(ADDON_ID in TelemetryEnvironment.currentEnvironment.addons.activeAddons)); Assert.equal(receivedNotifications, EXPECTED_NOTIFICATIONS, "We must only receive the notifications we expect."); @@ -783,15 +735,16 @@ add_task(function* test_pluginsWatch_Add() { return; } - yield TelemetryEnvironment.init(); + Assert.equal(TelemetryEnvironment.currentEnvironment.addons.activePlugins.length, 1); let newPlugin = new PluginTag(PLUGIN2_NAME, PLUGIN2_DESC, PLUGIN2_VERSION, true); gInstalledPlugins.push(newPlugin); let deferred = PromiseUtils.defer(); let receivedNotifications = 0; - let callback = () => { + let callback = (reason, data) => { receivedNotifications++; + Assert.equal(reason, "addons-changed"); deferred.resolve(); }; TelemetryEnvironment.registerChangeListener("testWatchPlugins_Add", callback); @@ -799,8 +752,9 @@ add_task(function* test_pluginsWatch_Add() { Services.obs.notifyObservers(null, PLUGIN_UPDATED_TOPIC, null); yield deferred.promise; + Assert.equal(TelemetryEnvironment.currentEnvironment.addons.activePlugins.length, 2); + TelemetryEnvironment.unregisterChangeListener("testWatchPlugins_Add"); - yield TelemetryEnvironment.shutdown(); Assert.equal(receivedNotifications, 1, "We must only receive one notification."); }); @@ -811,8 +765,6 @@ add_task(function* test_pluginsWatch_Remove() { return; } - yield TelemetryEnvironment.init(); - // Find the test plugin. let plugin = gInstalledPlugins.find(plugin => (plugin.name == PLUGIN2_NAME)); Assert.ok(plugin, "The test plugin must exist."); @@ -832,7 +784,6 @@ add_task(function* test_pluginsWatch_Remove() { yield deferred.promise; TelemetryEnvironment.unregisterChangeListener("testWatchPlugins_Remove"); - yield TelemetryEnvironment.shutdown(); Assert.equal(receivedNotifications, 1, "We must only receive one notification."); }); @@ -841,19 +792,25 @@ add_task(function* test_addonsWatch_NotInterestingChange() { // We are not interested to dictionary addons changes. const DICTIONARY_ADDON_INSTALL_URL = gDataRoot + "dictionary.xpi"; const INTERESTING_ADDON_INSTALL_URL = gDataRoot + "restartless.xpi"; - yield TelemetryEnvironment.init(); - let receivedNotifications = 0; + let receivedNotification = false; + let deferred = PromiseUtils.defer(); TelemetryEnvironment.registerChangeListener("testNotInteresting", - () => receivedNotifications++); + () => { + Assert.ok(!receivedNotification, "Should not receive multiple notifications"); + receivedNotification = true; + deferred.resolve(); + }); yield AddonTestUtils.installXPIFromURL(DICTIONARY_ADDON_INSTALL_URL); yield AddonTestUtils.installXPIFromURL(INTERESTING_ADDON_INSTALL_URL); - Assert.equal(receivedNotifications, 1, "We must receive only one notification."); + yield deferred.promise; + Assert.ok(!("telemetry-dictionary@tests.mozilla.org" in + TelemetryEnvironment.currentEnvironment.addons.activeAddons), + "Dictionaries should not appear in active addons."); TelemetryEnvironment.unregisterChangeListener("testNotInteresting"); - yield TelemetryEnvironment.shutdown(); }); add_task(function* test_addonsAndPlugins() { @@ -884,12 +841,10 @@ add_task(function* test_addonsAndPlugins() { clicktoplay: true, }; - yield TelemetryEnvironment.init(); - // Install an addon so we have some data. yield AddonTestUtils.installXPIFromURL(ADDON_INSTALL_URL); - let data = yield TelemetryEnvironment.getEnvironmentData(); + let data = TelemetryEnvironment.currentEnvironment; checkEnvironmentData(data); // Check addon data. @@ -919,8 +874,6 @@ add_task(function* test_addonsAndPlugins() { let personaId = (gIsGonk) ? null : PERSONA_ID; Assert.equal(data.addons.persona, personaId, "The correct Persona Id must be reported."); - - yield TelemetryEnvironment.shutdown(); }); add_task(function*() { diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index b5f0d862377..5ae8941690a 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -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 + 2; + const expectedSubsessions = sessionState.profileSubsessionCounter + 3; const expectedUUID = "009fd1ad-b85e-4817-b3e5-000000003785"; fakeGenerateUUID(generateUUID, () => expectedUUID); @@ -1246,6 +1246,8 @@ add_task(function* test_savedSessionData() { // Start TelemetrySession so that it loads the session data file. yield TelemetrySession.reset(); // Watch a test preference, trigger and environment change and wait for it to propagate. + + // _watchPreferences triggers a subsession notification TelemetryEnvironment._watchPreferences(prefsToWatch); let changePromise = new Promise(resolve => TelemetryEnvironment.registerChangeListener("test_fake_change", resolve)); From b8710a771d19aa657d5f8fd3cdc585bbfaddf97f Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 10/52] Bug 1140558 - Part 2 - Make the testing deepEqual implementation shared properly in ObjectUtils.jsm. r=yoric --- testing/modules/Assert.jsm | 89 +----------- testing/modules/tests/xpcshell/test_assert.js | 18 --- toolkit/modules/ObjectUtils.jsm | 131 ++++++++++++++++++ toolkit/modules/moz.build | 1 + .../tests/xpcshell/test_ObjectUtils.js | 92 ++++++++++++ toolkit/modules/tests/xpcshell/xpcshell.ini | 1 + 6 files changed, 227 insertions(+), 105 deletions(-) create mode 100644 toolkit/modules/ObjectUtils.jsm create mode 100644 toolkit/modules/tests/xpcshell/test_ObjectUtils.js diff --git a/testing/modules/Assert.jsm b/testing/modules/Assert.jsm index 2693828382b..923ca92ac88 100644 --- a/testing/modules/Assert.jsm +++ b/testing/modules/Assert.jsm @@ -17,6 +17,7 @@ this.EXPORTED_SYMBOLS = [ ]; Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); +Components.utils.import("resource://gre/modules/ObjectUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "Promise", "resource://gre/modules/Promise.jsm"); @@ -258,95 +259,9 @@ proto.notEqual = function notEqual(actual, expected, message) { * (string) Short explanation of the expected result */ proto.deepEqual = function deepEqual(actual, expected, message) { - this.report(!_deepEqual(actual, expected), actual, expected, message, "deepEqual"); + this.report(!ObjectUtils.deepEqual(actual, expected), actual, expected, message, "deepEqual"); }; -function _deepEqual(actual, expected) { - // 7.1. All identical values are equivalent, as determined by ===. - if (actual === expected) { - return true; - // 7.2. If the expected value is a Date object, the actual value is - // equivalent if it is also a Date object that refers to the same time. - } else if (instanceOf(actual, "Date") && instanceOf(expected, "Date")) { - if (isNaN(actual.getTime()) && isNaN(expected.getTime())) - return true; - return actual.getTime() === expected.getTime(); - // 7.3 If the expected value is a RegExp object, the actual value is - // equivalent if it is also a RegExp object with the same source and - // properties (`global`, `multiline`, `lastIndex`, `ignoreCase`). - } else if (instanceOf(actual, "RegExp") && instanceOf(expected, "RegExp")) { - return actual.source === expected.source && - actual.global === expected.global && - actual.multiline === expected.multiline && - actual.lastIndex === expected.lastIndex && - actual.ignoreCase === expected.ignoreCase; - // 7.4. Other pairs that do not both pass typeof value == "object", - // equivalence is determined by ==. - } else if (typeof actual != "object" && typeof expected != "object") { - return actual == expected; - // 7.5 For all other Object pairs, including Array objects, equivalence is - // determined by having the same number of owned properties (as verified - // with Object.prototype.hasOwnProperty.call), the same set of keys - // (although not necessarily the same order), equivalent values for every - // corresponding key, and an identical 'prototype' property. Note: this - // accounts for both named and indexed properties on Arrays. - } else { - return objEquiv(actual, expected); - } -} - -function isUndefinedOrNull(value) { - return value === null || value === undefined; -} - -function isArguments(object) { - return instanceOf(object, "Arguments"); -} - -function objEquiv(a, b) { - if (isUndefinedOrNull(a) || isUndefinedOrNull(b)) { - return false; - } - // An identical 'prototype' property. - if (a.prototype !== b.prototype) { - return false; - } - // Object.keys may be broken through screwy arguments passing. Converting to - // an array solves the problem. - if (isArguments(a)) { - if (!isArguments(b)) { - return false; - } - a = pSlice.call(a); - b = pSlice.call(b); - return _deepEqual(a, b); - } - let ka, kb, key, i; - try { - ka = Object.keys(a); - kb = Object.keys(b); - } catch (e) { - // Happens when one is a string literal and the other isn't - return false; - } - // Having the same number of owned properties (keys incorporates - // hasOwnProperty) - if (ka.length != kb.length) - return false; - // The same set of keys (although not necessarily the same order), - ka.sort(); - kb.sort(); - // Equivalent values for every corresponding key, and possibly expensive deep - // test - for (i = ka.length - 1; i >= 0; i--) { - key = ka[i]; - if (!_deepEqual(a[key], b[key])) { - return false; - } - } - return true; -} - /** * 8. The non-equivalence assertion tests for any deep inequality. * assert.notDeepEqual(actual, expected, message_opt); diff --git a/testing/modules/tests/xpcshell/test_assert.js b/testing/modules/tests/xpcshell/test_assert.js index c36aed2014c..60a2c582ab2 100644 --- a/testing/modules/tests/xpcshell/test_assert.js +++ b/testing/modules/tests/xpcshell/test_assert.js @@ -209,24 +209,6 @@ function run_test() { } }); - // Make sure deepEqual doesn't loop forever on circular refs - - let b = {}; - b.b = b; - - let c = {}; - c.b = c; - - let gotError = false; - try { - assert.deepEqual(b, c); - } catch (e) { - gotError = true; - } - - dump("All OK\n"); - assert.ok(gotError); - function testAssertionMessage(actual, expected) { try { assert.equal(actual, ""); diff --git a/toolkit/modules/ObjectUtils.jsm b/toolkit/modules/ObjectUtils.jsm new file mode 100644 index 00000000000..3a7a39237a4 --- /dev/null +++ b/toolkit/modules/ObjectUtils.jsm @@ -0,0 +1,131 @@ +/* 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/. */ + +// Portions of this file are originally from narwhal.js (http://narwhaljs.org) +// Copyright (c) 2009 Thomas Robinson <280north.com> +// MIT license: http://opensource.org/licenses/MIT + +"use strict"; + +this.EXPORTED_SYMBOLS = [ + "ObjectUtils" +]; + +const {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components; + +this.ObjectUtils = { + /** + * This tests objects & values for deep equality. + * + * We check using the most exact approximation of equality between two objects + * to keep the chance of false positives to a minimum. + * `JSON.stringify` is not designed to be used for this purpose; objects may + * have ambiguous `toJSON()` implementations that would influence the test. + * + * @param a (mixed) Object or value to be compared. + * @param b (mixed) Object or value to be compared. + * @return Boolean Whether the objects are deep equal. + */ + deepEqual: function(a, b) { + return _deepEqual(a, b); + }, +}; + +// ... Start of previously MIT-licensed code. +// This deepEqual implementation is originally from narwhal.js (http://narwhaljs.org) +// Copyright (c) 2009 Thomas Robinson <280north.com> +// MIT license: http://opensource.org/licenses/MIT + +function _deepEqual(a, b) { + // The numbering below refers to sections in the CommonJS spec. + + // 7.1 All identical values are equivalent, as determined by ===. + if (a === b) { + return true; + // 7.2 If the b value is a Date object, the a value is + // equivalent if it is also a Date object that refers to the same time. + } else if (instanceOf(a, "Date") && instanceOf(b, "Date")) { + if (isNaN(a.getTime()) && isNaN(b.getTime())) + return true; + return a.getTime() === b.getTime(); + // 7.3 If the b value is a RegExp object, the a value is + // equivalent if it is also a RegExp object with the same source and + // properties (`global`, `multiline`, `lastIndex`, `ignoreCase`). + } else if (instanceOf(a, "RegExp") && instanceOf(b, "RegExp")) { + return a.source === b.source && + a.global === b.global && + a.multiline === b.multiline && + a.lastIndex === b.lastIndex && + a.ignoreCase === b.ignoreCase; + // 7.4 Other pairs that do not both pass typeof value == "object", + // equivalence is determined by ==. + } else if (typeof a != "object" && typeof b != "object") { + return a == b; + // 7.5 For all other Object pairs, including Array objects, equivalence is + // determined by having the same number of owned properties (as verified + // with Object.prototype.hasOwnProperty.call), the same set of keys + // (although not necessarily the same order), equivalent values for every + // corresponding key, and an identical 'prototype' property. Note: this + // accounts for both named and indexed properties on Arrays. + } else { + return objEquiv(a, b); + } +} + +function instanceOf(object, type) { + return Object.prototype.toString.call(object) == "[object " + type + "]"; +} + +function isUndefinedOrNull(value) { + return value === null || value === undefined; +} + +function isArguments(object) { + return instanceOf(object, "Arguments"); +} + +function objEquiv(a, b) { + if (isUndefinedOrNull(a) || isUndefinedOrNull(b)) { + return false; + } + // An identical 'prototype' property. + if (a.prototype !== b.prototype) { + return false; + } + // Object.keys may be broken through screwy arguments passing. Converting to + // an array solves the problem. + if (isArguments(a)) { + if (!isArguments(b)) { + return false; + } + a = pSlice.call(a); + b = pSlice.call(b); + return _deepEqual(a, b); + } + let ka, kb; + try { + ka = Object.keys(a); + kb = Object.keys(b); + } catch (e) { + // Happens when one is a string literal and the other isn't + return false; + } + // Having the same number of owned properties (keys incorporates + // hasOwnProperty) + if (ka.length != kb.length) + return false; + // The same set of keys (although not necessarily the same order), + ka.sort(); + kb.sort(); + // Equivalent values for every corresponding key, and possibly expensive deep + // test + for (let key of ka) { + if (!_deepEqual(a[key], b[key])) { + return false; + } + } + return true; +} + +// ... End of previously MIT-licensed code. diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index 23c1d9e5185..9031d100381 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -30,6 +30,7 @@ EXTRA_JS_MODULES += [ 'LoadContextInfo.jsm', 'Log.jsm', 'NewTabUtils.jsm', + 'ObjectUtils.jsm', 'PageMenu.jsm', 'PageMetadata.jsm', 'PermissionsUtils.jsm', diff --git a/toolkit/modules/tests/xpcshell/test_ObjectUtils.js b/toolkit/modules/tests/xpcshell/test_ObjectUtils.js new file mode 100644 index 00000000000..c7716b49bb2 --- /dev/null +++ b/toolkit/modules/tests/xpcshell/test_ObjectUtils.js @@ -0,0 +1,92 @@ +Components.utils.import("resource://gre/modules/ObjectUtils.jsm"); + +function run_test() { + run_next_test(); +} + +add_task(function* test_deepEqual() { + let deepEqual = ObjectUtils.deepEqual.bind(ObjectUtils); + // CommonJS 7.2 + Assert.ok(deepEqual(new Date(2000, 3, 14), new Date(2000, 3, 14)), "deepEqual date"); + Assert.ok(deepEqual(new Date(NaN), new Date(NaN)), "deepEqual invalid dates"); + + Assert.ok(!deepEqual(new Date(), new Date(2000, 3, 14)), "deepEqual date"); + + // 7.3 + Assert.ok(deepEqual(/a/, /a/)); + Assert.ok(deepEqual(/a/g, /a/g)); + Assert.ok(deepEqual(/a/i, /a/i)); + Assert.ok(deepEqual(/a/m, /a/m)); + Assert.ok(deepEqual(/a/igm, /a/igm)); + Assert.ok(!deepEqual(/ab/, /a/)); + Assert.ok(!deepEqual(/a/g, /a/)); + Assert.ok(!deepEqual(/a/i, /a/)); + Assert.ok(!deepEqual(/a/m, /a/)); + Assert.ok(!deepEqual(/a/igm, /a/im)); + + let re1 = /a/; + re1.lastIndex = 3; + Assert.ok(!deepEqual(re1, /a/)); + + // 7.4 + Assert.ok(deepEqual(4, "4"), "deepEqual == check"); + Assert.ok(deepEqual(true, 1), "deepEqual == check"); + Assert.ok(!deepEqual(4, "5"), "deepEqual == check"); + + // 7.5 + // having the same number of owned properties && the same set of keys + Assert.ok(deepEqual({a: 4}, {a: 4})); + Assert.ok(deepEqual({a: 4, b: "2"}, {a: 4, b: "2"})); + Assert.ok(deepEqual([4], ["4"])); + Assert.ok(!deepEqual({a: 4}, {a: 4, b: true})); + Assert.ok(deepEqual(["a"], {0: "a"})); + + let a1 = [1, 2, 3]; + let a2 = [1, 2, 3]; + a1.a = "test"; + a1.b = true; + a2.b = true; + a2.a = "test"; + Assert.ok(!deepEqual(Object.keys(a1), Object.keys(a2))); + Assert.ok(deepEqual(a1, a2)); + + let nbRoot = { + toString: function() { return this.first + " " + this.last; } + }; + + function nameBuilder(first, last) { + this.first = first; + this.last = last; + return this; + } + nameBuilder.prototype = nbRoot; + + function nameBuilder2(first, last) { + this.first = first; + this.last = last; + return this; + } + nameBuilder2.prototype = nbRoot; + + let nb1 = new nameBuilder("Ryan", "Dahl"); + let nb2 = new nameBuilder2("Ryan", "Dahl"); + + Assert.ok(deepEqual(nb1, nb2)); + + nameBuilder2.prototype = Object; + nb2 = new nameBuilder2("Ryan", "Dahl"); + Assert.ok(!deepEqual(nb1, nb2)); + + // String literal + object + Assert.ok(!deepEqual("a", {})); + + // Make sure deepEqual doesn't loop forever on circular refs + + let b = {}; + b.b = b; + + let c = {}; + c.b = c; + + Assert.ok(!deepEqual(b, c)); +}); diff --git a/toolkit/modules/tests/xpcshell/xpcshell.ini b/toolkit/modules/tests/xpcshell/xpcshell.ini index f2fa7e635f4..595b4eab3c7 100644 --- a/toolkit/modules/tests/xpcshell/xpcshell.ini +++ b/toolkit/modules/tests/xpcshell/xpcshell.ini @@ -16,6 +16,7 @@ support-files = [test_Http.js] [test_Log.js] [test_NewTabUtils.js] +[test_ObjectUtils.js] [test_PermissionsUtils.js] [test_Preferences.js] [test_Promise.js] From 5bbacd17417a30ce55222ac0f2623fc93469ad6b Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 11/52] Bug 1140558 - Part 3 - Pass the old environment data to event listeners on environment changes. r=vladan --- .../telemetry/TelemetryEnvironment.jsm | 93 +++++++++---------- .../components/telemetry/TelemetryPing.jsm | 9 +- .../components/telemetry/TelemetrySession.jsm | 3 +- .../components/telemetry/docs/environment.rst | 12 ++- .../tests/unit/test_TelemetryEnvironment.js | 7 +- .../tests/unit/test_TelemetrySession.js | 6 +- 6 files changed, 71 insertions(+), 59 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index 3567eee3f84..3d2a48fa000 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -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 whether the environment changed. + * @returns Promise 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); } diff --git a/toolkit/components/telemetry/TelemetryPing.jsm b/toolkit/components/telemetry/TelemetryPing.jsm index ef5053cdddd..eb9f3037c83 100644 --- a/toolkit/components/telemetry/TelemetryPing.jsm +++ b/toolkit/components/telemetry/TelemetryPing.jsm @@ -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 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. diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index ea8f0b397e5..64acd85bd99 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -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); }, diff --git a/toolkit/components/telemetry/docs/environment.rst b/toolkit/components/telemetry/docs/environment.rst index 6b15b225061..c6f222ca658 100644 --- a/toolkit/components/telemetry/docs/environment.rst +++ b/toolkit/components/telemetry/docs/environment.rst @@ -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 `_. +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: , // 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 diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js index 7b620ce5e11..a1d43791722 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js @@ -630,17 +630,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, diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 5ae8941690a..aef7ce663b3 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -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); From eb983aa37e2d07ea64c655ea24d7a2cfa20a1e8c Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 12/52] Bug 1140558 - Part 4 - Fix TelemetryEnvironment returning NaN for GFX RAM on error. r=gfritzsche --- toolkit/components/telemetry/TelemetryEnvironment.jsm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index 3d2a48fa000..1e57aa135da 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -239,9 +239,11 @@ function getGfxField(aPropertyName, aDefault) { * @return An object containing the adapter properties. */ function getGfxAdapter(aSuffix = "") { - let memoryMB = getGfxField("adapterRAM" + aSuffix, null); - if (memoryMB) { - memoryMB = parseInt(memoryMB, 10); + // Note that gfxInfo, and so getGfxField, might return "Unknown" for the RAM on failures, + // not null. + let memoryMB = parseInt(getGfxField("adapterRAM" + aSuffix, null), 10); + if (Number.isNaN(memoryMB)) { + memoryMB = null; } return { From e6830ab5e716d8d902a5fc1ae25da30210b47eba Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 13/52] Bug 1143714 - Throttle Telemetry environment changes. r=vladan --- .../telemetry/TelemetryEnvironment.jsm | 23 ++++++- .../components/telemetry/tests/unit/head.js | 21 +++++++ .../tests/unit/test_TelemetryEnvironment.js | 63 +++++++++++++++++-- .../tests/unit/test_TelemetrySession.js | 32 ++++++---- 4 files changed, 120 insertions(+), 19 deletions(-) diff --git a/toolkit/components/telemetry/TelemetryEnvironment.jsm b/toolkit/components/telemetry/TelemetryEnvironment.jsm index 1e57aa135da..3f65c0ae244 100644 --- a/toolkit/components/telemetry/TelemetryEnvironment.jsm +++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm @@ -31,6 +31,15 @@ XPCOMUtils.defineLazyModuleGetter(this, "ProfileAge", XPCOMUtils.defineLazyModuleGetter(this, "UpdateChannel", "resource://gre/modules/UpdateChannel.jsm"); +const CHANGE_THROTTLE_INTERVAL_MS = 5 * 60 * 1000; + +/** + * This is a policy object used to override behavior for testing. + */ +let Policy = { + now: () => new Date(), +}; + var gGlobalEnvironment; function getGlobal() { if (!gGlobalEnvironment) { @@ -628,6 +637,9 @@ function EnvironmentCache() { // A map of listeners that will be called on environment changes. this._changeListeners = new Map(); + // The last change date for the environment, used to throttle environment changes. + this._lastEnvironmentChangeDate = null; + // A map of watched preferences which trigger an Environment change when // modified. Every entry contains a recording policy (RECORD_PREF_*). this._watchedPrefs = DEFAULT_ENVIRONMENT_PREFS; @@ -1065,7 +1077,16 @@ EnvironmentCache.prototype = { } // We are already skipping change events in _checkChanges if there is a pending change task running. - // Further throttling is coming in bug 1143714. + let now = Policy.now(); + if (this._lastEnvironmentChangeDate && + (CHANGE_THROTTLE_INTERVAL_MS >= + (now.getTime() - this._lastEnvironmentChangeDate.getTime()))) { + this._log.trace("_onEnvironmentChange - throttling changes, now: " + now + + ", last change: " + this._lastEnvironmentChangeDate); + return; + } + + this._lastEnvironmentChangeDate = now; for (let [name, listener] of this._changeListeners) { try { diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js index 39fff81df67..bc60ff80344 100644 --- a/toolkit/components/telemetry/tests/unit/head.js +++ b/toolkit/components/telemetry/tests/unit/head.js @@ -9,6 +9,10 @@ const gIsMac = ("@mozilla.org/xpcom/mac-utils;1" in Components.classes); const gIsAndroid = ("@mozilla.org/android/bridge;1" in Components.classes); const gIsGonk = ("@mozilla.org/cellbroadcast/gonkservice;1" in Components.classes); +const MILLISECONDS_PER_MINUTE = 60 * 1000; +const MILLISECONDS_PER_HOUR = 60 * MILLISECONDS_PER_MINUTE; +const MILLISECONDS_PER_DAY = 24 * MILLISECONDS_PER_HOUR; + const HAS_DATAREPORTINGSERVICE = "@mozilla.org/datareporting/service;1" in Components.classes; let gOldAppInfo = null; @@ -87,6 +91,23 @@ function fakeSchedulerTimer(set, clear) { session.Policy.clearSchedulerTickTimeout = clear; } +// Fake the current date. +function fakeNow(date) { + let session = Cu.import("resource://gre/modules/TelemetrySession.jsm"); + session.Policy.now = () => date; + let environment = Cu.import("resource://gre/modules/TelemetryEnvironment.jsm"); + environment.Policy.now = () => date; +} + +// Return a date that is |offset| ms in the future from |date|. +function futureDate(date, offset) { + return new Date(date.getTime() + offset); +} + +function truncateToDays(aMsec) { + return Math.floor(aMsec / MILLISECONDS_PER_DAY); +} + // Set logging preferences for all the tests. Services.prefs.setCharPref("toolkit.telemetry.log.level", "Trace"); Services.prefs.setBoolPref("toolkit.telemetry.log.dump", true); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js index a1d43791722..23968a6aaf3 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js @@ -27,6 +27,9 @@ let gHttpRoot = null; // The URL of the data directory, on the webserver. let gDataRoot = null; +let gNow = new Date(2010, 1, 1, 12, 0, 0); +fakeNow(gNow); + const PLATFORM_VERSION = "1.9.2"; const APP_VERSION = "1"; const APP_ID = "xpcshell@tests.mozilla.org"; @@ -43,7 +46,6 @@ const PARTNER_ID = "NicePartner-ID-3785"; const GFX_VENDOR_ID = "0xabcd"; const GFX_DEVICE_ID = "0x1234"; -const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000; // The profile reset date, in milliseconds (Today) const PROFILE_RESET_DATE_MS = Date.now(); // The profile creation date, in milliseconds (Yesterday). @@ -175,10 +177,6 @@ function spoofPartnerInfo() { } } -function truncateToDays(aMsec) { - return Math.floor(aMsec / MILLISECONDS_PER_DAY); -} - /** * Check that a value is a string and not empty. * @@ -613,6 +611,8 @@ add_task(function* test_prefWatchPolicies() { const PREF_TEST_4 = "toolkit.telemetry.test.pref_old"; const expectedValue = "some-test-value"; + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); let prefsToWatch = {}; prefsToWatch[PREF_TEST_1] = TelemetryEnvironment.RECORD_PREF_VALUE; @@ -662,6 +662,9 @@ add_task(function* test_prefWatch_prefReset() { // Set the preference to a non-default value. Preferences.set(PREF_TEST, false); + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + // Set the Environment preferences to watch. TelemetryEnvironment._watchPreferences(prefsToWatch); let deferred = PromiseUtils.defer(); @@ -689,6 +692,8 @@ add_task(function* test_addonsWatch_InterestingChange() { let receivedNotifications = 0; let registerCheckpointPromise = (aExpected) => { + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); return new Promise(resolve => TelemetryEnvironment.registerChangeListener( "testWatchAddons_Changes" + aExpected, (reason, data) => { Assert.equal(reason, "addons-changed"); @@ -738,6 +743,9 @@ add_task(function* test_pluginsWatch_Add() { return; } + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + Assert.equal(TelemetryEnvironment.currentEnvironment.addons.activePlugins.length, 1); let newPlugin = new PluginTag(PLUGIN2_NAME, PLUGIN2_DESC, PLUGIN2_VERSION, true); @@ -768,6 +776,9 @@ add_task(function* test_pluginsWatch_Remove() { return; } + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + // Find the test plugin. let plugin = gInstalledPlugins.find(plugin => (plugin.name == PLUGIN2_NAME)); Assert.ok(plugin, "The test plugin must exist."); @@ -796,6 +807,9 @@ add_task(function* test_addonsWatch_NotInterestingChange() { const DICTIONARY_ADDON_INSTALL_URL = gDataRoot + "dictionary.xpi"; const INTERESTING_ADDON_INSTALL_URL = gDataRoot + "restartless.xpi"; + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + let receivedNotification = false; let deferred = PromiseUtils.defer(); TelemetryEnvironment.registerChangeListener("testNotInteresting", @@ -879,6 +893,45 @@ add_task(function* test_addonsAndPlugins() { Assert.equal(data.addons.persona, personaId, "The correct Persona Id must be reported."); }); +add_task(function* test_changeThrottling() { + const PREF_TEST = "toolkit.telemetry.test.pref1"; + let prefsToWatch = {}; + prefsToWatch[PREF_TEST] = TelemetryEnvironment.RECORD_PREF_STATE; + Preferences.reset(PREF_TEST); + + gNow = futureDate(gNow, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + + // Set the Environment preferences to watch. + TelemetryEnvironment._watchPreferences(prefsToWatch); + let deferred = PromiseUtils.defer(); + let changeCount = 0; + TelemetryEnvironment.registerChangeListener("testWatchPrefs_throttling", () => { + ++changeCount; + deferred.resolve(); + }); + + // The first pref change should trigger a notification. + Preferences.set(PREF_TEST, 1); + yield deferred.promise; + Assert.equal(changeCount, 1); + + // We should only get a change notification for second of the following changes. + deferred = PromiseUtils.defer(); + gNow = futureDate(gNow, MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + Preferences.set(PREF_TEST, 2); + gNow = futureDate(gNow, 5 * MILLISECONDS_PER_MINUTE); + fakeNow(gNow); + Preferences.set(PREF_TEST, 3); + yield deferred.promise; + + Assert.equal(changeCount, 2); + + // Unregister the listener. + TelemetryEnvironment.unregisterChangeListener("testWatchPrefs_throttling"); +}); + add_task(function*() { do_test_finished(); }); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index aef7ce663b3..722d456c703 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -99,6 +99,13 @@ function generateUUID() { return str.substring(1, str.length - 1); } +function truncateDateToDays(date) { + return new Date(date.getFullYear(), + date.getMonth(), + date.getDate(), + 0, 0, 0, 0); +} + function sendPing() { TelemetrySession.gatherStartup(); if (gServerStarted) { @@ -123,15 +130,6 @@ function wrapWithExceptionHandler(f) { return wrapper; } -function futureDate(date, offset) { - return new Date(date.getTime() + offset); -} - -function fakeNow(date) { - let session = Cu.import("resource://gre/modules/TelemetrySession.jsm"); - session.Policy.now = () => date; -} - function fakeGenerateUUID(sessionFunc, subsessionFunc) { let session = Cu.import("resource://gre/modules/TelemetrySession.jsm"); session.Policy.generateSessionUUID = sessionFunc; @@ -1113,7 +1111,6 @@ add_task(function* test_environmentChange() { } let now = new Date(2040, 1, 1, 12, 0, 0); - let nowDay = new Date(2040, 1, 1, 0, 0, 0); let timerCallback = null; let timerDelay = null; @@ -1144,6 +1141,10 @@ add_task(function* test_environmentChange() { keyed.add("b", 1); // Trigger and collect environment-change ping. + let startDay = truncateDateToDays(now); + now = futureDate(now, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(now); + Preferences.set(PREF_TEST, 1); let request = yield gRequestIterator.next(); Assert.ok(!!request); @@ -1153,12 +1154,16 @@ add_task(function* test_environmentChange() { 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()); + Assert.equal(subsessionStartDate.toISOString(), startDay.toISOString()); Assert.equal(ping.payload.histograms[COUNT_ID].sum, 1); Assert.equal(ping.payload.keyedHistograms[KEYED_ID]["a"].sum, 1); // Trigger and collect another ping. The histograms should be reset. + startDay = truncateDateToDays(now); + now = futureDate(now, 10 * MILLISECONDS_PER_MINUTE); + fakeNow(now); + Preferences.set(PREF_TEST, 2); request = yield gRequestIterator.next(); Assert.ok(!!request); @@ -1168,7 +1173,7 @@ add_task(function* test_environmentChange() { 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()); + Assert.equal(subsessionStartDate.toISOString(), startDay.toISOString()); Assert.equal(ping.payload.histograms[COUNT_ID].sum, 0); Assert.deepEqual(ping.payload.keyedHistograms[KEYED_ID], {}); @@ -1248,6 +1253,7 @@ add_task(function* test_savedSessionData() { // Watch a test preference, trigger and environment change and wait for it to propagate. // _watchPreferences triggers a subsession notification + fakeNow(new Date(2050, 1, 1, 12, 0, 0)); TelemetryEnvironment._watchPreferences(prefsToWatch); let changePromise = new Promise(resolve => TelemetryEnvironment.registerChangeListener("test_fake_change", resolve)); @@ -1487,7 +1493,7 @@ add_task(function* test_schedulerEnvironmentReschedules() { gRequestIterator = Iterator(new Request()); // Set a fake current date and start Telemetry. - let nowDate = new Date(2009, 10, 18, 0, 00, 0); + let nowDate = new Date(2060, 10, 18, 0, 00, 0); fakeNow(nowDate); let schedulerTickCallback = null; fakeSchedulerTimer(callback => schedulerTickCallback = callback, () => {}); From 6321067e72fb4ea8e2b3e9937afe6e7619ba6b7e Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 14/52] Bug 1143796 - Increase TelemetryScheduler ticking interval when user is not active. r=gfritzsche --- .../components/telemetry/TelemetrySession.jsm | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 64acd85bd99..3c302ba2540 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -86,6 +86,8 @@ const TELEMETRY_DELAY = 60000; const TELEMETRY_TEST_DELAY = 100; // Execute a scheduler tick every 5 minutes. const SCHEDULER_TICK_INTERVAL_MS = 5 * 60 * 1000; +// When user is idle, execute a scheduler tick every 60 minutes. +const SCHEDULER_TICK_IDLE_INTERVAL_MS = 60 * 60 * 1000; // The maximum number of times a scheduled operation can fail. const SCHEDULER_RETRY_ATTEMPTS = 3; @@ -416,6 +418,8 @@ let TelemetryScheduler = { // The timer which drives the scheduler. _schedulerTimer: null, + // The interval used by the scheduler timer. + _schedulerInterval: 0, _shuttingDown: true, /** @@ -430,14 +434,16 @@ let TelemetryScheduler = { let now = Policy.now(); this._lastDailyPingTime = now.getTime(); this._lastSessionCheckpointTime = now.getTime(); + this._schedulerInterval = SCHEDULER_TICK_INTERVAL_MS; this._rescheduleTimeout(); + idleService.addIdleObserver(this, IDLE_TIMEOUT_SECONDS); }, /** * Reschedules the tick timer. */ _rescheduleTimeout: function() { - this._log.trace("_rescheduleTimeout"); + this._log.trace("_rescheduleTimeout - timeout: " + this._schedulerInterval); if (this._shuttingDown) { this._log.warn("_rescheduleTimeout - already shutdown"); return; @@ -448,7 +454,7 @@ let TelemetryScheduler = { } this._schedulerTimer = - Policy.setSchedulerTickTimeout(() => this._onSchedulerTick(), SCHEDULER_TICK_INTERVAL_MS); + Policy.setSchedulerTickTimeout(() => this._onSchedulerTick(), this._schedulerInterval); }, /** @@ -498,6 +504,25 @@ let TelemetryScheduler = { .catch(e => this._log.error("_saveAbortedPing - Failed", e)); }, + /** + * The notifications handler. + */ + observe: function(aSubject, aTopic, aData) { + this._log.trace("observe - aTopic: " + aTopic); + switch(aTopic) { + case "idle": + // If the user is idle, increase the tick interval. + this._schedulerInterval = SCHEDULER_TICK_IDLE_INTERVAL_MS; + this._rescheduleTimeout(); + break; + case "active": + // User is back to work, restore the original tick interval. + this._schedulerInterval = SCHEDULER_TICK_INTERVAL_MS; + this._rescheduleTimeout(); + break; + } + }, + /** * Performs a scheduler tick. This function manages Telemetry recurring operations. * @return {Promise} A promise, only used when testing, resolved when the scheduled @@ -650,6 +675,8 @@ let TelemetryScheduler = { this._schedulerTimer = null; } + idleService.removeIdleObserver(this, IDLE_TIMEOUT_SECONDS); + this._shuttingDown = true; } }; From 0328c20f7124f3cf705ca281c6d51ac23e99a6f6 Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 15/52] Bug 1143796 - Add test for TelemetryScheduler tick interval changing when user is idle. r=gfritzsche --- .../tests/unit/test_TelemetrySession.js | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 722d456c703..e86b0554f28 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -136,6 +136,11 @@ function fakeGenerateUUID(sessionFunc, subsessionFunc) { session.Policy.generateSubsessionUUID = subsessionFunc; } +function fakeIdleNotification(topic) { + let session = Cu.import("resource://gre/modules/TelemetrySession.jsm"); + session.TelemetryScheduler.observe(null, topic, null); +} + function registerPingHandler(handler) { gHttpServer.registerPrefixHandler("/submit/telemetry/", wrapWithExceptionHandler(handler)); @@ -1614,6 +1619,39 @@ add_task(function* test_pingExtendedStats() { "UITelemetry must be sent if the extended set is on."); }); +add_task(function* test_schedulerUserIdle() { + if (gIsAndroid || gIsGonk) { + // We don't have the aborted session or the daily ping here. + return; + } + + const SCHEDULER_TICK_INTERVAL_MS = 5 * 60 * 1000; + const SCHEDULER_TICK_IDLE_INTERVAL_MS = 60 * 60 * 1000; + + let schedulerTimeout = 0; + fakeSchedulerTimer((callback, timeout) => { + schedulerTimeout = timeout; + }, () => {}); + yield TelemetrySession.reset(); + + // When not idle, the scheduler should have a 5 minutes tick interval. + Assert.equal(schedulerTimeout, SCHEDULER_TICK_INTERVAL_MS); + + // Send an "idle" notification to the scheduler. + fakeIdleNotification("idle"); + + // When idle, the scheduler should have a 1hr tick interval. + Assert.equal(schedulerTimeout, SCHEDULER_TICK_IDLE_INTERVAL_MS); + + // Send an "active" notification to the scheduler. + fakeIdleNotification("active"); + + // When user is back active, the scheduler tick should be 5 minutes again. + Assert.equal(schedulerTimeout, SCHEDULER_TICK_INTERVAL_MS); + + yield TelemetrySession.shutdown(); +}); + add_task(function* stopServer(){ gHttpServer.stop(do_test_finished); }); From a24592d51eb70d1ad7540a577129726ef19e8bee Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 16/52] Bug 1139751 - Try to collect data for Telemetry pings when the user is idle. r=vladan --- .../components/telemetry/TelemetrySession.jsm | 106 ++++++++++++------ .../tests/unit/test_TelemetrySession.js | 76 ++++++++++++- 2 files changed, 142 insertions(+), 40 deletions(-) diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 3c302ba2540..1170964e090 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -47,9 +47,7 @@ const REASON_SHUTDOWN = "shutdown"; const ENVIRONMENT_CHANGE_LISTENER = "TelemetrySession::onEnvironmentChange"; -const SEC_IN_ONE_DAY = 24 * 60 * 60; -const MS_IN_ONE_DAY = SEC_IN_ONE_DAY * 1000; - +const MS_IN_ONE_HOUR = 60 * 60 * 1000; const MIN_SUBSESSION_LENGTH_MS = 10 * 60 * 1000; // This is the HG changeset of the Histogram.json file, used to associate @@ -196,6 +194,17 @@ function areTimesClose(t1, t2, tolerance) { return Math.abs(t1 - t2) <= tolerance; } +/** + * Get the next midnight for a date. + * @param {Object} date The date object to check. + * @return {Object} The Date object representing the next midnight. + */ +function getNextMidnight(date) { + let nextMidnight = new Date(truncateToDays(date)); + nextMidnight.setDate(nextMidnight.getDate() + 1); + return nextMidnight; +} + /** * Get the midnight which is closer to the provided date. * @param {Object} date The date object to check. @@ -208,8 +217,7 @@ function getNearestMidnight(date) { return lastMidnight; } - let nextMidnightDate = new Date(lastMidnight); - nextMidnightDate.setDate(nextMidnightDate.getDate() + 1); + const nextMidnightDate = getNextMidnight(date); if (areTimesClose(date.getTime(), nextMidnightDate.getTime(), SCHEDULER_MIDNIGHT_TOLERANCE_MS)) { return nextMidnightDate; } @@ -421,6 +429,7 @@ let TelemetryScheduler = { // The interval used by the scheduler timer. _schedulerInterval: 0, _shuttingDown: true, + _isUserIdle: false, /** * Initialises the scheduler and schedules the first daily/aborted session pings. @@ -429,12 +438,12 @@ let TelemetryScheduler = { this._log = Log.repository.getLoggerWithMessagePrefix(LOGGER_NAME, "TelemetryScheduler::"); this._log.trace("init"); this._shuttingDown = false; + this._isUserIdle = false; // Initialize the last daily ping and aborted session last due times to the current time. // Otherwise, we might end up sending daily pings even if the subsession is not long enough. let now = Policy.now(); this._lastDailyPingTime = now.getTime(); this._lastSessionCheckpointTime = now.getTime(); - this._schedulerInterval = SCHEDULER_TICK_INTERVAL_MS; this._rescheduleTimeout(); idleService.addIdleObserver(this, IDLE_TIMEOUT_SECONDS); }, @@ -443,7 +452,7 @@ let TelemetryScheduler = { * Reschedules the tick timer. */ _rescheduleTimeout: function() { - this._log.trace("_rescheduleTimeout - timeout: " + this._schedulerInterval); + this._log.trace("_rescheduleTimeout - isUserIdle: " + this._isUserIdle); if (this._shuttingDown) { this._log.warn("_rescheduleTimeout - already shutdown"); return; @@ -453,8 +462,31 @@ let TelemetryScheduler = { Policy.clearSchedulerTickTimeout(this._schedulerTimer); } + const now = Policy.now(); + let timeout = SCHEDULER_TICK_INTERVAL_MS; + + // When the user is idle we want to fire the timer less often. + if (this._isUserIdle) { + timeout = SCHEDULER_TICK_IDLE_INTERVAL_MS; + // We need to make sure though that we don't miss sending pings around + // midnight when we use the longer idle intervals. + const nextMidnight = getNextMidnight(now); + timeout = Math.min(timeout, nextMidnight.getTime() - now.getTime()); + } + + this._log.trace("_rescheduleTimeout - scheduling next tick for " + new Date(now.getTime() + timeout)); this._schedulerTimer = - Policy.setSchedulerTickTimeout(() => this._onSchedulerTick(), this._schedulerInterval); + Policy.setSchedulerTickTimeout(() => this._onSchedulerTick(), timeout); + }, + + _sentDailyPingToday: function(nowDate) { + // This is today's date and also the previous midnight (0:00). + const todayDate = truncateToDays(nowDate); + const nearestMidnight = getNearestMidnight(nowDate); + // If we are close to midnight, we check against that, otherwise against the last midnight. + const checkDate = nearestMidnight || todayDate; + // We consider a ping sent for today if it occured after midnight, or prior within the tolerance. + return (this._lastDailyPingTime >= (checkDate.getTime() - SCHEDULER_MIDNIGHT_TOLERANCE_MS)); }, /** @@ -463,31 +495,37 @@ let TelemetryScheduler = { * @return {Boolean} True if we can send the daily ping, false otherwise. */ _isDailyPingDue: function(nowDate) { - let nearestMidnight = getNearestMidnight(nowDate); - if (nearestMidnight) { - let subsessionLength = Math.abs(nowDate.getTime() - this._lastDailyPingTime); - if (subsessionLength < MIN_SUBSESSION_LENGTH_MS) { - // Generating a daily ping now would create a very short subsession. - return false; - } else if (areTimesClose(this._lastDailyPingTime, nearestMidnight.getTime(), - SCHEDULER_MIDNIGHT_TOLERANCE_MS)) { - // We've already sent a ping for this midnight. - return false; - } + const sentPingToday = this._sentDailyPingToday(nowDate); + + // The daily ping is not due if we already sent one today. + if (sentPingToday) { + this._log.trace("_isDailyPingDue - already sent one today"); + return false; + } + + const nearestMidnight = getNearestMidnight(nowDate); + if (!sentPingToday && !nearestMidnight) { + // Computer must have gone to sleep, the daily ping is overdue. + this._log.trace("_isDailyPingDue - daily ping is overdue... computer went to sleep?"); return true; } - let lastDailyPingDate = truncateToDays(new Date(this._lastDailyPingTime)); - // This is today's date and also the previous midnight (0:00). - let todayDate = truncateToDays(nowDate); - // Check that _lastDailyPingTime isn't today nor within SCHEDULER_MIDNIGHT_TOLERANCE_MS of the - // *previous* midnight. - if ((lastDailyPingDate.getTime() != todayDate.getTime()) && - !areTimesClose(this._lastDailyPingTime, todayDate.getTime(), SCHEDULER_MIDNIGHT_TOLERANCE_MS)) { - // Computer must have gone to sleep, the daily ping is overdue. - return true; + // Avoid overly short sessions. + const timeSinceLastDaily = nowDate.getTime() - this._lastDailyPingTime; + if (timeSinceLastDaily < MIN_SUBSESSION_LENGTH_MS) { + this._log.trace("_isDailyPingDue - delaying daily to keep minimum session length"); + return false; } - return false; + + // To fight jank, we allow daily pings to be collected on user idle before midnight + // within the tolerance interval. + if (!this._isUserIdle && (nowDate.getTime() < nearestMidnight.getTime())) { + this._log.trace("_isDailyPingDue - waiting for user idle period"); + return false; + } + + this._log.trace("_isDailyPingDue - is due"); + return true; }, /** @@ -512,13 +550,13 @@ let TelemetryScheduler = { switch(aTopic) { case "idle": // If the user is idle, increase the tick interval. - this._schedulerInterval = SCHEDULER_TICK_IDLE_INTERVAL_MS; - this._rescheduleTimeout(); + this._isUserIdle = true; + return this._onSchedulerTick(); break; case "active": // User is back to work, restore the original tick interval. - this._schedulerInterval = SCHEDULER_TICK_INTERVAL_MS; - this._rescheduleTimeout(); + this._isUserIdle = false; + return this._onSchedulerTick(); break; } }, @@ -531,7 +569,7 @@ let TelemetryScheduler = { _onSchedulerTick: function() { if (this._shuttingDown) { this._log.warn("_onSchedulerTick - already shutdown."); - return; + return Promise.reject(new Error("Already shutdown.")); } let promise = Promise.resolve(); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index e86b0554f28..00710702375 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -61,8 +61,8 @@ const RW_OWNER = parseInt("0600", 8); const NUMBER_OF_THREADS_TO_LAUNCH = 30; let gNumberOfThreadsLaunched = 0; -const SEC_IN_ONE_DAY = 24 * 60 * 60; -const MS_IN_ONE_DAY = SEC_IN_ONE_DAY * 1000; +const MS_IN_ONE_HOUR = 60 * 60 * 1000; +const MS_IN_ONE_DAY = 24 * MS_IN_ONE_HOUR; const PREF_BRANCH = "toolkit.telemetry."; const PREF_ENABLED = PREF_BRANCH + "enabled"; @@ -138,7 +138,7 @@ function fakeGenerateUUID(sessionFunc, subsessionFunc) { function fakeIdleNotification(topic) { let session = Cu.import("resource://gre/modules/TelemetrySession.jsm"); - session.TelemetryScheduler.observe(null, topic, null); + return session.TelemetryScheduler.observe(null, topic, null); } function registerPingHandler(handler) { @@ -1023,8 +1023,10 @@ add_task(function* test_dailyDuplication() { fakeSchedulerTimer(callback => schedulerTickCallback = callback, () => {}); yield TelemetrySession.setup(); - // Make sure the daily ping gets triggered just before midnight. - let firstDailyDue = new Date(2030, 1, 1, 23, 45, 0); + // Make sure the daily ping gets triggered at midnight. + // We need to make sure that we trigger this after the period where we wait for + // the user to become idle. + let firstDailyDue = new Date(2030, 1, 2, 0, 0, 0); fakeNow(firstDailyDue); // Run a scheduler tick: it should trigger the daily ping. @@ -1046,7 +1048,6 @@ add_task(function* test_dailyDuplication() { // Set the current time to a bit after midnight. let secondDailyDue = new Date(firstDailyDue); - secondDailyDue.setDate(firstDailyDue.getDate() + 1); secondDailyDue.setHours(0); secondDailyDue.setMinutes(15); fakeNow(secondDailyDue); @@ -1628,11 +1629,15 @@ add_task(function* test_schedulerUserIdle() { const SCHEDULER_TICK_INTERVAL_MS = 5 * 60 * 1000; const SCHEDULER_TICK_IDLE_INTERVAL_MS = 60 * 60 * 1000; + let now = new Date(2010, 1, 1, 11, 0, 0); + fakeNow(now); + let schedulerTimeout = 0; fakeSchedulerTimer((callback, timeout) => { schedulerTimeout = timeout; }, () => {}); yield TelemetrySession.reset(); + gRequestIterator = Iterator(new Request()); // When not idle, the scheduler should have a 5 minutes tick interval. Assert.equal(schedulerTimeout, SCHEDULER_TICK_INTERVAL_MS); @@ -1649,6 +1654,65 @@ add_task(function* test_schedulerUserIdle() { // When user is back active, the scheduler tick should be 5 minutes again. Assert.equal(schedulerTimeout, SCHEDULER_TICK_INTERVAL_MS); + // We should not miss midnight when going to idle. + now.setHours(23); + now.setMinutes(50); + fakeIdleNotification("idle"); + Assert.equal(schedulerTimeout, 10 * 60 * 1000); + + yield TelemetrySession.shutdown(); +}); + +add_task(function* test_sendDailyOnIdle() { + if (gIsAndroid || gIsGonk) { + // We don't have the aborted session or the daily ping here. + return; + } + + let now = new Date(2040, 1, 1, 11, 0, 0); + fakeNow(now); + + let schedulerTickCallback = 0; + fakeSchedulerTimer((callback, timeout) => { + schedulerTickCallback = callback; + }, () => {}); + yield TelemetrySession.reset(); + + // Make sure we are not sending a daily before midnight when active. + now = new Date(2040, 1, 1, 23, 55, 0); + fakeNow(now); + registerPingHandler((req, res) => { + Assert.ok(false, "No daily ping should be received yet when the user is active."); + }); + yield fakeIdleNotification("active"); + + // The Request constructor restores the previous ping handler. + gRequestIterator = Iterator(new Request()); + + // We should receive a daily ping after midnight. + now = new Date(2040, 1, 2, 0, 05, 0); + fakeNow(now); + yield schedulerTickCallback(); + + let request = yield gRequestIterator.next(); + Assert.ok(!!request); + let ping = decodeRequestPayload(request); + + Assert.equal(ping.type, PING_TYPE_MAIN); + Assert.equal(ping.payload.info.reason, REASON_DAILY); + + // We should also trigger a ping when going idle shortly before next midnight. + now = new Date(2040, 1, 2, 23, 55, 0); + fakeNow(now); + yield fakeIdleNotification("idle"); + + request = yield gRequestIterator.next(); + Assert.ok(!!request); + ping = decodeRequestPayload(request); + + Assert.equal(ping.type, PING_TYPE_MAIN); + Assert.equal(ping.payload.info.reason, REASON_DAILY); + yield TelemetrySession.shutdown(); }); From 3911791336f353e2af9f466283935cda11ad8942 Mon Sep 17 00:00:00 2001 From: Georg Fritzsche Date: Wed, 1 Apr 2015 21:06:19 +0200 Subject: [PATCH 17/52] Bug 1139751 - Fix aborted-session saves that can race to after the shutdown aborted-session removal. r=vladan --- toolkit/components/telemetry/TelemetrySession.jsm | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index 1170964e090..8ee271b474b 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1987,7 +1987,8 @@ let Impl = { #if !defined(MOZ_WIDGET_GONK) && !defined(MOZ_WIDGET_ANDROID) // If required, also save the payload as an aborted session. if (saveAsAborted) { - return promise.then(() => this._saveAbortedSessionPing(payload)); + let abortedPromise = this._saveAbortedSessionPing(payload); + promise = promise.then(() => abortedPromise); } #endif return promise; @@ -2094,8 +2095,13 @@ let Impl = { const FILE_PATH = OS.Path.join(OS.Constants.Path.profileDir, DATAREPORTING_DIRECTORY, ABORTED_SESSION_FILE_NAME); try { + this._log.trace("_removeAbortedSessionPing - success"); return OS.File.remove(FILE_PATH); - } catch (ex if ex.becauseNoSuchFile) { } + } catch (ex if ex.becauseNoSuchFile) { + this._log.trace("_removeAbortedSessionPing - no such file"); + } catch (ex) { + this._log.error("_removeAbortedSessionPing - error removing ping", ex) + } return Promise.resolve(); }, From 356538242a9d4d7fb420b9784b1c3be4ffd16582 Mon Sep 17 00:00:00 2001 From: Divjot Singh Date: Wed, 1 Apr 2015 21:43:19 +0530 Subject: [PATCH 18/52] Bug 1139026 - Use white background-color & inverted color for selected area. r=margaret --- toolkit/themes/windows/global/aboutReader.css | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/toolkit/themes/windows/global/aboutReader.css b/toolkit/themes/windows/global/aboutReader.css index 6c67a1aa7da..2f0597f4bad 100644 --- a/toolkit/themes/windows/global/aboutReader.css +++ b/toolkit/themes/windows/global/aboutReader.css @@ -234,6 +234,13 @@ body.loaded { .dark > .container > .content blockquote { -moz-border-start: 2px solid #eeeeee; } +.dark *::-moz-selection { + background-color: #FFFFFF; + color: #0095DD; +} +.dark a::-moz-selection { + color: #DD4800; +} .content ul, .content ol { From 88914fda601baac3d735529e11b9e046ff1feebd Mon Sep 17 00:00:00 2001 From: Jordan Santell Date: Wed, 25 Mar 2015 10:08:40 -0700 Subject: [PATCH 19/52] Bug 1143933 - Expose raw JIT optimization information in performance front end. r=vp,shu --- browser/app/profile/firefox.js | 1 + browser/devtools/jar.mn | 1 + .../performance/performance-controller.js | 9 + browser/devtools/performance/performance.xul | 78 ++-- browser/devtools/performance/test/browser.ini | 4 + .../test/browser_perf-jit-model-01.js | 70 +++ .../test/browser_perf-jit-model-02.js | 77 ++++ .../test/browser_perf-jit-view-01.js | 162 +++++++ .../test/browser_profiler_tree-model-06.js | 103 +++++ browser/devtools/performance/test/head.js | 2 + .../performance/views/details-js-call-tree.js | 52 ++- browser/devtools/performance/views/details.js | 2 +- .../performance/views/jit-optimizations.js | 409 ++++++++++++++++++ browser/devtools/shared/moz.build | 1 + browser/devtools/shared/options-view.js | 2 +- browser/devtools/shared/profiler/jit.js | 205 +++++++++ .../devtools/shared/profiler/tree-model.js | 65 ++- .../chrome/browser/devtools/profiler.dtd | 10 + .../browser/devtools/profiler.properties | 12 + .../shared/devtools/performance.inc.css | 125 +++++- 20 files changed, 1330 insertions(+), 60 deletions(-) create mode 100644 browser/devtools/performance/test/browser_perf-jit-model-01.js create mode 100644 browser/devtools/performance/test/browser_perf-jit-model-02.js create mode 100644 browser/devtools/performance/test/browser_perf-jit-view-01.js create mode 100644 browser/devtools/performance/test/browser_profiler_tree-model-06.js create mode 100644 browser/devtools/performance/views/jit-optimizations.js create mode 100644 browser/devtools/shared/profiler/jit.js diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index b88d846238d..eabf0aaee0e 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1440,6 +1440,7 @@ pref("devtools.performance.ui.show-platform-data", false); pref("devtools.performance.ui.show-idle-blocks", true); pref("devtools.performance.ui.enable-memory", false); pref("devtools.performance.ui.enable-framerate", true); +pref("devtools.performance.ui.show-jit-optimizations", false); // The default cache UI setting pref("devtools.cache.disabled", false); diff --git a/browser/devtools/jar.mn b/browser/devtools/jar.mn index 1336d186ec2..2e63d0f93a3 100644 --- a/browser/devtools/jar.mn +++ b/browser/devtools/jar.mn @@ -103,6 +103,7 @@ browser.jar: content/browser/devtools/performance/views/details-memory-call-tree.js (performance/views/details-memory-call-tree.js) content/browser/devtools/performance/views/details-memory-flamegraph.js (performance/views/details-memory-flamegraph.js) content/browser/devtools/performance/views/recordings.js (performance/views/recordings.js) + content/browser/devtools/performance/views/jit-optimizations.js (performance/views/jit-optimizations.js) content/browser/devtools/responsivedesign/resize-commands.js (responsivedesign/resize-commands.js) content/browser/devtools/commandline.css (commandline/commandline.css) content/browser/devtools/commandlineoutput.xhtml (commandline/commandlineoutput.xhtml) diff --git a/browser/devtools/performance/performance-controller.js b/browser/devtools/performance/performance-controller.js index ee374f75250..a3e52bdc76c 100644 --- a/browser/devtools/performance/performance-controller.js +++ b/browser/devtools/performance/performance-controller.js @@ -17,6 +17,8 @@ devtools.lazyRequireGetter(this, "EventEmitter", devtools.lazyRequireGetter(this, "DevToolsUtils", "devtools/toolkit/DevToolsUtils"); +devtools.lazyRequireGetter(this, "TreeWidget", + "devtools/shared/widgets/TreeWidget", true); devtools.lazyRequireGetter(this, "TIMELINE_BLUEPRINT", "devtools/shared/timeline/global", true); devtools.lazyRequireGetter(this, "L10N", @@ -39,6 +41,8 @@ devtools.lazyRequireGetter(this, "ThreadNode", "devtools/shared/profiler/tree-model", true); devtools.lazyRequireGetter(this, "FrameNode", "devtools/shared/profiler/tree-model", true); +devtools.lazyRequireGetter(this, "JITOptimizations", + "devtools/shared/profiler/jit", true); devtools.lazyRequireGetter(this, "OptionsView", "devtools/shared/options-view", true); @@ -100,6 +104,11 @@ const EVENTS = { // When the PerformanceController has new recording data TIMELINE_DATA: "Performance:TimelineData", + // Emitted by the JITOptimizationsView when it renders new optimization + // data and clears the optimization data + OPTIMIZATIONS_RESET: "Performance:UI:OptimizationsReset", + OPTIMIZATIONS_RENDERED: "Performance:UI:OptimizationsRendered", + // Emitted by the OverviewView when more data has been rendered OVERVIEW_RENDERED: "Performance:UI:OverviewRendered", FRAMERATE_GRAPH_RENDERED: "Performance:UI:OverviewFramerateRendered", diff --git a/browser/devtools/performance/performance.xul b/browser/devtools/performance/performance.xul index 41ef972679f..24bac6dad4c 100644 --- a/browser/devtools/performance/performance.xul +++ b/browser/devtools/performance/performance.xul @@ -27,6 +27,7 @@