Bug 882653 part 1. Improve error reporting for bogus this objects in WebIDL bindings. r=smaug

dom/bindings/BindingUtils.cpp

@@ -53,12 +53,54 @@ 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 {

dom/bindings/BindingUtils.h

@@ -59,6 +59,13 @@ 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);
 
 template<bool mainThread>
 inline bool

dom/bindings/Codegen.py

@@ -5070,7 +5070,7 @@ class CGAbstractBindingMethod(CGAbstractStaticMethod):
         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
@@ -5133,7 +5133,12 @@ class CGGenericMethod(CGAbstractBindingMethod):
     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(
@@ -5265,7 +5270,9 @@ class CGGenericGetter(CGAbstractBindingMethod):
                 "return true;")
         else:
             name = "genericGetter"
-            unwrapFailureCode = None
+            unwrapFailureCode = (
+                'return ThrowInvalidGetterThis(cx, "%s");' %
+                descriptor.interface.identifier.name)
         CGAbstractBindingMethod.__init__(self, descriptor, name, args,
                                          unwrapFailureCode)
 
@@ -5343,7 +5350,10 @@ class CGGenericSetter(CGAbstractBindingMethod):
                 "return true;")
         else:
             name = "genericSetter"
-            unwrapFailureCode = None
+            unwrapFailureCode = (
+                'return ThrowInvalidSetterThis(cx, "%s");' %
+                descriptor.interface.identifier.name)
+
         CGAbstractBindingMethod.__init__(self, descriptor, name, args,
                                          unwrapFailureCode)
 

dom/bindings/Errors.msg

@@ -24,6 +24,10 @@ 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.")