Bug 1718755 - Fix the SpecialPowers pref code to deal with ui / font prefs correctly after the previous change. r=kmag

This allows us to remove the timeout (which was there for these prefs)
and makes the code a bit more explicit.

Unconditionally making all tests wait for rAF causes some timing changes
which cause some tests to time out / fail, see:

  https://treeherder.mozilla.org/jobs?repo=try&revision=3aef4c168c6ab7d762dd360f49d4f56dff686c03

So this only does it when changing the prefs that care about it. Fix
some tests that were relying on the timeout to get this green.

Differential Revision: https://phabricator.services.mozilla.com/D119040

browser/components/search/test/browser/browser_search_telemetry_content.js

@@ -127,9 +127,7 @@ add_task(async function test_about_newtab() {
   await SpecialPowers.pushPrefEnv({
     set: [
       [
-        [
-          "browser.newtabpage.activity-stream.improvesearch.handoffToAwesomebar",
-        ],
+        "browser.newtabpage.activity-stream.improvesearch.handoffToAwesomebar",
         false,
       ],
     ],

dom/canvas/test/test_hitregion_event.html

@@ -16,7 +16,8 @@
 </div>
 <pre id="test">
 <script type="application/javascript">
-SpecialPowers.pushPrefEnv({"set": [["canvas.hitregions.enabled", true]]}).then(function() {
+SpecialPowers.pushPrefEnv({"set": [["canvas.hitregions.enabled", true]]}).then(async function() {
+  await SpecialPowers.promiseTimeout(0);
 
   var input = document.getElementById("input");
   var regionId = "";

dom/tests/mochitest/chrome/test_MozDomFullscreen_event.xhtml

@@ -35,7 +35,7 @@ SpecialPowers.pushPrefEnv({"set": [
   ['full-screen-api.allow-trusted-requests-only', false],
   ['full-screen-api.transition-duration.enter', '0 0'],
   ['full-screen-api.transition-duration.leave', '0 0']
-]}, setup);
+]}).then(setup);
 
 function setup() {
    newwindow = window.browsingContext.topChromeWindow.openDialog(

dom/tests/mochitest/dom-level0/test_background_loading_iframes.html

@@ -34,7 +34,9 @@ window.onload = function() {
   myLoadTime = performance.now();
 }
 
-SpecialPowers.pushPrefEnv({"set":[["dom.background_loading_iframe", true]]}).then(function () {
+SpecialPowers.pushPrefEnv({"set":[["dom.background_loading_iframe", true]]}).then(async function () {
+  await SpecialPowers.promiseTimeout(0);
+
   var iframe1 = document.createElement("iframe");
   var iframe2 = document.createElement("iframe");
   var iframe3 = document.createElement("iframe");

dom/xul/test/test_bug757137.xhtml

@@ -17,7 +17,7 @@ SimpleTest.waitForExplicitFinish();
 // Force off out-of-process mozbrowser because we need to grab its
 // |window| synchronously from here.  Out-of-process docshell creation
 // for mozbrowser haves entirely differently.
-SpecialPowers.pushPrefEnv({"set":[["dom.ipc.tabs.disabled", true]]}, startTest);
+SpecialPowers.pushPrefEnv({"set":[["dom.ipc.tabs.disabled", true]]}).then(startTest);
 
 function startTest() {
   var otherWindow = window.browsingContext.topChromeWindow.open("window_bug757137.xhtml", "", "chrome");

js/xpconnect/tests/browser/browser_parent_process_hang_telemetry.js

@@ -14,6 +14,7 @@ add_task(async function test_browser_hang() {
   await SpecialPowers.pushPrefEnv({
     set: [["dom.max_chrome_script_run_time", 2]]
   });
+  await SpecialPowers.promiseTimeout(0);
 
   // Hang for 1.2 seconds.
   let now = Date.now();

layout/base/tests/test_after_paint_pref.html

@@ -66,7 +66,7 @@ function step1(event)
   div.style.backgroundColor = "blue";
 }
 
-function step2(event)
+async function step2(event)
 {
   ok(true, "step2 reached: " + print_event(event));
   window.removeEventListener("MozAfterPaint", step2);
@@ -81,7 +81,9 @@ function step2(event)
   // When there was previously another page in our window, we seem to
   // get duplicate events, simultaneously, so we need to register our
   // next listener after a zero timeout.
-  SpecialPowers.pushPrefEnv({'set': [['dom.send_after_paint_to_content', false]]}, function() {setTimeout(step3, 0, timeout);});
+  await SpecialPowers.pushPrefEnv({'set': [['dom.send_after_paint_to_content', false]]});
+  await SpecialPowers.promiseTimeout(0);
+  setTimeout(step3, 0, timeout);
 }
 function step3(timeout)
 {

layout/style/test/test_dont_use_document_colors.html

@@ -64,14 +64,7 @@ var cs9 = getComputedStyle(document.getElementById("nine"), "");
 var cs10 = getComputedStyle(document.getElementById("ten"), "");
 
 function pushPrefEnvAndWait(args, cb) {
-  SpecialPowers.pushPrefEnv(args, _ => {
-    // The nsPresContext delays applying a preference change until after a 0ms
-    // nsITimer fires.  The SpecialPowers.pushPrefEnv() uses setTimeout(f, 0)
-    // which may execute as a simple runnable dispatch and fire before the
-    // 0ms nsITimer.  Therefore wait an additional 1ms to allow the nsPresContext
-    // to apply the preferences.
-    setTimeout(cb, 1);
-  });
+  SpecialPowers.pushPrefEnv(args).then(cb)
 }
 
 pushPrefEnvAndWait({'set': [['browser.display.document_color_use', 1]]}, part1);

testing/specialpowers/content/SpecialPowersChild.jsm

@@ -897,18 +897,46 @@ class SpecialPowersChild extends JSWindowActorChild {
   }
 
   async pushPrefEnv(inPrefs, callback = null) {
-    await this.sendQuery("PushPrefEnv", inPrefs).then(callback);
-    await this.promiseTimeout(0);
+    let { requiresRefresh } = await this.sendQuery("PushPrefEnv", inPrefs);
+    if (callback) {
+      await callback();
+    }
+    if (requiresRefresh) {
+      await this._promiseEarlyRefresh();
+    }
   }
 
   async popPrefEnv(callback = null) {
-    await this.sendQuery("PopPrefEnv").then(callback);
-    await this.promiseTimeout(0);
+    let { popped, requiresRefresh } = await this.sendQuery("PopPrefEnv");
+    if (callback) {
+      await callback(popped);
+    }
+    if (requiresRefresh) {
+      await this._promiseEarlyRefresh();
+    }
   }
 
   async flushPrefEnv(callback = null) {
-    await this.sendQuery("FlushPrefEnv").then(callback);
-    await this.promiseTimeout(0);
+    let { requiresRefresh } = await this.sendQuery("FlushPrefEnv");
+    if (callback) {
+      await callback();
+    }
+    if (requiresRefresh) {
+      await this._promiseEarlyRefresh();
+    }
+  }
+
+  _promiseEarlyRefresh() {
+    return new Promise(r => {
+      // for mochitest-browser
+      if (typeof this.chromeWindow != "undefined") {
+        this.chromeWindow.requestAnimationFrame(r);
+      }
+      // for mochitest-plain
+      else {
+        this.contentWindow.requestAnimationFrame(r);
+      }
+    });
   }
 
   _addObserverProxy(notification) {

testing/specialpowers/content/SpecialPowersParent.jsm

@@ -402,15 +402,26 @@ class SpecialPowersParent extends JSWindowActorParent {
   /*
     Iterate through one atomic set of pref actions and perform sets/clears as appropriate.
     All actions performed must modify the relevant pref.
+
+    Returns whether we need to wait for a refresh driver tick for the pref to
+    have effect. This is only needed for ui. and font. prefs, which affect the
+    look and feel code and have some change-coalescing going on.
   */
   _applyPrefs(actions) {
+    let requiresRefresh = false;
     for (let pref of actions) {
+      requiresRefresh =
+        requiresRefresh ||
+        pref.name.startsWith("ui.") ||
+        pref.name.startsWith("browser.display.") ||
+        pref.name.startsWith("font.");
       if (pref.action == "set") {
         this._setPref(pref.name, pref.type, pref.value, pref.iid);
       } else if (pref.action == "clear") {
         Services.prefs.clearUserPref(pref.name);
       }
     }
+    return requiresRefresh;
   }
 
   /**
@@ -492,7 +503,8 @@ class SpecialPowersParent extends JSWindowActorParent {
       }
 
       prefUndoStack.push(cleanupActions);
-      this._applyPrefs(pendingActions);
+      let requiresRefresh = this._applyPrefs(pendingActions);
+      return { requiresRefresh };
     });
   }
 
@@ -500,17 +512,19 @@ class SpecialPowersParent extends JSWindowActorParent {
     return doPrefEnvOp(() => {
       let env = prefUndoStack.pop();
       if (env) {
-        this._applyPrefs(env);
-        return true;
+        let requiresRefresh = this._applyPrefs(env);
+        return { popped: true, requiresRefresh };
       }
-      return false;
+      return { popped: false, requiresRefresh: false };
     });
   }
 
   flushPrefEnv() {
+    let requiresRefresh = false;
     while (prefUndoStack.length) {
-      this.popPrefEnv();
+      requiresRefresh |= this.popPrefEnv().requiresRefresh;
     }
+    return { requiresRefresh };
   }
 
   _setPref(name, type, value, iid) {

toolkit/components/aboutmemory/tests/test_aboutmemory5.xhtml

@@ -36,7 +36,7 @@
     ["memory.report_concurrency", 2]        // Cover more child handling cases
   ];
 
-  SpecialPowers.pushPrefEnv({"set": prefs}, function() {
+  SpecialPowers.pushPrefEnv({"set": prefs}).then(function() {
     for (let i = 0; i < numRemotes; i++) {
       let w = remotes[i] = window.browsingContext.topChromeWindow.open("remote.xhtml", "", "chrome");
 
@@ -54,7 +54,7 @@
         mm.loadFrameScript("data:," + encodeURI("sendAsyncMessage('test:ready');"), true);
       });
     }
-  }, {once: true});
+  });
 
   // Load the given file into the frame, then copy+paste the entire frame and
   // check that the cut text matches what we expect.

toolkit/components/aboutmemory/tests/test_dumpGCAndCCLogsToFile.xhtml

@@ -21,8 +21,7 @@
   // Create some remote processes, and set up message-passing so that
   // we know when each child is fully initialized.
   let remotes = [];
-  SpecialPowers.pushPrefEnv({"set": [["dom.ipc.processCount", numRemotes]]},
-			    function() {
+  SpecialPowers.pushPrefEnv({"set": [["dom.ipc.processCount", numRemotes]]}).then(function() {
     for (let i = 0; i < numRemotes; i++) {
       let w = remotes[i] = window.browsingContext.topChromeWindow.open("remote.xhtml", "", "chrome");
 

toolkit/components/aboutmemory/tests/test_memoryReporters2.xhtml

@@ -28,7 +28,7 @@
   // Create some remote processes, and set up message-passing so that
   // we know when each child is fully initialized.
   let remotes = [];
-  SpecialPowers.pushPrefEnv({"set": [["dom.ipc.processCount", 3]]}, function() {
+  SpecialPowers.pushPrefEnv({"set": [["dom.ipc.processCount", 3]]}).then(function() {
     for (let i = 0; i < numToOpen; i++) {
       let w = remotes[i] = window.browsingContext.topChromeWindow.open("remote.xhtml", "", "chrome");
 

toolkit/content/tests/chrome/test_preferences_beforeaccept.xhtml

@@ -10,41 +10,40 @@
   <script type="application/javascript">
   <![CDATA[
     SimpleTest.waitForExplicitFinish();
-    SpecialPowers.pushPrefEnv({"set":[["browser.preferences.instantApply", false]]}, function() {
+    SpecialPowers.pushPrefEnv({"set":[["browser.preferences.instantApply", false]]}).then(function() {
+      SimpleTest.registerCleanupFunction(() => {
+        SpecialPowers.clearUserPref("tests.beforeaccept.dialogShown");
+        SpecialPowers.clearUserPref("tests.beforeaccept.called");
+      });
 
-    SimpleTest.registerCleanupFunction(() => {
-      SpecialPowers.clearUserPref("tests.beforeaccept.dialogShown");
-      SpecialPowers.clearUserPref("tests.beforeaccept.called");
+      // No instant-apply for this test
+      var prefWindow = window.browsingContext.topChromeWindow.openDialog("window_preferences_beforeaccept.xhtml", "", "", windowOnload);
+
+      function windowOnload() {
+        var dialogShown = prefWindow.Preferences.get("tests.beforeaccept.dialogShown");
+        var called = prefWindow.Preferences.get("tests.beforeaccept.called");
+        is(dialogShown.value, true, "dialog opened, shown pref set");
+        is(dialogShown.valueFromPreferences, null, "shown pref not committed");
+        is(called.value, null, "beforeaccept not yet called");
+        is(called.valueFromPreferences, null, "beforeaccept not yet called, pref not committed");
+
+        // try to accept the dialog, should fail the first time
+        prefWindow.document.getElementById("beforeaccept_dialog").acceptDialog();
+        is(prefWindow.closed, false, "window not closed");
+        is(dialogShown.value, true, "shown pref still set");
+        is(dialogShown.valueFromPreferences, null, "shown pref still not committed");
+        is(called.value, true, "beforeaccept called");
+        is(called.valueFromPreferences, null, "called pref not committed");
+
+        // try again, this one should succeed
+        prefWindow.document.getElementById("beforeaccept_dialog").acceptDialog();
+        is(prefWindow.closed, true, "window now closed");
+        is(dialogShown.valueFromPreferences, true, "shown pref committed");
+        is(called.valueFromPreferences, true, "called pref committed");
+
+        SimpleTest.finish();
+      }
     });
-
-    // No instant-apply for this test
-    var prefWindow = window.browsingContext.topChromeWindow.openDialog("window_preferences_beforeaccept.xhtml", "", "", windowOnload);
-
-    function windowOnload() {
-      var dialogShown = prefWindow.Preferences.get("tests.beforeaccept.dialogShown");
-      var called = prefWindow.Preferences.get("tests.beforeaccept.called");
-      is(dialogShown.value, true, "dialog opened, shown pref set");
-      is(dialogShown.valueFromPreferences, null, "shown pref not committed");
-      is(called.value, null, "beforeaccept not yet called");
-      is(called.valueFromPreferences, null, "beforeaccept not yet called, pref not committed");
-
-      // try to accept the dialog, should fail the first time
-      prefWindow.document.getElementById("beforeaccept_dialog").acceptDialog();
-      is(prefWindow.closed, false, "window not closed");
-      is(dialogShown.value, true, "shown pref still set");
-      is(dialogShown.valueFromPreferences, null, "shown pref still not committed");
-      is(called.value, true, "beforeaccept called");
-      is(called.valueFromPreferences, null, "called pref not committed");
-
-      // try again, this one should succeed
-      prefWindow.document.getElementById("beforeaccept_dialog").acceptDialog();
-      is(prefWindow.closed, true, "window now closed");
-      is(dialogShown.valueFromPreferences, true, "shown pref committed");
-      is(called.valueFromPreferences, true, "called pref committed");
-
-      SimpleTest.finish();
-    }
-});
   ]]>
   </script>