зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1330954 - fix leakiness of SHIELD system add-on and re-enable test, r=kmag,mythmon
There are 3 issues this tackles: - we were keeping a reference to the notification we added (this.notice), which kept windows alive. - we had the CleanupManager keep a reference to our close method, which kept a reference to us, which kept a reference to the window. - the fact that we use timeouts to call this.close() in 2 places meant this.close would get called multiple times, which meant we errored out on later occasions, because various things had been nulled out. This tidies up the timeouts when cleanup is called to avoid re-entrancy/errors/leaks. MozReview-Commit-ID: EYvK7bQEh3X --HG-- extra : rebase_source : 8646b7fe965b7110a69a728b54e5c149c750f9c3
This commit is contained in:
Родитель
b8dee21ff7
Коммит
3d0449b2ff
|
@ -6,16 +6,24 @@
|
|||
|
||||
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);
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
|
|
|
@ -100,6 +100,7 @@ this.Heartbeat = class {
|
|||
|
||||
// so event handlers are consistent
|
||||
this.handleWindowClosed = this.handleWindowClosed.bind(this);
|
||||
this.close = this.close.bind(this);
|
||||
|
||||
if (this.options.engagementButtonLabel) {
|
||||
this.buttons = [{
|
||||
|
@ -208,7 +209,7 @@ this.Heartbeat = class {
|
|||
}, surveyDuration);
|
||||
|
||||
this.sandboxManager.addHold("heartbeat");
|
||||
CleanupManager.addCleanupHandler(() => this.close());
|
||||
CleanupManager.addCleanupHandler(this.close);
|
||||
}
|
||||
|
||||
maybeNotifyHeartbeat(name, data = {}) {
|
||||
|
@ -281,10 +282,7 @@ this.Heartbeat = class {
|
|||
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;
|
||||
|
@ -315,12 +313,16 @@ this.Heartbeat = class {
|
|||
this.chromeWindow.gBrowser.selectedTab = this.chromeWindow.gBrowser.addTab(this.options.postAnswerUrl.toString());
|
||||
}
|
||||
|
||||
if (this.surveyEndTimer) {
|
||||
clearTimeout(this.surveyEndTimer);
|
||||
this.surveyEndTimer = null;
|
||||
}
|
||||
this.endTimerIfPresent("surveyEndTimer");
|
||||
|
||||
setTimeout(() => this.close(), NOTIFICATION_TIME);
|
||||
this.engagementCloseTimer = setTimeout(() => this.close(), NOTIFICATION_TIME);
|
||||
}
|
||||
|
||||
endTimerIfPresent(timerName) {
|
||||
if (this[timerName]) {
|
||||
clearTimeout(this[timerName]);
|
||||
this[timerName] = null;
|
||||
}
|
||||
}
|
||||
|
||||
handleWindowClosed() {
|
||||
|
@ -333,6 +335,10 @@ this.Heartbeat = class {
|
|||
}
|
||||
|
||||
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);
|
||||
|
@ -340,7 +346,10 @@ this.Heartbeat = class {
|
|||
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);
|
||||
}
|
||||
};
|
||||
|
|
|
@ -3,7 +3,6 @@
|
|||
[browser_EventEmitter.js]
|
||||
[browser_Storage.js]
|
||||
[browser_Heartbeat.js]
|
||||
skip-if = true # bug 1325409
|
||||
[browser_NormandyApi.js]
|
||||
support-files =
|
||||
test_server.sjs
|
||||
|
|
Загрузка…
Ссылка в новой задаче