Bug 743843 - Introduce the options object for Components.Exception. r=Ms2ger,sr=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Sun, 15 Apr 2012 17:51:37 -0700
changeset 93542 39cf985bb1ce61ea97ef630168d9ca6b083e5a19
parent 93541 b80749916305ca8e0578e085389a75d25ecb0436
child 93543 69fdff02f85d4b7b42981a81318e3db7b2301d07
push idunknown
push userunknown
push dateunknown
reviewersMs2ger, mrbkap
bugs743843
milestone14.0a1
Bug 743843 - Introduce the options object for Components.Exception. r=Ms2ger,sr=mrbkap
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/tests/chrome/Makefile.in
js/xpconnect/tests/chrome/test_bug743843.xul
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -1852,25 +1852,40 @@ struct NS_STACK_CLASS ExceptionArgParser
 
     // Parse the constructor arguments into the above |eFoo| parameter values.
     bool parse(uint32_t argc, JS::Value *argv) {
         /*
          * The Components.Exception takes a series of arguments, all of them
          * optional:
          *
          * Argument 0: Exception message (defaults to 'exception').
-         * Argument 1: Result code (defaults to NS_ERROR_FAILURE).
+         * Argument 1: Result code (defaults to NS_ERROR_FAILURE) _or_ options
+         *             object (see below).
          * Argument 2: Stack (defaults to the current stack, which we trigger
          *                    by leaving this NULL in the parser).
          * Argument 3: Optional user data (defaults to NULL).
+         *
+         * To dig our way out of this clunky API, we now support passing an
+         * options object as the second parameter (as opposed to a result code).
+         * If this is the case, all subsequent arguments are ignored, and the
+         * following properties are parsed out of the object (using the
+         * associated default if the property does not exist):
+         *
+         *   result:    Result code (see argument 1).
+         *   stack:     Call stack (see argument 2).
+         *   data:      User data (see argument 3).
          */
         if (argc > 0 && !parseMessage(argv[0]))
             return false;
-        if (argc > 1 && !parseResult(argv[1]))
-            return false;
+        if (argc > 1) {
+            if (argv[1].isObject())
+                return parseOptionsObject(argv[1].toObject());
+            if (!parseResult(argv[1]))
+                return false;
+        }
         if (argc > 2 && !parseStack(argv[2]))
             return false;
         if (argc > 3 && !parseData(argv[3]))
             return false;
         return true;
     }
 
   protected:
@@ -1910,16 +1925,50 @@ struct NS_STACK_CLASS ExceptionArgParser
             return true;
         }
 
         return NS_SUCCEEDED(xpc->WrapJS(cx, &v.toObject(),
                                         NS_GET_IID(nsISupports),
                                         getter_AddRefs(eData)));
     }
 
+    bool parseOptionsObject(JSObject &obj) {
+        JS::Value v;
+
+        if (!getOption(obj, "result", &v) ||
+            (!v.isUndefined() && !parseResult(v)))
+            return false;
+
+        if (!getOption(obj, "stack", &v) ||
+            (!v.isUndefined() && !parseStack(v)))
+            return false;
+
+        if (!getOption(obj, "data", &v) ||
+            (!v.isUndefined() && !parseData(v)))
+            return false;
+
+        return true;
+    }
+
+    bool getOption(JSObject &obj, const char *name, JS::Value *rv) {
+        // Look for the property.
+        JSBool found;
+        if (!JS_HasProperty(cx, &obj, name, &found))
+            return false;
+
+        // If it wasn't found, indicate with undefined.
+        if (!found) {
+            *rv = JSVAL_VOID;
+            return true;
+        }
+
+        // Get the property.
+        return JS_GetProperty(cx, &obj, name, rv);
+    }
+
     /*
      * Internal data members.
      */
 
     // If there's a non-default exception string, hold onto the allocated bytes.
     JSAutoByteString messageBytes;
 
     // Various bits and pieces that are helpful to have around.
--- a/js/xpconnect/tests/chrome/Makefile.in
+++ b/js/xpconnect/tests/chrome/Makefile.in
@@ -63,23 +63,24 @@ include $(topsrcdir)/config/rules.mk
 		test_bug614757.xul \
 		test_bug616992.xul \
 		test_bug618176.xul \
 		file_bug618176.xul \
 		test_bug596580.xul \
 		test_bug654370.xul \
 		test_bug658560.xul \
 		test_bug679861.xul \
+		test_bug664689.xul \
+		test_bug706301.xul \
+		test_bug743843.xul \
 		test_APIExposer.xul \
-		test_bug664689.xul \
 		test_precisegc.xul \
 		test_nodelists.xul \
 		test_getweakmapkeys.xul \
 		test_weakmaps.xul \
-		test_bug706301.xul \
 		test_exnstack.xul \
 		test_weakref.xul \
 		$(NULL)
 
 # Disabled until this test gets updated to test the new proxy based
 # wrappers.
 #		test_wrappers-2.xul \
 
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug743843.xul
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=743843
+-->
+<window title="Mozilla Bug 743843"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=743843"
+     target="_blank">Mozilla Bug 743843</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript">
+  <![CDATA[
+  /** Test for Components.Exception options objects. **/
+
+  // Note: We pass |window| as the 'data' field here because Components.Exception
+  // doesn't handle JS Objects here all that nicely. See bug 743121.
+
+  // Test the old interface.
+  var e1 = Components.Exception('foo', Components.results.NS_BINDING_ABORTED, null, window);
+  is(e1.result, Components.results.NS_BINDING_ABORTED, "Result should get set properly");
+  is(e1.data, window, "User data should get set properly");
+
+  // Test the options object.
+  var e2 = Components.Exception('foo', { result: Components.results.NS_BINDING_ABORTED,
+                                         data: window,
+                                         foobar: 2 });
+  is(e2.result, Components.results.NS_BINDING_ABORTED, "Result should get set properly");
+  is(e2.data.window, window, "User data should get set properly");
+
+  ]]>
+  </script>
+</window>