Bug 701060 - Fix race condition with pref observers causing test orange. r=ehsan.
authorJonathan Watt <jwatt@jwatt.org>
Fri, 27 Jan 2012 22:16:44 +0000
changeset 85621 8df0708b2fe3b06158d0a820a9b3e16e0c5e2c73
parent 85620 be23da096c4085bc1c05376d10f0af785cb893ac
child 85622 117f2280bd374a99b6344a0641ac16281d9f1aa4
push id21940
push userjdrew@mozilla.com
push dateSun, 29 Jan 2012 02:43:03 +0000
treeherdermozilla-central@ec666b4c8d84 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs701060
milestone12.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 701060 - Fix race condition with pref observers causing test orange. r=ehsan.
layout/style/test/test_bug389464.html
layout/style/test/test_bug401046.html
testing/mochitest/tests/SimpleTest/specialpowersAPI.js
--- a/layout/style/test/test_bug389464.html
+++ b/layout/style/test/test_bug389464.html
@@ -26,17 +26,17 @@
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 SimpleTest.waitForExplicitFinish();
 
 var cs1 = getComputedStyle(document.getElementById("one"), "");
 var cs2 = getComputedStyle(document.getElementById("two"), "");
 
-SpecialPowers.pushPrefEnv({'set': [['variable.x-western', 25], ['fixed.x-western', 20]]}, function() setTimeout(part1, 0));
+SpecialPowers.pushPrefEnv({'set': [['variable.x-western', 25], ['fixed.x-western', 20]]}, part1);
 
 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");
 
     SimpleTest.finish();
--- a/layout/style/test/test_bug401046.html
+++ b/layout/style/test/test_bug401046.html
@@ -51,34 +51,34 @@ 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.pushPrefEnv({'clear': [['font.minimum-size.x-western']]}, function() setTimeout(step1, 0));
+SpecialPowers.pushPrefEnv({'clear': [['font.minimum-size.x-western']]}, step1);
 
 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.pushPrefEnv({'set': [['font.minimum-size.x-western', 7]]}, function() setTimeout(step2, 0));
+    SpecialPowers.pushPrefEnv({'set': [['font.minimum-size.x-western', 7]]}, step2);
 }
 
 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.pushPrefEnv({'set': [['font.minimum-size.x-western', 18]]}, function() setTimeout(step3, 0));
+    SpecialPowers.pushPrefEnv({'set': [['font.minimum-size.x-western', 18]]}, step3);
 }
 
 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");
 
--- a/testing/mochitest/tests/SimpleTest/specialpowersAPI.js
+++ b/testing/mochitest/tests/SimpleTest/specialpowersAPI.js
@@ -541,28 +541,49 @@ SpecialPowersAPI.prototype = {
         } else {
           cleanupTodo.action = 'set';
         }
         cleanupActions.push(cleanupTodo);
       }
     }
 
     if (pendingActions.length > 0) {
+      // The callback needs to be delayed twice. One delay is because the pref
+      // service doesn't guarantee the order it calls its observers in, so it
+      // may notify the observer holding the callback before the other
+      // observers have been notified and given a chance to make the changes
+      // that the callback checks for. The second delay is because pref
+      // observers often defer making their changes by posting an event to the
+      // event loop.
+      function delayedCallback() {
+        function delayAgain() {
+          content.window.setTimeout(callback, 0);
+        }
+        content.window.setTimeout(delayAgain, 0);
+      }
       this._prefEnvUndoStack.push(cleanupActions);
-      this._pendingPrefs.push([pendingActions, callback]);
+      this._pendingPrefs.push([pendingActions, delayedCallback]);
       this._applyPrefs();
     } else {
       content.window.setTimeout(callback, 0);
     }
   },
 
   popPrefEnv: function(callback) {
     if (this._prefEnvUndoStack.length > 0) {
+      // See pushPrefEnv comment regarding delay.
+      function delayedCallback() {
+        function delayAgain() {
+          content.window.setTimeout(callback, 0);
+        }
+        content.window.setTimeout(delayAgain, 0);
+      }
+      let cb = callback ? delayedCallback : null; 
       /* Each pop will have a valid block of preferences */
-      this._pendingPrefs.push([this._prefEnvUndoStack.pop(), callback]);
+      this._pendingPrefs.push([this._prefEnvUndoStack.pop(), cb]);
       this._applyPrefs();
     } else {
       content.window.setTimeout(callback, 0);
     }
   },
 
   flushPrefEnv: function(callback) {
     while (this._prefEnvUndoStack.length > 1)