Skip to content

Commit

Permalink
Merge branch 'feature/broadcaster-leaks' into develop
Browse files Browse the repository at this point in the history 
* feature/broadcaster-leaks:
  Disallow multiple suspension manager initializations
  Use promises to initialize background script, kill switches to remove broadcast listeners
  Fix broadcast event listener/handler memory leaks/dead objects from popup, as reported by firefox
  • Loading branch information
@joelpurra
joelpurra committed Apr 25, 2017
2 parents 7a9818c + f3f6549 commit c124b75
Show file tree
Hide file tree
Showing 6 changed files with 233 additions and 96 deletions.
198 changes: 113 additions & 85 deletions src/background/background.js
View file
Expand Up @@ -98,9 +98,6 @@ function main() {
const metadataManager = new MetadataManager();
const configuration = new Configuration(metadataManager, configurationObject);

logDebug("Locale (@@ui_locale)", configuration.uiLocale);
logDebug("Locale (messages.json)", configuration.messagesLocale);

const broadcaster = new Broadcaster();

const onlyLastCaller = new OnlyLastCaller();
Expand Down Expand Up @@ -195,104 +192,135 @@ function main() {
}
}());

// TODO: put initialization promise on the root chain?
return Promise.resolve()
.then(() => suspensionManager.initialize())
.then(() => {
(function registerBroadcastListeners() {
broadcaster.registerListeningAction(knownEvents.stopSpeaking, () => onlyLastCaller.incrementCallerId());
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => onlyLastCaller.incrementCallerId());
const killSwitches = [];

const executeKillSwitches = () => {
// NOTE: expected to have only synchronous methods for the relevant parts.
killSwitches.forEach((killSwitch) => {
try {
killSwitch();
} catch (error) {
logError("executeKillSwitches", error);
}
});
};

// NOTE: synchronous version.
window.addEventListener("unload", () => {
executeKillSwitches();
});

return Promise.all([
broadcaster.registerListeningAction(knownEvents.stopSpeaking, () => onlyLastCaller.incrementCallerId()),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => onlyLastCaller.incrementCallerId()),

broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => plug.once()
.catch((error) => {
// NOTE: swallowing any plug.once() errors.
// NOTE: reduced logging for known tab/page access problems.
// NOTE: swallowing any plug.once() errors.
// NOTE: reduced logging for known tab/page access problems.
if (error && typeof error.message === "string" && error.message.startsWith("Cannot access")) {
logDebug("plug.once", "Error swallowed", error);
} else {
logInfo("plug.once", "Error swallowed", error);
}

return undefined;
}));

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => speakingStatus.setActiveTabAsSpeaking());
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => speakingStatus.setDoneSpeaking());

// NOTE: setting icons async.
broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => { setTimeout(() => iconManager.setIconModePlaying(), 10); return undefined; });
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => { setTimeout(() => iconManager.setIconModeStopped(), 10); return undefined; });

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => buttonPopupManager.disablePopup());
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => buttonPopupManager.enablePopup());

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => suspensionManager.preventExtensionSuspend());
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => suspensionManager.allowExtensionSuspend());

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => progress.resetProgress(0, actionData.text.length, 0));
broadcaster.registerListeningAction(knownEvents.beforeSpeakingPart, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => progress.startSegment(actionData.textPart.length));
broadcaster.registerListeningAction(knownEvents.afterSpeakingPart, () => progress.endSegment());
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => progress.finishProgress());
}());

(function addChromeListeners() {
browser.tabs.onRemoved.addListener(() => talkieBackground.onTabRemovedHandler());
browser.tabs.onUpdated.addListener(() => talkieBackground.onTabUpdatedHandler());

// NOTE: not supported in Firefox (2017-03-15).
// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onSuspend#Browser_compatibility
if (browser.runtime.onSuspend) {
browser.runtime.onSuspend.addListener(() => talkieBackground.onExtensionSuspendHandler());
browser.runtime.onSuspend.addListener(() => suspensionManager.unintialize());
}

// NOTE: used when the popup has been disabled.
browser.browserAction.onClicked.addListener(() => talkieBackground.startStopSpeakSelectionOnPage());

browser.contextMenus.onClicked.addListener((info) => contextMenuManager.contextMenuClickAction(info));

// NOTE: might throw an unexpected error in Firefox due to command configuration in manifest.json.
// Does not seem to happen in Chrome.
// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/commands/onCommand
try {
browser.commands.onCommand.addListener((command) => shortcutKeyManager.handler(command));
} catch (error) {
logError("browser.commands.onCommand.addListener(...)", error);
}
}());

(function exportBackgroundFunctions() {
window.broadcaster = broadcaster;
window.progress = progress;

window.logDebug = logDebug;
window.logInfo = logInfo;
window.logWarn = logWarn;
window.logError = logError;
window.setLoggingLevel = setLevel;

window.getAllVoices = () => talkieSpeaker.getAllVoices();
window.iconClick = () => talkieBackground.startStopSpeakSelectionOnPage();
window.stopSpeakFromFrontend = () => talkieBackground.stopSpeakingAction();
window.startSpeakFromFrontend = (text, voice) => talkieBackground.startSpeakingTextInVoiceAction(text, voice);
window.getVersionName = () => metadataManager.getVersionName();
window.isFreeVersion = () => metadataManager.isFreeVersion();
window.isPremiumVersion = () => metadataManager.isPremiumVersion();
window.getEffectiveVoiceForLanguage = (languageName) => voiceManager.getEffectiveVoiceForLanguage(languageName);
window.isLanguageVoiceOverrideName = (languageName, voiceName) => voiceManager.isLanguageVoiceOverrideName(languageName, voiceName);
window.toggleLanguageVoiceOverrideName = (languageName, voiceName) => voiceManager.toggleLanguageVoiceOverrideName(languageName, voiceName);
window.getVoiceRateDefault = (voiceName) => voiceManager.getVoiceRateDefault(voiceName);
window.setVoiceRateOverride = (voiceName, rate) => voiceManager.setVoiceRateOverride(voiceName, rate);
window.getEffectiveRateForVoice = (voiceName) => voiceManager.getEffectiveRateForVoice(voiceName);
window.getVoicePitchDefault = (voiceName) => voiceManager.getVoicePitchDefault(voiceName);
window.setVoicePitchOverride = (voiceName, pitch) => voiceManager.setVoicePitchOverride(voiceName, pitch);
window.getEffectivePitchForVoice = (voiceName) => voiceManager.getEffectivePitchForVoice(voiceName);
window.getStoredValue = (key) => storageManager.getStoredValue(key);
window.setStoredValue = (key, value) => storageManager.setStoredValue(key, value);
window.getConfigurationValue = (path) => configuration.get(path);
}());
})),

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => speakingStatus.setActiveTabAsSpeaking()),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => speakingStatus.setDoneSpeaking()),

// NOTE: setting icons async.
broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => { setTimeout(() => iconManager.setIconModePlaying(), 10); return undefined; }),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => { setTimeout(() => iconManager.setIconModeStopped(), 10); return undefined; }),

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => buttonPopupManager.disablePopup()),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => buttonPopupManager.enablePopup()),

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, () => suspensionManager.preventExtensionSuspend()),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => suspensionManager.allowExtensionSuspend()),

