604804 - nsUpdateTimerManager.js should run a timer at the optimal frequency

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Alon Zakai (:azakai)]

Attachment: patch

Currently a constant timer is run there, even if a much lower frequency would be good enough. Running at the optimal frequency would save power.

(The timer frequency is currently set at 1 in 10 minutes in firefox and 1 minute in fennec.)

Comment 1

[Robert Strong (they/them - no direct email)]

Thanks, adding this functionality has been on the back burner for a while now. Would you be able to create an xpcshell test for this?
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test_timermanager/

Comment 2

[Robert Strong (they/them - no direct email)]

I was planning on fixing this in bug 517156 as time permits. Could you make it so only one registered timer is fired per notification and reschedule to run in 60 seconds (should be able to control this by a pref as well) if there are additional timers that would have normally fired in the same notification thereby staggering them.

Comment 3

[Alon Zakai (:azakai)]

Hmm, so only one timer should be called each time, at 60 second intervals, even if several want to be called at the same time? I guess in order to spread out the CPU usage/disk IO?

Comment 4

[Robert Strong (they/them - no direct email)]

Yes... after starting up all of the consumers of nsUpdateTimerManager can get notified at the same time and some of them perform async actions so more than one can be doing work at the same time. I am especially concerned about this happening on mobile devices.

Comment 5

[Alon Zakai (:azakai)]

Attachment: Patch v2

New version with staggering, and updated the test to check for staggering.

I didn't add automatic testing for whether the timer keeps firing unnecessarily; that will happen anyhow in a more general way in bug 604527 (where we will check for any timers whatsoever being fired).

Comment 6

[Robert Strong (they/them - no direct email)]

Sorry I haven't got to this yet... I will by Monday at the latest.

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 484096 [details] [diff] [review]
Patch v2

