Bug 883358 part 2. Use the new information in bindings error reporting. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 17 Jun 2013 16:31:13 -0400
changeset 146895 bfc3d2853ee3ef96f9f033ca1fc5adebbad0e786
parent 146894 1cec8b5a9ac2e131f00ca32623a3826f4915cbb3
child 146896 4bbca87e3009a82691ce86def47a27b406ad8f48
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs883358
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 883358 part 2. Use the new information in bindings error reporting. r=smaug
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Errors.msg
dom/bindings/test/Makefile.in
dom/bindings/test/test_exception_messages.html
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -55,57 +55,36 @@ ThrowErrorMessage(JSContext* aCx, const 
   va_start(ap, aErrorNumber);
   JS_ReportErrorNumberVA(aCx, GetErrorMessage, nullptr,
                          static_cast<const unsigned>(aErrorNumber), ap);
   va_end(ap);
   return false;
 }
 
 bool
-ThrowInvalidMethodThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                       const char* aInterfaceName)
+ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
+                 const ErrNum aErrorNumber,
+                 const char* aInterfaceName)
 {
   NS_ConvertASCIItoUTF16 ifaceName(aInterfaceName);
-  // This should only be called for DOM methods, which are JSNative-backed
-  // functions, so we can assume that JS_ValueToFunction and
-  // JS_GetFunctionDisplayId will both return non-null and that
-  // JS_GetStringCharsZ returns non-null.
+  // This should only be called for DOM methods/getters/setters, which
+  // are JSNative-backed functions, so we can assume that
+  // JS_ValueToFunction and JS_GetFunctionDisplayId will both return
+  // non-null and that JS_GetStringCharsZ returns non-null.
   JS::Rooted<JSFunction*> func(aCx, JS_ValueToFunction(aCx, aArgs.calleev()));
   MOZ_ASSERT(func);
   JS::Rooted<JSString*> funcName(aCx, JS_GetFunctionDisplayId(func));
   MOZ_ASSERT(funcName);
   JS_ReportErrorNumberUC(aCx, GetErrorMessage, nullptr,
-                         static_cast<const unsigned>(MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE),
+                         static_cast<const unsigned>(aErrorNumber),
                          JS_GetStringCharsZ(aCx, funcName),
                          ifaceName.get());
   return false;
 }
 
-bool
-ThrowInvalidGetterThis(JSContext* aCx, const char* aInterfaceName)
-{
-  // Sadly for getters we have no way to get the name of the property.
-  NS_ConvertASCIItoUTF16 ifaceName(aInterfaceName);
-  JS_ReportErrorNumberUC(aCx, GetErrorMessage, nullptr,
-                         static_cast<const unsigned>(MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE),
-                         ifaceName.get());
-  return false;
-}
-
-bool
-ThrowInvalidSetterThis(JSContext* aCx, const char* aInterfaceName)
-{
-  // Sadly for setters we have no way to get the name of the property.
-  NS_ConvertASCIItoUTF16 ifaceName(aInterfaceName);
-  JS_ReportErrorNumberUC(aCx, GetErrorMessage, nullptr,
-                         static_cast<const unsigned>(MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE),
-                         ifaceName.get());
-  return false;
-}
-
 } // namespace dom
 
 struct ErrorResult::Message {
   nsTArray<nsString> mArgs;
   dom::ErrNum mErrorNumber;
 };
 
 void
@@ -845,28 +824,28 @@ XrayResolveAttribute(JSContext* cx, JS::
           const JSPropertySpec& attrSpec = attributeSpecs[i];
           // Because of centralization, we need to make sure we fault in the
           // JitInfos as well. At present, until the JSAPI changes, the easiest
           // way to do this is wrap them up as functions ourselves.
           desc->attrs = attrSpec.flags & ~JSPROP_NATIVE_ACCESSORS;
           // They all have getters, so we can just make it.
           JS::Rooted<JSObject*> global(cx, JS_GetGlobalForObject(cx, wrapper));
           JS::Rooted<JSFunction*> fun(cx,
-                                      JS_NewFunction(cx, (JSNative)attrSpec.getter.op,
-                                                     0, 0, global, nullptr));
+                                      JS_NewFunctionById(cx, (JSNative)attrSpec.getter.op,
+                                                         0, 0, global, id));
           if (!fun)
             return false;
           SET_JITINFO(fun, attrSpec.getter.info);
           JSObject *funobj = JS_GetFunctionObject(fun);
           desc->getter = js::CastAsJSPropertyOp(funobj);
           desc->attrs |= JSPROP_GETTER;
           if (attrSpec.setter.op) {
             // We have a setter! Make it.
-            fun = JS_NewFunction(cx, (JSNative)attrSpec.setter.op, 1, 0,
-                                 global, nullptr);
+            fun = JS_NewFunctionById(cx, (JSNative)attrSpec.setter.op, 1, 0,
+                                     global, id);
             if (!fun)
               return false;
             SET_JITINFO(fun, attrSpec.setter.info);
             funobj = JS_GetFunctionObject(fun);
             desc->setter = js::CastAsJSStrictPropertyOp(funobj);
             desc->attrs |= JSPROP_SETTER;
           } else {
             desc->setter = nullptr;
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -55,22 +55,19 @@ UnwrapArg(JSContext* cx, jsval v, Interf
                                     vp);
   *ppArgRef = static_cast<StrongRefType*>(argRef);
   return rv;
 }
 
 bool
 ThrowErrorMessage(JSContext* aCx, const ErrNum aErrorNumber, ...);
 bool
-ThrowInvalidMethodThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                       const char* aInterfaceName);
-bool
-ThrowInvalidGetterThis(JSContext* aCx, const char* aInterfaceName);
-bool
-ThrowInvalidSetterThis(JSContext* aCx, const char* aInterfaceName);
+ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
+                 const ErrNum aErrorNumber,
+                 const char* aInterfaceName);
 
 template<bool mainThread>
 inline bool
 Throw(JSContext* cx, nsresult rv)
 {
   using mozilla::dom::workers::exceptions::ThrowDOMExceptionForNSResult;
 
   // XXX Introduce exception machinery.
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -5180,17 +5180,17 @@ def MakeNativeName(name):
 class CGGenericMethod(CGAbstractBindingMethod):
     """
     A class for generating the C++ code for an IDL method..
     """
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'), Argument('unsigned', 'argc'),
                 Argument('JS::Value*', 'vp')]
         unwrapFailureCode = (
-            'return ThrowInvalidMethodThis(cx, args, \"%s\");' %
+            'return ThrowInvalidThis(cx, args, MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE, \"%s\");' %
             descriptor.interface.identifier.name)
         CGAbstractBindingMethod.__init__(self, descriptor, 'genericMethod',
                                          args,
                                          unwrapFailureCode=unwrapFailureCode)
 
     def generate_code(self):
         return CGIndenter(CGGeneric(
             "const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(JS_CALLEE(cx, vp));\n"
@@ -5317,17 +5317,17 @@ class CGGenericGetter(CGAbstractBindingM
                 "if (!ReportLenientThisUnwrappingFailure(cx, obj)) {\n"
                 "  return false;\n"
                 "}\n"
                 "args.rval().set(JS::UndefinedValue());\n"
                 "return true;")
         else:
             name = "genericGetter"
             unwrapFailureCode = (
-                'return ThrowInvalidGetterThis(cx, "%s");' %
+                'return ThrowInvalidThis(cx, args, MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, "%s");' %
                 descriptor.interface.identifier.name)
         CGAbstractBindingMethod.__init__(self, descriptor, name, args,
                                          unwrapFailureCode)
 
     def generate_code(self):
         return CGIndenter(CGGeneric(
             "const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(JS_CALLEE(cx, vp));\n"
             "MOZ_ASSERT(info->type == JSJitInfo::Getter);\n"
@@ -5397,17 +5397,17 @@ class CGGenericSetter(CGAbstractBindingM
                 "if (!ReportLenientThisUnwrappingFailure(cx, obj)) {\n"
                 "  return false;\n"
                 "}\n"
                 "args.rval().set(JS::UndefinedValue());\n"
                 "return true;")
         else:
             name = "genericSetter"
             unwrapFailureCode = (
-                'return ThrowInvalidSetterThis(cx, "%s");' %
+                'return ThrowInvalidThis(cx, args, MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, "%s");' %
                 descriptor.interface.identifier.name)
 
         CGAbstractBindingMethod.__init__(self, descriptor, name, args,
                                          unwrapFailureCode)
 
     def generate_code(self):
         return CGIndenter(CGGeneric(
                 "if (args.length() == 0) {\n"
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -19,19 +19,19 @@
  * be replaced with a string value when the error is reported.
  */
 
 MSG_DEF(MSG_INVALID_ENUM_VALUE, 3, "{0} '{1}' is not a valid value for enumeration {2}.")
 MSG_DEF(MSG_MISSING_ARGUMENTS, 1, "Not enough arguments to {0}.")
 MSG_DEF(MSG_NOT_OBJECT, 1, "{0} is not an object.")
 MSG_DEF(MSG_NOT_CALLABLE, 1, "{0} is not callable.")
 MSG_DEF(MSG_DOES_NOT_IMPLEMENT_INTERFACE, 2, "{0} does not implement interface {1}.")
-MSG_DEF(MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, "{0} called on an object that does not implement interface {1}.")
-MSG_DEF(MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 1, "getter called on an object that does not implement interface {0}.")
-MSG_DEF(MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 1, "setter called on an object that does not implement interface {0}.")
+MSG_DEF(MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, "'{0}' called on an object that does not implement interface {1}.")
+MSG_DEF(MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, "'{0}' getter called on an object that does not implement interface {1}.")
+MSG_DEF(MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, "'{0}' setter called on an object that does not implement interface {1}.")
 MSG_DEF(MSG_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 1, "\"this\" object does not implement interface {0}.")
 MSG_DEF(MSG_NOT_IN_UNION, 2, "{0} could not be converted to any of: {1}.")
 MSG_DEF(MSG_ILLEGAL_CONSTRUCTOR, 0, "Illegal constructor.")
 MSG_DEF(MSG_NO_PROPERTY_SETTER, 1, "{0} doesn't have an indexed property setter.")
 MSG_DEF(MSG_ENFORCE_RANGE_NON_FINITE, 1, "Non-finite value is out of range for {0}.")
 MSG_DEF(MSG_ENFORCE_RANGE_OUT_OF_RANGE, 1, "Value is out of range for {0}.")
 MSG_DEF(MSG_NOT_SEQUENCE, 1, "{0} can't be converted to a sequence.")
 MSG_DEF(MSG_NOT_DICTIONARY, 1, "{0} can't be converted to a dictionary.")
--- a/dom/bindings/test/Makefile.in
+++ b/dom/bindings/test/Makefile.in
@@ -70,16 +70,17 @@ MOCHITEST_FILES := \
   test_bug759621.html \
   test_queryInterface.html \
   test_exceptionThrowing.html \
   test_bug852846.html \
   test_bug862092.html \
   test_bug560072.html \
   test_lenientThis.html \
   test_ByteString.html \
+  test_exception_messages.html \
   $(NULL)
 
 MOCHITEST_CHROME_FILES = \
   test_bug775543.html \
   $(NULL)
 
 ifdef GNU_CC
 CXXFLAGS += -Wno-uninitialized
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_exception_messages.html
@@ -0,0 +1,82 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=882653
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 882653</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 882653 **/
+  // Each test is a string to eval, the expected exception message, and the
+  // test description.
+  var tests = [
+      [ 'document.documentElement.appendChild.call({}, new Image())',
+        "'appendChild' called on an object that does not implement interface Node.",
+        "bogus method this object" ],
+      [ 'Object.getOwnPropertyDescriptor(Document.prototype, "documentElement").get.call({})',
+        "'documentElement' getter called on an object that does not implement interface Document.",
+        "bogus getter this object" ],
+      [ 'Object.getOwnPropertyDescriptor(Element.prototype, "innerHTML").set.call({})',
+        "'innerHTML' setter called on an object that does not implement interface Element.",
+        "bogus setter this object" ],
+      [ 'document.documentElement.appendChild(5)',
+        "Argument 1 of Node.appendChild is not an object.",
+        "bogus interface argument" ],
+      [ 'document.documentElement.appendChild(null)',
+        "Argument 1 of Node.appendChild is not an object.",
+        "null interface argument" ],
+      [ 'document.createTreeWalker(document).currentNode = 5',
+        "Value being assigned to TreeWalker.currentNode is not an object.",
+        "interface setter call" ],
+      [ 'document.documentElement.appendChild({})',
+        "Argument 1 of Node.appendChild does not implement interface Node.",
+        "wrong interface argument" ],
+      [ 'document.createTreeWalker(document).currentNode = {}',
+        "Value being assigned to TreeWalker.currentNode does not implement interface Node.",
+        "wrong interface setter call" ],
+      [ 'document.createElement("canvas").getContext("2d").fill("bogus")',
+        "Argument 1 of CanvasRenderingContext2D.fill 'bogus' is not a valid value for enumeration CanvasWindingRule.",
+        "bogus enum value" ],
+      [ 'document.createTreeWalker(document, 0xFFFFFFFF, { acceptNode: 5 }).nextNode()',
+        "Property 'acceptNode' is not callable.",
+        "non-callable callback interface operation property" ],
+      [ '(new TextEncoder).encode("", new RegExp())',
+        "Argument 2 of TextEncoder.encode can't be converted to a dictionary.",
+        "regexp passed for a dictionary" ],
+      [ 'URL.createObjectURL(null, null)',
+        "Argument 1 is not valid for any of the 2-argument overloads of URL.createObjectURL.",
+        "overload resolution failure" ],
+      [ 'document.createElement("select").add({})',
+        "Argument 1 of HTMLSelectElement.add could not be converted to any of: HTMLOptionElement, HTMLOptGroupElement.",
+        "invalid value passed for union" ],
+      [ 'document.createElement("canvas").getContext("2d").createLinearGradient(0, 1, 0, 1).addColorStop(NaN, "")',
+        "Argument 1 of CanvasGradient.addColorStop is not a finite floating-point value.",
+        "invalid float" ]
+  ];
+
+  for (var i = 0; i < tests.length; ++i) {
+      msg = "Correct exception should be thrown for " + tests[i][2];
+      try {
+          eval(tests[i][0]);
+          ok(false, msg);
+      } catch (e) {
+          is(e.message, tests[i][1], msg);
+      }
+  }
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=882653">Mozilla Bug 882653</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>