Bug 1180290 - Part 5: Remove getter/setter variant for ThrowInvalidThis message. r=bz
authorTooru Fujisawa <arai_a@mac.com>
Wed, 06 Jan 2016 17:54:00 +0900
changeset 278813 7bc312c15f1f
parent 278812 a59c19e0b14d
child 278814 384bf27d0f58
push id29860
push usercbook@mozilla.com
push dateThu, 07 Jan 2016 10:51:20 +0000
treeherdermozilla-central@e0bcd16e1d4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1180290
milestone46.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 1180290 - Part 5: Remove getter/setter variant for ThrowInvalidThis message. r=bz
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/Errors.msg
dom/bindings/test/test_exception_messages.html
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -88,45 +88,47 @@ ThrowErrorMessage(JSContext* aCx, const 
   JS_ReportErrorNumberVA(aCx, GetErrorMessage, nullptr,
                          static_cast<const unsigned>(aErrorNumber), ap);
   va_end(ap);
   return false;
 }
 
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                 const ErrNum aErrorNumber,
-                 const char* aInterfaceName)
+                 bool aSecurityError, const char* aInterfaceName)
 {
   NS_ConvertASCIItoUTF16 ifaceName(aInterfaceName);
   // 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);
   nsAutoJSString funcNameStr;
   if (!funcNameStr.init(aCx, funcName)) {
     return false;
   }
-  MOZ_RELEASE_ASSERT(GetErrorArgCount(aErrorNumber) <= 2);
+  const ErrNum errorNumber = aSecurityError ?
+                             MSG_METHOD_THIS_UNWRAPPING_DENIED :
+                             MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE;
+  MOZ_RELEASE_ASSERT(GetErrorArgCount(errorNumber) <= 2);
   JS_ReportErrorNumberUC(aCx, GetErrorMessage, nullptr,
-                         static_cast<const unsigned>(aErrorNumber),
+                         static_cast<const unsigned>(errorNumber),
                          funcNameStr.get(), ifaceName.get());
   return false;
 }
 
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                 const ErrNum aErrorNumber,
+                 bool aSecurityError,
                  prototypes::ID aProtoId)
 {
-  return ThrowInvalidThis(aCx, aArgs, aErrorNumber,
+  return ThrowInvalidThis(aCx, aArgs, aSecurityError,
                           NamesOfInterfacesWithProtos(aProtoId));
 }
 
 bool
 ThrowNoSetterArg(JSContext* aCx, prototypes::ID aProtoId)
 {
   nsPrintfCString errorMessage("%s attribute setter",
                                NamesOfInterfacesWithProtos(aProtoId));
@@ -2622,28 +2624,26 @@ EnforceNotInPrerendering(JSContext* aCx,
 
 bool
 GenericBindingGetter(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
-    return ThrowInvalidThis(cx, args,
-                            MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE,
-                            protoID);
+    return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
   void* self;
   {
     nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
-                              GetInvalidThisErrorForGetter(rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO),
+                              rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
 
   MOZ_ASSERT(info->type() == JSJitInfo::Getter);
   JSJitGetterOp getter = info->getter;
   bool ok = getter(cx, obj, self, JSJitGetterCallArgs(args));
 #ifdef DEBUG
@@ -2656,28 +2656,26 @@ GenericBindingGetter(JSContext* cx, unsi
 
 bool
 GenericBindingSetter(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
-    return ThrowInvalidThis(cx, args,
-                            MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE,
-                            protoID);
+    return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
   void* self;
   {
     nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
-                              GetInvalidThisErrorForSetter(rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO),
+                              rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
   if (args.length() == 0) {
     return ThrowNoSetterArg(cx, protoID);
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Setter);
   JSJitSetterOp setter = info->setter;
@@ -2693,28 +2691,26 @@ GenericBindingSetter(JSContext* cx, unsi
 
 bool
 GenericBindingMethod(JSContext* cx, unsigned argc, JS::Value* vp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
-    return ThrowInvalidThis(cx, args,
-                            MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE,
-                            protoID);
+    return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
   void* self;
   {
     nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
-                              GetInvalidThisErrorForMethod(rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO),
+                              rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Method);
   JSJitMethodOp method = info->method;
   bool ok = method(cx, obj, self, JSJitMethodCallArgs(args));
 #ifdef DEBUG
   if (ok) {
@@ -2731,30 +2727,27 @@ GenericPromiseReturningBindingMethod(JSC
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   JS::Rooted<JSObject*> callee(cx, &args.callee());
 
   // We could invoke GenericBindingMethod here, but that involves an
   // extra call.  Manually inline it instead.
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
-    ThrowInvalidThis(cx, args,
-                     MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE,
-                     protoID);
+    ThrowInvalidThis(cx, args, false, protoID);
     return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                      args.rval());
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
   void* self;
   {
     nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
     if (NS_FAILED(rv)) {
-      ThrowInvalidThis(cx, args,
-                       GetInvalidThisErrorForMethod(rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO),
+      ThrowInvalidThis(cx, args, rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                        protoID);
       return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                        args.rval());
     }
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Method);
   JSJitMethodOp method = info->method;
   bool ok = method(cx, obj, self, JSJitMethodCallArgs(args));
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -56,46 +56,23 @@ UnwrapArgImpl(JS::Handle<JSObject*> src,
 template <class Interface>
 inline nsresult
 UnwrapArg(JS::Handle<JSObject*> src, Interface** ppArg)
 {
   return UnwrapArgImpl(src, NS_GET_TEMPLATE_IID(Interface),
                        reinterpret_cast<void**>(ppArg));
 }
 
-inline const ErrNum
-GetInvalidThisErrorForMethod(bool aSecurityError)
-{
-  return aSecurityError ? MSG_METHOD_THIS_UNWRAPPING_DENIED :
-                          MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE;
-}
-
-inline const ErrNum
-GetInvalidThisErrorForGetter(bool aSecurityError)
-{
-  return aSecurityError ? MSG_GETTER_THIS_UNWRAPPING_DENIED :
-                          MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE;
-}
-
-inline const ErrNum
-GetInvalidThisErrorForSetter(bool aSecurityError)
-{
-  return aSecurityError ? MSG_SETTER_THIS_UNWRAPPING_DENIED :
-                          MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE;
-}
+bool
+ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
+                 bool aSecurityError, const char* aInterfaceName);
 
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                 const ErrNum aErrorNumber,
-                 const char* aInterfaceName);
-
-bool
-ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
-                 const ErrNum aErrorNumber,
-                 prototypes::ID aProtoId);
+                 bool aSecurityError, prototypes::ID aProtoId);
 
 // Returns true if the JSClass is used for DOM objects.
 inline bool
 IsDOMClass(const JSClass* clasp)
 {
   return clasp->flags & JSCLASS_IS_DOMJSCLASS;
 }
 
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -8100,17 +8100,17 @@ class CGGenericMethod(CGAbstractBindingM
     """
     A class for generating the C++ code for an IDL method.
 
     If allowCrossOriginThis is true, then this-unwrapping will first do an
     UncheckedUnwrap and after that operate on the result.
     """
     def __init__(self, descriptor, allowCrossOriginThis=False):
         unwrapFailureCode = (
-            'return ThrowInvalidThis(cx, args, GetInvalidThisErrorForMethod(%%(securityError)s), "%s");\n' %
+            'return ThrowInvalidThis(cx, args, %%(securityError)s, "%s");\n' %
             descriptor.interface.identifier.name)
         name = "genericCrossOriginMethod" if allowCrossOriginThis else "genericMethod"
         CGAbstractBindingMethod.__init__(self, descriptor, name,
                                          JSNativeArguments(),
                                          unwrapFailureCode=unwrapFailureCode,
                                          allowCrossOriginThis=allowCrossOriginThis)
 
     def generate_code(self):
@@ -8131,17 +8131,17 @@ class CGGenericMethod(CGAbstractBindingM
 class CGGenericPromiseReturningMethod(CGAbstractBindingMethod):
     """
     A class for generating the C++ code for an IDL method that returns a Promise.
 
     Does not handle cross-origin this.
     """
     def __init__(self, descriptor):
         unwrapFailureCode = dedent("""
-            ThrowInvalidThis(cx, args, GetInvalidThisErrorForMethod(%%(securityError)s), "%s");\n
+            ThrowInvalidThis(cx, args, %%(securityError)s, "%s");\n
             return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                              args.rval());\n""" %
                                    descriptor.interface.identifier.name)
 
         name = "genericPromiseReturningMethod"
         customCallArgs = dedent("""
             JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
             // Make sure to save the callee before someone maybe messes with rval().
@@ -8472,17 +8472,17 @@ class CGGenericGetter(CGAbstractBindingM
                 return true;
                 """)
         else:
             if allowCrossOriginThis:
                 name = "genericCrossOriginGetter"
             else:
                 name = "genericGetter"
             unwrapFailureCode = (
-                'return ThrowInvalidThis(cx, args, GetInvalidThisErrorForGetter(%%(securityError)s), "%s");\n' %
+                'return ThrowInvalidThis(cx, args, %%(securityError)s, "%s");\n' %
                 descriptor.interface.identifier.name)
         CGAbstractBindingMethod.__init__(self, descriptor, name, JSNativeArguments(),
                                          unwrapFailureCode,
                                          allowCrossOriginThis=allowCrossOriginThis)
 
     def generate_code(self):
         return CGGeneric(dedent("""
             const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
@@ -8603,17 +8603,17 @@ class CGGenericSetter(CGAbstractBindingM
                 return true;
                 """)
         else:
             if allowCrossOriginThis:
                 name = "genericCrossOriginSetter"
             else:
                 name = "genericSetter"
             unwrapFailureCode = (
-                'return ThrowInvalidThis(cx, args, GetInvalidThisErrorForSetter(%%(securityError)s), "%s");\n' %
+                'return ThrowInvalidThis(cx, args, %%(securityError)s, "%s");\n' %
                 descriptor.interface.identifier.name)
 
         CGAbstractBindingMethod.__init__(self, descriptor, name, JSNativeArguments(),
                                          unwrapFailureCode,
                                          allowCrossOriginThis=allowCrossOriginThis)
 
     def generate_code(self):
         return CGGeneric(fill(
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -24,20 +24,16 @@
 
 MSG_DEF(MSG_INVALID_ENUM_VALUE, 3, JSEXN_TYPEERR, "{0} '{1}' is not a valid value for enumeration {2}.")
 MSG_DEF(MSG_MISSING_ARGUMENTS, 1, JSEXN_TYPEERR, "Not enough arguments to {0}.")
 MSG_DEF(MSG_NOT_OBJECT, 1, JSEXN_TYPEERR, "{0} is not an object.")
 MSG_DEF(MSG_NOT_CALLABLE, 1, JSEXN_TYPEERR, "{0} is not callable.")
 MSG_DEF(MSG_DOES_NOT_IMPLEMENT_INTERFACE, 2, JSEXN_TYPEERR, "{0} does not implement interface {1}.")
 MSG_DEF(MSG_METHOD_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, JSEXN_TYPEERR, "'{0}' called on an object that does not implement interface {1}.")
 MSG_DEF(MSG_METHOD_THIS_UNWRAPPING_DENIED, 1, JSEXN_TYPEERR, "Permission to call '{0}' denied.")
-MSG_DEF(MSG_GETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, JSEXN_TYPEERR, "'{0}' getter called on an object that does not implement interface {1}.")
-MSG_DEF(MSG_GETTER_THIS_UNWRAPPING_DENIED, 1, JSEXN_TYPEERR, "Permission to call '{0}' getter denied.")
-MSG_DEF(MSG_SETTER_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 2, JSEXN_TYPEERR, "'{0}' setter called on an object that does not implement interface {1}.")
-MSG_DEF(MSG_SETTER_THIS_UNWRAPPING_DENIED, 1, JSEXN_TYPEERR, "Permission to call '{0}' setter denied.")
 MSG_DEF(MSG_THIS_DOES_NOT_IMPLEMENT_INTERFACE, 1, JSEXN_TYPEERR, "\"this\" object does not implement interface {0}.")
 MSG_DEF(MSG_NOT_IN_UNION, 2, JSEXN_TYPEERR, "{0} could not be converted to any of: {1}.")
 MSG_DEF(MSG_ILLEGAL_CONSTRUCTOR, 0, JSEXN_TYPEERR, "Illegal constructor.")
 MSG_DEF(MSG_CONSTRUCTOR_WITHOUT_NEW, 1, JSEXN_TYPEERR, "Constructor {0} requires 'new'")
 MSG_DEF(MSG_ENFORCE_RANGE_NON_FINITE, 1, JSEXN_TYPEERR, "Non-finite value is out of range for {0}.")
 MSG_DEF(MSG_ENFORCE_RANGE_OUT_OF_RANGE, 1, JSEXN_TYPEERR, "Value is out of range for {0}.")
 MSG_DEF(MSG_NOT_SEQUENCE, 1, JSEXN_TYPEERR, "{0} can't be converted to a sequence.")
 MSG_DEF(MSG_NOT_DICTIONARY, 1, JSEXN_TYPEERR, "{0} can't be converted to a dictionary.")
--- a/dom/bindings/test/test_exception_messages.html
+++ b/dom/bindings/test/test_exception_messages.html
@@ -13,20 +13,20 @@ https://bugzilla.mozilla.org/show_bug.cg
   /** 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({})',
-        "'get documentElement' getter called on an object that does not implement interface Document.",
+        "'get documentElement' called on an object that does not implement interface Document.",
         "bogus getter this object" ],
       [ 'Object.getOwnPropertyDescriptor(Element.prototype, "innerHTML").set.call({})',
-        "'set innerHTML' setter called on an object that does not implement interface Element.",
+        "'set innerHTML' 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',