From b459f53a11232cdd78a7c514ea4299e99cb3f66a Mon Sep 17 00:00:00 2001
From: Kris Maglione <maglione.k@gmail.com>
Date: Mon, 24 Jun 2019 13:47:53 -0700
Subject: [PATCH] Bug 1561061: Move SpecialPowers pref env code to parent and
 make sane-ish. r=aswan

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

--HG--
extra : rebase_source : ec33af8c17048c3828d4ca4643e2e17bd2a854c0
extra : source : c2d0956f41d82e76c682f829807e818863cd802a
---
 browser/base/content/test/general/head.js     |   8 +-
 .../browser_inspector_extension_sidebar.js    |   2 +-
 devtools/client/shared/test/shared-head.js    |   8 +-
 dom/base/test/chrome/test_bug914381.html      |   2 +-
 ..._exception_options_from_jsimplemented.html |   4 +-
 ...promise_rejections_from_jsimplemented.html |   6 +-
 dom/canvas/test/test_hitregion_event.html     |   2 +-
 dom/html/test/file_fullscreen-denied.html     |   1 +
 .../test/test_eme_request_notifications.html  |   2 +-
 .../test_background_loading_iframes.html      |   2 +-
 layout/base/tests/test_bug394057.html         |   4 +-
 .../content/SpecialPowersAPI.jsm              | 244 ++----------------
 .../content/SpecialPowersAPIParent.jsm        | 227 +++++++++++++---
 .../mochitest/test_ext_storage_cleanup.html   |   4 +-
 14 files changed, 227 insertions(+), 289 deletions(-)

