Bug 1511401 part 4. Use CallArgs::requireAtLeast in the DOM. r=nbp,qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 10 Dec 2018 14:13:06 -0500
changeset 506832 39b04fe4eae8f8a2995808e73757e1e2be033f03
parent 506831 696e7b4f7535209266af79ad7250f27f3d59cd40
child 506833 13f891b92db19ea8ec85ef329eff7a793c8b368f
child 506883 6915aee134cb2b579b67e7718579c7a59b15e1d8
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, qdot
bugs1511401
milestone65.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 1511401 part 4. Use CallArgs::requireAtLeast in the DOM. r=nbp,qdot
dom/bindings/BindingUtils.cpp
dom/bindings/Codegen.py
dom/bindings/Errors.msg
js/public/CallArgs.h
js/src/jsfriendapi.h
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -157,20 +157,21 @@ bool ThrowInvalidThis(JSContext* aCx, co
 }
 
 bool ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
                       bool aSecurityError, prototypes::ID aProtoId) {
   return ThrowInvalidThis(aCx, aArgs, aSecurityError,
                           NamesOfInterfacesWithProtos(aProtoId));
 }
 
-bool ThrowNoSetterArg(JSContext* aCx, prototypes::ID aProtoId) {
+bool ThrowNoSetterArg(JSContext* aCx, const JS::CallArgs& aArgs,
+                      prototypes::ID aProtoId) {
   nsPrintfCString errorMessage("%s attribute setter",
                                NamesOfInterfacesWithProtos(aProtoId));
-  return ThrowErrorMessage(aCx, MSG_MISSING_ARGUMENTS, errorMessage.get());
+  return aArgs.requireAtLeast(aCx, errorMessage.get(), 1);
 }
 
 }  // namespace dom
 
 namespace binding_danger {
 
 template <typename CleanupPolicy>
 struct TErrorResult<CleanupPolicy>::Message {
@@ -3003,17 +3004,17 @@ bool GenericSetter(JSContext* cx, unsign
     nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(
         wrapper, self, protoID, info->depth);
     if (NS_FAILED(rv)) {
       return ThisPolicy::HandleInvalidThis(
           cx, args, rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO, protoID);
     }
   }
   if (args.length() == 0) {
-    return ThrowNoSetterArg(cx, protoID);
+    return ThrowNoSetterArg(cx, args, protoID);
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Setter);
   JSJitSetterOp setter = info->setter;
   if (!setter(cx, obj, self, JSJitSetterCallArgs(args))) {
     return false;
   }
   args.rval().setUndefined();
 #ifdef DEBUG
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -8037,18 +8037,18 @@ class CGMethodCall(CGThing):
             requiredArgs = requiredArgCount(signature)
 
             # Skip required arguments check for maplike/setlike interfaces, as
             # they can have arguments which are not passed, and are treated as
             # if undefined had been explicitly passed.
             if requiredArgs > 0 and not method.isMaplikeOrSetlikeOrIterableMethod():
                 code = fill(
                     """
-                    if (MOZ_UNLIKELY(args.length() < ${requiredArgs})) {
-                      return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "${methodName}");
+                    if (!args.requireAtLeast(cx, "${methodName}", ${requiredArgs})) {
+                      return false;
                     }
                     """,
                     requiredArgs=requiredArgs,
                     methodName=methodName)
                 self.cgRoot.prepend(CGGeneric(code))
             return
 
         # Need to find the right overload
@@ -8380,18 +8380,25 @@ class CGMethodCall(CGThing):
 
         overloadCGThings = []
         overloadCGThings.append(
             CGGeneric("unsigned argcount = std::min(args.length(), %du);\n" %
                       maxArgCount))
         overloadCGThings.append(
             CGSwitch("argcount",
                      argCountCases,
-                     CGGeneric('return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "%s");\n' %
-                               methodName)))
+                     CGGeneric(fill(
+                         """
+                         // Using nsPrintfCString here would require including that
+                         // header.  Let's not worry about it.
+                         nsAutoCString argCountStr;
+                         argCountStr.AppendPrintf("%u", args.length());
+                         return ThrowErrorMessage(cx, MSG_INVALID_OVERLOAD_ARGCOUNT, "${methodName}", argCountStr.get());
+                         """,
+                         methodName=methodName))))
         overloadCGThings.append(
             CGGeneric('MOZ_CRASH("We have an always-returning default case");\n'
                       'return false;\n'))
         self.cgRoot = CGList(overloadCGThings)
 
     def define(self):
         return self.cgRoot.define()
 
