Skip to content
This repository has been archived by the owner on Oct 18, 2018. It is now read-only.

Updated to use memory.notify rather than memory.log for Firefox 16 and above. Fixes issue #134 #141

Closed
wants to merge 5 commits into from
Closed

Updated to use memory.notify rather than memory.log for Firefox 16 and above. Fixes issue #134 #141

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1993336
Updated to use memory.notify rather than memory.log for Firefox 16 an…
davehunt Jul 16, 2012
ae2d12f
Reduced code duplication and improved comments.
davehunt Jul 17, 2012
56aa1d0
Made GC preference public so it can be accessed from the tests.
davehunt Jul 23, 2012
aca803d
Moved the GC notification preference out of global scope.
davehunt Jul 25, 2012
4fa5b11
Moved preference getter below constructor as suggested.
davehunt Aug 7, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion extension/lib/config.js
View file
Open in desktop
Expand Up @@ -19,7 +19,8 @@ const PREFERENCES = {
modified_prefs: 'extensions.' + self.id + '.modifiedPrefs',

// Application preferences
memory_log: 'javascript.options.mem.log'
memory_log: 'javascript.options.mem.log',
memory_notify: 'javascript.options.mem.notify'
}


Expand Down
25 changes: 17 additions & 8 deletions extension/lib/garbage-collector.js
View file
Open in desktop
Expand Up @@ -19,6 +19,9 @@ const unload = require("api-utils/unload");
const config = require("config");


var pref_gc_notifications;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable should still be on the reporter object and not stay in the global scope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed on IRC, I have attempted to move this out of global scope, but this causes the test to fail.



const reporter = EventEmitter.compose({
constructor: function Reporter() {
// Report unhandled errors from listeners
Expand All @@ -27,9 +30,15 @@ const reporter = EventEmitter.compose({
// Make sure we clean-up correctly
unload.ensure(this, 'unload');

// For now the logger preference has to be enabled to be able to
// parse the GC / CC information from the console service messages
this._isEnabled = prefs.get(config.preferences.memory_log);
if (config.application.branch >= 16) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should maintain those different paths multiple times in our code. Simply assign the right pref to an internal property like _pref_gc_notifications or similar. That way we don't need another if/else in _enable.

pref_gc_notifications = config.preferences.memory_notify;
}
else {
pref_gc_notifications = config.preferences.memory_log;
}

// Ensure GC/CC observer and console messages preference is enabled
this._isEnabled = prefs.get(pref_gc_notifications);
if (!this._isEnabled)
this._enable();

Expand Down Expand Up @@ -67,15 +76,13 @@ const reporter = EventEmitter.compose({
},

_enable: function Reporter__enable() {
let logging_pref= config.preferences.memory_log;

var modifiedPrefs = JSON.parse(prefs.get(config.preferences.modified_prefs,
"{}"));
if (!modifiedPrefs.hasOwnProperty(logging_pref)) {
modifiedPrefs[logging_pref] = prefs.get(logging_pref);
if (!modifiedPrefs.hasOwnProperty(pref_gc_notifications)) {
modifiedPrefs[pref_gc_notifications] = prefs.get(pref_gc_notifications);
}

prefs.set(logging_pref, true);
prefs.set(pref_gc_notifications, true);
prefs.set(config.preferences.modified_prefs, JSON.stringify(modifiedPrefs));
this._isEnabled = true;
},
Expand Down Expand Up @@ -160,6 +167,8 @@ var doGlobalGC = function () {
}


exports.pref_gc_notifications = pref_gc_notifications;

exports.reporter = reporter;

exports.doCC = doCC;
Expand Down
6 changes: 3 additions & 3 deletions extension/test/test-garbage-collector.js
View file
Open in desktop
Expand Up @@ -2,9 +2,9 @@ const prefs = require("api-utils/preferences-service");

const config = require('memchaser/config');

exports.test_javascript_mem_log_enabled = function (test) {
exports.test_javascript_memory_pref_enabled = function (test) {
// We have to require the garbage collector to initialize the module
require("memchaser/garbage-collector");
var gc = require("garbage-collector");

test.assert(prefs.get(config.preferences.memory_log));
test.assert(prefs.get(gc.pref_gc_notifications));
}