mirror of
https://github.com/servo/servo.git
synced 2025-08-03 04:30:10 +01:00
Auto merge of #19644 - Xanewok:root-seq-any, r=jdm
Root sequence<any> params using CustomAutoRooter
<!-- Please describe your changes on the following line: -->
Attempt at https://github.com/servo/servo/issues/16678. At the moment this pulls an external [remove-handle-conv](https://github.com/Xanewok/rust-mozjs/tree/remove-handle-conv) branch for rust-mozjs (removing `FromJSValConvertible` implementation for `HandleValue` and adding one for `*mut JSObject`)
In short, this roots the `sequence<any>` and `sequence<object>` function arguments using `CustomAutoRooter` (introduced in https://github.com/servo/rust-mozjs/pull/382).
As per https://github.com/servo/servo/issues/16678#issuecomment-302224110 the underlying vector contains raw gc thing pointers instead of handles. To do that, this introduces new possible `isMember = "AutoRoot"` value in CodegenRust.py.
The rooted argument is passed to a function wrapped in a `CustomAutoRooterGuard` [RAII root guard](ed5e37a288/src/rust.rs (L586-L588)
) (e.g. as `CustomAutoRooterGuard<Vec<JSVal>>`)
I felt quite out of my depth when trying to adapt CodegenRust.py code, so I'd appreciate any help with how can be done better/in a more clean manner.
Also the name `CustomAutoRooterGuard` is quite lengthy, so I'm also open to changing it (`AutoRoot`? `AutoRootGuard`?)
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [x] These changes fix #16678 (github issue number if applicable).
<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19644)
<!-- Reviewable:end -->
This commit is contained in:
commit
989d2fd532
8 changed files with 91 additions and 10 deletions
|
@ -562,6 +562,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
|
|||
isDefinitelyObject=False,
|
||||
isMember=False,
|
||||
isArgument=False,
|
||||
isAutoRooted=False,
|
||||
invalidEnumValueFatal=True,
|
||||
defaultValue=None,
|
||||
treatNullAs="Default",
|
||||
|
@ -702,7 +703,8 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
|
|||
if type.isSequence() or type.isRecord():
|
||||
innerInfo = getJSToNativeConversionInfo(innerContainerType(type),
|
||||
descriptorProvider,
|
||||
isMember=isMember)
|
||||
isMember=isMember,
|
||||
isAutoRooted=isAutoRooted)
|
||||
declType = wrapInNativeContainerType(type, innerInfo.declType)
|
||||
config = getConversionConfigForType(type, isEnforceRange, isClamp, treatNullAs)
|
||||
|
||||
|
@ -1038,10 +1040,14 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
|
|||
assert not isEnforceRange and not isClamp
|
||||
assert isMember != "Union"
|
||||
|
||||
if isMember == "Dictionary":
|
||||
if isMember == "Dictionary" or isAutoRooted:
|
||||
# TODO: Need to properly root dictionaries
|
||||
# https://github.com/servo/servo/issues/6381
|
||||
declType = CGGeneric("Heap<JSVal>")
|
||||
if isMember == "Dictionary":
|
||||
declType = CGGeneric("Heap<JSVal>")
|
||||
# AutoRooter can trace properly inner raw GC thing pointers
|
||||
else:
|
||||
declType = CGGeneric("JSVal")
|
||||
|
||||
if defaultValue is None:
|
||||
default = None
|
||||
|
@ -1238,6 +1244,7 @@ class CGArgumentConverter(CGThing):
|
|||
isEnforceRange=argument.enforceRange,
|
||||
isClamp=argument.clamp,
|
||||
isMember="Variadic" if argument.variadic else False,
|
||||
isAutoRooted=type_needs_auto_root(argument.type),
|
||||
allowTreatNonObjectAsNull=argument.allowTreatNonCallableAsNull())
|
||||
template = info.template
|
||||
default = info.default
|
||||
|
@ -1260,8 +1267,15 @@ class CGArgumentConverter(CGThing):
|
|||
else:
|
||||
assert not default
|
||||
|
||||
arg = "arg%d" % index
|
||||
|
||||
self.converter = instantiateJSToNativeConversionTemplate(
|
||||
template, replacementVariables, declType, "arg%d" % index)
|
||||
template, replacementVariables, declType, arg)
|
||||
|
||||
# The auto rooting is done only after the conversion is performed
|
||||
if type_needs_auto_root(argument.type):
|
||||
self.converter.append(CGGeneric("auto_root!(in(cx) let %s = %s);" % (arg, arg)))
|
||||
|
||||
else:
|
||||
assert argument.optional
|
||||
variadicConversion = {
|
||||
|
@ -5698,6 +5712,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
|
|||
'js::panic::maybe_resume_unwind',
|
||||
'js::panic::wrap_panic',
|
||||
'js::rust::GCMethods',
|
||||
'js::rust::CustomAutoRooterGuard',
|
||||
'js::rust::define_methods',
|
||||
'js::rust::define_properties',
|
||||
'js::rust::get_object_class',
|
||||
|
@ -6421,9 +6436,24 @@ def type_needs_tracing(t):
|
|||
assert False, (t, type(t))
|
||||
|
||||
|
||||
def type_needs_auto_root(t):
|
||||
"""
|
||||
Certain IDL types, such as `sequence<any>` or `sequence<object>` need to be
|
||||
traced and wrapped via (Custom)AutoRooter
|
||||
"""
|
||||
assert isinstance(t, IDLObject), (t, type(t))
|
||||
|
||||
if t.isType():
|
||||
if t.isSequence() and (t.inner.isAny() or t.inner.isObject()):
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
def argument_type(descriptorProvider, ty, optional=False, defaultValue=None, variadic=False):
|
||||
info = getJSToNativeConversionInfo(
|
||||
ty, descriptorProvider, isArgument=True)
|
||||
ty, descriptorProvider, isArgument=True,
|
||||
isAutoRooted=type_needs_auto_root(ty))
|
||||
declType = info.declType
|
||||
|
||||
if variadic:
|
||||
|
@ -6437,6 +6467,9 @@ def argument_type(descriptorProvider, ty, optional=False, defaultValue=None, var
|
|||
if ty.isDictionary() and not type_needs_tracing(ty):
|
||||
declType = CGWrapper(declType, pre="&")
|
||||
|
||||
if type_needs_auto_root(ty):
|
||||
declType = CGTemplatedType("CustomAutoRooterGuard", declType)
|
||||
|
||||
return declType.define()
|
||||
|
||||
|
||||
|
|
|
@ -38,6 +38,7 @@ use dom_struct::dom_struct;
|
|||
use js::jsapi::{HandleObject, HandleValue, Heap, JSContext, JSObject};
|
||||
use js::jsapi::{JS_NewPlainObject, JS_NewUint8ClampedArray};
|
||||
use js::jsval::{JSVal, NullValue};
|
||||
use js::rust::CustomAutoRooterGuard;
|
||||
use script_traits::MsDuration;
|
||||
use servo_config::prefs::PREFS;
|
||||
use std::borrow::ToOwned;
|
||||
|
@ -449,6 +450,14 @@ impl TestBindingMethods for TestBinding {
|
|||
fn PassCallbackFunction(&self, _: Rc<Function>) {}
|
||||
fn PassCallbackInterface(&self, _: Rc<EventListener>) {}
|
||||
fn PassSequence(&self, _: Vec<i32>) {}
|
||||
#[allow(unsafe_code)]
|
||||
unsafe fn PassAnySequence(&self, _: *mut JSContext, _: CustomAutoRooterGuard<Vec<JSVal>>) {}
|
||||
#[allow(unsafe_code)]
|
||||
unsafe fn AnySequencePassthrough(&self, _: *mut JSContext, seq: CustomAutoRooterGuard<Vec<JSVal>>) -> Vec<JSVal> {
|
||||
(*seq).clone()
|
||||
}
|
||||
#[allow(unsafe_code)]
|
||||
unsafe fn PassObjectSequence(&self, _: *mut JSContext, _: CustomAutoRooterGuard<Vec<*mut JSObject>>) {}
|
||||
fn PassStringSequence(&self, _: Vec<DOMString>) {}
|
||||
fn PassInterfaceSequence(&self, _: Vec<DomRoot<Blob>>) {}
|
||||
|
||||
|
|
|
@ -263,6 +263,9 @@ interface TestBinding {
|
|||
void passCallbackFunction(Function fun);
|
||||
void passCallbackInterface(EventListener listener);
|
||||
void passSequence(sequence<long> seq);
|
||||
void passAnySequence(sequence<any> seq);
|
||||
sequence<any> anySequencePassthrough(sequence<any> seq);
|
||||
void passObjectSequence(sequence<object> seq);
|
||||
void passStringSequence(sequence<DOMString> seq);
|
||||
void passInterfaceSequence(sequence<Blob> seq);
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue