Bug 883358 part 2. Use the new information in bindings error reporting. r=smaug
☠☠ backed out by 04b177645a6b ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 17 Jun 2013 16:31:13 -0400
changeset 146852 b5e6522257cb72dc5822a8d18bcd98fdf3aec026
parent 146851 757a3f2e5de671b9f638692139826f3cba3cf0e6
child 146853 ec253175bb51751b8223fbf651497f525eaf5b6a
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
--- 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" ],
+      [ 'document.createElement("canvas").getContext("webgl").uniform1fv(null, new RegExp())',
+	"Argument 2 is not valid for any of the 2-argument overloads of WebGLRenderingContext.uniform1fv.",
+	"regexp passed for a sequence" ],
+      [ '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>