Unions contain unsafe JS<T> values

[jdm]

What we really want are unions that contain JSRef<T> values when used as arguments, and Temporary<T> values when used as return values. This is kind of awkward - maybe we make two variants for each interface that part of a union, eFooArg and eFooRetval. Alternatively, maybe we generate two different unions, FooOrStringArg and FooOrStringRetval.

[Ms2ger]

Two unions, I think, to make sure the compiler checks us.

[jdm]

@nox suggested we could define our unions as enum NodeOrString<T: Assignable<Node>> { eNode(T), eString(DOMString). This would allow us to store NodeOrString<JS<Node>> values, accept NodeOrString<JSRef<Node>> arguments, and return NodeOrString<Temporary<Node>> values. That's pretty compelling.

[Manishearth]

That sounds like a good idea :)

[nox]

If we do this and ever need unions of multiple interfaces, a lint should probably be added to make sure the various type parameters are actually of the same flavour of Assignable.

[jdm]

Such as http://mxr.mozilla.org/servo/source/components/script/dom/webidls/CanvasRenderingContext2D.webidl#9

[nox]

The Assignable trick doesn't work because it doesn't have an impl for Unrooted.

[jdm]

I don't think there's a problem with adding one.

[nox]

@jdm I forgot that it's the same for Root.

[jdm]

Perhaps to get around the Root problem we can instantiate JSRef values from them ahead of time?

[nox]

So my idea is to generate something like this:

pub enum BlobOrString<TBlob: Assignable<Blob>> {
    eBlob(TBlob),
    eString(TDOMString),
}

impl<TBlob: ToJSValConvertible> ToJSValConvertible for BlobOrString<TBlob> {
    fn to_jsval(&self, cx: *mut JSContext) -> JSVal {
        match *self {
            BlobOrString::eBlob(ref inner) => inner.to_jsval(cx),
            BlobOrString::eString(ref inner) => inner.to_jsval(cx),
        }
    }
}

pub struct BlobOrStringConfig<'a> {
    eBlob: &'a mut RootedVec<JS<Blob>>,
}

impl<'a> FromJSValConvertible for BlobOrString<JSRef<'a, Blob>> {
    type Config = BlobOrStringConfig<'a>;
    fn from_jsval(cx: *mut JSContext, value: JSVal, config: BlobOrStringConfig<'a>) -> Result<Self, ()> {
        if value.is_object() {
            match BlobOrString::TryConvertToBlob(cx, value) {
                Err(_) => return Err(()),
                Ok(Some(value)) => {
                    config.eBlob.push(JS::from_unrooted(value));
                    return Ok(BlobOrString::eBlob(???));
                },
                Ok(None) => (),
            }

        }

        match BlobOrString::TryConvertToString(cx, value) {
            Err(_) => return Err(()),
            Ok(Some(value)) => return Ok(BlobOrString::eString(value)),
            Ok(None) => (),
        }


        throw_not_in_union(cx, "Blob, String");
        Err(())
    }
}

It's an… interesting use of FromJSValConvertible::Config to say the least, but that lets us pass RootedVec values from the method bindings and share them between different values from vectors of unions ((Node or String)...) and even from different union arguments ((Foo or Bar) bar, (Foo or Qux) qux). The problem is that to return the JSRef while inserting it to the RootedVec, I either need to use get_unsound_ref_forever or to somehow append it from a &'a value. I'm thinking something similar to a TypedArena would be a fit here.

[nox]

Following what I said, here is what I came with:

/// An arena of items that are rooted for the lifetime of this struct.
#[allow(unrooted_must_root)]
#[jstraceable]
pub struct RootedArena<T: Reflectable> {
    v: UnsafeCell<RootedVec<JS<T>>>,
}

impl<T: Reflectable> RootedArena<T> {
    /// Create an arena of items of type T.
    pub fn new() -> Self {
        RootedArena { v: UnsafeCell::new(RootedVec::new()) }
    }

    /// Push a value into an arena and return a `JSRef` of it.
    #[allow(unrooted_must_root)]
    pub fn push<U: Assignable<T>>(&self, value: U) -> JSRef<T> {
        let value = unsafe { value.get_js() };
        unsafe { &mut *self.v.get() }.push(value);
        JSRef {
            ptr: value.ptr,
            chain: PhantomData,
        }
    }
}

It compiles but I've yet to test it.

[nox]

Can't I simplify everything and make RootedArena store a list of Reflector or even *mut JSObject?

[nox]

Sounds like I just want a RootCollection after all.

[nox]

/// An arena of items that are rooted for the lifetime of this struct.
#[allow(unrooted_must_root)]
#[jstraceable]
pub struct RootedArena {
    collection: RootCollection,
}

impl RootedArena {
    /// Create an arena of items.
    pub fn new() -> Self {
        RootedArena { collection: RootCollection::new() }
    }

    /// Push a value into an arena and return a `JSRef` of it.
    #[allow(unrooted_must_root)]
    pub fn root<T: Reflectable, U: Assignable<T>>(&self, value: U) -> JSRef<T> {
        let value = value.get_js();
        self.collection.root(value.reflector().get_jsobject());
        JSRef {
            ptr: value.ptr,
            chain: PhantomData,
        }
    }
}

Something like this would allow me to use this structure for all union members, wouldn't it?

[nox]

@jdm Implementing Assignable on Unrooted and Root might looks weird because functions such as JS::from_rooted() use that trait. "Unrooted" doesn't sound like it should be an argument to JS::from_rooted().

[jdm]

Yes, that's true. It's why I originally suggested getting a JSRef from those values and using those in places that took Assignable, instead of implementing that trait.

[nox]

I see. After coding I realised I don't need the additional Assignable impls anyway.

[nox]

I am waiting for #6150 to land before resuming work on this.