Bug 1346234 - Part 43: Remove test harness export for "optionsClear" functions. r=sfink
authorAndré Bargull <andre.bargull@gmail.com>
Tue, 31 Oct 2017 08:03:04 -0700
changeset 689748 6b6102b5d8bff6a7c0a3848efeec2232c2dd5512
parent 689747 3477b6b49e67b02d4181247551f490d1f56f30a1
child 689749 917e3724bdedd26a62dd542f38469f6a8ff4d1f1
push id87097
push userdholbert@mozilla.com
push dateTue, 31 Oct 2017 22:39:07 +0000
reviewerssfink
bugs1346234
milestone58.0a1
Bug 1346234 - Part 43: Remove test harness export for "optionsClear" functions. r=sfink
js/src/tests/browser.js
js/src/tests/shell.js
--- a/js/src/tests/browser.js
+++ b/js/src/tests/browser.js
@@ -232,17 +232,16 @@
   var NodePrototypeAppendChild = global.Node.prototype.appendChild;
   var NodePrototypeTextContentSetter =
     ObjectGetOwnPropertyDescriptor(global.Node.prototype, "textContent").set;
   var setTimeout = global.setTimeout;
 
   // Saved harness functions.
   var dump = global.dump;
   var gczeal = global.gczeal;
-  var optionsClear = global.optionsClear;
   var print = global.print;
   var reportFailure = global.reportFailure;
   var TestCase = global.TestCase;
 
   var SpecialPowers = global.SpecialPowers;
   var SpecialPowersCu = SpecialPowers.Cu;
   var SpecialPowersForceGC = SpecialPowers.forceGC;
 
@@ -284,16 +283,27 @@
   function CreateElement(name) {
     return ReflectApply(DocumentCreateElement, document, [name]);
   }
 
   function SetTextContent(element, text) {
     ReflectApply(NodePrototypeTextContentSetter, element, [text]);
   }
 
+  // Object containing the set options.
+  var currentOptions;
+
+  // browser.js version of shell.js' |shellOptionsClear| function.
+  function browserOptionsClear() {
+    for (var optionName in currentOptions) {
+      delete currentOptions[optionName];
+      SpecialPowersCu[optionName] = false;
+    }
+  }
+
   // This function is *only* used by shell.js's for-browsers |print()| function!
   // It's only defined/exported here because it needs CreateElement and friends,
   // only defined here, and we're not yet ready to move them to shell.js.
   function AddPrintOutput(s) {
     var msgDiv = CreateElement("div");
     SetTextContent(msgDiv, s);
     AppendChild(printOutputContainer, msgDiv);
   }
