Bug 621363 - SpecialPowers ipc setter code does not receive new value locally until next event loop run. r=jdm,cjones
☠☠ backed out by 4dc1e45afdd7 ☠ ☠
authorJoel Maher <jmaher@mozilla.com>
Fri, 26 Aug 2011 15:47:34 -0400
changeset 75960 14463b9d2d143045b7d88b601207f9dcd5a0e45d
parent 75959 97d43d93ab55b2b00db0ca9ed3217764f758a739
child 75961 728cf3529cba2a1758853c2a133a1df80ca57c9d
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersjdm, cjones
bugs621363
milestone9.0a1
Bug 621363 - SpecialPowers ipc setter code does not receive new value locally until next event loop run. r=jdm,cjones
layout/style/test/test_bug389464.html
layout/style/test/test_bug401046.html
testing/mochitest/tests/SimpleTest/TestRunner.js
testing/mochitest/tests/SimpleTest/specialpowersAPI.js
--- a/layout/style/test/test_bug389464.html
+++ b/layout/style/test/test_bug389464.html
@@ -22,42 +22,26 @@
 <p><font id="two" size="-1">text</font></p>
 
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 SimpleTest.waitForExplicitFinish();
 
-function get_pref(pref)
-{
-    return SpecialPowers.getIntPref("font.size." + pref);
-}
-
-function set_pref(pref, val)
-{
-    SpecialPowers.setIntPref("font.size." + pref, val);
-}
-
 var cs1 = getComputedStyle(document.getElementById("one"), "");
 var cs2 = getComputedStyle(document.getElementById("two"), "");
 
-var oldVariable = get_pref("variable.x-western");
-var oldFixed = get_pref("fixed.x-western");
-set_pref("variable.x-western", 25);
-set_pref("fixed.x-western", 20);
-setTimeout(part1, 0);
+SpecialPowers.pushPrefEnv({'set': [['variable.x-western', 25], ['fixed.x-western', 20]]}, function() setTimeout(part1, 0));
 
 function part1()
 {
     var fs1 = cs1.fontSize.match(/(.*)px/)[1];
     var fs2 = cs2.fontSize.match(/(.*)px/)[1];
     ok(fs1 < fs2, "<font size=-1> shrinks relative to font-family: -moz-fixed");
 
-    set_pref("variable.x-western", oldVariable);
-    set_pref("fixed.x-western", oldFixed);
     SimpleTest.finish();
 }
 
 </script>
 </pre>
 </body>
 </html>
