Bug 882653 part 1. Improve error reporting for bogus this objects in WebIDL bindings. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 17 Jun 2013 13:07:03 -0400
changeset 146802 814684b8e50af95f86b51975ccdbc89169734c60
parent 146801 9432471a1c1221cbea618161428683584c2d5818
child 146803 4ce6a3c97d61afd3074673f0ed4d335bdd476a7b
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
bugs882653
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 882653 part 1. Improve error reporting for bogus this objects in WebIDL bindings. r=smaug
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Errors.msg
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -48,22 +48,64 @@ GetErrorMessage(void* aUserRef, const ch
   return &ErrorFormatString[aErrorNumber];
 }
 
 bool
 ThrowErrorMessage(JSContext* aCx, const ErrNum aErrorNumber, ...)
 {
   va_list ap;
   va_start(ap, aErrorNumber);
-  JS_ReportErrorNumberVA(aCx, GetErrorMessage, NULL,
+  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)
+{
+  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.
+  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),
+                         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
@@ -54,16 +54,23 @@ UnwrapArg(JSContext* cx, jsval v, Interf
                                     reinterpret_cast<void**>(ppArg), &argRef,
                                     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);
 
 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
@@ -5065,17 +5065,17 @@ class CGAbstractBindingMethod(CGAbstract
     CGThing which is already properly indented.
     """
     def __init__(self, descriptor, name, args, unwrapFailureCode=None,
                  getThisObj="args.computeThis(cx).toObjectOrNull()",
                  callArgs="JS::CallArgs args = JS::CallArgsFromVp(argc, vp);"):
         CGAbstractStaticMethod.__init__(self, descriptor, name, "JSBool", args)
 
         if unwrapFailureCode is None:
-            self.unwrapFailureCode = 'return ThrowErrorMessage(cx, MSG_DOES_NOT_IMPLEMENT_INTERFACE, "%s");' % self.descriptor.name
+            self.unwrapFailureCode = 'return ThrowErrorMessage(cx, MSG_THIS_DOES_NOT_IMPLEMENT_INTERFACE, "%s");' % descriptor.interface.identifier.name
         else:
             self.unwrapFailureCode = unwrapFailureCode
         self.getThisObj = getThisObj
         self.callArgs = callArgs
 
     def definition_body(self):
         # Our descriptor might claim that we're not castable, simply because
         # we're someone's consequential interface.  But for this-unwrapping, we
@@ -5128,17 +5128,22 @@ 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')]
-        CGAbstractBindingMethod.__init__(self, descriptor, 'genericMethod', args)
+        unwrapFailureCode = (
+            'return ThrowInvalidMethodThis(cx, args, \"%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"
             "MOZ_ASSERT(info->type == JSJitInfo::Method);\n"
             "JSJitMethodOp method = info->method;\n"
             "return method(cx, obj, self, JSJitMethodCallArgs(args));"))
 
@@ -5260,17 +5265,19 @@ class CGGenericGetter(CGAbstractBindingM
                 "MOZ_ASSERT(!JS_IsExceptionPending(cx));\n"
                 "if (!ReportLenientThisUnwrappingFailure(cx, obj)) {\n"
                 "  return false;\n"
                 "}\n"
                 "args.rval().set(JS::UndefinedValue());\n"
                 "return true;")
         else:
             name = "genericGetter"
-            unwrapFailureCode = None
+            unwrapFailureCode = (
+                'return ThrowInvalidGetterThis(cx, "%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"
             "JSJitGetterOp getter = info->getter;\n"
@@ -5338,17 +5345,20 @@ class CGGenericSetter(CGAbstractBindingM
                 "MOZ_ASSERT(!JS_IsExceptionPending(cx));\n"
                 "if (!ReportLenientThisUnwrappingFailure(cx, obj)) {\n"
                 "  return false;\n"
                 "}\n"
                 "args.rval().set(JS::UndefinedValue());\n"
                 "return true;")
         else:
             name = "genericSetter"
-            unwrapFailureCode = None
+            unwrapFailureCode = (
+                'return ThrowInvalidSetterThis(cx, "%s");' %
+                descriptor.interface.identifier.name)
+
         CGAbstractBindingMethod.__init__(self, descriptor, name, args,
                                          unwrapFailureCode)
 
     def generate_code(self):
         return CGIndenter(CGGeneric(
                 "if (args.length() == 0) {\n"
                 '  return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "%s attribute setter");\n'
                 "}\n"
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -19,16 +19,20 @@
  * be replaced with a string value when the error is reported.
  */
 
 MSG_DEF(MSG_INVALID_ENUM_VALUE, 2, "Value '{0}' is not a valid value for enumeration {1}.")
 MSG_DEF(MSG_MISSING_ARGUMENTS, 1, "Not enough arguments to {0}.")
 MSG_DEF(MSG_NOT_OBJECT, 0, "Value is not an object.")
 MSG_DEF(MSG_NOT_CALLABLE, 0, "Value is not callable.")
 MSG_DEF(MSG_DOES_NOT_IMPLEMENT_INTERFACE, 1, "Value 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, 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_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 1, "\"this\" object does not implement interface {0}.")
 MSG_DEF(MSG_NOT_IN_UNION, 1, "Value could not be converted to any of: {0}.")
 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, 0, "Value can not be converted to a sequence.")
 MSG_DEF(MSG_NOT_DICTIONARY, 0, "Value can not be converted to a dictionary.")
 MSG_DEF(MSG_INVALID_ARG, 2, "Argument {0} is not valid for any of the {1}-argument overloads.")