@@ -323,20 +333,18 @@
 
   // Possible values:
   // - "Unknown" if no error is expected,
   // - "error" if no specific error type is expected,
   // - otherwise the error name, e.g. "TypeError" or "RangeError".
   var expectedError;
 
   window.onerror = function (msg, page, line, column, error) {
-    // Restore options in case a test case used this common variable name.
-    global.options = options;
-
-    optionsClear();
+    // Unset all options even when the test finished with an error.
+    browserOptionsClear();
 
     if (DESCRIPTION === undefined) {
       DESCRIPTION = "Unknown";
     }
 
     var actual = "error";
     var expected = expectedError;
     if (expected !== "error" && expected !== "Unknown") {
@@ -369,66 +377,65 @@
   }
   global.gc = gc;
 
   function options(aOptionName) {
     // return value of options() is a comma delimited list
     // of the previously set values
 
     var value = "";
-    for (var optionName in options.currvalues) {
+    for (var optionName in currentOptions) {
       if (value)
         value += ",";
       value += optionName;
     }
 
     if (aOptionName) {
       if (!HasOwnProperty(SpecialPowersCu, aOptionName)) {
         // This test is trying to flip an unsupported option, so it's
         // likely no longer testing what it was supposed to.  Fail it
         // hard.
         throw "Unsupported JSContext option '" + aOptionName + "'";
       }
 
-      if (aOptionName in options.currvalues) {
+      if (aOptionName in currentOptions) {
         // option is set, toggle it to unset
-        delete options.currvalues[aOptionName];
+        delete currentOptions[aOptionName];
         SpecialPowersCu[aOptionName] = false;
       } else {
         // option is not set, toggle it to set
-        options.currvalues[aOptionName] = true;
+        currentOptions[aOptionName] = true;
         SpecialPowersCu[aOptionName] = true;
       }
     }
 
     return value;
   }
   global.options = options;
 
   /****************************************
    * HARNESS SETUP AND TEARDOWN FUNCTIONS *
    ****************************************/
 
   function jsTestDriverBrowserInit() {
     // Unset all options before running any test code, cf. the call to
-    // |optionsClear| in shell.js' set-up code.
+    // |shellOptionsClear| in shell.js' set-up code.
     for (var optionName of ["strict", "werror", "strict_mode"]) {
       if (!HasOwnProperty(SpecialPowersCu, optionName))
         throw "options is out of sync with Components.utils";
 
       // Option is set, toggle it to unset. (Reading an option is a cheap
       // operation, but setting is relatively expensive, so only assign if
       // necessary.)
       if (SpecialPowersCu[optionName])
         SpecialPowersCu[optionName] = false;
     }
 
-    // Hash containing the set options, initially empty because we just turned
-    // off all options.
-    options.currvalues = Object.create(null);
+    // Initialize with an empty set, because we just turned off all options.
+    currentOptions = Object.create(null);
 
     if (document.location.search.indexOf("?") !== 0) {
       // not called with a query string
       return;
     }
 
     var properties = Object.create(null);
     var fields = document.location.search.slice(1).split(";");
@@ -609,24 +616,18 @@
     // jsTestDriverEnd()
 
     if (gDelayTestDriverEnd) {
       return;
     }
 
     window.onerror = null;
 
-    // Restore options in case a test case used this common variable name.
-    global.options = options;
-
-    try {
-      optionsClear();
-    } catch (ex) {
-      dump("jsTestDriverEnd " + ex);
-    }
+    // Unset all options when the test has finished.
+    browserOptionsClear();
 
     if (window.opener && window.opener.runNextTest) {
       if (window.opener.reportCallBack) {
         window.opener.reportCallBack(window.opener.gWindow);
       }
 
       setTimeout("window.opener.runNextTest()", 250);
     } else {
--- a/js/src/tests/shell.js
+++ b/js/src/tests/shell.js
@@ -41,16 +41,17 @@
     // Certain cached functionality only exists (and is only needed) when
     // running in the browser.  Segregate that caching here.
 
     var SpecialPowersSetGCZeal =
       global.SpecialPowers ? global.SpecialPowers.setGCZeal : undefined;
   }
 
   var evaluate = global.evaluate;
+  var options = global.options;
 
   /****************************
    * GENERAL HELPER FUNCTIONS *
    ****************************/
 
   // We *cannot* use Array.prototype.push for this, because that function sets
   // the new trailing element, which could invoke a setter (left by a test) on
   // Array.prototype or Object.prototype.
@@ -86,16 +87,31 @@
         return parts;
       }
 
       ArrayPush(parts, ReflectApply(StringPrototypeSubstring, str, [last, i]));
       last = i + delimiter.length;
     }
   }
 
+  function shellOptionsClear() {
+    assertEq(runningInBrowser, false, "Only called when running in the shell.");
+
+    // Return early if no options are set.
+    var currentOptions = options();
+    if (currentOptions === "")
+      return;
+
+    // Turn off current settings.
+    var optionNames = StringSplit(currentOptions, ",");
+    for (var i = 0; i < optionNames.length; i++) {
+      options(optionNames[i]);
+    }
+  }
+
   /****************************
    * TESTING FUNCTION EXPORTS *
    ****************************/
 
   function SameValue(v1, v2) {
     // We could |return Object.is(v1, v2);|, but that's less portable.
     if (v1 === 0 && v2 === 0)
       return 1 / v1 === 1 / v2;
@@ -559,50 +575,26 @@
     // fired. They are responsible for setting gDelayTestDriverEnd = true then
     // when completed, setting gDelayTestDriverEnd = false then calling
     // jsTestDriverEnd()
 
     if (gDelayTestDriverEnd) {
       return;
     }
 
-    try {
-      optionsClear();
-    } catch(ex) {
-      dump('jsTestDriverEnd ' + ex);
-    }
+    // Unset all options when the test has finished.
+    shellOptionsClear();
 
     var testCases = getTestCases();
     for (var i = 0; i < testCases.length; i++) {
       testCases[i].dump();
     }
   }
   global.jsTestDriverEnd = jsTestDriverEnd;
 
-  // Harness internal function, only exported for browser.js.
-  // XXX: This function is only exported for the window.onerror handler in
-  // browser.js. If the handler doesn't actually need to clear the options, we
-  // can remove this export.
-  function optionsClear() {
-    // Return early if no options are set.
-    var currentOptions = options();
-    if (currentOptions === "")
-      return;
-
-    // Turn off current settings.
-    var optionNames = StringSplit(currentOptions, ",");
-    for (var i = 0; i < optionNames.length; i++) {
-      var optionName = optionNames[i];
-      if (optionName) {
-        options(optionName);
-      }
-    }
-  }
-  global.optionsClear = optionsClear;
-
   /************************************
    * PROMISE TESTING FUNCTION EXPORTS *
    ************************************/
 
   function getPromiseResult(promise) {
     var result, error, caught = false;
     promise.then(r => { result = r; },
                  e => { caught = true; error = e; });
@@ -630,14 +622,14 @@
 
   /*******************************************
    * RUN ONCE CODE TO SETUP ADDITIONAL STATE *
    *******************************************/
 
   // Clear all options before running any tests. browser.js performs this
   // set-up as part of its jsTestDriverBrowserInit function.
   if (!runningInBrowser)
-    optionsClear();
+    shellOptionsClear();
 })(this);
 
 var gDelayTestDriverEnd = false;
 
 var DESCRIPTION;