diff --git a/browser/base/content/test/general/head.js b/browser/base/content/test/general/head.js
index 6c881d367dde..8b65cbb0d728 100644
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@@ -161,15 +161,11 @@ function setTestPluginEnabledState(newEnabledState, pluginName) {
 }
 
 function pushPrefs(...aPrefs) {
-  return new Promise(resolve => {
-    SpecialPowers.pushPrefEnv({"set": aPrefs}, resolve);
-  });
+  return SpecialPowers.pushPrefEnv({"set": aPrefs});
 }
 
 function popPrefs() {
-  return new Promise(resolve => {
-    SpecialPowers.popPrefEnv(resolve);
-  });
+  return SpecialPowers.popPrefEnv();
 }
 
 function updateBlocklist(aCallback) {
diff --git a/devtools/client/inspector/extensions/test/browser_inspector_extension_sidebar.js b/devtools/client/inspector/extensions/test/browser_inspector_extension_sidebar.js
index e06ef6309036..761d308228d6 100644
--- a/devtools/client/inspector/extensions/test/browser_inspector_extension_sidebar.js
+++ b/devtools/client/inspector/extensions/test/browser_inspector_extension_sidebar.js
@@ -273,7 +273,7 @@ add_task(async function testActiveTabOnNonExistingSidebar() {
   // to simulate the scenario where an extension has installed a sidebar
   // which has been saved in the preference but it doesn't exist anymore.
   await SpecialPowers.pushPrefEnv({
-    set: [["devtools.inspector.activeSidebar"], "unexisting-sidebar-id"],
+    set: [["devtools.inspector.activeSidebar", "unexisting-sidebar-id"]],
   });
 
   const res = await openInspectorForURL("about:blank");
diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js
index 4f419abd30bd..1dc40d6d8cf6 100644
--- a/devtools/client/shared/test/shared-head.js
+++ b/devtools/client/shared/test/shared-head.js
@@ -456,6 +456,8 @@ var closeTabAndToolbox = async function(tab = gBrowser.selectedTab) {
   }
 
   await removeTab(tab);
+
+  await new Promise(resolve => setTimeout(resolve, 0));
 };
 
 /**
@@ -596,10 +598,8 @@ function waitForClipboardPromise(setup, expected) {
  * @return {Promise} resolves when the preferences have been updated
  */
 function pushPref(preferenceName, value) {
-  return new Promise(resolve => {
-    const options = {"set": [[preferenceName, value]]};
-    SpecialPowers.pushPrefEnv(options, resolve);
-  });
+  const options = {"set": [[preferenceName, value]]};
+  return SpecialPowers.pushPrefEnv(options);
 }
 
 /**
diff --git a/dom/base/test/chrome/test_bug914381.html b/dom/base/test/chrome/test_bug914381.html
index 8c2601f405b6..7f8448ad2c9f 100644
--- a/dom/base/test/chrome/test_bug914381.html
+++ b/dom/base/test/chrome/test_bug914381.html
@@ -34,7 +34,7 @@ function createFileWithData(fileData) {
 /** Test for Bug 914381. File's created in JS using an nsIFile should allow mozGetFullPathInternal calls to succeed **/
 var file = createFileWithData("Test bug 914381");
 
-SpecialPowers.pushPrefEnv({ set: [ "dom.file.createInChild" ]})
+SpecialPowers.pushPrefEnv({ set: [["dom.file.createInChild", true]]})
 .then(() => {
   return File.createFromNsIFile(file);
 })
diff --git a/dom/bindings/test/test_exception_options_from_jsimplemented.html b/dom/bindings/test/test_exception_options_from_jsimplemented.html
index c19e35e29445..3bba95d831c1 100644
--- a/dom/bindings/test/test_exception_options_from_jsimplemented.html
+++ b/dom/bindings/test/test_exception_options_from_jsimplemented.html
@@ -16,10 +16,10 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
 
   function doTest() {
     var file = location.href;
+
     var asyncFrame;
     /* Async parent frames from pushPrefEnv don't show up in e10s.  */
-    var isE10S = !SpecialPowers.isMainProcess();
-    if (!isE10S && SpecialPowers.getBoolPref("javascript.options.asyncstack")) {
+    if (SpecialPowers.getBoolPref("javascript.options.asyncstack")) {
       asyncFrame = `Async*@${file}:153:17
 `;
     } else {
diff --git a/dom/bindings/test/test_promise_rejections_from_jsimplemented.html b/dom/bindings/test/test_promise_rejections_from_jsimplemented.html
index d70beb6a467c..e21117199753 100644
--- a/dom/bindings/test/test_promise_rejections_from_jsimplemented.html
+++ b/dom/bindings/test/test_promise_rejections_from_jsimplemented.html
@@ -37,12 +37,12 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
 
   function doTest() {
     var t = new TestInterfaceJS();
-    /* Async parent frames from pushPrefEnv don't show up in e10s.  */
-    var isE10S = !SpecialPowers.isMainProcess();
+
+
     var asyncStack = SpecialPowers.getBoolPref("javascript.options.asyncstack");
     var ourFile = location.href;
     var unwrapError = "Promise rejection value is a non-unwrappable cross-compartment wrapper.";
-    var parentFrame = (asyncStack && !isE10S) ? `Async*@${ourFile}:130:17
+    var parentFrame = asyncStack ? `Async*@${ourFile}:130:17
 ` : "";
 
     Promise.all([
diff --git a/dom/canvas/test/test_hitregion_event.html b/dom/canvas/test/test_hitregion_event.html
index 383523e269e7..d98b35b70614 100644
--- a/dom/canvas/test/test_hitregion_event.html
+++ b/dom/canvas/test/test_hitregion_event.html
@@ -16,7 +16,7 @@
 </div>
 <pre id="test">
 <script type="application/javascript">
-SpecialPowers.pushPrefEnv({"set": [["canvas.hitregions.enabled", true]]}, function() {
+SpecialPowers.pushPrefEnv({"set": [["canvas.hitregions.enabled", true]]}).then(function() {
 
   var input = document.getElementById("input");
   var regionId = "";
diff --git a/dom/html/test/file_fullscreen-denied.html b/dom/html/test/file_fullscreen-denied.html
index a01ce3ee31ef..f6952ef6b151 100644
--- a/dom/html/test/file_fullscreen-denied.html
+++ b/dom/html/test/file_fullscreen-denied.html
@@ -152,6 +152,7 @@ async function testFullscreenMouseBtn(event, button, next) {
   // Restore the pref environment we changed before
   // entering testNonTrustContext.
   await SpecialPowers.popPrefEnv();
+  await SpecialPowers.popPrefEnv();
   finish();
 }
 
diff --git a/dom/media/test/test_eme_request_notifications.html b/dom/media/test/test_eme_request_notifications.html
index ddc92ef655cd..441b27e8affd 100644
--- a/dom/media/test/test_eme_request_notifications.html
+++ b/dom/media/test/test_eme_request_notifications.html
@@ -60,7 +60,7 @@ var tests = [
   {
     keySystem: "com.widevine.alpha",
     expectedStatus: 'cdm-not-installed',
-    prefs: [["media.eme.enabled", true], , ["media.gmp-widevinecdm.enabled", true]]
+    prefs: [["media.eme.enabled", true], ["media.gmp-widevinecdm.enabled", true]]
   },
   {
     keySystem: CLEARKEY_KEYSYSTEM,
diff --git a/dom/tests/mochitest/dom-level0/test_background_loading_iframes.html b/dom/tests/mochitest/dom-level0/test_background_loading_iframes.html
index 7eea8d4910c0..311af252077b 100644
--- a/dom/tests/mochitest/dom-level0/test_background_loading_iframes.html
+++ b/dom/tests/mochitest/dom-level0/test_background_loading_iframes.html
@@ -34,7 +34,7 @@ window.onload = function() {
   myLoadTime = performance.now();
 }
 
-SpecialPowers.pushPrefEnv({"set":[["dom.background_loading_iframe", true]]}, function () {
+SpecialPowers.pushPrefEnv({"set":[["dom.background_loading_iframe", true]]}).then(function () {
   var iframe1 = document.createElement("iframe");
   var iframe2 = document.createElement("iframe");
   var iframe3 = document.createElement("iframe");
diff --git a/layout/base/tests/test_bug394057.html b/layout/base/tests/test_bug394057.html
index 2b395944f5d5..7cadd4075362 100644
--- a/layout/base/tests/test_bug394057.html
+++ b/layout/base/tests/test_bug394057.html
@@ -63,12 +63,12 @@ if (serifWidth == monospaceWidth) {
 isnot(serifWidth, monospaceWidth,
       "can't find serif and monospace fonts of different width");
 
-SpecialPowers.pushPrefEnv({'set': [['font.name.serif.x-western', serifFonts[serifIdx]]]}, step2);
+SpecialPowers.pushPrefEnv({'set': [['font.name.serif.x-western', serifFonts[serifIdx]]]}).then(step2);
 
 var serifWidthFromPref;
 function step2() {
     serifWidthFromPref = tableElement.offsetWidth;
-    SpecialPowers.pushPrefEnv({'set': [['font.name.serif.x-western', monospaceFonts[monospaceIdx]]]}, step3);
+    SpecialPowers.pushPrefEnv({'set': [['font.name.serif.x-western', monospaceFonts[monospaceIdx]]]}).then(step3);
 }
 var monospaceWidthFromPref;
 function step3() {
diff --git a/testing/specialpowers/content/SpecialPowersAPI.jsm b/testing/specialpowers/content/SpecialPowersAPI.jsm
index acffd055f72e..5726c347c4a5 100644
--- a/testing/specialpowers/content/SpecialPowersAPI.jsm
+++ b/testing/specialpowers/content/SpecialPowersAPI.jsm
@@ -118,7 +118,6 @@ class SpecialPowersAPI extends JSWindowActorChild {
     this._unexpectedCrashDumpFiles = { };
     this._crashDumpDir = null;
     this._mfl = null;
-    this._applyingPrefs = false;
     this._applyingPermissions = false;
     this._observingPermissions = false;
     this._asyncObservers = new WeakMap();
@@ -517,6 +516,12 @@ class SpecialPowersAPI extends JSWindowActorChild {
       this.contentWindow.setTimeout(callback, 0);
   }
 
+  promiseTimeout(delay) {
+    return new Promise(resolve => {
+      this._setTimeout(resolve, delay);
+    });
+  }
+
   _delayCallbackTwice(callback) {
      let delayedCallback = () => {
        let delayAgain = (aCallback) => {
@@ -737,236 +742,19 @@ class SpecialPowersAPI extends JSWindowActorChild {
     }
   }
 
-  /**
-   * Helper to resolve a promise by calling the resolve function and call an
-   * optional callback.
-   */
-  _resolveAndCallOptionalCallback(resolveFn, callback = null) {
-    resolveFn();
-
-    if (callback) {
-      callback();
-    }
+  async pushPrefEnv(inPrefs, callback = null) {
+    await this.sendQuery("PushPrefEnv", inPrefs).then(callback);
+    await this.promiseTimeout(0);
   }
 
-  /**
-   * Take in a list of pref changes to make, then invokes |callback| and resolves
-   * the returned Promise once those changes have taken effect.  When the test
-   * finishes, these changes are reverted.
-   *
-   * |inPrefs| must be an object with up to two properties: "set" and "clear".
-   * pushPrefEnv will set prefs as indicated in |inPrefs.set| and will unset
-   * the prefs indicated in |inPrefs.clear|.
-   *
-   * For example, you might pass |inPrefs| as:
-   *
-   *  inPrefs = {'set': [['foo.bar', 2], ['magic.pref', 'baz']],
-   *             'clear': [['clear.this'], ['also.this']] };
-   *
-   * Notice that |set| and |clear| are both an array of arrays.  In |set|, each
-   * of the inner arrays must have the form [pref_name, value] or [pref_name,
-   * value, iid].  (The latter form is used for prefs with "complex" values.)
-   *
-   * In |clear|, each inner array should have the form [pref_name].
-   *
-   * If you set the same pref more than once (or both set and clear a pref),
-   * the behavior of this method is undefined.
-   *
-   * (Implementation note: _prefEnvUndoStack is a stack of values to revert to,
-   * not values which have been set!)
-   *
-   * TODO: complex values for original cleanup?
-   *
-   */
-  async _pushPrefEnv(inPrefs) {
-    var prefs = Services.prefs;
-
-    var pref_string = [];
-    pref_string[prefs.PREF_INT] = "INT";
-    pref_string[prefs.PREF_BOOL] = "BOOL";
-    pref_string[prefs.PREF_STRING] = "CHAR";
-
-    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);
-      }
-    }
-
-    return new Promise(resolve => {
-      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.
-        this._prefEnvUndoStack.push(cleanupActions);
-        this._pendingPrefs.push([pendingActions,
-                                 this._delayCallbackTwice(resolve)]);
-        this._applyPrefs();
-      } else {
-        this._setTimeout(resolve);
-      }
-    });
+  async popPrefEnv(callback = null) {
+    await this.sendQuery("PopPrefEnv").then(callback);
+    await this.promiseTimeout(0);
   }
 
-  pushPrefEnv(inPrefs, callback = null) {
-    let promise = this._pushPrefEnv(inPrefs);
-    if (callback) {
-      promise.then(callback);
-    }
-    return promise;
-  }
-
-  popPrefEnv(callback = null) {
-    return new Promise(resolve => {
-      let done = this._resolveAndCallOptionalCallback.bind(this, resolve, callback);
-      if (this._prefEnvUndoStack.length > 0) {
-        // See pushPrefEnv comment regarding delay.
-        let cb = this._delayCallbackTwice(done);
-        /* Each pop will have a valid block of preferences */
-        this._pendingPrefs.push([this._prefEnvUndoStack.pop(), cb]);
-        this._applyPrefs();
-      } else {
-        this._setTimeout(done);
-      }
-    });
-  }
-
-  flushPrefEnv(callback = null) {
-    while (this._prefEnvUndoStack.length > 1)
-      this.popPrefEnv(null);
-
-    return new Promise(resolve => {
-      let done = this._resolveAndCallOptionalCallback.bind(this, resolve, callback);
-      this.popPrefEnv(done);
-    });
-  }
-
-  _isPrefActionNeeded(prefAction) {
-    if (prefAction.action === "clear") {
-      return Services.prefs.prefHasUserValue(prefAction.name);
-    } else if (prefAction.action === "set") {
-      try {
-        let currentValue = this._getPref(prefAction.name, prefAction.type, {});
-        return currentValue != prefAction.value;
-      } catch (e) {
-        // If the preference is not defined yet, setting the value will have an effect.
-        return true;
-      }
-    }
-    // Only "clear" and "set" actions are supported.
-    return false;
-  }
-
-  /*
-    Iterate through one atomic set of pref actions and perform sets/clears as appropriate.
-    All actions performed must modify the relevant pref.
-  */
-  _applyPrefs() {
-    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];
-
-    // Filter out all the pending actions that will not have any effect.
-    pendingActions = pendingActions.filter(action => {
-      return this._isPrefActionNeeded(action);
-    });
-
-
-    var self = this;
-    let onPrefActionsApplied = function() {
-      self._setTimeout(callback);
-      self._setTimeout(function() {
-        self._applyingPrefs = false;
-        // Now apply any prefs that may have been queued while we were applying
-        self._applyPrefs();
-      });
-    };
-
-    // If no valid action remains, call onPrefActionsApplied directly and bail out.
-    if (pendingActions.length === 0) {
-      onPrefActionsApplied();
-      return;
-    }
-
-    var lastPref = pendingActions[pendingActions.length - 1];
-
-    var pb = Services.prefs;
-    pb.addObserver(lastPref.name, function prefObs(subject, topic, data) {
-      pb.removeObserver(lastPref.name, prefObs);
-      onPrefActionsApplied();
-    });
-
-    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);
-      }
-    }
+  async flushPrefEnv(callback = null) {
+    await this.sendQuery("FlushPrefEnv").then(callback);
+    await this.promiseTimeout(0);
   }
 
   _addObserverProxy(notification) {
@@ -2075,8 +1863,6 @@ SpecialPowersAPI.prototype.EARLY_BETA_OR_EARLIER = AppConstants.EARLY_BETA_OR_EA
 // code depends on all SpecialPowers instances using the same arrays for
 // these.
 Object.assign(SpecialPowersAPI.prototype, {
-  _prefEnvUndoStack: [],
-  _pendingPrefs: [],
   _permissionsUndoStack: [],
   _pendingPermissions: [],
 });
diff --git a/testing/specialpowers/content/SpecialPowersAPIParent.jsm b/testing/specialpowers/content/SpecialPowersAPIParent.jsm
index 1f466eb1aa3e..3449af873cd9 100644
--- a/testing/specialpowers/content/SpecialPowersAPIParent.jsm
+++ b/testing/specialpowers/content/SpecialPowersAPIParent.jsm
@@ -72,6 +72,33 @@ function getTestPlugin(pluginName) {
   return null;
 }
 
+const PREF_TYPES = {
+  [Ci.nsIPrefBranch.PREF_INVALID]: "INVALID",
+  [Ci.nsIPrefBranch.PREF_INT]: "INT",
+  [Ci.nsIPrefBranch.PREF_BOOL]: "BOOL",
+  [Ci.nsIPrefBranch.PREF_STRING]: "CHAR",
+  "number": "INT",
+  "boolean": "BOOL",
+  "string": "CHAR",
+};
+
+// We share a single preference environment stack between all
+// SpecialPowers instances, across all processes.
+let prefUndoStack = [];
+let inPrefEnvOp = false;
+
+function doPrefEnvOp(fn) {
+  if (inPrefEnvOp) {
+    throw new Error("Reentrant preference environment operations not supported");
+  }
+  inPrefEnvOp = true;
+  try {
+    return fn();
+  } finally {
+    inPrefEnvOp = false;
+  }
+}
+
 class SpecialPowersAPIParent extends JSWindowActorParent {
   constructor() {
     super();
@@ -245,6 +272,156 @@ class SpecialPowersAPIParent extends JSWindowActorParent {
     });
   }
 
+  /*
+    Iterate through one atomic set of pref actions and perform sets/clears as appropriate.
+    All actions performed must modify the relevant pref.
+  */
+  _applyPrefs(actions) {
+    for (let pref of actions) {
+      if (pref.action == "set") {
+        this._setPref(pref.name, pref.type, pref.value, pref.iid);
+      } else if (pref.action == "clear") {
+        Services.prefs.clearUserPref(pref.name);
+      }
+    }
+  }
+
+  /**
+   * Take in a list of pref changes to make, pushes their current values
+   * onto the restore stack, and makes the changes.  When the test
+   * finishes, these changes are reverted.
+   *
+   * |inPrefs| must be an object with up to two properties: "set" and "clear".
+   * pushPrefEnv will set prefs as indicated in |inPrefs.set| and will unset
+   * the prefs indicated in |inPrefs.clear|.
+   *
+   * For example, you might pass |inPrefs| as:
+   *
+   *  inPrefs = {'set': [['foo.bar', 2], ['magic.pref', 'baz']],
+   *             'clear': [['clear.this'], ['also.this']] };
+   *
+   * Notice that |set| and |clear| are both an array of arrays.  In |set|, each
+   * of the inner arrays must have the form [pref_name, value] or [pref_name,
+   * value, iid].  (The latter form is used for prefs with "complex" values.)
+   *
+   * In |clear|, each inner array should have the form [pref_name].
+   *
+   * If you set the same pref more than once (or both set and clear a pref),
+   * the behavior of this method is undefined.
+   */
+  pushPrefEnv(inPrefs) {
+    return doPrefEnvOp(() => {
+      let pendingActions = [];
+      let cleanupActions = [];
+
+      for (let [action, prefs] of Object.entries(inPrefs)) {
+        for (let pref of prefs) {
+          let name = pref[0];
+          let value = null;
+          let iid = null;
+          let type = PREF_TYPES[Services.prefs.getPrefType(name)];
+          let originalValue = null;
+
+          if (pref.length == 3) {
+            value = pref[1];
+            iid = pref[2];
+          } else if (pref.length == 2) {
+            value = pref[1];
+          }
+
+
+          /* If pref is not found or invalid it doesn't exist. */
+          if (type !== "INVALID") {
+            if ((Services.prefs.prefHasUserValue(name) && action == "clear") ||
+                action == "set") {
+              originalValue = this._getPref(name, type);
+            }
+          } else if (action == "set") {
+            /* name doesn't exist, so 'clear' is pointless */
+            if (iid) {
+              type = "COMPLEX";
+            }
+          }
+
+          if (type === "INVALID") {
+            type = PREF_TYPES[typeof value];
+          }
+          if (type === "INVALID") {
+            throw new Error("Unexpected preference type");
+          }
+
+          pendingActions.push({action, type, name, value, iid});
+
+          /* Push original preference value or clear into cleanup array */
+          var cleanupTodo = {type, name, value: originalValue, iid};
+          if (originalValue == null) {
+            cleanupTodo.action = "clear";
+          } else {
+            cleanupTodo.action = "set";
+          }
+          cleanupActions.push(cleanupTodo);
+        }
+      }
+
+      prefUndoStack.push(cleanupActions);
+      this._applyPrefs(pendingActions);
+    });
+  }
+
+  async popPrefEnv() {
+    return doPrefEnvOp(() => {
+      let env = prefUndoStack.pop();
+      if (env) {
+        this._applyPrefs(env);
+        return true;
+      }
+      return false;
+    });
+  }
+
+  flushPrefEnv() {
+    while (prefUndoStack.length) {
+      this.popPrefEnv();
+    }
+  }
+
+  _setPref(name, type, value, iid) {
+    switch (type) {
+      case "BOOL":
+        return Services.prefs.setBoolPref(name, value);
+      case "INT":
+        return Services.prefs.setIntPref(name, value);
+      case "CHAR":
+        return Services.prefs.setCharPref(name, value);
+      case "COMPLEX":
+        return Services.prefs.setComplexValue(name, iid, value);
+    }
+    throw new Error(`Unexpected preference type: ${type}`);
+  }
+
+  _getPref(name, type, defaultValue, iid) {
+    switch (type) {
+      case "BOOL":
+        if (defaultValue !== undefined) {
+          return Services.prefs.getBoolPref(name, defaultValue);
+        }
+        return Services.prefs.getBoolPref(name);
+      case "INT":
+        if (defaultValue !== undefined) {
+          return Services.prefs.getIntPref(name, defaultValue);
+        }
+        return Services.prefs.getIntPref(name);
+      case "CHAR":
+        if (defaultValue !== undefined) {
+          return Services.prefs.getCharPref(name, defaultValue);
+        }
+        return Services.prefs.getCharPref(name);
+      case "COMPLEX":
+        return Services.prefs.getComplexValue(name, iid);
+    }
+    throw new Error(`Unexpected preference type: ${type}`);
+  }
+
   /**
    * messageManager callback function
    * This will get requests from our API in the window and process them in chrome for it
@@ -254,6 +431,15 @@ class SpecialPowersAPIParent extends JSWindowActorParent {
     // doesn't trigger a flurry of warnings about "does not always return
     // a value".
     switch (aMessage.name) {
+      case "PushPrefEnv":
+        return this.pushPrefEnv(aMessage.data);
+
+      case "PopPrefEnv":
+        return this.popPrefEnv();
+
+      case "FlushPrefEnv":
+        return this.flushPrefEnv();
+
       case "SPPrefService": {
         let prefs = Services.prefs;
         let prefType = aMessage.json.prefType.toUpperCase();
@@ -266,52 +452,21 @@ class SpecialPowersAPIParent extends JSWindowActorParent {
           // return null if the pref doesn't exist
           if (defaultValue === undefined && prefs.getPrefType(prefName) == prefs.PREF_INVALID)
             return null;
+          return this._getPref(prefName, prefType, defaultValue, iid);
         } else if (aMessage.json.op == "set") {
           if (!prefName || !prefType || prefValue === undefined)
             throw new SpecialPowersError("Invalid parameters for set in SPPrefService");
+
+          return this._setPref(prefName, prefType, prefValue, iid);
         } else if (aMessage.json.op == "clear") {
           if (!prefName)
             throw new SpecialPowersError("Invalid parameters for clear in SPPrefService");
+
+          prefs.clearUserPref(prefName);
         } else {
           throw new SpecialPowersError("Invalid operation for SPPrefService");
         }
 
-        // Now we make the call
-        switch (prefType) {
-          case "BOOL":
-            if (aMessage.json.op == "get") {
-              if (defaultValue !== undefined) {
-                return prefs.getBoolPref(prefName, defaultValue);
-              }
-              return prefs.getBoolPref(prefName);
-            }
-            return prefs.setBoolPref(prefName, prefValue);
-          case "INT":
-            if (aMessage.json.op == "get") {
-              if (defaultValue !== undefined) {
-                return prefs.getIntPref(prefName, defaultValue);
-              }
-              return prefs.getIntPref(prefName);
-            }
-            return prefs.setIntPref(prefName, prefValue);
-          case "CHAR":
-            if (aMessage.json.op == "get") {
-              if (defaultValue !== undefined) {
-                return prefs.getCharPref(prefName, defaultValue);
-              }
-              return prefs.getCharPref(prefName);
-            }
-            return prefs.setCharPref(prefName, prefValue);
-          case "COMPLEX":
-            if (aMessage.json.op == "get")
-              return prefs.getComplexValue(prefName, iid);
-            return prefs.setComplexValue(prefName, iid, prefValue);
-          case "":
-            if (aMessage.json.op == "clear") {
-              prefs.clearUserPref(prefName);
-              return undefined;
-            }
-        }
         return undefined; // See comment at the beginning of this function.
       }
 
diff --git a/toolkit/components/extensions/test/mochitest/test_ext_storage_cleanup.html b/toolkit/components/extensions/test/mochitest/test_ext_storage_cleanup.html
index 477bde79acac..e02b01641913 100644
--- a/toolkit/components/extensions/test/mochitest/test_ext_storage_cleanup.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_storage_cleanup.html
@@ -212,7 +212,7 @@ add_task(async function test_uninistall_with_storage_local_file_backend() {
     ...(storageTestHelpers.storageLocal),
   });
 
-  await SpecialPowers.pushPrefEnv();
+  await SpecialPowers.popPrefEnv();
 });
 
 // Repeat the cleanup test when the storage.local IndexedDB backend is enabled.
@@ -226,7 +226,7 @@ add_task(async function test_uninistall_with_storage_local_idb_backend() {
     ...(storageTestHelpers.storageLocal),
   });
 
-  await SpecialPowers.pushPrefEnv();
+  await SpecialPowers.popPrefEnv();
 });
 
 </script>