broadcaster.registerListeningAction(knownEvents.beforeSpeaking, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => progress.resetProgress(0, actionData.text.length, 0)),
broadcaster.registerListeningAction(knownEvents.beforeSpeakingPart, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => progress.startSegment(actionData.textPart.length)),
broadcaster.registerListeningAction(knownEvents.afterSpeakingPart, () => progress.endSegment()),
broadcaster.registerListeningAction(knownEvents.afterSpeaking, () => progress.finishProgress()),
])
.then((registeredKillSwitches) => {
// NOTE: don't want to replace the existing killSwitches array.
registeredKillSwitches.forEach((registeredKillSwitch) => killSwitches.push(registeredKillSwitch));

return undefined;
});
})
.then(() => {
browser.tabs.onRemoved.addListener(() => talkieBackground.onTabRemovedHandler());
browser.tabs.onUpdated.addListener(() => talkieBackground.onTabUpdatedHandler());

// NOTE: not supported in Firefox (2017-03-15).
// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onSuspend#Browser_compatibility
if (browser.runtime.onSuspend) {
browser.runtime.onSuspend.addListener(() => talkieBackground.onExtensionSuspendHandler());
browser.runtime.onSuspend.addListener(() => suspensionManager.unintialize());
}

// NOTE: used when the popup has been disabled.
browser.browserAction.onClicked.addListener(() => talkieBackground.startStopSpeakSelectionOnPage());

browser.contextMenus.onClicked.addListener((info) => contextMenuManager.contextMenuClickAction(info));

// NOTE: might throw an unexpected error in Firefox due to command configuration in manifest.json.
// Does not seem to happen in Chrome.
// https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/commands/onCommand
try {
browser.commands.onCommand.addListener((command) => shortcutKeyManager.handler(command));
} catch (error) {
logError("browser.commands.onCommand.addListener(...)", error);
}

return undefined;
})
.then(() => {
window.broadcaster = broadcaster;
window.progress = progress;

window.logDebug = logDebug;
window.logInfo = logInfo;
window.logWarn = logWarn;
window.logError = logError;
window.setLoggingLevel = setLevel;

window.getAllVoices = () => talkieSpeaker.getAllVoices();
window.iconClick = () => talkieBackground.startStopSpeakSelectionOnPage();
window.stopSpeakFromFrontend = () => talkieBackground.stopSpeakingAction();
window.startSpeakFromFrontend = (text, voice) => talkieBackground.startSpeakingTextInVoiceAction(text, voice);
window.getVersionName = () => metadataManager.getVersionName();
window.isFreeVersion = () => metadataManager.isFreeVersion();
window.isPremiumVersion = () => metadataManager.isPremiumVersion();
window.getEffectiveVoiceForLanguage = (languageName) => voiceManager.getEffectiveVoiceForLanguage(languageName);
window.isLanguageVoiceOverrideName = (languageName, voiceName) => voiceManager.isLanguageVoiceOverrideName(languageName, voiceName);
window.toggleLanguageVoiceOverrideName = (languageName, voiceName) => voiceManager.toggleLanguageVoiceOverrideName(languageName, voiceName);
window.getVoiceRateDefault = (voiceName) => voiceManager.getVoiceRateDefault(voiceName);
window.setVoiceRateOverride = (voiceName, rate) => voiceManager.setVoiceRateOverride(voiceName, rate);
window.getEffectiveRateForVoice = (voiceName) => voiceManager.getEffectiveRateForVoice(voiceName);
window.getVoicePitchDefault = (voiceName) => voiceManager.getVoicePitchDefault(voiceName);
window.setVoicePitchOverride = (voiceName, pitch) => voiceManager.setVoicePitchOverride(voiceName, pitch);
window.getEffectivePitchForVoice = (voiceName) => voiceManager.getEffectivePitchForVoice(voiceName);
window.getStoredValue = (key) => storageManager.getStoredValue(key);
window.setStoredValue = (key, value) => storageManager.setStoredValue(key, value);
window.getConfigurationValue = (path) => configuration.get(path);

return undefined;
})
.then(() => {
buttonPopupManager.enablePopup();

logInfo("Locale (@@ui_locale)", configuration.uiLocale);
logInfo("Locale (messages.json)", configuration.messagesLocale);

logDebug("Done", "Main background function");

return undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/background/suspension-connector-manager.js
View file
Expand Up @@ -68,7 +68,7 @@ export default class SuspensionConnectorManager {
logDebug("Start", "_disconnectToDie");

if (this.talkiePreventSuspensionPort === null) {
// TODO: investigate if this should happen during normal operation, or no.
// TODO: investigate if this should happen during normal operation, or not.
// throw new Error("this.talkiePreventSuspensionPort is null");
logDebug("Done", "_disconnectToDie", "already null");

Expand Down
24 changes: 16 additions & 8 deletions src/background/suspension-manager.js
View file
Expand Up @@ -77,6 +77,10 @@ export default class SuspensionManager {
() => {
logDebug("Start", "SuspensionManager.initialize");

if (this._initialized === true) {
throw new Error("Already initialized.");
}

return this._injectBackgroundFrame()
.then(() => {
logDebug("Done", "SuspensionManager.initialize");
Expand All @@ -94,6 +98,10 @@ export default class SuspensionManager {
() => {
logDebug("Start", "SuspensionManager.unintialize");

if (this._initialized === false) {
throw new Error("Not initialized.");
}

return this._removeBackgroundFrame()
.then(() => {
logDebug("Done", "SuspensionManager.unintialize");
Expand All @@ -111,15 +119,11 @@ export default class SuspensionManager {
() => {
logInfo("SuspensionManager.preventExtensionSuspend");

return Promise.resolve()
.then(() => {
if (this._initialized === false) {
throw new Error("Not initialized.");
}
if (this._initialized === false) {
throw new Error("Not initialized.");
}

return undefined;
})
.then(() => this.suspensionConnectorManager._connectToStayAlive());
return this.suspensionConnectorManager._connectToStayAlive();
}
);
}
Expand All @@ -129,6 +133,10 @@ export default class SuspensionManager {
() => {
logInfo("SuspensionManager.allowExtensionSuspend");

if (this._initialized === false) {
throw new Error("Not initialized.");
}

return this.suspensionConnectorManager._disconnectToDie();
}
);
Expand Down
24 changes: 23 additions & 1 deletion src/popup/popup.js
View file
Expand Up @@ -95,10 +95,24 @@ const updateProgress = (data) => promiseTry(
}
);

const killSwitches = [];

const executeKillSwitches = () => {
// NOTE: expected to have only synchronous methods for the relevant parts.
killSwitches.forEach((killSwitch) => {
try {
killSwitch();
} catch (error) {
dualLogger.dualLogError("executeKillSwitches", error);
}
});
};

const registerBroadcastListeners = () => promiseTry(
() => {
return getBackgroundPage()
.then((background) => background.broadcaster.registerListeningAction(knownEvents.updateProgress, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => updateProgress(actionData)));
.then((background) => background.broadcaster.registerListeningAction(knownEvents.updateProgress, (/* eslint-disable no-unused-vars*/actionName/* eslint-enable no-unused-vars*/, actionData) => updateProgress(actionData)))
.then((killSwitch) => killSwitches.push(killSwitch));
}
);

Expand All @@ -114,6 +128,7 @@ const start = () => promiseTry(

const stop = () => promiseTry(
() => {
// NOTE: probably won't be correctly executed as unload doesn't allow asynchronous calls.
return Promise.resolve()
.then(() => stopFrontend());
}
Expand All @@ -122,4 +137,11 @@ const stop = () => promiseTry(
registerUnhandledRejectionHandler();

document.addEventListener("DOMContentLoaded", eventToPromise.bind(null, start));

// NOTE: probably won't be correctly executed as unload doesn't allow asynchronous calls.
window.addEventListener("unload", eventToPromise.bind(null, stop));

// NOTE: synchronous version.
window.addEventListener("unload", () => {
executeKillSwitches();
});

0 comments on commit c124b75

Please sign in to comment.