Bug 1197310 - Return a promise from pushPrefEnv/popPrefEnv/flushPrefEnv. r=jmaher draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Fri, 13 May 2016 21:43:50 -0700
changeset 367082 4f56ce4db54a3d3fef2e3930504e4e5b414e0c1a
parent 364815 043082cb7bd8490c60815f67fbd1f33323ad7663
child 520910 04f2f846506fd1f14a083d727b800a9d8f13eaa8
push id18134
push usermozilla@noorenberghe.ca
push dateSat, 14 May 2016 04:44:11 +0000
reviewersjmaher
bugs1197310
milestone49.0a1
Bug 1197310 - Return a promise from pushPrefEnv/popPrefEnv/flushPrefEnv. r=jmaher MozReview-Commit-ID: HQxXeo37XS5
testing/mochitest/tests/Harness_sanity/test_SpecialPowersPushPrefEnv.html
testing/specialpowers/content/specialpowersAPI.js
--- a/testing/mochitest/tests/Harness_sanity/test_SpecialPowersPushPrefEnv.html
+++ b/testing/mochitest/tests/Harness_sanity/test_SpecialPowersPushPrefEnv.html
@@ -59,21 +59,21 @@ function test1(aCount) {
 }
 
 function test2() {
   // test non-changing values
   SpecialPowers.pushPrefEnv({"set": [["test.bool", true], ["test.int", 1], ["test.char", "test"]]}, test3);
 }
 
 function test3() {
-  // test changing char pref
+  // test changing char pref using the Promise
   is(SpecialPowers.getBoolPref('test.bool'), true, 'test.bool should be true');
   is(SpecialPowers.getIntPref('test.int'), 1, 'test.int should be 1');
   is(SpecialPowers.getCharPref('test.char'), 'test', 'test.char should be test');
-  SpecialPowers.pushPrefEnv({"set": [["test.bool", true], ["test.int", 1], ["test.char", "test2"]]}, test4);
+  SpecialPowers.pushPrefEnv({"set": [["test.bool", true], ["test.int", 1], ["test.char", "test2"]]}).then(test4);
 }
 
 function test4() {
   // test changing all values and adding test.char2 pref
   is(SpecialPowers.getCharPref('test.char'), 'test2', 'test.char should be test2');
   SpecialPowers.pushPrefEnv({"set": [["test.bool", false], ["test.int", 10], ["test.char", "test2"], ["test.char2", "test"]]}, test5);
 }
 
@@ -132,17 +132,17 @@ function test7() {
 
   try {
     SpecialPowers.getCharPref('test.char2');
     ok(false, 'This ok should not be reached!');
   } catch(e) {
     ok(true, 'getCharPref("test.char2") should throw');
   }
 
-  SpecialPowers.flushPrefEnv(test8);
+  SpecialPowers.flushPrefEnv().then(test8);
 }
 
 function test8() {
   is(SpecialPowers.getBoolPref('test.bool'), true, 'test.bool should be true');
   is(typeof SpecialPowers.getBoolPref('test.bool'), typeof true, 'test.bool should be boolean');
   is(SpecialPowers.getIntPref('test.int'), 1, 'test.int should be 1');
   is(typeof SpecialPowers.getIntPref('test.int'), typeof 1, 'test.int should be integer');
   is(SpecialPowers.getCharPref('test.char'), 'test', 'test.char should be test');
--- a/testing/specialpowers/content/specialpowersAPI.js
+++ b/testing/specialpowers/content/specialpowersAPI.js
@@ -963,20 +963,32 @@ SpecialPowersAPI.prototype = {
     }
 
     for (var idx in pendingActions) {
       var perm = pendingActions[idx];
       this._sendSyncMessage('SPPermissionManager', perm)[0];
     }
   },
 
-  /*
-   * Take in a list of pref changes to make, and invoke |callback| once those
-   * changes have taken effect.  When the test finishes, these changes are
-   * reverted.
+  /**
+   * Helper to resolve a promise by calling the resolve function and call an
+   * optional callback.
+   */
+  _resolveAndCallOptionalCallback(resolveFn, callback = null) {
+    resolveFn();
+
+    if (callback) {
+      callback();
+    }
+  },
+
+  /**
+   * 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']],
@@ -992,17 +1004,17 @@ SpecialPowersAPI.prototype = {
    * 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?
    *
    */
-  pushPrefEnv: function(inPrefs, callback) {
+  pushPrefEnv: function(inPrefs, callback = null) {
     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 = [];
@@ -1060,50 +1072,59 @@ SpecialPowersAPI.prototype = {
           cleanupTodo.action = 'clear';
         } 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.
-      this._prefEnvUndoStack.push(cleanupActions);
-      this._pendingPrefs.push([pendingActions,
-                               this._delayCallbackTwice(callback)]);
-      this._applyPrefs();
-    } else {
-      this._setTimeout(callback);
-    }
+    return new Promise(resolve => {
+      let done = this._resolveAndCallOptionalCallback.bind(this, resolve, callback);
+      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(done)]);
+        this._applyPrefs();
+      } else {
+        this._setTimeout(done);
+      }
+    });
   },
 
-  popPrefEnv: function(callback) {
-    if (this._prefEnvUndoStack.length > 0) {
-      // See pushPrefEnv comment regarding delay.
-      let cb = callback ? this._delayCallbackTwice(callback) : null;
-      /* Each pop will have a valid block of preferences */
-      this._pendingPrefs.push([this._prefEnvUndoStack.pop(), cb]);
-      this._applyPrefs();
-    } else {
-      this._setTimeout(callback);
-    }
+  popPrefEnv: function(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: function(callback) {
+  flushPrefEnv: function(callback = null) {
     while (this._prefEnvUndoStack.length > 1)
       this.popPrefEnv(null);
 
-    this.popPrefEnv(callback);
+    return new Promise(resolve => {
+      let done = this._resolveAndCallOptionalCallback.bind(this, resolve, callback);
+      this.popPrefEnv(done);
+    });
   },
 
   /*
     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) {