Backed out changeset 7ef4465f20bd (bug 777196) for OSX asserts on a CLOSED TREE.

toolkit/components/contentprefs/ContentPrefService2.jsm

@@ -196,7 +196,7 @@ ContentPrefService2.prototype = {
       this._pbStore.set(group, name, value);
       this._schedule(function () {
         cbHandleCompletion(callback, Ci.nsIContentPrefCallback2.COMPLETE_OK);
-        this._cps._notifyPrefSet(group, name, value);
+        this._cps._broadcastPrefSet(group, name, value);
       });
       return;
     }
@@ -266,7 +266,7 @@ ContentPrefService2.prototype = {
           this._cache.setWithCast(group, name, value);
         cbHandleCompletion(callback, reason);
         if (ok)
-          this._cps._notifyPrefSet(group, name, value);
+          this._cps._broadcastPrefSet(group, name, value);
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
@@ -356,7 +356,7 @@ ContentPrefService2.prototype = {
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, , ] in prefs) {
-            this._cps._notifyPrefRemoved(sgroup, name);
+            this._cps._broadcastPrefRemoved(sgroup, name);
           }
         }
       },
@@ -448,7 +448,7 @@ ContentPrefService2.prototype = {
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, sname, ] in prefs) {
-            this._cps._notifyPrefRemoved(sgroup, sname);
+            this._cps._broadcastPrefRemoved(sgroup, sname);
           }
         }
       },
@@ -505,7 +505,7 @@ ContentPrefService2.prototype = {
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, sname, ] in prefs) {
-            this._cps._notifyPrefRemoved(sgroup, sname);
+            this._cps._broadcastPrefRemoved(sgroup, sname);
           }
         }
       },
@@ -585,7 +585,7 @@ ContentPrefService2.prototype = {
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, , ] in prefs) {
-            this._cps._notifyPrefRemoved(sgroup, name);
+            this._cps._broadcastPrefRemoved(sgroup, name);
           }
         }
       },

toolkit/components/contentprefs/nsContentPrefService.js

@@ -9,6 +9,25 @@ const Cu = Components.utils;
 
 const CACHE_MAX_GROUP_ENTRIES = 100;
 
+
+// We have a whitelist for getting/setting. This is because
+// there are potential privacy issues with a compromised
+// content process checking the user's content preferences
+// and using that to discover all the websites visited, etc.
+// Also there are both potential race conditions (if two processes
+// set more than one value in succession, and the values
+// only make sense together), as well as security issues, if
+// a compromised content process can send arbitrary setPref
+// messages. The whitelist contains only those settings that
+// are not at risk for either.
+// We currently whitelist saving/reading the last directory of file
+// uploads, and the last current spellchecker dictionary which are so far
+// the only need we have identified.
+const REMOTE_WHITELIST = [
+  "browser.upload.lastDir",
+  "spellcheck.lang",
+];
+
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 /**
@@ -22,11 +41,75 @@ function electrolify(service) {
   service.wrappedJSObject = service;
 
   var appInfo = Cc["@mozilla.org/xre/app-info;1"];
-  if (appInfo && appInfo.getService(Ci.nsIXULRuntime).processType !=
+  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
-      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT)
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
-  {
+    // Parent process
+
+    service.messageManager = Cc["@mozilla.org/parentprocessmessagemanager;1"].
+                             getService(Ci.nsIMessageBroadcaster);
+
+    // Setup listener for child messages. We don't need to call
+    // addMessageListener as the wakeup service will do that for us.
+    service.receiveMessage = function(aMessage) {
+      var json = aMessage.json;
+
+      if (REMOTE_WHITELIST.indexOf(json.name) == -1)
+        return { succeeded: false };
+
+      switch (aMessage.name) {
+        case "ContentPref:getPref":
+          return { succeeded: true,
+                   value: service.getPref(json.group, json.name, json.value) };
+
+        case "ContentPref:setPref":
+          service.setPref(json.group, json.name, json.value);
+          return { succeeded: true };
+      }
+    };
+  } else {
     // Child process
+
     service._dbInit = function(){}; // No local DB
+
+    service.messageManager = Cc["@mozilla.org/childprocessmessagemanager;1"].
+                             getService(Ci.nsISyncMessageSender);
+
+    // Child method remoting
+    [
+      ['getPref', ['group', 'name'], ['_parseGroupParam']],
+      ['setPref', ['group', 'name', 'value'], ['_parseGroupParam']],
+    ].forEach(function(data) {
+      var method = data[0];
+      var params = data[1];
+      var parsers = data[2];
+      service[method] = function __remoted__() {
+        var json = {};
+        for (var i = 0; i < params.length; i++) {
+          if (params[i]) {
+            json[params[i]] = arguments[i];
+            if (parsers[i])
+              json[params[i]] = this[parsers[i]](json[params[i]]);
+          }
+        }
+        var ret = service.messageManager.sendSyncMessage('ContentPref:' + method, json)[0];
+        if (!ret.succeeded)
+          throw "ContentPrefs remoting failed to pass whitelist";
+        return ret.value;
+      };
+    });
+
+    // Listen to preference change notifications from the parent and notify
+    // observers in the child process according to the change
+    service.messageManager.addMessageListener("ContentPref:notifyPrefSet",
+      function(aMessage) {
+        var json = aMessage.json;
+        service._notifyPrefSet(json.group, json.name, json.value);
+      });
+    service.messageManager.addMessageListener("ContentPref:notifyPrefRemoved",
+      function(aMessage) {
+        var json = aMessage.json;
+        service._notifyPrefRemoved(json.group, json.name);
+      });
   }
 }
 
@@ -267,7 +350,7 @@ ContentPrefService.prototype = {
 
     if (aContext && aContext.usePrivateBrowsing) {
       this._privModeStorage.setWithCast(group, aName, aValue);
-      this._notifyPrefSet(group, aName, aValue);
+      this._broadcastPrefSet(group, aName, aValue);
       return;
     }
 
@@ -289,7 +372,7 @@ ContentPrefService.prototype = {
       this._insertPref(groupID, settingID, aValue);
 
     this._cache.setWithCast(group, aName, aValue);
-    this._notifyPrefSet(group, aName, aValue);
+    this._broadcastPrefSet(group, aName, aValue);
   },
 
   hasPref: function ContentPrefService_hasPref(aGroup, aName, aContext) {
@@ -317,7 +400,7 @@ ContentPrefService.prototype = {
 
     if (aContext && aContext.usePrivateBrowsing) {
       this._privModeStorage.remove(group, aName);
-      this._notifyPrefRemoved(group, aName);
+      this._broadcastPrefRemoved(group, aName);
       return;
     }
 
@@ -340,7 +423,7 @@ ContentPrefService.prototype = {
       this._deleteGroupIfUnused(groupID);
 
     this._cache.remove(group, aName);
-    this._notifyPrefRemoved(group, aName);
+    this._broadcastPrefRemoved(group, aName);
   },
 
   removeGroupedPrefs: function ContentPrefService_removeGroupedPrefs(aContext) {
@@ -375,7 +458,7 @@ ContentPrefService.prototype = {
       for (let [group, name, ] in this._privModeStorage) {
         if (name === aName) {
           this._privModeStorage.remove(group, aName);
-          this._notifyPrefRemoved(group, aName);
+          this._broadcastPrefRemoved(group, aName);
         }
       }
     }
@@ -417,7 +500,7 @@ ContentPrefService.prototype = {
       if (groupNames[i]) // ie. not null, which will be last (and i == groupIDs.length)
         this._deleteGroupIfUnused(groupIDs[i]);
       if (!aContext || !aContext.usePrivateBrowsing) {
-        this._notifyPrefRemoved(groupNames[i], aName);
+        this._broadcastPrefRemoved(groupNames[i], aName);
       }
     }
   },
@@ -536,6 +619,38 @@ ContentPrefService.prototype = {
     }
   },
 
+  /**
+   * Notify all observers in the current process about the removal of a
+   * preference and send a message to all other processes so that they can in
+   * turn notify their observers about the change. This is meant to be called
+   * only in the parent process. Only whitelisted preferences are broadcast to
+   * the child processes.
+   */
+  _broadcastPrefRemoved: function ContentPrefService__broadcastPrefRemoved(aGroup, aName) {
+    this._notifyPrefRemoved(aGroup, aName);
+
+    if (REMOTE_WHITELIST.indexOf(aName) != -1) {
+      this.messageManager.broadcastAsyncMessage('ContentPref:notifyPrefRemoved',
+        { "group": aGroup, "name": aName } );
+    }
+  },
+
+  /**
+   * Notify all observers in the current process about a preference change and
+   * send a message to all other processes so that they can in turn notify
+   * their observers about the change. This is meant to be called only in the
+   * parent process. Only whitelisted preferences are broadcast to the child
+   * processes.
+   */
+  _broadcastPrefSet: function ContentPrefService__broadcastPrefSet(aGroup, aName, aValue) {
+    this._notifyPrefSet(aGroup, aName, aValue);
+
+    if (REMOTE_WHITELIST.indexOf(aName) != -1) {
+      this.messageManager.broadcastAsyncMessage('ContentPref:notifyPrefSet',
+        { "group": aGroup, "name": aName, "value": aValue } );
+    }
+  },
+
   _grouper: null,
   get grouper() {
     if (!this._grouper)

toolkit/components/contentprefs/tests/moz.build

@@ -7,3 +7,7 @@
 MODULE = 'test_toolkit_contentprefs'
 
 XPCSHELL_TESTS_MANIFESTS += ['unit/xpcshell.ini', 'unit_cps2/xpcshell.ini']
+
+# FIXME/bug 575918: out-of-process xpcshell is broken on OS X
+if CONFIG['OS_ARCH'] != 'Darwin':
+    XPCSHELL_TESTS_MANIFESTS += ['unit_ipc/xpcshell.ini']

toolkit/components/contentprefs/tests/unit_ipc/contentPrefs_childipc.js

@@ -0,0 +1,39 @@
+
+function run_test() {
+  do_check_true(inChildProcess(), "test harness should never call us directly");
+
+  var cps = Cc["@mozilla.org/content-pref/service;1"].
+            createInstance(Ci.nsIContentPrefService);
+
+  // Cannot get general values
+  try {
+    cps.getPref("group", "name")
+    do_check_false(true, "Must have thrown exception on getting general value");
+  }
+  catch(e) { }
+
+  // Cannot set general values
+  try {
+    cps.setPref("group", "name", "someValue2");
+    do_check_false(true, "Must have thrown exception on setting general value");
+  }
+  catch(e) { }
+
+  // Can set&get whitelisted values
+  cps.setPref("group", "browser.upload.lastDir", "childValue", null);
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir", null), "childValue");
+
+  // Test sending URI
+  var ioSvc = Cc["@mozilla.org/network/io-service;1"].
+              getService(Ci.nsIIOService);
+  var uri = ioSvc.newURI("http://mozilla.org", null, null);
+  cps.setPref(uri, "browser.upload.lastDir", "childValue2", null);
+  do_check_eq(cps.getPref(uri, "browser.upload.lastDir", null), "childValue2");
+
+  // Previous value
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir", null), "childValue");
+
+  // Tell parent to finish and clean up
+  cps.wrappedJSObject.messageManager.sendSyncMessage('ContentPref:QUIT');
+}
+

toolkit/components/contentprefs/tests/unit_ipc/head_contentPrefs.js

@@ -0,0 +1,5 @@
+// initializing profile here because do_get_profile cannot be called
+// from a content process
+do_get_profile();
+
+load("../unit/head_contentPrefs.js");

toolkit/components/contentprefs/tests/unit_ipc/tail_contentPrefs.js

@@ -0,0 +1,3 @@
+
+load("../unit/tail_contentPrefs.js");
+

toolkit/components/contentprefs/tests/unit_ipc/test_contentPrefs_parentipc.js

@@ -0,0 +1,88 @@
+
+function run_test() {
+  // Check received messages
+
+  var cps = Cc["@mozilla.org/content-pref/service;1"].
+            createInstance(Ci.nsIContentPrefService).
+            wrappedJSObject;
+
+  var messageHandler = cps;
+  // FIXME: For now, use the wrappedJSObject hack, until bug
+  //        593407 which will clean that up. After that, use
+  //        the commented out line below it.
+  messageHandler = cps.wrappedJSObject;
+  //messageHandler = cps.QueryInterface(Ci.nsIMessageListener);
+
+  // Cannot get values
+  do_check_false(messageHandler.receiveMessage({
+    name: "ContentPref:getPref",
+    json: { group: 'group2', name: 'name' } }).succeeded);
+
+  // Cannot set general values
+  messageHandler.receiveMessage({ name: "ContentPref:setPref",
+    json: { group: 'group2', name: 'name', value: 'someValue' } });
+  do_check_eq(cps.getPref('group', 'name', null), undefined);
+
+  // Can set whitelisted values
+  do_check_true(messageHandler.receiveMessage({ name: "ContentPref:setPref",
+    json: { group: 'group2', name: 'browser.upload.lastDir',
+            value: 'someValue' } }).succeeded);
+  do_check_eq(cps.getPref('group2', 'browser.upload.lastDir', null), 'someValue');
+
+  // Prepare for child tests
+
+  // Manually listen to messages - the wakeup manager should do this
+  // for us, but it doesn't run in xpcshell tests.
+  var messageProxy = {
+    receiveMessage: function(aMessage) {
+      if (aMessage.name == 'ContentPref:QUIT') {
+        // Undo mock storage.
+        delete cps._mockStorage;
+        delete cps._messageProxy;
+        cps.setPref = cps.old_setPref;
+        cps.getPref = cps.old_getPref;
+        cps._dbInit = cps.old__dbInit;
+        // Unlisten to messages
+        mM.removeMessageListener("ContentPref:setPref", messageProxy);
+        mM.removeMessageListener("ContentPref:getPref", messageProxy);
+        mM.removeMessageListener("ContentPref:QUIT", messageProxy);
+        do_test_finished();
+        return true;
+      } else {
+        return messageHandler.receiveMessage(aMessage);
+      }
+    },
+  };
+
+  var mM = Cc["@mozilla.org/parentprocessmessagemanager;1"].
+           getService(Ci.nsIMessageListenerManager);
+  mM.addMessageListener("ContentPref:setPref", messageProxy);
+  mM.addMessageListener("ContentPref:getPref", messageProxy);
+  mM.addMessageListener("ContentPref:QUIT", messageProxy);
+
+  // Mock storage. This is necessary because
+  // the IPC xpcshell setup doesn't do well with the normal storage
+  // engine.
+
+  cps = cps.wrappedJSObject;
+  cps._mockStorage = {};
+
+  cps.old_setPref = cps.setPref;
+  cps.setPref = function(aGroup, aName, aValue) {
+    this._mockStorage[aGroup+':'+aName] = aValue;
+  }
+
+  cps.old_getPref = cps.getPref;
+  cps.getPref = function(aGroup, aName) {
+    return this._mockStorage[aGroup+':'+aName];
+  }
+
+  cps.old__dbInit = cps._dbInit;
+  cps._dbInit = function(){};
+
+  cps._messageProxy = messageProxy; // keep it alive
+  do_test_pending();
+
+  run_test_in_child("contentPrefs_childipc.js");
+}
+

toolkit/components/contentprefs/tests/unit_ipc/xpcshell.ini

@@ -0,0 +1,5 @@
+[DEFAULT]
+head = head_contentPrefs.js
+tail = tail_contentPrefs.js
+
+[test_contentPrefs_parentipc.js]