@@ -9132,18 +9139,18 @@ class CGStaticSetter(CGAbstractStaticBin
         name = 'set_' + IDLToCIdentifier(attr.identifier.name)
         CGAbstractStaticBindingMethod.__init__(self, descriptor, name)
 
     def generate_code(self):
         nativeName = CGSpecializedSetter.makeNativeName(self.descriptor,
                                                         self.attr)
         checkForArg = CGGeneric(fill(
             """
-            if (args.length() == 0) {
-              return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "${name} setter");
+            if (!args.requireAtLeast(cx, "${name} setter", 1)) {
+              return false;
             }
             """,
             name=self.attr.identifier.name))
         call = CGSetterCall(self.attr.type, nativeName, self.descriptor,
                             self.attr)
         return CGList([checkForArg, call])
 
     def auto_profiler_label(self):
@@ -15354,18 +15361,18 @@ class CGJSImplClass(CGBindingImplClass):
         # XXXbz we could try to get parts of this (e.g. the argument
         # conversions) auto-generated by somehow creating an IDLMethod and
         # adding it to our interface, but we'd still need to special-case the
         # implementation slightly to have it not try to forward to the JS
         # object...
         return fill(
             """
             JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-            if (args.length() < 2) {
-              return ThrowErrorMessage(cx, MSG_MISSING_ARGUMENTS, "${ifaceName}._create");
+            if (!args.requireAtLeast(cx, "${ifaceName}._create", 2)) {
+              return false;
             }
             if (!args[0].isObject()) {
               return ThrowErrorMessage(cx, MSG_NOT_OBJECT, "Argument 1 of ${ifaceName}._create");
             }
             if (!args[1].isObject()) {
               return ThrowErrorMessage(cx, MSG_NOT_OBJECT, "Argument 2 of ${ifaceName}._create");
             }
 
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -18,17 +18,17 @@
  * engine should throw.
  *
  * <FORMAT_STRING> is a string literal, containing <ARGUMENT_COUNT> sequences
  * {X} where X  is an integer representing the argument number that will
  * be replaced with a string value when the error is reported.
  */
 
 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_INVALID_OVERLOAD_ARGCOUNT, 2, JSEXN_TYPEERR, "{1} is not a valid argument count for any overload of {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_NOT_CONSTRUCTOR, 1, JSEXN_TYPEERR, "{0} is not a constructor.")
 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_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}.")
--- a/js/public/CallArgs.h
+++ b/js/public/CallArgs.h
@@ -241,16 +241,23 @@ class MOZ_STACK_CLASS CallArgsBase {
    * so eventually.)  You don't need to use or change this if your method
    * fails.
    */
   MutableHandleValue rval() const {
     this->setUsedRval();
     return MutableHandleValue::fromMarkedLocation(&argv_[-2]);
   }
 
+  /*
+   * Returns true if there are at least |required| arguments passed in. If
+   * false, it reports an error message on the context.
+   */
+  JS_PUBLIC_API inline bool requireAtLeast(JSContext* cx, const char* fnname,
+                                           unsigned required) const;
+
  public:
   // These methods are publicly exposed, but they are *not* to be used when
   // implementing a JSNative method and encapsulating access to |vp| within
   // it.  You probably don't want to use these!
 
   void setCallee(const Value& aCalleev) const {
     this->clearUsedRval();
     argv_[-2] = aCalleev;
@@ -304,39 +311,40 @@ class MOZ_STACK_CLASS CallArgs
     MOZ_ASSERT(ValueIsNotGray(args.calleev()));
     for (unsigned i = 0; i < argc; ++i) {
       MOZ_ASSERT(ValueIsNotGray(argv[i]));
     }
 #endif
     return args;
   }
 
+ public:
   /*
-   * Helper for requireAtLeast to report the actual exception.
+   * Helper for requireAtLeast to report the actual exception.  Public
+   * so we can call it from CallArgsBase and not need multiple
+   * per-template instantiations of it.
    */
   static JS_PUBLIC_API void reportMoreArgsNeeded(JSContext* cx,
                                                  const char* fnname,
                                                  unsigned required,
                                                  unsigned actual);
+};
 
- public:
-  /*
-   * Returns true if there are at least |required| arguments passed in. If
-   * false, it reports an error message on the context.
-   */
-  JS_PUBLIC_API inline bool requireAtLeast(JSContext* cx, const char* fnname,
-                                           unsigned required) const {
-    if (MOZ_LIKELY(required <= length())) {
-      return true;
-    }
+namespace detail {
+template <class WantUsedRval>
+JS_PUBLIC_API inline bool CallArgsBase<WantUsedRval>::requireAtLeast(
+    JSContext* cx, const char* fnname, unsigned required) const {
+  if (MOZ_LIKELY(required <= length())) {
+    return true;
+  }
 
-    reportMoreArgsNeeded(cx, fnname, required, length());
-    return false;
-  }
-};
+  CallArgs::reportMoreArgsNeeded(cx, fnname, required, length());
+  return false;
+}
+}  // namespace detail
 
 MOZ_ALWAYS_INLINE CallArgs CallArgsFromVp(unsigned argc, Value* vp) {
   return CallArgs::create(argc, vp + 2, vp[1].isMagic(JS_IS_CONSTRUCTING));
 }
 
 // This method is only intended for internal use in SpiderMonkey.  We may
 // eventually move it to an internal header.  Embedders should use
 // JS::CallArgsFromVp!
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -2051,16 +2051,23 @@ class JSJitMethodCallArgs
 
   JSObject& callee() const {
     // We can't use Base::callee() because that will try to poke at
     // this->usedRval_, which we don't have.
     return argv_[-2].toObject();
   }
 
   JS::HandleValue get(unsigned i) const { return Base::get(i); }
+
+  bool requireAtLeast(JSContext* cx, const char* fnname,
+                      unsigned required) const {
+    // Can just forward to Base, since it only needs the length and we
+    // forward that already.
+    return Base::requireAtLeast(cx, fnname, required);
+  }
 };
 
 struct JSJitMethodCallArgsTraits {
   static const size_t offsetOfArgv = offsetof(JSJitMethodCallArgs, argv_);
   static const size_t offsetOfArgc = offsetof(JSJitMethodCallArgs, argc_);
 };
 
 typedef bool (*JSJitGetterOp)(JSContext* cx, JS::HandleObject thisObj,