584842 - E10s nsIContentPrefService remoting implementation r=myk, a=blocking-fennec

dom/interfaces/base/nsIContentPrefService.idl

@@ -82,6 +82,9 @@ interface nsIContentPrefService : nsISupports
    * to NULL in the database, as well as undefined (nsIDataType::VTYPE_VOID),
    * which means there is no record for this pref in the database.
    *
+   * This method can be called from content processes in electrolysis builds.
+   * We have a whitelist of values that can be read in such a way.
+   *
    * @param    aGroup      the group for which to get the pref, as an nsIURI
    *                       from which the hostname will be used, a string
    *                       (typically in the format of a hostname), or null 
@@ -101,6 +104,9 @@ interface nsIContentPrefService : nsISupports
   /**
    * Set a pref.
    *
+   * This method can be called from content processes in electrolysis builds.
+   * We have a whitelist of values that can be set in such a way.
+   *
    * @param    aGroup      the group for which to set the pref, as an nsIURI
    *                       from which the hostname will be used, a string
    *                       (typically in the format of a hostname), or null

toolkit/components/contentprefs/src/nsContentPrefService.js

@@ -43,7 +43,88 @@ const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+/**
+ * Remotes the service. All the remoting/electrolysis code is in here,
+ * so the regular service code below remains uncluttered and maintainable.
+ */
+function electrolify(service) {
+  // FIXME: For now, use the wrappedJSObject hack, until bug
+  //        593407 which will clean that up.
+  service.wrappedJSObject = service;
+
+  var appInfo = Cc["@mozilla.org/xre/app-info;1"];
+  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
+    // Parent process
+
+    // 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;
+      // 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, which is so far the only need we have identified.
+
+      const NAME_WHITELIST = ["browser.upload.lastDir"];
+      if (NAME_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;
+      };
+    });
+  }
+}
+
 function ContentPrefService() {
+  electrolify(this);
+
   // If this throws an exception, it causes the getService call to fail,
   // but the next time a consumer tries to retrieve the service, we'll try
   // to initialize the database again, which might work if the failure
@@ -59,7 +140,8 @@ ContentPrefService.prototype = {
   // XPCOM Plumbing
 
   classID:          Components.ID("{e6a3f533-4ffa-4615-8eb4-d4e72d883fa7}"),
-  QueryInterface:   XPCOMUtils.generateQI([Ci.nsIContentPrefService]),
+  QueryInterface:   XPCOMUtils.generateQI([Ci.nsIContentPrefService,
+                                           Ci.nsIFrameMessageListener]),
 
 
   //**************************************************************************//
@@ -137,15 +219,10 @@ ContentPrefService.prototype = {
       throw Components.Exception("aName cannot be null or an empty string",
                                  Cr.NS_ERROR_ILLEGAL_VALUE);
 
-    if (aGroup == null)
+    var group = this._parseGroupParam(aGroup);
+    if (group == null)
       return this._selectGlobalPref(aName, aCallback);
-    if (aGroup.constructor.name == "String")
+    return this._selectPref(group, aName, aCallback);
-      return this._selectPref(aGroup.toString(), aName, aCallback);
-    if (aGroup instanceof Ci.nsIURI)
-      return this._selectPref(this.grouper.group(aGroup), aName, aCallback);
-
-    throw Components.Exception("aGroup is not a string, nsIURI or null",
-                               Cr.NS_ERROR_ILLEGAL_VALUE);
   },
 
   setPref: function ContentPrefService_setPref(aGroup, aName, aValue) {
@@ -168,26 +245,15 @@ ContentPrefService.prototype = {
     }
 
     var settingID = this._selectSettingID(aName) || this._insertSetting(aName);
-    var group, groupID, prefID;
+    var group = this._parseGroupParam(aGroup);
-    if (aGroup == null) {
+    var groupID, prefID;
-      group = null;
+    if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
     }
-    else if (aGroup.constructor.name == "String") {
-      group = aGroup.toString();
-      groupID = this._selectGroupID(group) || this._insertGroup(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
-    else if (aGroup instanceof Ci.nsIURI) {
-      group = this.grouper.group(aGroup);
-      groupID = this._selectGroupID(group) || this._insertGroup(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
     else {
-      // Should never get here, due to earlier getPref call
+      groupID = this._selectGroupID(group) || this._insertGroup(group);
-      throw Components.Exception("aGroup is not a string, nsIURI or null",
+      prefID = this._selectPrefID(groupID, settingID);
-                                 Cr.NS_ERROR_ILLEGAL_VALUE);
     }
 
     // Update the existing record, if any, or create a new one.
@@ -217,27 +283,17 @@ ContentPrefService.prototype = {
     if (!this.hasPref(aGroup, aName))
       return;
 
+
     var settingID = this._selectSettingID(aName);
-    var group, groupID, prefID;
+    var group = this._parseGroupParam(aGroup);
-    if (aGroup == null) {
+    var groupID, prefID;
-      group = null;
+    if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
     }
-    else if (aGroup.constructor.name == "String") {
-      group = aGroup.toString();
-      groupID = this._selectGroupID(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
-    else if (aGroup instanceof Ci.nsIURI) {
-      group = this.grouper.group(aGroup);
-      groupID = this._selectGroupID(group);
-      prefID = this._selectPrefID(groupID, settingID);
-    }
     else {
-      // Should never get here, due to earlier hasPref call
+      groupID = this._selectGroupID(group);
-      throw Components.Exception("aGroup is not a string, nsIURI or null",
+      prefID = this._selectPrefID(groupID, settingID);
-                                 Cr.NS_ERROR_ILLEGAL_VALUE);
     }
 
     this._deletePref(prefID);
@@ -312,18 +368,10 @@ ContentPrefService.prototype = {
   },
 
   getPrefs: function ContentPrefService_getPrefs(aGroup) {
-    if (aGroup == null)
+    var group = this._parseGroupParam(aGroup);
+    if (group == null)
       return this._selectGlobalPrefs();
-    if (aGroup.constructor.name == "String") {
+    return this._selectPrefs(group);
-      group = aGroup.toString();
-      return this._selectPrefs(group);
-    }
-    if (aGroup instanceof Ci.nsIURI) {
-      var group = this.grouper.group(aGroup);
-      return this._selectPrefs(group);
-    }
-    throw Components.Exception("aGroup is not a string, nsIURI or null",
-                               Cr.NS_ERROR_ILLEGAL_VALUE);
   },
 
   getPrefsByName: function ContentPrefService_getPrefsByName(aName) {
@@ -1011,8 +1059,19 @@ ContentPrefService.prototype = {
 
   _dbMigrate2To3: function ContentPrefService__dbMigrate2To3(aDBConnection) {
     this._dbCreateIndices(aDBConnection);
-  }
+  },
 
+  _parseGroupParam: function ContentPrefService__parseGroupParam(aGroup) {
+    if (aGroup == null)
+      return null;
+    if (aGroup.constructor.name == "String")
+      return aGroup.toString();
+    if (aGroup instanceof Ci.nsIURI)
+      return this.grouper.group(aGroup);
+
+    throw Components.Exception("aGroup is not a string, nsIURI or null",
+                               Cr.NS_ERROR_ILLEGAL_VALUE);
+  },
 };
 
 

toolkit/components/contentprefs/src/nsContentPrefService.manifest

@@ -2,3 +2,5 @@ component {e6a3f533-4ffa-4615-8eb4-d4e72d883fa7} nsContentPrefService.js
 contract @mozilla.org/content-pref/service;1 {e6a3f533-4ffa-4615-8eb4-d4e72d883fa7}
 component {8df290ae-dcaa-4c11-98a5-2429a4dc97bb} nsContentPrefService.js
 contract @mozilla.org/content-pref/hostname-grouper;1 {8df290ae-dcaa-4c11-98a5-2429a4dc97bb}
+category wakeup-request nsContentPrefService @mozilla.org/content-pref/service;1,nsIContentPrefService,getService,ContentPref:getPref,ContentPref:setPref
+

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

@@ -167,7 +167,13 @@ var ContentPrefTest = {
 ContentPrefTest.deleteDatabase();
 
 // Turn on logging for the content preferences service so we can troubleshoot
-// problems with the tests.
+// problems with the tests. This is only useful in regular (parent)
-var prefBranch = Cc["@mozilla.org/preferences-service;1"].
+// processes - child processes are not able to do that and will crash.
-                 getService(Ci.nsIPrefBranch);
+var appInfo = Cc["@mozilla.org/xre/app-info;1"];
-prefBranch.setBoolPref("browser.preferences.content.log", true);
+if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
+    Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
+  var prefBranch = Cc["@mozilla.org/preferences-service;1"].
+                   getService(Ci.nsIPrefBranch);
+  prefBranch.setBoolPref("browser.preferences.content.log", true);
+}
+

toolkit/components/contentprefs/tests/unit/test_contentPrefs_childipc.js

@@ -0,0 +1,39 @@
+function run_test() {
+  var appInfo = Cc["@mozilla.org/xre/app-info;1"];
+  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
+    return; // test harness also calls us in the parent process - ignore that
+  }
+
+  let 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");
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir"), "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");
+  do_check_eq(cps.getPref(uri, "browser.upload.lastDir"), "childValue2");
+
+  // Previous value
+  do_check_eq(cps.getPref("group", "browser.upload.lastDir"), "childValue");
+}
+

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

@@ -0,0 +1,54 @@
+
+function run_test() {
+  let cps = Cc["@mozilla.org/content-pref/service;1"].
+            createInstance(Ci.nsIContentPrefService);
+
+  // Check received messages
+
+  let 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.nsIFrameMessageListener);
+
+  // 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'), 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'), 'someValue');
+
+  // Prepare for child tests
+
+  // Mock construction for set/get for child. This is necessary because
+  // the IPC xpcshell setup doens't do well with the normal storage
+  // engine. But even with this mock interface, we are testing
+  // the whitelisting functionality properly, which is all we need here.
+  var mockStorage = {};
+  cps.wrappedJSObject.setPref = function(aGroup, aName, aValue) {
+    mockStorage[aGroup+':'+aName] = aValue;
+  }
+  cps.wrappedJSObject.getPref = function(aGroup, aName) {
+    return mockStorage[aGroup+':'+aName];
+  }
+
+  // Manually listen to messages - the wakeup manager should do this
+  // for us, but it doens't run in xpcshell tests
+  var mM = Cc["@mozilla.org/parentprocessmessagemanager;1"].
+           getService(Ci.nsIFrameMessageManager);
+  mM.addMessageListener("ContentPref:setPref", messageHandler);
+  mM.addMessageListener("ContentPref:getPref", messageHandler);
+
+  run_test_in_child("test_contentPrefs_childipc.js");
+}
+