author | Gijs Kruitbosch <gijskruitbosch@gmail.com> |
Fri, 13 Jan 2017 13:42:47 +0000 | |
changeset 329491 | a6b1db06ced9a83d00630a1e10b7d782639ca8b1 |
parent 329490 | 4374afb7411ed698716013a6b2c5f8a4235aeb3b |
child 329492 | 4ed09f5f11991b28f838d657e999992d9ad22d46 |
push id | 31210 |
push user | philringnalda@gmail.com |
push date | Sun, 15 Jan 2017 20:32:23 +0000 |
treeherder | mozilla-central@829f16159b89 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | kmag, mythmon |
bugs | 1330954 |
milestone | 53.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/extensions/shield-recipe-client/lib/CleanupManager.jsm +++ b/browser/extensions/shield-recipe-client/lib/CleanupManager.jsm @@ -1,21 +1,29 @@ /* 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/. */ "use strict"; this.EXPORTED_SYMBOLS = ["CleanupManager"]; -const cleanupHandlers = []; +const cleanupHandlers = new Set(); this.CleanupManager = { addCleanupHandler(handler) { - cleanupHandlers.push(handler); + cleanupHandlers.add(handler); + }, + + removeCleanupHandler(handler) { + cleanupHandlers.delete(handler); }, cleanup() { for (const handler of cleanupHandlers) { - handler(); + try { + handler(); + } catch (ex) { + Cu.reportError(ex); + } } }, };
--- a/browser/extensions/shield-recipe-client/lib/Heartbeat.jsm +++ b/browser/extensions/shield-recipe-client/lib/Heartbeat.jsm @@ -95,16 +95,17 @@ this.Heartbeat = class { this.eventEmitter = eventEmitter; this.sandboxManager = sandboxManager; this.options = options; this.surveyResults = {}; this.buttons = null; // so event handlers are consistent this.handleWindowClosed = this.handleWindowClosed.bind(this); + this.close = this.close.bind(this); if (this.options.engagementButtonLabel) { this.buttons = [{ label: this.options.engagementButtonLabel, callback: () => { // Let the consumer know user engaged. this.maybeNotifyHeartbeat("Engaged"); @@ -203,17 +204,17 @@ this.Heartbeat = class { const surveyDuration = Preferences.get(PREF_SURVEY_DURATION, 300) * 1000; this.surveyEndTimer = setTimeout(() => { this.maybeNotifyHeartbeat("SurveyExpired"); this.close(); }, surveyDuration); this.sandboxManager.addHold("heartbeat"); - CleanupManager.addCleanupHandler(() => this.close()); + CleanupManager.addCleanupHandler(this.close); } maybeNotifyHeartbeat(name, data = {}) { if (this.pingSent) { log.warn("Heartbeat event recieved after Telemetry ping sent. name:", name, "data:", data); return; } @@ -276,20 +277,17 @@ this.Heartbeat = class { addClientId: true, addEnvironment: true, }); // only for testing this.eventEmitter.emit("TelemetrySent", Cu.cloneInto(payload, this.sandboxManager.sandbox)); // Survey is complete, clear out the expiry timer & survey configuration - if (this.surveyEndTimer) { - clearTimeout(this.surveyEndTimer); - this.surveyEndTimer = null; - } + this.endTimerIfPresent("surveyEndTimer"); this.pingSent = true; this.surveyResults = null; } if (cleanup) { this.cleanup(); } @@ -310,37 +308,48 @@ this.Heartbeat = class { if (this.options.postAnswerUrl) { for (const key in engagementParams) { this.options.postAnswerUrl.searchParams.append(key, engagementParams[key]); } // Open the engagement URL in a new tab. this.chromeWindow.gBrowser.selectedTab = this.chromeWindow.gBrowser.addTab(this.options.postAnswerUrl.toString()); } - if (this.surveyEndTimer) { - clearTimeout(this.surveyEndTimer); - this.surveyEndTimer = null; + this.endTimerIfPresent("surveyEndTimer"); + + this.engagementCloseTimer = setTimeout(() => this.close(), NOTIFICATION_TIME); + } + + endTimerIfPresent(timerName) { + if (this[timerName]) { + clearTimeout(this[timerName]); + this[timerName] = null; } - - setTimeout(() => this.close(), NOTIFICATION_TIME); } handleWindowClosed() { this.maybeNotifyHeartbeat("WindowClosed"); } close() { this.notificationBox.removeNotification(this.notice); this.cleanup(); } cleanup() { + // Kill the timers which might call things after we've cleaned up: + this.endTimerIfPresent("surveyEndTimer"); + this.endTimerIfPresent("engagementCloseTimer"); + this.sandboxManager.removeHold("heartbeat"); // remove listeners this.chromeWindow.removeEventListener("SSWindowClosing", this.handleWindowClosed); // remove references for garbage collection this.chromeWindow = null; this.notificationBox = null; this.notification = null; + this.notice = null; this.eventEmitter = null; this.sandboxManager = null; + // Ensure we don't re-enter and release the CleanupManager's reference to us: + CleanupManager.removeCleanupHandler(this.close); } };
--- a/browser/extensions/shield-recipe-client/test/browser.ini +++ b/browser/extensions/shield-recipe-client/test/browser.ini @@ -1,9 +1,8 @@ [browser_driver_uuids.js] [browser_env_expressions.js] [browser_EventEmitter.js] [browser_Storage.js] [browser_Heartbeat.js] -skip-if = true # bug 1325409 [browser_NormandyApi.js] support-files = test_server.sjs