From 05bd182538a9358b2f6cee8d41696e1fd516c3d6 Mon Sep 17 00:00:00 2001 From: Mukilan Thiyagarajan Date: Sat, 8 Nov 2014 20:55:09 +0530 Subject: [PATCH 1/2] Fix binding generation for Callback Functions and Callback Interfaces --- .../dom/bindings/codegen/CodegenRust.py | 407 ++++-------------- components/script/dom/testbinding.rs | 12 + .../script/dom/webidls/TestBinding.webidl | 10 + 3 files changed, 98 insertions(+), 331 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index aaac17a9002dc..9a2cb83bcb1f1 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -601,11 +601,13 @@ def wrapObjectTemplate(templateBody, isDefinitelyObject, type, if descriptor.interface.isCallback(): name = descriptor.nativeType - declType = CGGeneric("Option<%s>" % name); - conversion = ("Some(%s::new((${val}).to_object()))" % name) + declType = CGGeneric(name) + template = "%s::new((${val}).to_object())" % name + if type.nullable(): + declType = CGWrapper(declType, pre="Option<", post=">") + template = wrapObjectTemplate("Some(%s)" % template, isDefinitelyObject, type, + failureCode) - template = wrapObjectTemplate(conversion, isDefinitelyObject, type, - failureCode) return handleOptional(template, declType, handleDefaultNull("None")) if isMember: @@ -983,15 +985,14 @@ def __init__(self, argument, index, argv, argc, descriptorProvider, needsRooting) seqType = CGTemplatedType("Vec", declType) + variadicConversion = string.Template( - "{\n" - " let mut vector: ${seqType} = Vec::with_capacity((${argc} - ${index}) as uint);\n" - " for variadicArg in range(${index}, ${argc}) {\n" + "let mut vector: ${seqType} = Vec::with_capacity((${argc} - ${index}) as uint);\n" + "for variadicArg in range(${index}, ${argc}) {\n" "${inner}\n" " vector.push(slot);\n" - " }\n" - " vector\n" - "}" + "}\n" + "vector" ).substitute({ "index": index, "argc": argc, @@ -999,6 +1000,10 @@ def __init__(self, argument, index, argv, argc, descriptorProvider, "inner": CGIndenter(innerConverter, 4).define(), }) + variadicConversion = CGIfElseWrapper(condition, + CGGeneric(variadicConversion), + CGGeneric("Vec::new()")).define() + self.converter = instantiateJSToNativeConversionTemplate( variadicConversion, replacementVariables, seqType, "arg%d" % index, False) @@ -3993,44 +3998,14 @@ class CGInterfaceTrait(CGThing): def __init__(self, descriptor): CGThing.__init__(self) - def argument_type(ty, optional=False, defaultValue=None, variadic=False): - _, _, declType, _ = getJSToNativeConversionTemplate( - ty, descriptor, isArgument=True) - - if variadic: - declType = CGWrapper(declType, pre="Vec<", post=">") - elif optional and not defaultValue: - declType = CGWrapper(declType, pre="Option<", post=">") - - if ty.isDictionary(): - declType = CGWrapper(declType, pre="&") - - return declType.define() def attribute_arguments(needCx, argument=None): if needCx: yield "cx", "*mut JSContext" if argument: - yield "value", argument_type(argument) + yield "value", argument_type(descriptor, argument) - def method_arguments(returnType, arguments, trailing=None): - if needCx(returnType, arguments, True): - yield "cx", "*mut JSContext" - - for argument in arguments: - ty = argument_type(argument.type, argument.optional, - argument.defaultValue, argument.variadic) - yield CGDictionary.makeMemberName(argument.identifier.name), ty - - if trailing: - yield trailing - - def return_type(rettype, infallible): - result = getRetvalDeclarationForType(rettype, descriptor) - if not infallible: - result = CGWrapper(result, pre="Fallible<", post=">") - return result.define() def members(): for m in descriptor.interface.members: @@ -4039,14 +4014,14 @@ def members(): name = CGSpecializedMethod.makeNativeName(descriptor, m) infallible = 'infallible' in descriptor.getExtendedAttributes(m) for idx, (rettype, arguments) in enumerate(m.signatures()): - arguments = method_arguments(rettype, arguments) - rettype = return_type(rettype, infallible) + arguments = method_arguments(descriptor, rettype, arguments) + rettype = return_type(descriptor, rettype, infallible) yield name + ('_' * idx), arguments, rettype elif m.isAttr() and not m.isStatic(): name = CGSpecializedGetter.makeNativeName(descriptor, m) infallible = 'infallible' in descriptor.getExtendedAttributes(m, getter=True) needCx = typeNeedsCx(m.type) - yield name, attribute_arguments(needCx), return_type(m.type, infallible) + yield name, attribute_arguments(needCx), return_type(descriptor, m.type, infallible) if not m.readonly: name = CGSpecializedSetter.makeNativeName(descriptor, m) @@ -4067,10 +4042,10 @@ def members(): infallible = 'infallible' in descriptor.getExtendedAttributes(operation) if operation.isGetter(): - arguments = method_arguments(rettype, arguments, ("found", "&mut bool")) + arguments = method_arguments(descriptor, rettype, arguments, trailing=("found", "&mut bool")) else: - arguments = method_arguments(rettype, arguments) - rettype = return_type(rettype, infallible) + arguments = method_arguments(descriptor, rettype, arguments) + rettype = return_type(descriptor, rettype, infallible) yield name, arguments, rettype def fmt(arguments): @@ -4573,6 +4548,38 @@ def __init__(self, config, prefix, webIDLFile): def define(self): return stripTrailingWhitespace(self.root.define()) +def argument_type(descriptorProvdider, ty, optional=False, defaultValue=None, variadic=False): + _, _, declType, _ = getJSToNativeConversionTemplate( + ty, descriptorProvdider, isArgument=True) + + if variadic: + declType = CGWrapper(declType, pre="Vec<", post=">") + elif optional and not defaultValue: + declType = CGWrapper(declType, pre="Option<", post=">") + + if ty.isDictionary(): + declType = CGWrapper(declType, pre="&") + + return declType.define() + +def method_arguments(descriptorProvider, returnType, arguments, passJSBits=True, trailing=None): + if needCx(returnType, arguments, passJSBits): + yield "cx", "*mut JSContext" + + for argument in arguments: + ty = argument_type(descriptorProvider, argument.type, argument.optional, + argument.defaultValue, argument.variadic) + yield CGDictionary.makeMemberName(argument.identifier.name), ty + + if trailing: + yield trailing + +def return_type(descriptorProvider, rettype, infallible): + result = getRetvalDeclarationForType(rettype, descriptorProvider) + if not infallible: + result = CGWrapper(result, pre="Fallible<", post=">") + return result.define() + class CGNativeMember(ClassMethod): def __init__(self, descriptorProvider, member, name, signature, extendedAttrs, breakAfter=True, passJSBitsAsNeeded=True, visibility="public", @@ -4592,7 +4599,7 @@ def __init__(self, descriptorProvider, member, name, signature, extendedAttrs, self.variadicIsSequence = variadicIsSequence breakAfterSelf = "\n" if breakAfter else "" ClassMethod.__init__(self, name, - self.getReturnType(signature[0], False), + self.getReturnType(signature[0]), self.getArgs(signature[0], signature[1]), static=member.isStatic(), # Mark our getters, which are attrs that @@ -4603,274 +4610,16 @@ def __init__(self, descriptorProvider, member, name, signature, extendedAttrs, breakAfterSelf=breakAfterSelf, visibility=visibility) - def getReturnType(self, type, isMember): - return self.getRetvalInfo(type, isMember)[0] - - def getRetvalInfo(self, type, isMember): - """ - Returns a tuple: - - The first element is the type declaration for the retval - - The second element is a template for actually returning a value stored in - "${declName}". This means actually returning it if - we're not outparam, else assigning to the "retval" outparam. If - isMember is true, this can be None, since in that case the caller will - never examine this value. - """ - if type.isVoid(): - typeDecl, template = "", "" - elif type.isPrimitive() and type.tag() in builtinNames: - result = CGGeneric(builtinNames[type.tag()]) - if type.nullable(): - raise TypeError("Nullable primitives are not supported here.") - - typeDecl, template = result.define(), "return Ok(${declName});" - elif type.isDOMString(): - if isMember: - # No need for a third element in the isMember case - typeDecl, template = "nsString", None - # Outparam - else: - typeDecl, template = "void", "retval = ${declName};" - elif type.isByteString(): - if isMember: - # No need for a third element in the isMember case - typeDecl, template = "nsCString", None - # Outparam - typeDecl, template = "void", "retval = ${declName};" - elif type.isEnum(): - enumName = type.unroll().inner.identifier.name - if type.nullable(): - enumName = CGTemplatedType("Nullable", - CGGeneric(enumName)).define() - typeDecl, template = enumName, "return ${declName};" - elif type.isGeckoInterface(): - iface = type.unroll().inner; - nativeType = self.descriptorProvider.getDescriptor( - iface.identifier.name).nativeType - # Now trim off unnecessary namespaces - nativeType = nativeType.split("::") - if nativeType[0] == "mozilla": - nativeType.pop(0) - if nativeType[0] == "dom": - nativeType.pop(0) - result = CGWrapper(CGGeneric("::".join(nativeType)), post="*") - # Since we always force an owning type for callback return values, - # our ${declName} is an OwningNonNull or nsRefPtr. So we can just - # .forget() to get our already_AddRefed. - typeDecl, template = result.define(), "return ${declName}.forget();" - elif type.isCallback(): - typeDecl, template = \ - ("already_AddRefed<%s>" % type.unroll().identifier.name, - "return ${declName}.forget();") - elif type.isAny(): - typeDecl, template = "JSVal", "return Ok(${declName});" - elif type.isObject(): - typeDecl, template = "JSObject*", "return ${declName};" - elif type.isSpiderMonkeyInterface(): - if type.nullable(): - returnCode = "return ${declName}.IsNull() ? nullptr : ${declName}.Value().Obj();" - else: - returnCode = "return ${declName}.Obj();" - typeDecl, template = "JSObject*", returnCode - elif type.isSequence(): - # If we want to handle sequence-of-sequences return values, we're - # going to need to fix example codegen to not produce nsTArray - # for the relevant argument... - assert not isMember - # Outparam. - if type.nullable(): - returnCode = ("if (${declName}.IsNull()) {\n" - " retval.SetNull();\n" - "} else {\n" - " retval.SetValue().SwapElements(${declName}.Value());\n" - "}") - else: - returnCode = "retval.SwapElements(${declName});" - typeDecl, template = "void", returnCode - elif type.isDate(): - result = CGGeneric("Date") - if type.nullable(): - result = CGTemplatedType("Nullable", result) - typeDecl, template = result.define(), "return ${declName};" - else: - raise TypeError("Don't know how to declare return value for %s" % type) - - if not 'infallible' in self.extendedAttrs: - if typeDecl: - typeDecl = "Fallible<%s>" % typeDecl - else: - typeDecl = "ErrorResult" - if not template: - template = "return Ok(());" - return typeDecl, template + def getReturnType(self, type): + infallible = 'infallible' in self.extendedAttrs + typeDecl = return_type(self.descriptorProvider, type, infallible) + return typeDecl def getArgs(self, returnType, argList): - args = [self.getArg(arg) for arg in argList] - # Now the outparams - if returnType.isDOMString(): - args.append(Argument("nsString&", "retval")) - if returnType.isByteString(): - args.append(Argument("nsCString&", "retval")) - elif returnType.isSequence(): - nullable = returnType.nullable() - if nullable: - returnType = returnType.inner - # And now the actual underlying type - elementDecl = self.getReturnType(returnType.inner, True) - type = CGTemplatedType("nsTArray", CGGeneric(elementDecl)) - if nullable: - type = CGTemplatedType("Nullable", type) - args.append(Argument("%s&" % type.define(), "retval")) - # The legacycaller thisval - if self.member.isMethod() and self.member.isLegacycaller(): - # If it has an identifier, we can't deal with it yet - assert self.member.isIdentifierLess() - args.insert(0, Argument("JS::Value", "aThisVal")) - # And jscontext bits. - if needCx(returnType, argList, self.passJSBitsAsNeeded): - args.insert(0, Argument("JSContext*", "cx")) - # And if we're static, a global - if self.member.isStatic(): - args.insert(0, Argument("const GlobalObject&", "global")) - return args - - def doGetArgType(self, type, optional, isMember): - """ - The main work of getArgType. Returns a string type decl, whether this - is a const ref, as well as whether the type should be wrapped in - Nullable as needed. - - isMember can be false or one of the strings "Sequence" or "Variadic" - """ - if type.isArray(): - raise TypeError("Can't handle array arguments yet") - - if type.isSequence(): - nullable = type.nullable() - if nullable: - type = type.inner - elementType = type.inner - argType = self.getArgType(elementType, False, "Sequence")[0] - decl = CGTemplatedType("Sequence", argType) - return decl.define(), True, True - - if type.isUnion(): - return union_native_type(type), False, True - - if type.isGeckoInterface() and not type.isCallbackInterface(): - iface = type.unroll().inner - argIsPointer = type.nullable() - forceOwningType = iface.isCallback() or isMember - if argIsPointer: - if (optional or isMember) and forceOwningType: - typeDecl = "nsRefPtr<%s>" - else: - typeDecl = "*%s" - else: - if optional or isMember: - if forceOwningType: - typeDecl = "OwningNonNull<%s>" - else: - typeDecl = "NonNull<%s>" - else: - typeDecl = "%s" - descriptor = self.descriptorProvider.getDescriptor(iface.identifier.name) - return (typeDecl % descriptor.argumentType, - False, False) - - if type.isSpiderMonkeyInterface(): - if self.jsObjectsArePtr: - return "JSObject*", False, False - - return type.name, True, True - - if type.isDOMString(): - declType = "DOMString" - return declType, True, False - - if type.isByteString(): - declType = "nsCString" - return declType, True, False - - if type.isEnum(): - return type.unroll().inner.identifier.name, False, True - - if type.isCallback() or type.isCallbackInterface(): - forceOwningType = optional or isMember - if type.nullable(): - if forceOwningType: - declType = "nsRefPtr<%s>" - else: - declType = "%s*" - else: - if forceOwningType: - declType = "OwningNonNull<%s>" - else: - declType = "%s&" - if type.isCallback(): - name = type.unroll().identifier.name - else: - name = type.unroll().inner.identifier.name - return declType % name, False, False - - if type.isAny(): - # Don't do the rooting stuff for variadics for now - if isMember: - declType = "JS::Value" - else: - declType = "JSVal" - return declType, False, False - - if type.isObject(): - if isMember: - declType = "JSObject*" - else: - declType = "JS::Handle" - return declType, False, False - - if type.isDictionary(): - typeName = CGDictionary.makeDictionaryName(type.inner) - return typeName, True, True - - if type.isDate(): - return "Date", False, True - - assert type.isPrimitive() - - return builtinNames[type.tag()], False, True - - def getArgType(self, type, optional, isMember): - """ - Get the type of an argument declaration. Returns the type CGThing, and - whether this should be a const ref. - - isMember can be False, "Sequence", or "Variadic" - """ - (decl, ref, handleNullable) = self.doGetArgType(type, optional, - isMember) - decl = CGGeneric(decl) - if handleNullable and type.nullable(): - decl = CGTemplatedType("Nullable", decl) - if isMember == "Variadic": - arrayType = "Sequence" if self.variadicIsSequence else "nsTArray" - decl = CGTemplatedType(arrayType, decl) - elif optional: - # Note: All variadic args claim to be optional, but we can just use - # empty arrays to represent them not being present. - decl = CGTemplatedType("Option", decl) - return decl - - def getArg(self, arg): - """ - Get the full argument declaration for an argument - """ - decl = self.getArgType(arg.type, - arg.optional and not arg.defaultValue, - "Variadic" if arg.variadic else False) - - return Argument(decl.define(), arg.identifier.name) + return [Argument(arg[1], arg[0]) for arg in method_arguments(self.descriptorProvider, + returnType, + argList, + self.passJSBitsAsNeeded)] class CGCallback(CGClass): def __init__(self, idlObject, descriptorProvider, baseName, methods, @@ -5052,8 +4801,8 @@ def __init__(self, sig, name, descriptorProvider, needThisHandling, rethrowConte lastArg = args[self.argCount-1] if lastArg.variadic: self.argCountStr = ( - "(%d - 1) + %s.Length()" % (self.argCount, - lastArg.identifier.name)) + "(%d - 1) + %s.len()" % (self.argCount, + lastArg.identifier.name)) else: self.argCountStr = "%d" % self.argCount self.needThisHandling = needThisHandling @@ -5111,7 +4860,6 @@ def getImpl(self): def getResultConversion(self): replacements = { "val": "rval", - "declName": "rvalDecl", } template, _, declType, needsRooting = getJSToNativeConversionTemplate( @@ -5125,10 +4873,12 @@ def getResultConversion(self): convertType = instantiateJSToNativeConversionTemplate( template, replacements, declType, "rvalDecl", needsRooting) - assignRetval = string.Template( - self.getRetvalInfo(self.retvalType, - False)[1]).substitute(replacements) - return convertType.define() + "\n" + assignRetval + "\n" + if self.retvalType is None or self.retvalType.isVoid(): + retval = "()" + else: + retval = "rvalDecl" + + return "%s\nOk(%s)\n" % (convertType.define(), retval) def getArgConversions(self): # Just reget the arglist from self.originalSig, because our superclasses @@ -5139,12 +4889,7 @@ def getArgConversions(self): # 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="loop {\n", - post="\nbreak;}\n") - for c in argConversions] + argConversions = [CGGeneric(c) for c in argConversions] if self.argCount > 0: argConversions.insert(0, self.getArgcDecl()) # And slap them together. @@ -5163,13 +4908,13 @@ def getArgConversion(self, i, arg): conversion = wrapForType("argv[%s]" % jsvalIndex, result=argval, - successCode="continue;" if arg.variadic else "break;") + successCode="") if arg.variadic: conversion = string.Template( - "for (uint32_t idx = 0; idx < ${arg}.Length(); ++idx) {\n" + + "for idx in range(0, ${arg}.len()) {\n" + CGIndenter(CGGeneric(conversion)).define() + "\n" "}\n" - "break;").substitute({ "arg": arg.identifier.name }) + ).substitute({ "arg": arg.identifier.name }) elif arg.optional and not arg.defaultValue: conversion = ( CGIfWrapper(CGGeneric(conversion), @@ -5220,7 +4965,7 @@ def getCallSetup(self): }) def getArgcDecl(self): - return CGGeneric("let mut argc = %su32;" % self.argCountStr); + return CGGeneric("let mut argc = %s as u32;" % self.argCountStr); @staticmethod def ensureASCIIName(idlObject): diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs index 073a5721fa10b..9101cbf4356a6 100644 --- a/components/script/dom/testbinding.rs +++ b/components/script/dom/testbinding.rs @@ -5,6 +5,8 @@ use dom::bindings::codegen::Bindings::TestBindingBinding::TestBindingMethods; use dom::bindings::codegen::Bindings::TestBindingBinding::TestEnum; use dom::bindings::codegen::Bindings::TestBindingBinding::TestEnumValues::_empty; +use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener; +use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::codegen::UnionTypes::BlobOrString::BlobOrString; use dom::bindings::codegen::UnionTypes::EventOrString::{EventOrString, eString}; use dom::bindings::codegen::UnionTypes::HTMLElementOrLong::{HTMLElementOrLong, eLong}; @@ -164,6 +166,8 @@ impl<'a> TestBindingMethods for JSRef<'a, TestBinding> { fn PassUnion2(self, _: EventOrString) {} fn PassUnion3(self, _: BlobOrString) {} fn PassAny(self, _: *mut JSContext, _: JSVal) {} + fn PassCallbackFunction(self, _: Function) {} + fn PassCallbackInterface(self, _: EventListener) {} fn PassNullableBoolean(self, _: Option) {} fn PassNullableByte(self, _: Option) {} @@ -182,6 +186,8 @@ impl<'a> TestBindingMethods for JSRef<'a, TestBinding> { fn PassNullableInterface(self, _: Option>) {} fn PassNullableUnion(self, _: Option) {} fn PassNullableUnion2(self, _: Option) {} + fn PassNullableCallbackFunction(self, _: Option) {} + fn PassNullableCallbackInterface(self, _: Option) {} fn PassOptionalBoolean(self, _: Option) {} fn PassOptionalByte(self, _: Option) {} @@ -201,6 +207,8 @@ impl<'a> TestBindingMethods for JSRef<'a, TestBinding> { fn PassOptionalUnion(self, _: Option) {} fn PassOptionalUnion2(self, _: Option) {} fn PassOptionalAny(self, _: *mut JSContext, _: JSVal) {} + fn PassOptionalCallbackFunction(self, _: Option) {} + fn PassOptionalCallbackInterface(self, _: Option) {} fn PassOptionalNullableBoolean(self, _: Option>) {} fn PassOptionalNullableByte(self, _: Option>) {} @@ -219,6 +227,8 @@ impl<'a> TestBindingMethods for JSRef<'a, TestBinding> { fn PassOptionalNullableInterface(self, _: Option>>) {} fn PassOptionalNullableUnion(self, _: Option>) {} fn PassOptionalNullableUnion2(self, _: Option>) {} + fn PassOptionalNullableCallbackFunction(self, _: Option>) {} + fn PassOptionalNullableCallbackInterface(self, _: Option>) {} fn PassOptionalBooleanWithDefault(self, _: bool) {} fn PassOptionalByteWithDefault(self, _: i8) {} @@ -249,6 +259,8 @@ impl<'a> TestBindingMethods for JSRef<'a, TestBinding> { fn PassOptionalNullableInterfaceWithDefault(self, _: Option>) {} fn PassOptionalNullableUnionWithDefault(self, _: Option) {} fn PassOptionalNullableUnion2WithDefault(self, _: Option) {} + // fn PassOptionalNullableCallbackFunctionWithDefault(self, _: Option) {} + fn PassOptionalNullableCallbackInterfaceWithDefault(self, _: Option) {} fn PassOptionalAnyWithDefault(self, _: *mut JSContext, _: JSVal) {} fn PassOptionalNullableBooleanWithNonNullDefault(self, _: Option) {} diff --git a/components/script/dom/webidls/TestBinding.webidl b/components/script/dom/webidls/TestBinding.webidl index e8ef05d8242fe..48a0fd1071d2d 100644 --- a/components/script/dom/webidls/TestBinding.webidl +++ b/components/script/dom/webidls/TestBinding.webidl @@ -148,6 +148,8 @@ interface TestBinding { void passUnion2((Event or DOMString) data); void passUnion3((Blob or DOMString) data); void passAny(any arg); + void passCallbackFunction(Function fun); + void passCallbackInterface(EventListener listener); void passNullableBoolean(boolean? arg); void passNullableByte(byte? arg); @@ -166,6 +168,8 @@ interface TestBinding { void passNullableInterface(Blob? arg); void passNullableUnion((HTMLElement or long)? arg); void passNullableUnion2((Event or DOMString)? data); + void passNullableCallbackFunction(Function? fun); + void passNullableCallbackInterface(EventListener? listener); void passOptionalBoolean(optional boolean arg); void passOptionalByte(optional byte arg); @@ -185,6 +189,8 @@ interface TestBinding { void passOptionalUnion(optional (HTMLElement or long) arg); void passOptionalUnion2(optional (Event or DOMString) data); void passOptionalAny(optional any arg); + void passOptionalCallbackFunction(optional Function fun); + void passOptionalCallbackInterface(optional EventListener listener); void passOptionalNullableBoolean(optional boolean? arg); void passOptionalNullableByte(optional byte? arg); @@ -203,6 +209,8 @@ interface TestBinding { void passOptionalNullableInterface(optional Blob? arg); void passOptionalNullableUnion(optional (HTMLElement or long)? arg); void passOptionalNullableUnion2(optional (Event or DOMString)? data); + void passOptionalNullableCallbackFunction(optional Function? fun); + void passOptionalNullableCallbackInterface(optional EventListener? listener); void passOptionalBooleanWithDefault(optional boolean arg = false); void passOptionalByteWithDefault(optional byte arg = 0); @@ -233,6 +241,8 @@ interface TestBinding { void passOptionalNullableInterfaceWithDefault(optional Blob? arg = null); void passOptionalNullableUnionWithDefault(optional (HTMLElement or long)? arg = null); void passOptionalNullableUnion2WithDefault(optional (Event or DOMString)? data = null); + // void passOptionalNullableCallbackFunctionWithDefault(optional Function? fun = null); + void passOptionalNullableCallbackInterfaceWithDefault(optional EventListener? listener = null); void passOptionalAnyWithDefault(optional any arg = null); void passOptionalNullableBooleanWithNonNullDefault(optional boolean? arg = false); From 4b2b0d072378aa1b66bf2383e7c64a732ab854c8 Mon Sep 17 00:00:00 2001 From: Mukilan Thiyagarajan Date: Sat, 8 Nov 2014 21:03:59 +0530 Subject: [PATCH 2/2] Allow passing arguments to setTimeout/setInterval callbacks --- .../script/dom/dedicatedworkerglobalscope.rs | 2 +- components/script/dom/webidls/Function.webidl | 14 ++++++++ components/script/dom/webidls/Window.webidl | 6 ++-- components/script/dom/window.rs | 14 ++++---- components/script/dom/workerglobalscope.rs | 18 +++++----- components/script/script_task.rs | 2 +- components/script/timers.rs | 33 +++++++++---------- .../wpt/metadata/html/dom/interfaces.html.ini | 24 ++++++++++++++ .../compile-error-in-setInterval.html.ini | 5 +-- .../compile-error-in-setTimeout.html.ini | 5 +-- .../runtime-error-in-setInterval.html.ini | 5 +-- .../runtime-error-in-setTimeout.html.ini | 5 +-- 12 files changed, 88 insertions(+), 45 deletions(-) create mode 100644 components/script/dom/webidls/Function.webidl diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 1c935bead98f2..9d22529f49f19 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -146,7 +146,7 @@ impl DedicatedWorkerGlobalScope { Worker::handle_release(addr) }, Ok(FireTimerMsg(FromWorker, timer_id)) => { - scope.handle_fire_timer(timer_id, js_context.ptr); + scope.handle_fire_timer(timer_id); } Ok(_) => panic!("Unexpected message"), Err(_) => break, diff --git a/components/script/dom/webidls/Function.webidl b/components/script/dom/webidls/Function.webidl new file mode 100644 index 0000000000000..55bec0811c71b --- /dev/null +++ b/components/script/dom/webidls/Function.webidl @@ -0,0 +1,14 @@ +/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * The origin of this IDL file is + * http://www.whatwg.org/specs/web-apps/current-work/#functiocn + * + * © Copyright 2004-2011 Apple Computer, Inc., Mozilla Foundation, and + * Opera Software ASA. You are granted a license to use, reproduce + * and create derivative works of this document. + */ + +callback Function = any(any... arguments); diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl index 9cd6ed1c045ef..7a94be619733f 100644 --- a/components/script/dom/webidls/Window.webidl +++ b/components/script/dom/webidls/Window.webidl @@ -64,13 +64,11 @@ Window implements WindowEventHandlers; // http://www.whatwg.org/html/#windowtimers [NoInterfaceObject/*, Exposed=Window,Worker*/] interface WindowTimers { - //long setTimeout(Function handler, optional long timeout = 0, any... arguments); + long setTimeout(Function handler, optional long timeout = 0, any... arguments); //long setTimeout(DOMString handler, optional long timeout = 0, any... arguments); - long setTimeout(any handler, optional long timeout = 0); void clearTimeout(optional long handle = 0); - //long setInterval(Function handler, optional long timeout = 0, any... arguments); + long setInterval(Function handler, optional long timeout = 0, any... arguments); //long setInterval(DOMString handler, optional long timeout = 0, any... arguments); - long setInterval(any handler, optional long timeout = 0); void clearInterval(optional long handle = 0); }; Window implements WindowTimers; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 046029da9e699..da6b788db7214 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -4,6 +4,7 @@ use dom::bindings::cell::DOMRefCell; use dom::bindings::codegen::Bindings::EventHandlerBinding::{OnErrorEventHandlerNonNull, EventHandlerNonNull}; +use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::codegen::Bindings::WindowBinding; use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods; use dom::bindings::codegen::InheritTypes::EventTargetCast; @@ -223,8 +224,9 @@ impl<'a> WindowMethods for JSRef<'a, Window> { self.navigator.get().unwrap() } - fn SetTimeout(self, _cx: *mut JSContext, callback: JSVal, timeout: i32) -> i32 { + fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec) -> i32 { self.timers.set_timeout_or_interval(callback, + args, timeout, false, // is_interval FromWindow(self.page.id.clone()), @@ -235,8 +237,9 @@ impl<'a> WindowMethods for JSRef<'a, Window> { self.timers.clear_timeout_or_interval(handle); } - fn SetInterval(self, _cx: *mut JSContext, callback: JSVal, timeout: i32) -> i32 { + fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec) -> i32 { self.timers.set_timeout_or_interval(callback, + args, timeout, true, // is_interval FromWindow(self.page.id.clone()), @@ -317,7 +320,7 @@ pub trait WindowHelpers { fn wait_until_safe_to_modify_dom(self); fn init_browser_context(self, doc: JSRef); fn load_url(self, href: DOMString); - fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext); + fn handle_fire_timer(self, timer_id: TimerId); fn evaluate_js_with_result(self, code: &str) -> JSVal; fn evaluate_script_with_result(self, code: &str, filename: &str) -> JSVal; } @@ -380,9 +383,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> { } } - fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext) { - let this_value = self.reflector().get_jsobject(); - self.timers.fire_timer(timer_id, this_value, cx); + fn handle_fire_timer(self, timer_id: TimerId) { + self.timers.fire_timer(timer_id, self.clone()); self.flush_layout(); } } diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 5c352739c98e1..4b490da86748a 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::codegen::Bindings::WorkerGlobalScopeBinding::WorkerGlobalScopeMethods; +use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::error::{ErrorResult, Fallible, Syntax, Network, FailureUnknown}; use dom::bindings::global; use dom::bindings::js::{MutNullableJS, JSRef, Temporary, OptionalSettable}; @@ -155,8 +156,9 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> { base64_atob(atob) } - fn SetTimeout(self, _cx: *mut JSContext, handler: JSVal, timeout: i32) -> i32 { - self.timers.set_timeout_or_interval(handler, + fn SetTimeout(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec) -> i32 { + self.timers.set_timeout_or_interval(callback, + args, timeout, false, // is_interval FromWorker, @@ -167,8 +169,9 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> { self.timers.clear_timeout_or_interval(handle); } - fn SetInterval(self, _cx: *mut JSContext, handler: JSVal, timeout: i32) -> i32 { - self.timers.set_timeout_or_interval(handler, + fn SetInterval(self, _cx: *mut JSContext, callback: Function, timeout: i32, args: Vec) -> i32 { + self.timers.set_timeout_or_interval(callback, + args, timeout, true, // is_interval FromWorker, @@ -181,14 +184,13 @@ impl<'a> WorkerGlobalScopeMethods for JSRef<'a, WorkerGlobalScope> { } pub trait WorkerGlobalScopeHelpers { - fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext); + fn handle_fire_timer(self, timer_id: TimerId); } impl<'a> WorkerGlobalScopeHelpers for JSRef<'a, WorkerGlobalScope> { - fn handle_fire_timer(self, timer_id: TimerId, cx: *mut JSContext) { - let this_value = self.reflector().get_jsobject(); - self.timers.fire_timer(timer_id, this_value, cx); + fn handle_fire_timer(self, timer_id: TimerId) { + self.timers.fire_timer(timer_id, self.clone()); } } diff --git a/components/script/script_task.rs b/components/script/script_task.rs index c407795bd63ba..4ab559682379a 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -665,7 +665,7 @@ impl ScriptTask { pipeline ID not associated with this script task. This is a bug."); let frame = page.frame(); let window = frame.as_ref().unwrap().window.root(); - window.handle_fire_timer(timer_id, self.get_cx()); + window.handle_fire_timer(timer_id); } /// Handles a notification that reflow completed. diff --git a/components/script/timers.rs b/components/script/timers.rs index 57480eb1cde58..988bb42797887 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -3,16 +3,17 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::cell::DOMRefCell; +use dom::bindings::callback::ReportExceptions; +use dom::bindings::codegen::Bindings::FunctionBinding::Function; +use dom::bindings::js::JSRef; +use dom::bindings::utils::Reflectable; use script_task::{FireTimerMsg, ScriptChan}; use script_task::{TimerSource, FromWindow, FromWorker}; use servo_util::task::spawn_named; -use js::jsapi::JS_CallFunctionValue; -use js::jsapi::{JSContext, JSObject}; -use js::jsval::{JSVal, NullValue}; -use js::rust::with_compartment; +use js::jsval::JSVal; use std::cell::Cell; use std::cmp; @@ -21,7 +22,6 @@ use std::comm::{channel, Sender}; use std::comm::Select; use std::hash::{Hash, sip}; use std::io::timer::Timer; -use std::ptr; use std::time::duration::Duration; #[deriving(PartialEq, Eq)] @@ -69,11 +69,14 @@ impl Drop for TimerManager { // Holder for the various JS values associated with setTimeout // (ie. function value to invoke and all arguments to pass // to the function when calling it) +// TODO: Handle rooting during fire_timer when movable GC is turned on #[jstraceable] #[privatize] +#[deriving(Clone)] struct TimerData { is_interval: bool, - funval: JSVal, + funval: Function, + args: Vec } impl TimerManager { @@ -85,7 +88,8 @@ impl TimerManager { } pub fn set_timeout_or_interval(&self, - callback: JSVal, + callback: Function, + arguments: Vec, timeout: i32, is_interval: bool, source: TimerSource, @@ -142,6 +146,7 @@ impl TimerManager { data: TimerData { is_interval: is_interval, funval: callback, + args: arguments } }; self.active_timers.borrow_mut().insert(timer_id, timer); @@ -156,21 +161,15 @@ impl TimerManager { } } - pub fn fire_timer(&self, timer_id: TimerId, this: *mut JSObject, cx: *mut JSContext) { + pub fn fire_timer(&self, timer_id: TimerId, this: JSRef) { let data = match self.active_timers.borrow().get(&timer_id) { None => return, - Some(timer_handle) => timer_handle.data, + Some(timer_handle) => timer_handle.data.clone(), }; - // TODO: Support extra arguments. This requires passing a `*JSVal` array as `argv`. - with_compartment(cx, this, || { - let mut rval = NullValue(); - unsafe { - JS_CallFunctionValue(cx, this, data.funval, - 0, ptr::null_mut(), &mut rval); - } - }); + // TODO: Must handle rooting of funval and args when movable GC is turned on + let _ = data.funval.Call_(this, data.args, ReportExceptions); if !data.is_interval { self.active_timers.borrow_mut().remove(&timer_id); diff --git a/tests/wpt/metadata/html/dom/interfaces.html.ini b/tests/wpt/metadata/html/dom/interfaces.html.ini index 957c1886542d6..f61d940b9b29b 100644 --- a/tests/wpt/metadata/html/dom/interfaces.html.ini +++ b/tests/wpt/metadata/html/dom/interfaces.html.ini @@ -9948,3 +9948,27 @@ [HTMLFontElement interface: document.createElement("font") must inherit property "size" with the proper type (2)] expected: FAIL + [Window interface: operation setTimeout(Function,long,any)] + expected: FAIL + + [Window interface: operation setTimeout(DOMString,long,any)] + expected: FAIL + + [Window interface: operation setInterval(Function,long,any)] + expected: FAIL + + [Window interface: operation setInterval(DOMString,long,any)] + expected: FAIL + + [WorkerGlobalScope interface: operation setTimeout(Function,long,any)] + expected: FAIL + + [WorkerGlobalScope interface: operation setTimeout(DOMString,long,any)] + expected: FAIL + + [WorkerGlobalScope interface: operation setInterval(Function,long,any)] + expected: FAIL + + [WorkerGlobalScope interface: operation setInterval(DOMString,long,any)] + expected: FAIL + diff --git a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setInterval.html.ini b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setInterval.html.ini index 0e052aa4acf85..39d32eaa2f3f5 100644 --- a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setInterval.html.ini +++ b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setInterval.html.ini @@ -1,8 +1,9 @@ [compile-error-in-setInterval.html] type: testharness + expected: TIMEOUT [window.onerror - compile error in setInterval] - expected: FAIL + expected: NOTRUN [window.onerror - compile error in setInterval (column)] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setTimeout.html.ini b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setTimeout.html.ini index 0b28074c4e34d..3e0d53c7a65b2 100644 --- a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setTimeout.html.ini +++ b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/compile-error-in-setTimeout.html.ini @@ -1,8 +1,9 @@ [compile-error-in-setTimeout.html] type: testharness + expected: TIMEOUT [window.onerror - compile error in setTimeout] - expected: FAIL + expected: NOTRUN [window.onerror - compile error in setTimeout (column)] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setInterval.html.ini b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setInterval.html.ini index 9f4aef723f18a..39b8112204661 100644 --- a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setInterval.html.ini +++ b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setInterval.html.ini @@ -1,8 +1,9 @@ [runtime-error-in-setInterval.html] type: testharness + expected: TIMEOUT [window.onerror - runtime error in setInterval] - expected: FAIL + expected: NOTRUN [window.onerror - runtime error in setInterval (column)] - expected: FAIL + expected: NOTRUN diff --git a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setTimeout.html.ini b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setTimeout.html.ini index 4ca3ceff69d05..c50330d0e7def 100644 --- a/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setTimeout.html.ini +++ b/tests/wpt/metadata/html/webappapis/scripting/processing-model-2/runtime-error-in-setTimeout.html.ini @@ -1,8 +1,9 @@ [runtime-error-in-setTimeout.html] type: testharness + expected: TIMEOUT [window.onerror - runtime error in setTimeout] - expected: FAIL + expected: NOTRUN [window.onerror - runtime error in setTimeout (column)] - expected: FAIL + expected: NOTRUN