Bug 883358 part 2. Use the new information in bindings error reporting. r=smaug

2013-06-17 16:31:13 -04:00 · 2013-06-17 16:31:13 -04:00 · c23542725a
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@ -60,47 +60,26 @@ ThrowErrorMessage(JSContext* aCx, const ErrNum aErrorNumber, ...)
 }

 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 {
@ -850,8 +829,8 @@ XrayResolveAttribute(JSContext* cx, JS::Handle<JSObject*> wrapper,
          // 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);
@ -860,8 +839,8 @@ XrayResolveAttribute(JSContext* cx, JS::Handle<JSObject*> wrapper,
          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);
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@ -60,12 +60,9 @@ UnwrapArg(JSContext* cx, jsval v, Interface** ppArg,
 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
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@ -5185,7 +5185,7 @@ class CGGenericMethod(CGAbstractBindingMethod):
        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,
@ -5322,7 +5322,7 @@ class CGGenericGetter(CGAbstractBindingMethod):
        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)
@ -5402,7 +5402,7 @@ class CGGenericSetter(CGAbstractBindingMethod):
        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,
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@ -24,9 +24,9 @@ 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.")
--- a/dom/bindings/test/Makefile.in
+++ b/dom/bindings/test/Makefile.in
@ -75,6 +75,7 @@ MOCHITEST_FILES := \
  test_bug560072.html \
  test_lenientThis.html \
  test_ByteString.html \
+  test_exception_messages.html \
  $(NULL)

 MOCHITEST_CHROME_FILES = \
--- a/dom/bindings/test/test_exception_messages.html
+++ 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>