Bug 1425987 - part 1: Add support to Log.jsm for managing log levels via prefs. r?Gijs draft
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 18 Dec 2017 17:48:41 +1100
changeset 713784 e9149b63743ae157270ecfe601b9e76c02dc0221
parent 713235 1624b88874765bf57e9feba176d30149c748d9d2
child 713785 90504319823c48f7f1b075c535ebdc469b37da67
push id93743
push userbmo:markh@mozilla.com
push dateWed, 20 Dec 2017 23:34:46 +0000
reviewersGijs
bugs1425987
milestone59.0a1
Bug 1425987 - part 1: Add support to Log.jsm for managing log levels via prefs. r?Gijs MozReview-Commit-ID: 6xLSnb0etnl
toolkit/modules/Log.jsm
toolkit/modules/tests/xpcshell/test_Log.js
--- a/toolkit/modules/Log.jsm
+++ b/toolkit/modules/Log.jsm
@@ -260,30 +260,60 @@ function Logger(name, repository) {
     repository = Log.repository;
   this._name = name;
   this.children = [];
   this.ownAppenders = [];
   this.appenders = [];
   this._repository = repository;
 }
 Logger.prototype = {
+  _levelPrefName: null,
+  _levelPrefValue: null,
+
   get name() {
     return this._name;
   },
 
   _level: null,
   get level() {
+    if (this._levelPrefName) {
+      // We've been asked to use a preference to configure the logs. If the
+      // pref has a value we use it, otherwise we continue to use the parent.
+      const lpv = this._levelPrefValue;
+      if (lpv) {
+        const levelValue = Log.Level[lpv];
+        if (levelValue) {
+          // stash it in _level just in case a future value of the pref is
+          // invalid, in which case we end up continuing to use this value.
+          this._level = levelValue;
+          return levelValue;
+        }
+      } else {
+        // in case the pref has transitioned from a value to no value, we reset
+        // this._level and fall through to using the parent.
+        this._level = null;
+      }
+    }
     if (this._level != null)
       return this._level;
     if (this.parent)
       return this.parent.level;
     dumpError("Log warning: root logger configuration error: no level defined");
     return Log.Level.All;
   },
   set level(level) {
+    if (this._levelPrefName) {
+      // I guess we could honor this by nuking this._levelPrefValue, but it
+      // almost certainly implies confusion, so we'll warn and ignore.
+      dumpError(`Log warning: The log '${this.name}' is configured to use ` +
+                `the preference '${this._levelPrefName}' - you must adjust ` +
+                `the level by setting this preference, not by using the ` +
+                `level setter`);
+      return;
+    }
     this._level = level;
   },
 
   _parent: null,
   get parent() {
     return this._parent;
   },
   set parent(parent) {
@@ -297,16 +327,31 @@ Logger.prototype = {
         this._parent.children.splice(index, 1);
       }
     }
     this._parent = parent;
     parent.children.push(this);
     this.updateAppenders();
   },
 
+  manageLevelFromPref(prefName) {
+    if (prefName == this._levelPrefName) {
+      // We've already configured this log with an observer for that pref.
+      return;
+    }
+    if (this._levelPrefName) {
+      dumpError(`The log '${this.name}' is already configured with the ` +
+                `preference '${this._levelPrefName}' - ignoring request to ` +
+                `also use the preference '${prefName}'`);
+      return;
+    }
+    this._levelPrefName = prefName;
+    XPCOMUtils.defineLazyPreferenceGetter(this, "_levelPrefValue", prefName);
+  },
+
   updateAppenders: function updateAppenders() {
     if (this._parent) {
       let notOwnAppenders = this._parent.appenders.filter(function(appender) {
         return this.ownAppenders.indexOf(appender) == -1;
       }, this);
       this.appenders = notOwnAppenders.concat(this.ownAppenders);
     } else {
       this.appenders = this.ownAppenders.slice();
--- a/toolkit/modules/tests/xpcshell/test_Log.js
+++ b/toolkit/modules/tests/xpcshell/test_Log.js
@@ -583,8 +583,39 @@ add_task(function* format_errors() {
     do_check_true(str.includes(":1:11)"));
     // Make sure that we use human-readable stack traces
     // Check that the error doesn't contain any reference to "Promise.jsm" or "Task.jsm"
     do_check_false(str.includes("Promise.jsm"));
     do_check_false(str.includes("Task.jsm"));
     do_check_true(str.includes("format_errors"));
   }
 });
+
+/*
+ * Check that automatic support for setting the log level via preferences
+ * works.
+ */
+add_test(function test_prefs() {
+  let log = Log.repository.getLogger("error.logger");
+  log.level = Log.Level.Debug;
+  Services.prefs.setStringPref("logger.test", "Error");
+  log.manageLevelFromPref("logger.test");
+  // check initial pref value is set up.
+  equal(log.level, Log.Level.Error);
+  Services.prefs.setStringPref("logger.test", "Error");
+  // check changing the pref causes the level to change.
+  Services.prefs.setStringPref("logger.test", "Trace");
+  equal(log.level, Log.Level.Trace);
+  // invalid values cause nothing to happen.
+  Services.prefs.setStringPref("logger.test", "invalid-level-value");
+  equal(log.level, Log.Level.Trace);
+  Services.prefs.setIntPref("logger.test", 123);
+  equal(log.level, Log.Level.Trace);
+  // setting a "sub preference" shouldn't impact it.
+  Services.prefs.setStringPref("logger.test.foo", "Debug");
+  equal(log.level, Log.Level.Trace);
+  // clearing the level param should cause it to use the parent level.
+  Log.repository.getLogger("error").level = Log.Level.All;
+  Services.prefs.setStringPref("logger.test", "");
+  equal(log.level, Log.Level.All);
+
+  run_next_test();
+});