Bug 952365. Add a TreatNonObjectAsNull annotation for WebIDL callback functions and use it for event handlers, since web sites depend on assigning non-callable objects to them in some cases. r=peterv

This commit is contained in:
Boris Zbarsky 2014-01-13 15:08:56 -05:00
parent b79e424106
commit 0bc28e81b8
8 changed files with 168 additions and 28 deletions

View File

@ -29,7 +29,6 @@ public:
nsIGlobalObject* aIncumbentGlobal)
: CallbackObject(aCallable, aIncumbentGlobal)
{
MOZ_ASSERT(JS_ObjectIsCallable(nullptr, mCallback));
}
JS::Handle<JSObject*> Callable() const

View File

@ -2841,8 +2841,9 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
If lenientFloatCode is not None, it should be used in cases when
we're a non-finite float that's not unrestricted.
If allowTreatNonCallableAsNull is true, then [TreatNonCallableAsNull]
extended attributes on nullable callback functions will be honored.
If allowTreatNonCallableAsNull is true, then [TreatNonCallableAsNull] and
[TreatNonObjectAsNull] extended attributes on nullable callback functions
will be honored.
If isCallbackReturnValue is "JSImpl" or "Callback", then the declType may be
adjusted to make it easier to return from a callback. Since that type is
@ -3737,6 +3738,8 @@ for (uint32_t i = 0; i < length; ++i) {
if type.isCallback():
assert not isEnforceRange and not isClamp
assert not type.treatNonCallableAsNull() or type.nullable()
assert not type.treatNonObjectAsNull() or type.nullable()
assert not type.treatNonObjectAsNull() or not type.treatNonCallableAsNull()
name = type.unroll().identifier.name
if type.nullable():
@ -3758,6 +3761,17 @@ for (uint32_t i = 0; i < length; ++i) {
"} else {\n"
" ${declName} = nullptr;\n"
"}")
elif allowTreatNonCallableAsNull and type.treatNonObjectAsNull():
if not isDefinitelyObject:
haveObject = "${val}.isObject()"
if defaultValue is not None:
assert(isinstance(defaultValue, IDLNullValue))
haveObject = "${haveValue} && " + haveObject
template = CGIfElseWrapper(haveObject,
CGGeneric(conversion),
CGGeneric("${declName} = nullptr;")).define()
else:
template = conversion
else:
template = wrapObjectTemplate(
"if (JS_ObjectIsCallable(cx, &${val}.toObject())) {\n" +
@ -10774,6 +10788,7 @@ class CGCallback(CGClass):
getters=[], setters=[]):
self.baseName = baseName
self._deps = idlObject.getDeps()
self.idlObject = idlObject
name = idlObject.identifier.name
if isJSImplementedDescriptor(descriptorProvider):
name = jsImplName(name)
@ -10801,6 +10816,13 @@ class CGCallback(CGClass):
methods=realMethods+getters+setters)
def getConstructors(self):
if (not self.idlObject.isInterface() and
not self.idlObject._treatNonObjectAsNull):
body = "MOZ_ASSERT(JS_ObjectIsCallable(nullptr, mCallback));"
else:
# Not much we can assert about it, other than not being null, and
# CallbackObject does that already.
body = ""
return [ClassConstructor(
[Argument("JS::Handle<JSObject*>", "aCallback"), Argument("nsIGlobalObject*", "aIncumbentGlobal")],
bodyInHeader=True,
@ -10808,7 +10830,8 @@ class CGCallback(CGClass):
explicit=True,
baseConstructors=[
"%s(aCallback, aIncumbentGlobal)" % self.baseName,
])]
],
body=body)]
def getMethodImpls(self, method):
assert method.needThisHandling
@ -10872,6 +10895,7 @@ class CGCallback(CGClass):
class CGCallbackFunction(CGCallback):
def __init__(self, callback, descriptorProvider):
self.callback = callback
CGCallback.__init__(self, callback, descriptorProvider,
"CallbackFunction",
methods=[CallCallback(callback, descriptorProvider)])
@ -11176,7 +11200,8 @@ class CallbackMethod(CallbackMember):
replacements = {
"errorReturn" : self.getDefaultRetval(),
"thisObj": self.getThisObj(),
"getCallable": self.getCallableDecl()
"getCallable": self.getCallableDecl(),
"callGuard": self.getCallGuard()
}
if self.argCount > 0:
replacements["argv"] = "argv.begin()"
@ -11185,7 +11210,7 @@ class CallbackMethod(CallbackMember):
replacements["argv"] = "nullptr"
replacements["argc"] = "0"
return string.Template("${getCallable}"
"if (!JS_CallFunctionValue(cx, ${thisObj}, callable,\n"
"if (${callGuard}!JS_CallFunctionValue(cx, ${thisObj}, callable,\n"
" ${argc}, ${argv}, rval.address())) {\n"
" aRv.Throw(NS_ERROR_UNEXPECTED);\n"
" return${errorReturn};\n"
@ -11206,6 +11231,11 @@ class CallCallback(CallbackMethod):
def getPrettyName(self):
return self.callback.identifier.name
def getCallGuard(self):
if self.callback._treatNonObjectAsNull:
return "JS_ObjectIsCallable(cx, mCallback) && "
return ""
class CallbackOperationBase(CallbackMethod):
"""
Common class for implementing various callback operations.
@ -11244,6 +11274,9 @@ class CallbackOperationBase(CallbackMethod):
'%s'
'}\n' % CGIndenter(CGGeneric(getCallableFromProp)).define())
def getCallGuard(self):
return ""
class CallbackOperation(CallbackOperationBase):
"""
Codegen actual WebIDL operations on callback interfaces.

View File

@ -876,6 +876,9 @@ class IDLInterface(IDLObjectWithScope):
if identifier == "TreatNonCallableAsNull":
raise WebIDLError("TreatNonCallableAsNull cannot be specified on interfaces",
[attr.location, self.location])
if identifier == "TreatNonObjectAsNull":
raise WebIDLError("TreatNonObjectAsNull cannot be specified on interfaces",
[attr.location, self.location])
elif identifier == "NoInterfaceObject":
if not attr.noArguments():
raise WebIDLError("[NoInterfaceObject] must take no arguments",
@ -1411,6 +1414,10 @@ class IDLType(IDLObject):
assert self.tag() == IDLType.Tags.callback
return self.nullable() and self.inner._treatNonCallableAsNull
def treatNonObjectAsNull(self):
assert self.tag() == IDLType.Tags.callback
return self.nullable() and self.inner._treatNonObjectAsNull
def addExtendedAttributes(self, attrs):
assert len(attrs) == 0
@ -2690,10 +2697,7 @@ class IDLAttribute(IDLInterfaceMember):
def handleExtendedAttribute(self, attr):
identifier = attr.identifier()
if identifier == "TreatNonCallableAsNull":
raise WebIDLError("TreatNonCallableAsNull cannot be specified on attributes",
[attr.location, self.location])
elif identifier == "SetterThrows" and self.readonly:
if identifier == "SetterThrows" and self.readonly:
raise WebIDLError("Readonly attributes must not be flagged as "
"[SetterThrows]",
[self.location])
@ -2937,6 +2941,7 @@ class IDLCallbackType(IDLType, IDLObjectWithScope):
argument.resolve(self)
self._treatNonCallableAsNull = False
self._treatNonObjectAsNull = False
def isCallback(self):
return True
@ -2982,8 +2987,13 @@ class IDLCallbackType(IDLType, IDLObjectWithScope):
for attr in attrs:
if attr.identifier() == "TreatNonCallableAsNull":
self._treatNonCallableAsNull = True
elif attr.identifier() == "TreatNonObjectAsNull":
self._treatNonObjectAsNull = True
else:
unhandledAttrs.append(attr)
if self._treatNonCallableAsNull and self._treatNonObjectAsNull:
raise WebIDLError("Cannot specify both [TreatNonCallableAsNull] "
"and [TreatNonObjectAsNull]", [self.location])
if len(unhandledAttrs) != 0:
IDLType.addExtendedAttributes(self, unhandledAttrs)

View File

@ -54,3 +54,18 @@ def WebIDLTest(parser, harness):
threw = True
harness.ok(threw, "Should have thrown.")
parser = parser.reset()
threw = False
try:
parser.parse("""
[TreatNonCallableAsNull, TreatNonObjectAsNull]
callback Function = any(any... arguments);
""")
results = parser.finish()
except:
threw = True
harness.ok(threw, "Should have thrown.")

View File

@ -31,4 +31,5 @@ support-files =
[test_namedNoIndexed.html]
[test_queryInterface.html]
[test_sequence_wrapping.html]
[test_treat_non_object_as_null.html]
[test_traceProtos.html]

View File

@ -0,0 +1,39 @@
<!DOCTYPE HTML>
<html>
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=952365
-->
<head>
<meta charset="utf-8">
<title>Test for Bug 952365</title>
<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
<script type="application/javascript">
/** Test for Bug 952365 **/
var onvolumechange;
var x = {};
(function() {
onvolumechange = x;
ise(onvolumechange, x,
"Should preserve an object value when assigning to event handler");
// Test that we don't try to actually call the non-callable object
window.dispatchEvent(new Event("volumechange"));
onvolumechange = 5;
ise(onvolumechange, null,
"Non-object values should become null when assigning to event handler");
})();
</script>
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=952365">Mozilla Bug 952365</a>
<p id="display"></p>
<div id="content" style="display: none">
</div>
<pre id="test">
</pre>
</body>
</html>

View File

@ -3,20 +3,63 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<div id="log"></div>
<button type="button" id="test">Start test</button>
<script>
var t = async_test("Event handler listeners should be registered when they are first set.");
var i = 0;
var uncalled = "t.step(function() { assert_unreached('First event handler.') })"
var button = document.getElementById('test');
button.onclick = {};
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 1) }), false);
button.setAttribute('onclick', uncalled); // event handler listener is registered here
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 3) }), false);
button.onclick = t.step_func(function () { assert_equals(++i, 2); });
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 4) }), false);
button.click()
assert_equals(button.getAttribute("onclick"), uncalled)
assert_equals(i, 4);
t.done()
var objects = [{}, function() {}, new Number(42), new String()];
var primitives = [42, null, undefined, ""];
objects.forEach(function(object) {
var t = async_test("Event handler listeners should be registered when they " +
"are first set to a object value (" +
format_value(object) + ").");
t.step(function() {
var i = 0;
var uncalled = "t.step(function() { assert_unreached('First event handler.') })"
var button = document.createElement('button');
button.onclick = object; // event handler listener is registered here
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 2) }), false);
button.setAttribute('onclick', uncalled);
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 3) }), false);
button.onclick = t.step_func(function () { assert_equals(++i, 1); });
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 4) }), false);
button.click()
assert_equals(button.getAttribute("onclick"), uncalled)
assert_equals(i, 4);
t.done()
});
});
primitives.forEach(function(primitive) {
var t = async_test("Event handler listeners should be registered when they " +
"are first set to a object value (" +
format_value(primitive) + ").");
t.step(function() {
var i = 0;
var uncalled = "t.step(function() { assert_unreached('First event handler.') })"
var button = document.createElement('button');
button.onclick = primitive;
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 1) }), false);
button.setAttribute('onclick', uncalled); // event handler listener is registered here
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 3) }), false);
button.onclick = t.step_func(function () { assert_equals(++i, 2); });
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 4) }), false);
button.click()
assert_equals(button.getAttribute("onclick"), uncalled)
assert_equals(i, 4);
t.done()
});
});
var t = async_test("Event handler listeners should be registered when they " +
"are first set to an object value.");
t.step(function() {
var i = 0;
var uncalled = "t.step(function() { assert_unreached('First event handler.') })"
var button = document.createElement('button');
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 1) }), false);
button.setAttribute('onclick', uncalled); // event handler listener is registered here
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 3) }), false);
button.onclick = t.step_func(function () { assert_equals(++i, 2); });
button.addEventListener('click', t.step_func(function () { assert_equals(++i, 4) }), false);
button.click()
assert_equals(button.getAttribute("onclick"), uncalled)
assert_equals(i, 4);
t.done()
});
</script>

View File

@ -10,17 +10,17 @@
* Opera Software ASA. You are granted a license to use, reproduce
* and create derivative works of this document.
*/
[TreatNonCallableAsNull]
[TreatNonObjectAsNull]
callback EventHandlerNonNull = any (Event event);
typedef EventHandlerNonNull? EventHandler;
[TreatNonCallableAsNull]
[TreatNonObjectAsNull]
// https://www.w3.org/Bugs/Public/show_bug.cgi?id=23489
//callback OnBeforeUnloadEventHandlerNonNull = DOMString (Event event);
callback OnBeforeUnloadEventHandlerNonNull = DOMString? (Event event);
typedef OnBeforeUnloadEventHandlerNonNull? OnBeforeUnloadEventHandler;
[TreatNonCallableAsNull]
[TreatNonObjectAsNull]
callback OnErrorEventHandlerNonNull = boolean ((Event or DOMString) event, optional DOMString source, optional unsigned long lineno, optional unsigned long column);
typedef OnErrorEventHandlerNonNull? OnErrorEventHandler;