>diff --git a/toolkit/mozapps/update/nsUpdateTimerManager.js b/toolkit/mozapps/update/nsUpdateTimerManager.js
>--- a/toolkit/mozapps/update/nsUpdateTimerManager.js
>+++ b/toolkit/mozapps/update/nsUpdateTimerManager.js
>@@ -1,9 +1,8 @@
>-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> /*
> # ***** BEGIN LICENSE BLOCK *****
> # Version: MPL 1.1/GPL 2.0/LGPL 2.1
> #
> # The contents of this file are subject to the Mozilla Public License Version
> # 1.1 (the "License"); you may not use this file except in compliance with
> # the License. You may obtain a copy of the License at
> # http://www.mozilla.org/MPL/
>@@ -137,52 +136,53 @@ TimerManager.prototype = {
>     case "utm-test-init":
>       // Enforce a minimum timer interval of 500 ms for tests and fall through
>       // to profile-after-change to initialize the timer.
>       minInterval = 500;
>       minFirstInterval = 500;
>     case "profile-after-change":
>       // Cancel the timer if it has already been initialized. This is primarily
>       // for tests.
>-      if (this._timer) {
>-        this._timer.cancel();
>-        this._timer = null;
>-      }
>+      this.cancelTimer();
I don't think this is necessary now

>       this._timerInterval = Math.max(getPref("getIntPref", PREF_APP_UPDATE_TIMER, 600000),
>                                      minInterval);
>       let firstInterval = Math.max(getPref("getIntPref", PREF_APP_UPDATE_TIMERFIRSTINTERVAL,
>                                            this._timerInterval), minFirstInterval);
>-      this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
>-      this._timer.initWithCallback(this, firstInterval,
>-                                   Ci.nsITimer.TYPE_REPEATING_SLACK);
>+      this.ensureTimer(firstInterval);
>       break;
>     case "xpcom-shutdown":
>       Services.obs.removeObserver(this, "xpcom-shutdown");
> 
>       // Release everything we hold onto.
>-      if (this._timer) {
>-        this._timer.cancel();
>-        this._timer = null;
>-      }
>+      this.cancelTimer();
>       for (var timerID in this._timers)
>         delete this._timers[timerID];
>       this._timers = null;
>       break;
>     }
>   },
> 
>   /**
> #    Called when the checking timer fires.
>+#
>+#    We only fire one notification each time, so that the
>+#    operations are staggered. We don't want too many to happen
>+#    at once, which could negatively impact responsiveness.
nit: extend each line up to column 80

>+#
> #    @param   timer
> #             The checking timer that fired.
>    */
>   notify: function TM_notify(timer) {
>-    if (timer.delay != this._timerInterval)
>-      timer.delay = this._timerInterval;
>+    var nextDelay = null;
>+    function updateNextDelay(delay) {
>+      if (nextDelay === null || delay < nextDelay)
>+        nextDelay = delay;
>+    }
> 
>+    var fired = false;
>     var prefLastUpdate;
>     var lastUpdateTime;
>     var now = Math.round(Date.now() / 1000);
>     var catMan = Cc["@mozilla.org/categorymanager;1"].
>                  getService(Ci.nsICategoryManager);
>     var entries = catMan.enumerateCategory(CATEGORY_UPDATE_TIMER);
>     while (entries.hasMoreElements()) {
>       let entry = entries.getNext().QueryInterface(Ci.nsISupportsCString).data;
>@@ -204,34 +204,37 @@ TimerManager.prototype = {
>         lastUpdateTime = Services.prefs.getIntPref(prefLastUpdate);
>       }
>       else {
>         lastUpdateTime = now + this._fudge;
>         Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>         continue;
>       }
> 
>-      if ((now - lastUpdateTime) > interval) {
>+      if (!fired && (now - lastUpdateTime) > interval) {
>         try {
>           Components.classes[cid][method](Ci.nsITimerCallback).notify(timer);
>           LOG("TimerManager:notify - notified " + cid);
>         }
>         catch (e) {
>           LOG("TimerManager:notify - error notifying component id: " +
>               cid + " ,error: " + e);
>         }
>         lastUpdateTime = now + this._fudge;
>         Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>+        fired = true;
>       }
>+
>+      updateNextDelay(lastUpdateTime + interval - now);
>     }
> 
>     for (var timerID in this._timers) {
>       var timerData = this._timers[timerID];
> 
>-      if ((now - timerData.lastUpdateTime) > timerData.interval) {
>+      if (!fired && (now - timerData.lastUpdateTime) > timerData.interval) {
>         if (timerData.callback instanceof Ci.nsITimerCallback) {
>           try {
>             timerData.callback.notify(timer);
>             LOG("TimerManager:notify - notified timerID: " + timerID);
>           }
>           catch (e) {
>             LOG("TimerManager:notify - error notifying timerID: " + timerID +
>                 ", error: " + e);
>@@ -240,17 +243,50 @@ TimerManager.prototype = {
>         else {
>           LOG("TimerManager:notify - timerID: " + timerID + " doesn't " +
>               "implement nsITimerCallback - skipping");
>         }
>         lastUpdateTime = now + this._fudge;
>         timerData.lastUpdateTime = lastUpdateTime;
>         prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/, timerID);
>         Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>+        fired = true;
>       }
>+
>+      updateNextDelay(timerData.lastUpdateTime + timerData.interval - now);
>+    }
With this approach (both the category and register methods) it seems like it would be possible for a single consumer to starve the other consumers from being notified. How about setting the delay to 600000 milliseconds when there are consumers to be notified that are skipped and set the last update pref for each consumer that was skipped to now - interval + 60 * counter and increment the counter for each consumer that was skipped to stagger them?

>+
>+    if (nextDelay !== null) {
>+      timer.delay = Math.max(nextDelay*1000, this._timerInterval);
nit: space on both sides of the * e.g. nextDelay * 1000

I don't think that the timer interval pref makes any sense with this approach and I'm leaning towards removing it. The app.update.timerFirstInterval pref should suffice to get things going. What do you think?

>+      this.lastTimerReset = Date.now();
>+    } else {
>+      this.cancelTimer();
>+    }
>+  },
>+
>+  /**
>+   * Starts the timer, if necessary, and ensures that it will fire soon enough
>+   * to happen after time |interval| (in milliseconds).
>+   */
>+  ensureTimer: function(interval) {
nit: prefix internal helpers with _
 
>+    if (!this.timer) {
>+      this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
>+      this._timer.initWithCallback(this, interval,
>+                                   Ci.nsITimer.TYPE_REPEATING_SLACK);
>+      this.lastTimerReset = Date.now();
>+    } else {
>+      if (Date.now() + interval < this.lastTimerReset + this.timer.delay) 
>+        this.timer.delay = this.lastTimerReset + interval - Date.now();
>+    }
>+  },
>+
>+  cancelTimer: function() {
nit: please add a comment describing the function and prefix internal helpers with _

>+    if (this._timer) {
>+      this._timer.cancel();
>+      this._timer = null;
>     }
>   },
> 
>   /**
>    * See nsIUpdateTimerManager.idl
>    */
>   registerTimer: function TM_registerTimer(id, callback, interval) {
>     LOG("TimerManager:registerTimer - id: " + id);
>@@ -260,16 +296,18 @@ TimerManager.prototype = {
>       lastUpdateTime = Services.prefs.getIntPref(prefLastUpdate);
>     } else {
>       lastUpdateTime = Math.round(Date.now() / 1000) + this._fudge;
>       Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>     }
>     this._timers[id] = { callback       : callback,
>                          interval       : interval,
>                          lastUpdateTime : lastUpdateTime };
>+
>+    this.ensureTimer(interval*1000);
nit: space on both sides of the * e.g. interval * 1000

registerTimer can be called before profile-after-change and _ensureTimer should only be called after profile-after-change since profile preferences are needed. There are a few different ways that can be addressed such as a boolean set to true when profile-after-change is called.

>diff --git a/toolkit/mozapps/update/test_timermanager/unit/test_0010_timermanager.js b/toolkit/mozapps/update/test_timermanager/unit/test_0010_timermanager.js
>--- a/toolkit/mozapps/update/test_timermanager/unit/test_0010_timermanager.js
>+++ b/toolkit/mozapps/update/test_timermanager/unit/test_0010_timermanager.js
>@@ -112,16 +112,24 @@ const TESTS = [ {
> }, {
>   desc            : "Test Timer Callback 7",
>   timerID         : "test7-update-timer",
>   defaultInterval : CONSUMER_TIMER_INTERVAL,
>   contractID      : "@mozilla.org/test7/timercallback;1",
>   classID         : Components.ID("af878d4b-1d12-41f6-9a90-4e687367ecc1"),
>   notified        : false,
>   lastUpdateTime  : 0
>+}, {
>+  desc            : "Test Timer Callback 8",
>+  timerID         : "test8-update-timer",
>+  defaultInterval : CONSUMER_TIMER_INTERVAL,
>+  contractID      : "@mozilla.org/test8/timercallback;1",
>+  classID         : Components.ID("5136b201-d64c-4328-8cf1-1a63491cc117"),
>+  notified        : false,
>+  lastUpdateTime  : 0
> } ];
> 
> var gUTM;
> var gNextFunc;
> 
> XPCOMUtils.defineLazyServiceGetter(this, "gPref",
>                                    "@mozilla.org/preferences-service;1",
>                                    "nsIPrefBranch2");
>@@ -273,29 +281,43 @@ function check_test1thru6() {
>        "registered\n");
>   do_check_eq(count, 0);
> 
>   do_timeout(0, run_test7);
> }
> 
> function run_test7() {
>   gNextFunc = check_test7;
>-  gPref.setIntPref(PREF_BRANCH_LAST_UPDATE_TIME + TESTS[6].timerID, 1);
>-  gCompReg.registerFactory(TESTS[6].classID, TESTS[6].desc,
>-                           TESTS[6].contractID, gTest7Factory);
>-  gUTM.registerTimer(TESTS[6].timerID, gTest7TimerCallback,
>-                     TESTS[6].defaultInterval);
>+  for (var i = 0; i < 2; i++) {
>+    gPref.setIntPref(PREF_BRANCH_LAST_UPDATE_TIME + TESTS[6+i].timerID, 1);
>+    gCompReg.registerFactory(TESTS[6+i].classID, TESTS[6+i].desc,
>+                             TESTS[6+i].contractID, eval("gTest" + (7+i) + "Factory"));
>+    gUTM.registerTimer(TESTS[6+i].timerID, eval("gTest" + (7+i) + "TimerCallback"),
>+                       TESTS[6+i].defaultInterval);
>+  }
nit: space on both sides of the +'s e.g. TESTS[6 + i] and (7 + i)

> }
> 
> function check_test7() {
>-  dump("Testing: one registerTimer registered timer has fired\n");
>-  do_check_true(TESTS[6].notified);
>-  dump("Testing: one registerTimer registered timer last update time has " +
>+  var self = arguments.callee;
>+  self.timesCalled = (self.timesCalled || 0) + 1;
>+  if (self.timesCalled < 2) return;
nit: return on its own line

>+
>+  dump("Testing: two registerTimer registered timers have fired\n");
>+  for (var i = 0; i < 2; i++)
>+    do_check_true(TESTS[6+i].notified);
nit: space on both sides of the +'s e.g. TESTS[6 + i]

>+
>+  // Check that 'staggering' has happened: even though the two events wanted to fire at
>+  // the same time, we waited a full MAIN_TIMER_INTERVAL between them.
>+  // (to avoid sensitivity to random timing issues, we fudge by a factor of 0.5 here)
>+  do_check_true(Math.abs(TESTS[6].notifyTime - TESTS[7].notifyTime) >= MAIN_TIMER_INTERVAL*0.5);
nit: space on both sides of the * e.g. MAIN_TIMER_INTERVAL * 0.5

>+
>+  dump("Testing: two registerTimer registered timers last update time have " +
>        "been updated\n");
>-  do_check_neq(gPref.getIntPref(PREF_BRANCH_LAST_UPDATE_TIME + TESTS[6].timerID), 1);
>+  for (var i = 0; i < 2; i++)
>+    do_check_neq(gPref.getIntPref(PREF_BRANCH_LAST_UPDATE_TIME + TESTS[6+i].timerID), 1);
nit: space on both sides of the +'s e.g. TESTS[6 + i]

Really like what you've done here! Thanks

Comment 8

[Alon Zakai (:azakai)]

Attachment: patch v3

Updated patch.

Regarding the starvation scenario, I'm not sure I understood what you meant to do. What the fixed patch now does is it fires a single event each time, specifically the one that wanted to be fired earliest, and others are delayed. That way all events must eventually be fired (each will eventually become the one that wanted to be called earliest), and starvation can't happen. Is that ok?

Comment 9

[Robert Strong (they/them - no direct email)]

Sorry I haven't been able to get to this sooner... I came down with the flu this week. I'll hopefully be up for reviewing this over the weekend.

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 487001 [details] [diff] [review]
patch v3

>diff --git a/toolkit/mozapps/update/nsUpdateTimerManager.js b/toolkit/mozapps/update/nsUpdateTimerManager.js
>--- a/toolkit/mozapps/update/nsUpdateTimerManager.js
>+++ b/toolkit/mozapps/update/nsUpdateTimerManager.js
>@@ -137,120 +136,193 @@ TimerManager.prototype = {
>...
>+      // We do not need to updateNextDelay for the timer that actually fires;
>+      // we'll update right after it fires, with the proper intended time.
>+      // Note that we might select one, then select another later (with an
>+      // earlier intended time); it is still ok that we did not update for
>+      // the first one, since if we have skipped firings, the next delay
>+      // will be the minimal delay anyhow.
nit: minimum delay

>-      if ((now - timerData.lastUpdateTime) > timerData.interval) {
>+    for (let _timerID in this._timers) {
>+      let timerID = _timerID; // necessary for the closure to work properly
>+      let timerData = this._timers[timerID];
>+      tryFire(function() {
>         if (timerData.callback instanceof Ci.nsITimerCallback) {
>           try {
>             timerData.callback.notify(timer);
>             LOG("TimerManager:notify - notified timerID: " + timerID);
>           }
>           catch (e) {
>             LOG("TimerManager:notify - error notifying timerID: " + timerID +
>                 ", error: " + e);
>           }
>         }
>         else {
>           LOG("TimerManager:notify - timerID: " + timerID + " doesn't " +
>               "implement nsITimerCallback - skipping");
>         }
>-        lastUpdateTime = now + this._fudge;
>+        lastUpdateTime = now + self._fudge;
_fudge should no longer be necessary with the staggering. I believe this allows removing "var self = this;" as well

>         timerData.lastUpdateTime = lastUpdateTime;
>-        prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/, timerID);
>+        var prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/, timerID);
>         Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>-      }
>+        updateNextDelay(timerData.lastUpdateTime + timerData.interval - now);
>+      }, timerData.lastUpdateTime + timerData.interval);
>+    }
>+
>+    if (callbackToFire)
>+      callbackToFire();
>+
>+    if (nextDelay !== null) {
>+      if (skippedFirings)
>+        timer.delay = this._timerInterval;
This along with the PREF_APP_UPDATE_TIMER preference will make it have a 10 minute stagger between firings when skipping a nsITimerCallback callback which is too long. Instead of PREF_APP_UPDATE_TIMER I think it would be better to get rid of PREF_APP_UPDATE_TIMER and go with a PREF_APP_UPDATE_TIMERMINIMUMDELAY (app.update.timerMinimumDelay) with a default value of 120 in seconds. I think it would be best if the pref was represented in seconds to be consistent with the nsITimerCallback consumers interval pref. this._timerInterval would become this._timerMinimumDelay which itself could be in ms.

Does that sound good to you? I suspect the resubmitted patch will only take a couple of minutes to review. Thanks!

Comment 11

[Alon Zakai (:azakai)]

Attachment: patch v4

Sounds good, here's a patch with the requested changes.

The only additional thing I did was change the interval used in the test, from 500ms to 1000ms, because that is now stored in a pref which is measured in seconds, so I didn't want it to get rounded implicitly to 0s or 1s.

Comment 12

[Robert Strong (they/them - no direct email)]

I see you didn't remove _fudge... do you think it is still necessary?

Comment 13

[Robert Strong (they/them - no direct email)]

ahhh... my comment about removing it wasn't just for that one use. I don't think it is necessary anymore and can be removed entirely

Comment 14

[Alon Zakai (:azakai)]

I thought it was still useful for the initial times. That staggers them somewhat, which can lead to less overlap later (but that would be fixed later anyhow).

Do you want a new patch without the fudge entirely?

Comment 15

[Robert Strong (they/them - no direct email)]

Please. I'm reviewing it as is now but I'd like one attached to the bug anyway.

Comment 16

[Robert Strong (they/them - no direct email)]

Comment on attachment 492573 [details] [diff] [review]
patch v4

r=me with _fudge removed. Please attach an updated patch and thanks for doing this!

Comment 17

[Robert Strong (they/them - no direct email)]

Comment on attachment 492573 [details] [diff] [review]
patch v4

Sorry, I found a first run bug where nextDelay will be null due to the lastUpdateTime prefs not existing. I'd prefer to get that fixed before this lands.

Comment 18

[Robert Strong (they/them - no direct email)]

Comment on attachment 492573 [details] [diff] [review]
patch v4

>diff --git a/toolkit/mozapps/update/nsUpdateTimerManager.js b/toolkit/mozapps/update/nsUpdateTimerManager.js
>--- a/toolkit/mozapps/update/nsUpdateTimerManager.js
>+++ b/toolkit/mozapps/update/nsUpdateTimerManager.js
>...
>       let interval = getPref("getIntPref", prefInterval, defaultInterval);
>-      prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/,
>+      let prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/,
>                                                                   timerID);
>       if (Services.prefs.prefHasUserValue(prefLastUpdate)) {
>         lastUpdateTime = Services.prefs.getIntPref(prefLastUpdate);
>       }
>       else {
>-        lastUpdateTime = now + this._fudge;
>+        lastUpdateTime = now;
>         Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>         continue;
I am fairly certain the first run cancellation of the timer is due to the continue above.

Comment 19

[Robert Strong (they/them - no direct email)]

Comment on attachment 492573 [details] [diff] [review]
patch v4

r=me with _fudge removed and the continue removed.

Comment 20

[Alon Zakai (:azakai)]

Attachment: final patch

Fixed those last 2 things, carried r+. Will push this tomorrow.

Thanks for the reviewing!

Comment 21

[Robert Strong (they/them - no direct email)]

cc'ing some people regarding the removal of the app.update.timer preference and the addition of the app.update.timerMinimumDelay preference which is the minimum delay in seconds for the timer delay and it defaults to 120 seconds.

Comment 22

[Robert Strong (they/them - no direct email)]

Comment on attachment 492582 [details] [diff] [review]
final patch

>diff --git a/toolkit/mozapps/update/nsUpdateTimerManager.js b/toolkit/mozapps/update/nsUpdateTimerManager.js
>--- a/toolkit/mozapps/update/nsUpdateTimerManager.js
>+++ b/toolkit/mozapps/update/nsUpdateTimerManager.js
>...
>@@ -137,139 +125,212 @@ TimerManager.prototype = {
>     case "utm-test-init":
>       // Enforce a minimum timer interval of 500 ms for tests and fall through
>       // to profile-after-change to initialize the timer.
>       minInterval = 500;
>       minFirstInterval = 500;
>     case "profile-after-change":
>       // Cancel the timer if it has already been initialized. This is primarily
>       // for tests.
>-      if (this._timer) {
>-        this._timer.cancel();
>-        this._timer = null;
>-      }
>-      this._timerInterval = Math.max(getPref("getIntPref", PREF_APP_UPDATE_TIMER, 600000),
>-                                     minInterval);
>+      this._timerMinimumDelay = Math.max(1000*getPref("getIntPref", PREF_APP_UPDATE_TIMERMINIMUMDELAY, 120),
>+                                         minInterval);
missed a style nit... please add a space before and after *

Comment 23

[Alon Zakai (:azakai)]

Pushed with the last style nit,

http://hg.mozilla.org/mozilla-central/rev/ac613052b58f

Comment 25

[Robert Kaiser]

(In reply to comment #21)
> cc'ing some people regarding the removal of the app.update.timer preference and
> the addition of the app.update.timerMinimumDelay preference which is the
> minimum delay in seconds for the timer delay and it defaults to 120 seconds.

Umm, shouldn't the former have been removed from firefox.js then (as well as the counterparts in our other products, of course)?

Comment 26

[Robert Strong (they/them - no direct email)]

(In reply to comment #25)
> (In reply to comment #21)
> > cc'ing some people regarding the removal of the app.update.timer preference and
> > the addition of the app.update.timerMinimumDelay preference which is the
> > minimum delay in seconds for the timer delay and it defaults to 120 seconds.
> 
> Umm, shouldn't the former have been removed from firefox.js then (as well as
> the counterparts in our other products, of course)?
Umm, it doesn't cause any harm besides it being an unused preference - bug 614181 will handle that for Firefox. Also, in Firefox we try to include all the preferences even if the default is the same as the preference that is included and this notification allows other apps to choose to do the same thing as well for the new app.update.timerMinimumDelay preference.

Comment 27

[Robert Strong (they/them - no direct email)]

(In reply to comment #25)
> (In reply to comment #21)
> > cc'ing some people regarding the removal of the app.update.timer preference and
> > the addition of the app.update.timerMinimumDelay preference which is the
> > minimum delay in seconds for the timer delay and it defaults to 120 seconds.
> 
> Umm, shouldn't the former have been removed from firefox.js then (as well as
> the counterparts in our other products, of course)?
If you like I can give you a list of all of the app update preferences that SeaMonkey has added that are the same as the default value used by app update and you can then remove them... this way you won't have to worry about them in the future. Otherwise, expect that I will cc you on bugs where prefs are removed / changed like this bug so you can then remove / change the prefs SeaMonkey has set.

Comment 28

[Robert Kaiser]

(In reply to comment #26)
> Umm, it doesn't cause any harm besides it being an unused preference - bug
> 614181 will handle that for Firefox.

Ah, OK, there it is. I just had marked this bug as something I need to look at due to your CC and wondered where the lines to adopt on our side are, as I usually find those in one patch with the toolkit stuff. :)

And yes, please just keep CCing me or someone else from the SeaMonkey team, thanks for doing that!