Bug 777200 - SettingsChangeNotifier should only notify processes with settings-change listeners. r=fabrice

dom/settings/SettingsChangeNotifier.jsm

@@ -28,7 +28,12 @@ XPCOMUtils.defineLazyServiceGetter(this, "ppmm",
 let SettingsChangeNotifier = {
   init: function() {
     debug("init");
-    ppmm.addMessageListener("Settings:Changed", this);
+    this.children = [];
+    this._messages = ["Settings:Changed", "Settings:RegisterForMessages", "Settings:UnregisterForMessages"];
+    this._messages.forEach((function(msgName) {
+      ppmm.addMessageListener(msgName, this);
+    }).bind(this));
+
     Services.obs.addObserver(this, kXpcomShutdownObserverTopic, false);
     Services.obs.addObserver(this, kMozSettingsChangedObserverTopic, false);
   },
@@ -37,7 +42,9 @@ let SettingsChangeNotifier = {
     debug("observe");
     switch (aTopic) {
       case kXpcomShutdownObserverTopic:
-        ppmm.removeMessageListener("Settings:Changed", this);
+        this._messages.forEach((function(msgName) {
+          ppmm.removeMessageListener(msgName, this);
+        }).bind(this));
         Services.obs.removeObserver(this, kXpcomShutdownObserverTopic);
         Services.obs.removeObserver(this, kMozSettingsChangedObserverTopic);
         ppmm = null;
@@ -50,7 +57,7 @@ let SettingsChangeNotifier = {
         // messages that are notified from the internal SettingsChangeNotifier.
         if (setting.message && setting.message === kFromSettingsChangeNotifier)
           return;
-        ppmm.broadcastAsyncMessage("Settings:Change:Return:OK",
+        this.broadcastMessage("Settings:Change:Return:OK",
           { key: setting.key, value: setting.value });
         break;
       }
@@ -60,12 +67,29 @@ let SettingsChangeNotifier = {
     }
   },
 
+  broadcastMessage: function broadcastMessage(aMsgName, aContent) {
+    let i;
+    for (i = this.children.length - 1; i >= 0; i -= 1) {
+      let msgMgr = this.children[i];
+      try {
+        msgMgr.sendAsyncMessage(aMsgName, aContent);
+      } catch (e) {
+        let index;
+        if ((index = this.children.indexOf(msgMgr)) != -1) {
+          this.children.splice(index, 1);
+          dump("Remove dead MessageManager!\n");
+        }
+      }
+    };
+  },
+
   receiveMessage: function(aMessage) {
     debug("receiveMessage");
     let msg = aMessage.json;
+    let mm = aMessage.target;
     switch (aMessage.name) {
       case "Settings:Changed":
-        ppmm.broadcastAsyncMessage("Settings:Change:Return:OK",
+        this.broadcastMessage("Settings:Change:Return:OK",
           { key: msg.key, value: msg.value });
         Services.obs.notifyObservers(this, kMozSettingsChangedObserverTopic,
           JSON.stringify({
@@ -74,6 +98,19 @@ let SettingsChangeNotifier = {
             message: kFromSettingsChangeNotifier
           }));
         break;
+      case "Settings:RegisterForMessages":
+        debug("Register!");
+        if (this.children.indexOf(mm) == -1) {
+          this.children.push(mm);
+        }
+        break;
+      case "Settings:UnregisterForMessages":
+        debug("Unregister");
+        let index;
+        if ((index = this.children.indexOf(mm)) != -1) {
+          this.children.splice(index, 1);
+        }
+        break;
       default:
         debug("Wrong message: " + aMessage.name);
     }

dom/settings/SettingsManager.js

@@ -237,10 +237,14 @@ SettingsManager.prototype = {
   },
 
   set onsettingchange(aCallback) {
-    if (this.hasPrivileges)
+    if (this.hasPrivileges) {
+      if (!this._onsettingchange) {
+        cpmm.sendAsyncMessage("Settings:RegisterForMessages");
+      }
       this._onsettingchange = aCallback;
-    else
+    } else {
       throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
+    }
   },
 
   get onsettingchange() {
@@ -294,8 +298,10 @@ SettingsManager.prototype = {
 
   addObserver: function addObserver(aName, aCallback) {
     debug("addObserver " + aName);
-    if (!this._callbacks)
+    if (!this._callbacks) {
+      cpmm.sendAsyncMessage("Settings:RegisterForMessages");
       this._callbacks = {};
+    }
     if (!this._callbacks[aName]) {
       this._callbacks[aName] = [aCallback];
     } else {
@@ -336,6 +342,7 @@ SettingsManager.prototype = {
   observe: function(aSubject, aTopic, aData) {
     debug("Topic: " + aTopic);
     if (aTopic == "inner-window-destroyed") {
+      cpmm.sendAsyncMessage("Settings:UnregisterForMessages");
       let wId = aSubject.QueryInterface(Ci.nsISupportsPRUint64).data;
       if (wId == this.innerWindowID) {
         Services.obs.removeObserver(this, "inner-window-destroyed");

dom/settings/tests/Makefile.in

@@ -15,6 +15,7 @@ include $(DEPTH)/config/autoconf.mk
 MOCHITEST_FILES = \
   test_settings_basics.html \
   test_settings_events.html \
+  test_settings_onsettingchange.html \
   $(NULL)
 
 _CHROMEMOCHITEST_FILES = \

dom/settings/tests/test_settings_onsettingchange.html

@@ -0,0 +1,288 @@
+<!DOCTYPE html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id={678695}
+-->
+<head>
+  <title>Test for Bug {678695} Settings API</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id={678695}">Mozilla Bug {678695}</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+"use strict";
+
+var comp = SpecialPowers.wrap(Components);
+comp.utils.import("resource://gre/modules/SettingsChangeNotifier.jsm");
+SpecialPowers.setBoolPref("dom.mozSettings.enabled", true);
+SpecialPowers.addPermission("settings", true, document);
+
+var screenBright = {"screen.brightness": 0.7};
+
+function onFailure() {
+  ok(false, "in on Failure!");
+}
+
+function observer1(setting) {
+  dump("observer 1 called!\n");
+  is(setting.settingName, "screen.brightness", "Same settingName");
+  is(setting.settingValue, "0.7", "Same settingvalue");
+};
+
+function observer2(setting) {
+  dump("observer 2 called!\n");
+  is(setting.settingName, "screen.brightness", "Same settingName");
+  is(setting.settingValue, "0.7", "Same settingvalue");
+};
+
+var calls = 0;
+function observerOnlyCalledOnce(setting) {
+  is(++calls, 1, "Observer only called once!");
+};
+
+
+function observerWithNext(setting) {
+  dump("observer with next called!\n");
+  is(setting.settingName, "screen.brightness", "Same settingName");
+  is(setting.settingValue, "0.7", "Same settingvalue");
+  next();
+};
+
+function onsettingschangeWithNext(event) {
+  dump("onsettingschangewithnext called!\n");
+  is(event.settingName, "screen.brightness", "Same settingName");
+  is(event.settingValue, "0.7", "Same settingvalue");
+  next();
+};
+
+var req, req2;
+var index = 0;
+
+var mozSettings = window.navigator.mozSettings;
+
+var steps = [
+  function () {
+    ok(true, "Deleting database");
+    var lock = mozSettings.createLock();
+    req = lock.clear();
+    req.onsuccess = function () {
+      ok(true, "Deleted the database");
+    };
+    req.onerror = onFailure;
+    req2 = lock.set(screenBright);
+    req2.onsuccess = function () {
+      ok(true, "set done");
+      navigator.mozSettings.onsettingchange = onsettingschangeWithNext;
+      next();
+    }
+    req2.onerror = onFailure;
+  },
+  function() {
+    ok(true, "testing");
+    var lock = mozSettings.createLock();
+    req2 = lock.set(screenBright);
+    req2.onsuccess = function() {
+      ok(true, "end adding onsettingchange");
+    };
+    req2.onerror = onFailure;
+  },
+  function() {
+    ok(true, "test observers");
+    var lock = mozSettings.createLock();
+    req = lock.get("screen.brightness");
+    req.onsuccess = function () {
+      ok(true, "get done");
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "adding Observers 1");
+    navigator.mozSettings.addObserver("screen.brightness", observer1);
+    navigator.mozSettings.addObserver("screen.brightness", observer1);
+    navigator.mozSettings.addObserver("screen.brightness", observer2);
+    navigator.mozSettings.addObserver("screen.brightness", observerOnlyCalledOnce);
+    var lock = mozSettings.createLock();
+    req2 = lock.get("screen.brightness");
+    req2.onsuccess = function() {
+      ok(true, "set observeSetting done!");
+      next();
+    };
+    req2.onerror = onFailure;
+  },
+  function() {
+    ok(true, "test observers");
+    var lock = mozSettings.createLock();
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set1 done");
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "test observers");
+    var lock = mozSettings.createLock();
+    req = lock.get("screen.brightness");
+    navigator.mozSettings.removeObserver("screen.brightness", observerOnlyCalledOnce);
+    req.onsuccess = function () {
+      ok(true, "set1 done");
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "removing Event Listener");
+    var lock = mozSettings.createLock();
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set2 done");
+      navigator.mozSettings.removeObserver("screen.brightness", observer2);
+      navigator.mozSettings.removeObserver("screen.brightness", observer1);
+      navigator.mozSettings.removeObserver("screen.brightness", observer1);
+    }
+    req.onerror = onFailure;
+  },
+  
+  function() {
+    ok(true, "delete onsettingschange");
+    var lock = mozSettings.createLock();
+    navigator.mozSettings.onsettingchange = null;
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set0 done");
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function () {
+    ok(true, "Waiting for all set callbacks");
+    var lock = mozSettings.createLock();
+    req = lock.get("screen.brightness");
+    req.onsuccess = function() {
+      ok(true, "Done");
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "adding Observers 1");
+    navigator.mozSettings.addObserver("screen.brightness", observer1);
+    navigator.mozSettings.addObserver("screen.brightness", observer1);
+    navigator.mozSettings.addObserver("screen.brightness", observer2);
+    navigator.mozSettings.addObserver("screen.brightness", observerWithNext);
+    var lock = mozSettings.createLock();
+    req2 = lock.get("screen.brightness");
+    req2.onsuccess = function() {
+      ok(true, "set observeSetting done!");
+      next();
+    };
+    req2.onerror = onFailure;
+  },
+  function() {
+    ok(true, "test observers");
+    var lock = mozSettings.createLock();
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set1 done");
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "removing Event Listener");
+    var lock = mozSettings.createLock();
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set2 done");
+      navigator.mozSettings.removeObserver("screen.brightness", observer2);
+      navigator.mozSettings.removeObserver("screen.brightness", observer1);
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "test Event Listener");
+    var lock = mozSettings.createLock();
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set3 done");
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "removing Event Listener");
+    var lock = mozSettings.createLock();
+    navigator.mozSettings.removeObserver("screen.brightness", observerWithNext);
+    req = lock.set(screenBright);
+    req.onsuccess = function () {
+      ok(true, "set4 done");
+      navigator.mozSettings.removeObserver("screen.brightness", observer2);
+      navigator.mozSettings.removeObserver("screen.brightness", observer1);
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "removing Event Listener");
+    var lock = mozSettings.createLock();
+    req = lock.get("screen.brightness");
+    req.onsuccess = function () {
+      ok(true, "get5 done");
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function() {
+    ok(true, "Clear DB");
+    var lock = mozSettings.createLock();
+    req = lock.clear();
+    req.onsuccess = function () {
+      ok(true, "Deleted the database");
+      next();
+    };
+    req.onerror = onFailure;
+  },
+  function () {
+    ok(true, "all done!\n");
+    SimpleTest.finish();
+  }
+];
+
+function next() {
+  ok(true, "Begin!");
+  if (index >= steps.length) {
+    ok(false, "Shouldn't get here!");
+    return;
+  }
+  try {
+    steps[index]();
+  } catch(ex) {
+    ok(false, "Caught exception", ex);
+  }
+  index += 1;
+}
+
+function permissionTest() {
+  if (gSettingsEnabled) {
+    next();
+  } else {
+    is(mozSettings, null, "mozSettings is null when not enabled.");
+    SimpleTest.finish();
+  }
+}
+
+var gSettingsEnabled = SpecialPowers.getBoolPref("dom.mozSettings.enabled");
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(permissionTest);
+
+ok(true, "test passed");
+</script>
+</pre>
+</body>
+</html>