Bug 777200 - SettingsChangeNotifier should only notify processes with settings-change listeners. r=fabrice
authorGregor Wagner <anygregor@gmail.com>
Wed, 26 Sep 2012 14:10:32 -0700
changeset 108292 24e6cf3628aebc049380fd95d101b62a1d9eeadc
parent 108291 ac9fdc7330d7fba845400ec9be7e269443189b29
child 108293 898ae4d394b3339269dc93e760db4242b71d36c7
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersfabrice
bugs777200
milestone18.0a1
Bug 777200 - SettingsChangeNotifier should only notify processes with settings-change listeners. r=fabrice
dom/settings/SettingsChangeNotifier.jsm
dom/settings/SettingsManager.js
dom/settings/tests/Makefile.in
dom/settings/tests/test_settings_onsettingchange.html
--- a/dom/settings/SettingsChangeNotifier.jsm
+++ b/dom/settings/SettingsChangeNotifier.jsm
@@ -23,61 +23,98 @@ const kFromSettingsChangeNotifier      =
 
 XPCOMUtils.defineLazyServiceGetter(this, "ppmm",
                                    "@mozilla.org/parentprocessmessagemanager;1",
                                    "nsIMessageBroadcaster");
 
 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);
   },
 
   observe: function(aSubject, aTopic, aData) {
     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;
         break;
       case kMozSettingsChangedObserverTopic:
       {
         let setting = JSON.parse(aData);
         // To avoid redundantly broadcasting settings-changed events that are
         // just requested from content processes themselves, skip the observer
         // 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;
       }
       default:
         debug("Wrong observer topic: " + aTopic);
         break;
     }
   },
 
+  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({
             key: msg.key,
             value: msg.value,
             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);
     }
   }
 }
 
 SettingsChangeNotifier.init();
--- a/dom/settings/SettingsManager.js
+++ b/dom/settings/SettingsManager.js
@@ -232,20 +232,24 @@ SettingsManager.prototype = {
   nextTick: function nextTick(aCallback, thisObj) {
     if (thisObj)
       aCallback = aCallback.bind(thisObj);
 
     Services.tm.currentThread.dispatch(aCallback, Ci.nsIThread.DISPATCH_NORMAL);
   },
 
   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() {
     return this._onsettingchange;
   },
 
   createLock: function() {
     debug("get lock!");
@@ -289,18 +293,20 @@ SettingsManager.prototype = {
         break;
       default: 
         debug("Wrong message: " + aMessage.name);
     }
   },
 
   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 {
       this._callbacks[aName].push(aCallback);
     }
   },
 
   removeObserver: function removeObserver(aName, aCallback) {
@@ -331,16 +337,17 @@ SettingsManager.prototype = {
     let perm = Services.perms.testExactPermissionFromPrincipal(aWindow.document.nodePrincipal, "settings");
     this.hasPrivileges = perm == Ci.nsIPermissionManager.ALLOW_ACTION;
     debug("has privileges :" + this.hasPrivileges);
   },
 
   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");
         cpmm.removeMessageListener("Settings:Change:Return:OK", this);
         this._requests = null;
         this._window = null;
         this._innerWindowID = null;
         this._onsettingchange = null;
--- a/dom/settings/tests/Makefile.in
+++ b/dom/settings/tests/Makefile.in
@@ -10,14 +10,15 @@ VPATH            = @srcdir@
 relativesrcdir   = @relativesrcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 
 MOCHITEST_FILES = \
   test_settings_basics.html \
   test_settings_events.html \
+  test_settings_onsettingchange.html \
   $(NULL)
 
 _CHROMEMOCHITEST_FILES = \
   $(NULL)
 
 include $(topsrcdir)/config/rules.mk
new file mode 100644
--- /dev/null
+++ b/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>