Bug 779048 part 8. Handling of arguments for callbacks. r=peterv

Note that we're now using the array object as the "obj" when wrapping sequence members, because we don't have an 'obj' around in our code... and every single existing sequence-of-interfaces consumer relied on there being an 'obj' floating about. Also note that I needed to rearrange the various wrapping helpers so that we can wrap things that are hanging out in const smartpointers or in const OwningNonNull or in plain object references.
2024-09-13 09:24:08 -07:00 · 2012-11-09 07:43:58 -08:00 · 2012-11-09 07:43:58 -08:00 · 499ff69d95
commit 499ff69d95
parent a087a0f7d6
5 changed files with 189 additions and 42 deletions
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@ -517,15 +517,6 @@ WrapNewBindingObject(JSContext* cx, JSObject* scope, T* value, JS::Value* vp)
  return JS_WrapValue(cx, vp);
 }

-// Helper for smart pointers (nsAutoPtr/nsRefPtr/nsCOMPtr).
-template <template <typename> class SmartPtr, class T>
-inline bool
-WrapNewBindingObject(JSContext* cx, JSObject* scope, const SmartPtr<T>& value,
-                     JS::Value* vp)
-{
-  return WrapNewBindingObject(cx, scope, value.get(), vp);
-}
-
 template <class T>
 inline bool
 WrapNewBindingNonWrapperCachedObject(JSContext* cx, JSObject* scope, T* value,
@ -781,7 +772,7 @@ WrapObject(JSContext* cx, JSObject* scope, T* p, JS::Value* vp)

 template<class T>
 inline bool
-WrapObject(JSContext* cx, JSObject* scope, nsCOMPtr<T> &p, const nsIID* iid,
+WrapObject(JSContext* cx, JSObject* scope, const nsCOMPtr<T> &p, const nsIID* iid,
           JS::Value* vp)
 {
  return WrapObject(cx, scope, p.get(), iid, vp);
@ -789,14 +780,14 @@ WrapObject(JSContext* cx, JSObject* scope, nsCOMPtr<T> &p, const nsIID* iid,

 template<class T>
 inline bool
-WrapObject(JSContext* cx, JSObject* scope, nsCOMPtr<T> &p, JS::Value* vp)
+WrapObject(JSContext* cx, JSObject* scope, const nsCOMPtr<T> &p, JS::Value* vp)
 {
  return WrapObject(cx, scope, p, NULL, vp);
 }

 template<class T>
 inline bool
-WrapObject(JSContext* cx, JSObject* scope, nsRefPtr<T> &p, const nsIID* iid,
+WrapObject(JSContext* cx, JSObject* scope, const nsRefPtr<T> &p, const nsIID* iid,
           JS::Value* vp)
 {
  return WrapObject(cx, scope, p.get(), iid, vp);
@ -804,7 +795,7 @@ WrapObject(JSContext* cx, JSObject* scope, nsRefPtr<T> &p, const nsIID* iid,

 template<class T>
 inline bool
-WrapObject(JSContext* cx, JSObject* scope, nsRefPtr<T> &p, JS::Value* vp)
+WrapObject(JSContext* cx, JSObject* scope, const nsRefPtr<T> &p, JS::Value* vp)
 {
  return WrapObject(cx, scope, p, NULL, vp);
 }
@ -821,6 +812,22 @@ bool
 WrapCallbackInterface(JSContext *cx, JSObject *scope, nsISupports* callback,
                      JS::Value* vp);

+static inline bool
+WrapCallbackInterface(JSContext *cx, JSObject *scope, nsISupports& callback,
+                      JS::Value* vp)
+{
+  return WrapCallbackInterface(cx, scope, &callback, vp);
+}
+
+// Helper for smart pointers (nsAutoPtr/nsRefPtr/nsCOMPtr).
+template <template <typename> class SmartPtr, class T>
+inline bool
+WrapCallbackInterface(JSContext* cx, JSObject* scope, const SmartPtr<T>& value,
+                      JS::Value* vp)
+{
+  return WrapCallbackInterface(cx, scope, value.get(), vp);
+}
+
 template<typename T>
 static inline JSObject*
 WrapNativeISupportsParent(JSContext* cx, JSObject* scope, T* p,
@ -933,6 +940,38 @@ WrapCallThisObject(JSContext* cx, JSObject* scope, const T& p)
  return obj;
 }

+// Helper for calling WrapNewBindingObject with smart pointers
+// (nsAutoPtr/nsRefPtr/nsCOMPtr) or references.
+HAS_MEMBER(get)
+
+template <class T, bool isSmartPtr=HasgetMember<T>::Value>
+struct WrapNewBindingObjectHelper
+{
+  static inline bool Wrap(JSContext* cx, JSObject* scope, const T& value,
+                          JS::Value* vp)
+  {
+    return WrapNewBindingObject(cx, scope, value.get(), vp);
+  }
+};
+
+template <class T>
+struct WrapNewBindingObjectHelper<T, false>
+{
+  static inline bool Wrap(JSContext* cx, JSObject* scope, T& value,
+                          JS::Value* vp)
+  {
+    return WrapNewBindingObject(cx, scope, &value, vp);
+  }
+};
+
+template<class T>
+inline bool
+WrapNewBindingObject(JSContext* cx, JSObject* scope, T& value,
+                     JS::Value* vp)
+{
+  return WrapNewBindingObjectHelper<T>::Wrap(cx, scope, value, vp);
+}
+
 static inline bool
 InternJSString(JSContext* cx, jsid& id, const char* chars)
 {
@ -1034,6 +1073,13 @@ public:
    return ptr;
  }

+  // Make us work with smart-ptr helpers that expect a get()
+  T* get() const {
+    MOZ_ASSERT(inited);
+    MOZ_ASSERT(ptr);
+    return ptr;
+  }
+
 protected:
  T* ptr;
 #ifdef DEBUG
@ -1072,6 +1118,13 @@ public:
    return ptr.forget();
  }

+  // Make us work with smart-ptr helpers that expect a get()
+  T* get() const {
+    MOZ_ASSERT(inited);
+    MOZ_ASSERT(ptr);
+    return ptr;
+  }
+
 protected:
  template<typename U>
  void init(U t) {
@ -1088,15 +1141,6 @@ protected:
 #endif
 };

-// Helper for OwningNonNull
-template <class T>
-inline bool
-WrapNewBindingObject(JSContext* cx, JSObject* scope, OwningNonNull<T>& value,
-                     JS::Value* vp)
-{
-  return WrapNewBindingObject(cx, scope, &static_cast<T&>(value), vp);
-}
-
 // A struct that has the same layout as an nsDependentString but much
 // faster constructor and destructor behavior
 struct FakeDependentString {
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@ -3099,7 +3099,8 @@ if (%s.IsNull()) {
                'jsvalRef': "tmp",
                'jsvalPtr': "&tmp",
                'isCreator': isCreator,
-                'exceptionCode': exceptionCode
+                'exceptionCode': exceptionCode,
+                'obj': "returnArray"
                }
            )
        innerTemplate = CGIndenter(CGGeneric(innerTemplate), 6).define()
@ -3138,7 +3139,7 @@ if (!returnArray) {
            wrappingCode = ""

        if descriptor.interface.isCallback():
-            wrap = "WrapCallbackInterface(cx, obj, %s, ${jsvalPtr})" % result
+            wrap = "WrapCallbackInterface(cx, ${obj}, %s, ${jsvalPtr})" % result
            failed = None
        elif not descriptor.interface.isExternal() and not descriptor.skipGen:
            if descriptor.wrapperCache:
@ -6618,13 +6619,19 @@ class CGBindingRoot(CGThing):

 class CGNativeMember(ClassMethod):
    def __init__(self, descriptor, member, name, signature, extendedAttrs,
-                 breakAfter=True, passCxAsNeeded=True, visibility="public"):
+                 breakAfter=True, passCxAsNeeded=True, visibility="public",
+                 jsObjectsArePtr=False):
+        """
+        If jsObjectsArePtr is true, typed arrays and "object" will be
+        passed as JSObject*
+        """
        self.descriptor = descriptor
        self.member = member
        self.extendedAttrs = extendedAttrs
        self.resultAlreadyAddRefed = isResultAlreadyAddRefed(self.descriptor,
                                                             self.extendedAttrs)
        self.passCxAsNeeded = passCxAsNeeded
+        self.jsObjectsArePtr = jsObjectsArePtr
        breakAfterSelf = "\n" if breakAfter else ""
        ClassMethod.__init__(self, name,
                             self.getReturnType(signature[0], False),
@ -6843,13 +6850,17 @@ class CGNativeMember(ClassMethod):

        if type.isSpiderMonkeyInterface():
            assert not isMember
-            if type.nullable():
-                typeDecl = "%s*"
+            if self.jsObjectsArePtr:
+                typeDecl = "JSObject*"
            else:
-                typeDecl = "%s"
-                if not optional:
-                    typeDecl += "&"
-            return (typeDecl % type.name), False, False
+                if type.nullable():
+                    typeDecl = "%s*"
+                else:
+                    typeDecl = "%s"
+                    if not optional:
+                        typeDecl += "&"
+                typeDecl = typeDecl % type.name
+            return typeDecl, False, False

        if type.isString():
            if isMember:
@ -6875,7 +6886,7 @@ class CGNativeMember(ClassMethod):
            return "JS::Value", False, False

        if type.isObject():
-            if type.nullable():
+            if type.nullable() or self.jsObjectsArePtr:
                declType = "%s*"
            else:
                if optional:
@ -7200,6 +7211,7 @@ class CallCallback(CGNativeMember):
    def __init__(self, callback, descriptorProvider):
        sig = callback.signatures()[0]
        self.retvalType = sig[0]
+        self.callback = callback
        args = sig[1]
        self.argCount = len(args)
        class FakeMember():
@ -7220,16 +7232,20 @@ class CallCallback(CGNativeMember):
                                "Call", (self.retvalType, args),
                                extendedAttrs={},
                                passCxAsNeeded=False,
-                                visibility="private")
+                                visibility="private",
+                                jsObjectsArePtr=True)
        # We have to do all the generation of our body now, because
        # the caller relies on us throwing if we can't manage it.
+        self.exceptionCode=("aRv.Throw(NS_ERROR_UNEXPECTED);\n"
+                            "return%s;" % self.getDefaultRetval())
        self.body = self.getImpl()

    def getImpl(self):
        replacements = {
            "errorReturn" : self.getDefaultRetval(),
            "argCount": self.argCount,
-            "returnResult": self.getResultConversion()
+            "returnResult": self.getResultConversion(),
+            "convertArgs": self.getArgConversions()
            }
        if self.argCount > 0:
            replacements["argvDecl"] = string.Template(
@ -7243,8 +7259,9 @@ class CallCallback(CGNativeMember):
        return string.Template(
            "JS::Value rval = JSVAL_VOID;\n"
            "${argvDecl}" # Newlines and semicolons are in the value
+            "${convertArgs}"
            "if (!JS_CallFunctionValue(cx, aThisObj, JS::ObjectValue(*mCallable),\n"
-            "                          ${argCount}, ${argv}, &rval)) {\n"
+            "                          argc, ${argv}, &rval)) {\n"
            "  aRv.Throw(NS_ERROR_UNEXPECTED);\n"
            "  return${errorReturn};\n"
            "}\n"
@ -7257,19 +7274,76 @@ class CallCallback(CGNativeMember):
            "holderName" : "rvalHolder",
            "declName" : "rvalDecl"
            }
-        exceptionCode=("aRv.Throw(NS_ERROR_UNEXPECTED);\n"
-                       "return%s;" % self.getDefaultRetval())

        convertType = instantiateJSToNativeConversionTemplate(
            getJSToNativeConversionTemplate(self.retvalType,
                                            self.descriptor,
-                                            exceptionCode=exceptionCode),
+                                            exceptionCode=self.exceptionCode),
            replacements)
        assignRetval = string.Template(
            self.getRetvalInfo(self.retvalType,
                               False)[2]).substitute(replacements)
        return convertType.define() + "\n" + assignRetval

+    def getArgConversions(self):
+        # Just reget the arglist from self.callback, because our superclasses
+        # just have way to many members they like to clobber, so I can't find a
+        # safe member name to store it in.
+        argConversions = [self.getArgConversion(i, arg) for (i, arg)
+                          in enumerate(self.callback.signatures()[0][1])]
+        # Do them back to front, so our argc modifications will work
+        # correctly, because we examine trailing arguments first.
+        argConversions.reverse();
+        # Wrap each one in a scope so that any locals it has don't leak out, and
+        # also so that we can just "break;" for our successCode.
+        argConversions = [CGWrapper(CGIndenter(CGGeneric(c)),
+                                    pre="do {\n",
+                                    post="\n} while (0);")
+                          for c in argConversions]
+        argConversions.insert(0,
+                              CGGeneric("unsigned argc = %d;" % self.argCount));
+        # And slap them together.
+        return CGList(argConversions, "\n\n").define() + "\n\n"
+
+    def getArgConversion(self, i, arg):
+        argval = arg.identifier.name
+        if arg.optional:
+            argval += ".Value()"
+        if arg.type.isString():
+            # XPConnect string-to-JS conversion wants to mutate the string.  So
+            # let's give it a string it can mutate
+            # XXXbz if we try to do a sequence of strings, this will kinda fail.
+            result = "mutableStr"
+            prepend = "nsString mutableStr(%s);\n" % argval
+        else:
+            result = argval
+            prepend = ""
+
+        conversion = prepend + wrapForType(
+            arg.type, self.descriptor,
+            {
+                'result' : result,
+                'successCode' : "break;",
+                'jsvalRef' : "argv[%d]" % i,
+                'jsvalPtr' : "&argv[%d]" % i,
+                # XXXbz we don't have anything better to use for 'obj',
+                # really...
+                'obj' : 'mCallable',
+                'isCreator': False,
+                'exceptionCode' : self.exceptionCode
+                })
+        if arg.optional:
+            conversion = (
+                CGIfWrapper(CGGeneric(conversion),
+                            "%s.WasPassed()" % arg.identifier.name).define() +
+                " else if (argc == %d) {\n"
+                "  // This is our current trailing argument; reduce argc\n"
+                "  --argc;\n"
+                "} else {\n"
+                "  argv[%d] = JS::UndefinedValue();\n"
+                "}" % (i+1, i))
+        return conversion
+
    def getDefaultRetval(self):
        default = self.getRetvalInfo(self.retvalType, False)[1]
        if len(default) != 0:
--- a/dom/bindings/test/TestBindingHeader.h
+++ b/dom/bindings/test/TestBindingHeader.h
@ -422,6 +422,7 @@ public:
  void PassDictionaryOrLong(int32_t);
  void PassDictContainingDict(const DictContainingDict&);
  void PassDictContainingSequence(const DictContainingSequence&);
+  void ReceiveDictContainingSequence(DictContainingSequence&);

  // Typedefs
  void ExerciseTypedefInterfaces1(TestInterface&);
--- a/dom/bindings/test/TestCodeGen.webidl
+++ b/dom/bindings/test/TestCodeGen.webidl
@ -34,7 +34,7 @@ callback TestIntegerReturn = long();
 callback TestNullableIntegerReturn = long?();
 callback TestBooleanReturn = boolean();
 callback TestFloatReturn = float();
-callback TestStringReturn = DOMString();
+callback TestStringReturn = DOMString(long arg);
 callback TestEnumReturn = TestEnum();
 callback TestInterfaceReturn = TestInterface();
 callback TestNullableInterfaceReturn = TestInterface?();
@ -50,6 +50,31 @@ callback TestTypedArrayReturn = ArrayBuffer();
 callback TestNullableTypedArrayReturn = ArrayBuffer?();
 callback TestSequenceReturn = sequence<boolean>();
 callback TestNullableSequenceReturn = sequence<boolean>?();
+// Callback argument tests
+callback TestIntegerArguments = sequence<long>(long arg1, long? arg2,
+                                               sequence<long> arg3,
+                                               sequence<long?>? arg4);
+callback TestInterfaceArguments = void(TestInterface arg1, TestInterface? arg2,
+                                       TestExternalInterface arg3,
+                                       TestExternalInterface? arg4,
+                                       TestCallbackInterface arg5,
+                                       TestCallbackInterface? arg6,
+                                       sequence<TestInterface> arg7,
+                                       sequence<TestInterface?>? arg8,
+                                       sequence<TestExternalInterface> arg9,
+                                       sequence<TestExternalInterface?>? arg10,
+                                       sequence<TestCallbackInterface> arg11,
+                                       sequence<TestCallbackInterface?>? arg12);
+callback TestStringEnumArguments = void(DOMString myString, DOMString? nullString,
+                                        TestEnum myEnum);
+callback TestObjectArguments = void(object anObj, object? anotherObj,
+                                    ArrayBuffer buf, ArrayBuffer? buf2);
+callback TestOptionalArguments = void(optional DOMString aString,
+                                      optional object something,
+                                      optional sequence<TestInterface> aSeq,
+                                      optional TestInterface? anInterface,
+                                      optional TestInterface anotherInterface,
+                                      optional long aLong);

 TestInterface implements ImplementedInterface;

@ -344,6 +369,7 @@ interface TestInterface {

  void passDictContainingDict(optional DictContainingDict arg);
  void passDictContainingSequence(optional DictContainingSequence arg);
+  DictContainingSequence receiveDictContainingSequence();

  // EnforceRange/Clamp tests
  void dontEnforceRangeOrClamp(byte arg);
@ -443,6 +469,7 @@ dictionary DictContainingDict {

 dictionary DictContainingSequence {
  sequence<long> ourSequence;
+  sequence<TestInterface> ourSequence2;
 };

 interface TestIndexedGetterInterface {
--- a/dom/webidl/EventHandler.webidl
+++ b/dom/webidl/EventHandler.webidl
@ -14,6 +14,7 @@
 callback EventHandlerNonNull = any (Event event);
 typedef EventHandlerNonNull? EventHandler;

-[TreatNonCallableAsNull]
-callback OnErrorEventHandlerNonNull = any ((Event or DOMString) event, DOMString source, unsigned long lineno, unsigned long column);
-typedef OnErrorEventHandlerNonNull? OnErrorEventHandler;
+// We don't support JS-wrapping of unions yet
+//[TreatNonCallableAsNull]
+//callback OnErrorEventHandlerNonNull = any ((Event or DOMString) event, DOMString source, unsigned long lineno, unsigned long column);
+//typedef OnErrorEventHandlerNonNull? OnErrorEventHandler;