--- a/layout/style/test/test_bug401046.html
+++ b/layout/style/test/test_bug401046.html
@@ -51,53 +51,42 @@ var elts = [
 
 function fs(idx) {
   // The computed font size actually *doesn't* currently reflect the
   // minimum font size preference, but things in em units do.  Not sure
   // if this is how it ought to be...
   return getComputedStyle(elts[idx], "").marginBottom;
 }
 
-SpecialPowers.clearUserPref('font.minimum-size.x-western');
-
-// preference change is async (although one setTimeout might be enough?)
-setTimeout(setTimeout, 0, step1, 0);
+SpecialPowers.pushPrefEnv({'clear': [['font.minimum-size.x-western']]}, function() setTimeout(step1, 0));
 
 function step1() {
     is(fs(0), "0px", "at min font size 0, 0px should compute to 0px");
     is(fs(1), "4px", "at min font size 0, 4px should compute to 4px");
     is(fs(2), "12px", "at min font size 0, 12px should compute to 12px");
     is(fs(3), "28px", "at min font size 0, 28px should compute to 28px");
 
-    SpecialPowers.setIntPref('font.minimum-size.x-western', 7);
-
-    // preference change is async (although one setTimeout might be enough?)
-    setTimeout(setTimeout, 0, step2, 0);
+    SpecialPowers.pushPrefEnv({'set': [['font.minimum-size.x-western', 7]]}, function() setTimeout(step2, 0));
 }
 
 function step2() {
     is(fs(0), "0px", "at min font size 7, 0px should compute to 0px");
     is(fs(1), "7px", "at min font size 7, 4px should compute to 7px");
     is(fs(2), "12px", "at min font size 7, 12px should compute to 12px");
     is(fs(3), "28px", "at min font size 7, 28px should compute to 28px");
 
-    SpecialPowers.setIntPref('font.minimum-size.x-western', 18);
-
-    // preference change is async (although one setTimeout might be enough?)
-    setTimeout(setTimeout, 0, step3, 0);
+    SpecialPowers.pushPrefEnv({'set': [['font.minimum-size.x-western', 18]]}, function() setTimeout(step3, 0));
 }
 
 function step3() {
     is(fs(0), "0px", "at min font size 18, 0px should compute to 0px");
     is(fs(1), "18px", "at min font size 18, 4px should compute to 18px");
     is(fs(2), "18px", "at min font size 18, 12px should compute to 18px");
     is(fs(3), "28px", "at min font size 18, 28px should compute to 28px");
 
-    SpecialPowers.clearUserPref('font.minimum-size.x-western');
-
-    SimpleTest.finish();
+    SpecialPowers.pushPrefEnv({'clear': [['font.minimum-size.x-western']]}, SimpleTest.finish);
 }
 
 </script>
 </pre>
 </body>
 </html>
 
--- a/testing/mochitest/tests/SimpleTest/TestRunner.js
+++ b/testing/mochitest/tests/SimpleTest/TestRunner.js
@@ -380,17 +380,17 @@ TestRunner.testFinished = function(tests
 
         TestRunner.updateUI(tests);
         TestRunner._currentTest++;
         TestRunner.runNextTest();
     }
 
     SpecialPowers.executeAfterFlushingMessageQueue(function() {
         cleanUpCrashDumpFiles();
-        runNextTest();
+        SpecialPowers.flushPrefEnv(runNextTest);
     });
 };
 
 /**
  * Get the results.
  */
 TestRunner.countResults = function(tests) {
   var nOK = 0;
--- a/testing/mochitest/tests/SimpleTest/specialpowersAPI.js
+++ b/testing/mochitest/tests/SimpleTest/specialpowersAPI.js
@@ -43,16 +43,19 @@ var Ci = Components.interfaces;
 var Cc = Components.classes;
 
 function SpecialPowersAPI() { 
   this._consoleListeners = [];
   this._encounteredCrashDumpFiles = [];
   this._unexpectedCrashDumpFiles = { };
   this._crashDumpDir = null;
   this._mfl = null;
+  this._prefEnvUndoStack = [];
+  this._pendingPrefs = [];
+  this._applyingPrefs = false;
 }
 
 function bindDOMWindowUtils(aWindow) {
   if (!aWindow)
     return
 
   var util = aWindow.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
                    .getInterface(Components.interfaces.nsIDOMWindowUtils);
@@ -81,16 +84,55 @@ function bindDOMWindowUtils(aWindow) {
     rebind(desc, "get");
     rebind(desc, "set");
     rebind(desc, "value");
     Object.defineProperty(target, i, desc);
   }
   return target;
 }
 
+function Observer(specialPowers, aTopic, aCallback, aIsPref) {
+  this._sp = specialPowers;
+  this._topic = aTopic;
+  this._callback = aCallback;
+  this._isPref = aIsPref;
+}
+
+Observer.prototype = {
+  _sp: null,
+  _topic: null,
+  _callback: null,
+  _isPref: false,
+
+  observe: function(aSubject, aTopic, aData) {
+    if ((!this._isPref && aTopic == this._topic) ||
+        (this._isPref && aTopic == "nsPref:changed")) {
+      if (aData == this._topic) {
+       this.cleanup();
+        /* The callback must execute asynchronously after all the preference observers have run */
+        content.window.setTimeout(this._callback, 0);
+        content.window.setTimeout(this._sp._finishPrefEnv, 0);
+      }
+    }
+  },
+
+  cleanup: function() {
+    if (this._isPref) {
+      var os = Cc["@mozilla.org/preferences-service;1"].getService()
+               .QueryInterface(Ci.nsIPrefBranch2);
+      os.removeObserver(this._topic, this);
+    } else {
+      var os = Cc["@mozilla.org/observer-service;1"]
+              .getService(Ci.nsIObserverService)
+              .QueryInterface(Ci.nsIObserverService);
+      os.removeObserver(this, this._topic);
+    }
+  },
+};
+
 SpecialPowersAPI.prototype = {
 
   getDOMWindowUtils: function(aWindow) {
     if (aWindow == this.window && this.DOMWindowUtils != null)
       return this.DOMWindowUtils;
 
     return bindDOMWindowUtils(aWindow);
   },
@@ -118,16 +160,180 @@ SpecialPowersAPI.prototype = {
     };
     var crashDumpFiles = this._sendSyncMessage("SPProcessCrashService", message)[0];
     crashDumpFiles.forEach(function(aFilename) {
       self._unexpectedCrashDumpFiles[aFilename] = true;
     });
     return crashDumpFiles;
   },
 
+  /*
+   * Take in a list of prefs and put the original value on the _prefEnvUndoStack so we can undo
+   * preferences that we set.  Note, that this is a stack of values to revert to, not
+   * what we have set.
+   *
+   * prefs: {set|clear: [[pref, value], [pref, value, Iid], ...], set|clear: [[pref, value], ...], ...}
+   * ex: {'set': [['foo.bar', 2], ['browser.magic', '0xfeedface']], 'remove': [['bad.pref']] }
+   *
+   * In the scenario where our prefs specify the same pref more than once, we do not guarantee
+   * the behavior.  
+   *
+   * If a preference is not changing a value, we will ignore it.
+   *
+   * TODO: complex values for original cleanup?
+   *
+   */
+  pushPrefEnv: function(inPrefs, callback) {
+    var prefs = Components.classes["@mozilla.org/preferences-service;1"].
+                           getService(Components.interfaces.nsIPrefBranch);
+
+    var pref_string = [];
+    pref_string[prefs.PREF_INT] = "INT";
+    pref_string[prefs.PREF_BOOL] = "BOOL";
+    pref_string[prefs.PREF_STRING] = "STRING";
+
+    var pendingActions = [];
+    var cleanupActions = [];
+
+    for (var action in inPrefs) { /* set|clear */
+      for (var idx in inPrefs[action]) {
+        var aPref = inPrefs[action][idx];
+        var prefName = aPref[0];
+        var prefValue = null;
+        var prefIid = null;
+        var prefType = prefs.PREF_INVALID;
+        var originalValue = null;
+
+        if (aPref.length == 3) {
+          prefValue = aPref[1];
+          prefIid = aPref[2];
+        } else if (aPref.length == 2) {
+          prefValue = aPref[1];
+        }
+
+        /* If pref is not found or invalid it doesn't exist. */
+        if (prefs.getPrefType(prefName) != prefs.PREF_INVALID) {
+          prefType = pref_string[prefs.getPrefType(prefName)];
+          if ((prefs.prefHasUserValue(prefName) && action == 'clear') ||
+              (action == 'set'))
+            originalValue = this._getPref(prefName, prefType);
+        } else if (action == 'set') {
+          /* prefName doesn't exist, so 'clear' is pointless */
+          if (aPref.length == 3) {
+            prefType = "COMPLEX";
+          } else if (aPref.length == 2) {
+            if (typeof(prefValue) == "boolean") 
+              prefType = "BOOL";
+            else if (typeof(prefValue) == "number")
+              prefType = "INT";
+            else if (typeof(prefValue) == "string")
+              prefType = "CHAR";
+          }
+        }
+
+        /* PREF_INVALID: A non existing pref which we are clearing or invalid values for a set */
+        if (prefType == prefs.PREF_INVALID)
+          continue;
+
+        /* We are not going to set a pref if the value is the same */
+        if (originalValue == prefValue)
+          continue;
+
+        pendingActions.push({'action': action, 'type': prefType, 'name': prefName, 'value': prefValue, 'Iid': prefIid});
+
+        /* Push original preference value or clear into cleanup array */
+        var cleanupTodo = {'action': action, 'type': prefType, 'name': prefName, 'value': originalValue, 'Iid': prefIid};
+        if (originalValue == null) {
+          cleanupTodo.action = 'clear';
+        } else {
+          cleanupTodo.action = 'set';
+        }
+        cleanupActions.push(cleanupTodo);
+      }
+    }
+
+    if (pendingActions.length > 0) {
+      this._prefEnvUndoStack.push(cleanupActions);
+      this._pendingPrefs.push([pendingActions, callback]);
+      this._applyPrefs();
+    } else {
+      content.window.setTimeout(callback, 0);
+    }
+  },
+
+  popPrefEnv: function(callback) {
+    if (this._prefEnvUndoStack.length > 0) {
+      /* Each pop will have a valid block of preferences */
+      this._pendingPrefs.push([this._prefEnvUndoStack.pop(), callback]);
+      this._applyPrefs();
+    } else {
+      content.window.setTimeout(callback, 0);
+    }
+  },
+
+  flushPrefEnv: function(callback) {
+    while (this._prefEnvUndoStack.length > 1)
+      this.popPrefEnv(null);
+
+    this.popPrefEnv(callback);
+  },
+
+  /*
+    Iterate through one atomic set of pref actions and perform sets/clears as appropriate. 
+    All actions performed must modify the relevant pref.
+  */
+  _applyPrefs: function() {
+    if (this._applyingPrefs || this._pendingPrefs.length <= 0) {
+      return;
+    }
+
+    /* Set lock and get prefs from the _pendingPrefs queue */
+    this._applyingPrefs = true;
+    var transaction = this._pendingPrefs.shift();
+    var pendingActions = transaction[0];
+    var callback = transaction[1];
+
+    var lastPref = pendingActions[pendingActions.length-1];
+    this._addObserver(lastPref.name, callback, true);
+
+    for (var idx in pendingActions) {
+      var pref = pendingActions[idx];
+      if (pref.action == 'set') {
+        this._setPref(pref.name, pref.type, pref.value, pref.Iid);
+      } else if (pref.action == 'clear') {
+        this.clearUserPref(pref.name);
+      }
+    }
+  },
+
+  _addObserver: function(aTopic, aCallback, aIsPref) {
+    var observer = new Observer(this, aTopic, aCallback, aIsPref);
+
+    if (aIsPref) {
+      var os = Cc["@mozilla.org/preferences-service;1"].getService()
+               .QueryInterface(Ci.nsIPrefBranch2);	
+      os.addObserver(aTopic, observer, false);
+    } else {
+      var os = Cc["@mozilla.org/observer-service;1"]
+              .getService(Ci.nsIObserverService)
+              .QueryInterface(Ci.nsIObserverService);
+      os.addObserver(observer, aTopic, false);
+    }
+  },
+
+  /* called from the observer when we get a pref:changed.  */
+  _finishPrefEnv: function() {
+    /*
+      Any subsequent pref environment pushes that occurred while waiting 
+      for the preference update are pending, and will now be executed.
+    */
+    this.wrappedJSObject.SpecialPowers._applyingPrefs = false;
+    this.wrappedJSObject.SpecialPowers._applyPrefs();
+  },
+
   // Mimic the get*Pref API
   getBoolPref: function(aPrefName) {
     return (this._getPref(aPrefName, 'BOOL'));
   },
   getIntPref: function(aPrefName) {
     return (this._getPref(aPrefName, 'INT'));
   },
   getCharPref: function(aPrefName) {