Webxr: XRSessionInit has generic types and is not future proof

[cabanier]

XRSessionInit takes a sequence of DOMString but wouldn't it be better if it takes an enumeration? There's a predefined set of allowed values so it seems it should be restricted to those.

I think XRSessionInit should also be made a bit more forward looking. We might want to ask for additional features (anchors, meshing, hit testing, etc) and this would be a good place to do it.
What if we change the IDL to the following:

enum XRSpaceType {
  "local",
  "local-floor",
  "bounded-floor",
  "unbounded"
};

dictionary XRSessionFeatures {
  XRSpaceType space;
}

dictionary XRSessionInit {
  XRSessionFeatures requiredFeatures;
  XRSessionFeatures optionalFeatures;
};

Future extension spec can then extend the 'XRSessionFeatures' object with more than just a simple value ie

dictionary XRWorldMeshFeatures {
  readonly attribute FrozenArray<DOMPointReadOnly> boundsGeometry; 
  XRWorldMeshQuality quality;
};

dictionary XRSessionFeatures {
  XRSpaceType space;
  XRWorldMeshFeatures worldMesh;
};

Any thoughts @toji ?

Sorry that I didn't chime in earlier. I was on PTO in early July and didn't notice all these changes going in.

[Manishearth]

XRSessionInit takes a sequence of DOMString but wouldn't it be better if it takes an enumeration? There's a predefined set of allowed values so it seems it should be restricted to those.

This isn't forwards-compatible, if we add an enumeration value for an optional feature a browser that doesn't yet support it will throw a TypeError. Using strings lets us add allowed values without breaking forwards compatibility.

I think XRSessionInit should also be made a bit more forward looking. We might want to ask for additional features (anchors, meshing, hit testing, etc) and this would be a good place to do it.

This is not a problem that needs solving if XRSessionInit is forwards compatible using strings.

[cabanier]

XRSessionInit takes a sequence of DOMString but wouldn't it be better if it takes an enumeration? There's a predefined set of allowed values so it seems it should be restricted to those.

This isn't forwards-compatible, if we add an enumeration value for an optional feature a browser that doesn't yet support it will throw a TypeError. Using strings lets us add allowed values without breaking forwards compatibility.

That is not the case when it's an attribute (per the webidl spec).
The validation is done manually on the C++ side (at least in Chromium) so special handling can be added. For instance, I looked how new permission names are handled and they are being ignored.

I don't feel super strongly about having enum; it just seems a bit cleaner.

I think XRSessionInit should also be made a bit more forward looking. We might want to ask for additional features (anchors, meshing, hit testing, etc) and this would be a good place to do it.

This is not a problem that needs solving if XRSessionInit is forwards compatible using strings.

I don't follow. See my example of XRWorldMeshFeatures. How would you do that with strings?
Like this?

optionalFeatures: [ "world-meshing", "world-meshing-low-quality", "world-meshing-bounds-10-10-10"]

An alternate way of tackling this would be how the Permissions api works. In that case requiredFeatures/optionalFeatures would be list of a basic dictionary type. Each dictionary would have a key to indicate what feature it is. See the Permissions API where PermissionDescriptor is the basic dictionary type.

[Manishearth]

Looking at Firefox's code there is no special validation; it relies on the existing enum typeerror stuff. The error thrown navigator.permissions.query({name: "foo"}) is the generic enum TypeError and comes from generic webidl codegen.

Can you point to where webidl allows invalid enum values? In _interface_ attributes I think they silently fail to set, but I don't think that carries over to dicts.

[Manishearth]

The reason Chrome does manual validation is that Permissions.query accepts an _object_ instead of a dictionary. This is so that the validation error can happen through the promise instead of happening immediately.

I don't follow. See my example of XRWorldMeshFeatures. How would you do that with strings?
Like this?

I misunderstood that part of the proposal, sorry. Using more complex types would be interesting.

[toji]

@Manishearth captured the editors reasoning on this pretty well, and it's an approach that was vetted by ergonomics review within the Chrome org at least.

Obviously the ideal approach from a readability standpoint would be to use an enum to crisply define which values are expected. The fact that requiredFeatures and optionalFeatures should respond to unrecognized values in different ways, though, makes it tricky.

As Manish described, the reason for using strings for optionalFeatures is to allow unknown values to easily be ignored. Yes, it's probably possible to do some sort of special cased bindings override here to allow the enum type to used in the idl file, but that's quite a bit more work for implementers than simply passing a string and specifying a bit of validation on the native side. Also, at least in Chrome, we try to avoid special cased bindings whenever possible, as it makes the code harder to follow and maintain.

requiredFeatures, on the other hand, could actually use the enum directly and get the desired behavior of rejecting on an unknown string, but that assumes that we're only going to be using values from a single enum. This is true for the moment but I expect it won't be for much longer (and in fact we're already considering extending it as part of the ongoing AR module discussion). WebIDL does not allow for unions of enums, and as such the options for allowing features to come from multiple different enums are to accept them as strings and validate in the method algorithm (as we are doing now) or define a new enum which duplicates all the possible feature symbols into it. Given that developers like to avoid duplication of effort when possible and that we were already going to be using strings for the optionalFeatures we chose the former route.

As for future features that can't be cleanly expressed in a string, we've planned for that. We can do unions between strings and other object types, so it won't be difficult to say in the future that the features arrays take either a string or a, say, XRMinimumBoundsRequirement interface, giving us something like so:

navigator.xr.requestSession('immersive-vr', {
  // Experience requires a bounded space with at least 3m x 2m floorspace.
  requiredFeatures: ['bounded-floor', new XRMinimumBoundsRequirement(3, 2)]
}).then(/* ... */);

That is not the case when it's an attribute (per the webidl spec).

I'm not sure what you mean by this. Do you have a link to the section of the spec you're referring to?

[cabanier]

As for future features that can't be cleanly expressed in a string, we've planned for that. We _can_ do unions between strings and other object types, so it won't be difficult to say in the future that the features arrays take either a string or a, say, XRMinimumBoundsRequirement interface, giving us something like so:

navigator.xr.requestSession('immersive-vr', {
  // Experience requires a bounded space with at least 3m x 2m floorspace.
  requiredFeatures: ['bounded-floor', new XRMinimumBoundsRequirement(3, 2)]
}).then(/* ... */);

Wouldn't that also have problems with forward compatibility? For instance, what if you pass the bounds requirement as an optional feature, wouldn't it cause an exception in older browser?

Are people ok with that or do we assume that browser are always up to dat?

That is not the case when it's an attribute (per the webidl spec).

I'm not sure what you mean by this. Do you have a link to the section of the spec you're referring to?

From https://heycam.github.io/webidl/#idl-enums:

In the ECMAScript binding, assignment of an invalid string value to an attribute is ignored

If you look in the Chromium code, you can see that there are places where it ignores unknown enums (for instance in the Permissions API)

[Manishearth]

In the ECMAScript binding, assignment of an invalid string value to an attribute is ignored

Yes, that's _assignment_ to an _attribute_, this is _initialization_ of a _dictionary member_, it's different. Dictionaries throw type errors when they fail to initialize, attributes silently plod on if you assign invalid things to them.

If you look in the Chromium code, you can see that there are places where it ignores unknown enums (for instance in the Permissions API)

The Permissions code is built to work on object (not a dictionary) so it does a lot of the stuff manually, both browsers have webidl codegen that will error if using a dictionary.

Wouldn't that also have problems with forward compatibility?

No, we already ignore unknown strings: https://immersive-web.github.io/webxr/#dom-xrsessioninit-optionalfeatures

[cabanier]

In the ECMAScript binding, assignment of an invalid string value to an attribute is ignored

Yes, that's _assignment_ to an _attribute_, this is _initialization_ of a _dictionary member_, it's different. Dictionaries throw type errors when they fail to initialize, attributes silently plod on if you assign invalid things to them.

You are right! This would be a better way to pass information.
From the Permission API:

Let rootDesc be the object permissionDesc refers to, converted to an IDL value of type PermissionDescriptor. If this throws an exception, return a promise rejected with that exception and abort these steps.

We could use the same mechanism so we can pass in features as enums and be forward compatible. For required features, we'd reject the promise and for optional feature, we ignore them.

If you look in the Chromium code, you can see that there are places where it ignores unknown enums (for instance in the Permissions API)

The Permissions code is built to work on object (not a dictionary) so it does a lot of the stuff manually, both browsers have webidl codegen that will error if using a dictionary.

Wouldn't that also have problems with forward compatibility?

No, we already ignore unknown strings: https://immersive-web.github.io/webxr/#dom-xrsessioninit-optionalfeatures

I see. So the built-in ToString function would convert the dictionary to a string? That feels a little ugly :-)

[Manishearth]

You are right! This would be a better way to pass information.

It's not good API design to accept interfaces instead of dicts for initialization (they're more effort to construct)

We could use the same mechanism

This won't work for optional features though. The reason the Permissions API does that is so that it can move the error into the promise. It doesn't get the ability to ignore non-parsing enum variants from there.

If we wanted to do that we'd basically have to write a new version of this algorithm that ignores invalid enum variants (but only in optionalFeatures). And I don't see any strong benefit to using an enum here: it's not like JS is actually statically type safe, webidl enums are strings anyway.

It seems far more straightforward to do what we're doing now, and add to the list of acceptable strings as time progresses.

If webidl let you union enums with things all of this would have been possible with

dictionary XRSessionInit {
  sequence<XRFeature> requiredFeatures;
  sequence<XRFeature or DOMString> optionalFeatures;
};

but that's not possible right now AIUI

[cabanier]

You are right! This would be a better way to pass information.

It's not good API design to accept interfaces instead of dicts for initialization (they're more effort to construct)

We wouldn't do an interface. It would be an array of objects, or a dictionary of objects

We could use the same mechanism

This won't work for optional features though. The reason the Permissions API does that is so that it can move the error into the promise. It doesn't get the ability to ignore non-parsing enum variants from there.

Yes it does. The language in the permission spec defines custom error handling. For optional features we can specify to ignore the error.

If we wanted to do that we'd basically have to write a new version of this algorithm that ignores invalid enum variants (but only in optionalFeatures). And I don't see any strong benefit to using an enum here: it's not like JS is actually statically type safe, webidl enums are strings anyway.

It seems far more straightforward to do what we're doing now, and add to the list of acceptable strings as time progresses.

If webidl let you union enums with things all of this would have been possible with

dictionary XRSessionInit {
  sequence<XRFeature> requiredFeatures;
  sequence<XRFeature or DOMString> optionalFeatures;
};

but that's not possible right now AIUI

I think this is possible:

 dictionary XRSessionInit {
   sequence<object> requiredFeatures;
   sequence<object> optionalFeatures;
 };

and then define steps in prose as listed in the Permission spec.
ie convert each object to an XRFeature and if that doesn't work to a string (or enum). If that still doesn't work, ignore the object if it's in optionalFeatures or return promise rejected if it's in requiredFeatures.

If we can update the spec to this behavior, UAs that implement the current spec won't have to change their code but the spec would be forward compatible.

[Manishearth]

We wouldn't do an interface. It would be an array of objects, or a dictionary of objects

You can't get the specced behavior of ignoring invalid enums without interfaces.

For optional features we can specify to ignore the error.

Yes, by ignoring all errors wholesale. We have a two-field dictionary to parse, it has a lot of potential error cases, only _some_ of which we wish to ignore (what if you pass a non-array to one of the fields?). We can't use that algorithm directly; we'd have to duplicate its contents.

I think this is possible:

I don't see why this proposal is any better than the current system. What's the benefit of an enum if it isn't being used?

---

I _think_ what you're trying to get at is the following:

dictionary XRFeatureInit { 
   required DOMString name;
   // all other fields optional
 }

dictionary XRSessionInit {
   sequence<DOMString or XRFeatureInit> requiredFeatures;
   sequence<DOMString or XRFeatureInit> optionalFeatures;
 };

This could work, we can continue to use DOMstrings the way we currently do, and we have forward compatibility for more complex features.

There's no benefit to using enums here, I think enums are a distraction in this discussion.

[cabanier]

We wouldn't do an interface. It would be an array of objects, or a dictionary of objects

You can't get the specced behavior of ignoring invalid enums without interfaces.

For optional features we can specify to ignore the error.

Yes, by ignoring all errors wholesale. We have a two-field dictionary to parse, it has a lot of potential error cases, only _some_ of which we wish to ignore (what if you pass a non-array to one of the fields?). We can't use that algorithm directly; we'd have to duplicate its contents.

Yes, the algorithm would be slightly different for required and optional features

I think this is possible:

I don't see why this proposal is any better than the current system. What's the benefit of an enum if it isn't being used?

Look again, I did't put an enum in the IDL.

I _think_ what you're trying to get at is the following:

dictionary XRFeatureInit { 
   required DOMString name;
   // all other fields optional
 }

dictionary XRSessionInit {
   sequence<DOMString or XRFeatureInit> requiredFeatures;
   sequence<DOMString or XRFeatureInit> optionalFeatures;
 };

This could work, we can continue to use DOMstrings the way we currently do, and we have forward compatibility for more complex features.

Yes, that is what I want but earlier in the discussion you said this is not possible in IDL so I proposed to do it in prose (= write out the algorithm so it can be implemented in c++).

There's no benefit to using enums here, I think enums are a distraction in this discussion.

That's fine :-)

[Manishearth]

you said this is not possible in IDL

it's not possible with enums, I _think_ it's possible with dicts.

[cabanier]

you said this is not possible in IDL

it's not possible with enums, I _think_ it's possible with dicts.

Do the UA IDL parsers support this? I'd like to see this in the WebXR spec so we don't have to change it when we write our extension specs.

[toji]

Talked with @BFGeek today about the ergonomics angle and did some experimenting in Chrome today to see what the most reasonable approach to this is. Seems like using the 'any' type in IDL and then specifying in the algorithm which values are accepted is probably the cleanest option?

dictionary XRSessionInit {
  sequence<any> requiredFeatures;
  sequence<any> optionalFeatures;
};

[cabanier]

That seems very open-ended :-)

What was @bfgeek's concern about sequence<DOMString or XRFeatureInit> ?

[toji]

WebGL uses any quite a bit to encapsulate multiple different return values types into a single function. (It's what allows WebGL to have just gl.getParameter() instead separate functions for glGetBooleanv(), glGetFloatv(), glGetIntegerv(), etc.)

I did not discuss the DOMString or XRFeatureInit variant with Ian today because any (or object as you listed above, which is what I initially presented to him) covers that use case equally well without requiring us to preemptively declare an XRFeatureInit type which we don't yet have an explicit use for.

[mounirlamouri]

Do we have example of any usage that isn't WebGL or storage related? As far as I can tell, it's very uncommon for specifications to use any and I'm surprised we believe this is something we believe is going to be an issue here especially given that there are various options to handle this in the future that wouldn't require us to do premature optimisations. For example, we could imagine using or whenever we want to add "advanced" features or even adding a new property to the dictionary.

[mounirlamouri]

And by the way, because permissions was mentioned, the approach that was taken there was that most permissions would mostly be a name but a permission is still described as a dictionary because we knew that some permissions would be more complex. I don't think we should do that here because we have no real "complex" features lined up but that would be better ergonomics than a mix and match of strings and dictionaries.

[rcabanier]

@mounirlamouri I agree. Using any feels too generic.

[toji]

To ensure that we're not losing sight of the reason why we made this change: the concern that @rcabanier raised in the opening comment of this thread was that the API design at the time, which only took DOMString types, was potentially not forwards compatible if we needed to introduce features that could not be captured as simple enums. (Which the editors view as very likely.) We take the forward compatibility of this API very seriously, and there appeared to be a simple way to resolve it, so we took action after the following the topic:

• This thread, which included a suggestion to use arrays of object prior to the WG call.
• Brought up as an agenda item on a WG call, where the discussion culminated in a request that I discuss potential approaches with API ergonomics experts within Chrome.
• Me meeting with @bfgeek to discuss the various approaches we might take.
• Notification of the approach we intended to take on this thread.
• A pull request that was open for review for 2 days prior to merging.

Given the scope of this particular change, the above felt more than adequate. We tend to reserve longer discussion periods for topics of greater impact on the functionality and use of the API.

I'm surprised we believe this is something we believe is going to be an issue here especially given that there are various options to handle this in the future that wouldn't require us to do premature optimisations.

To be precise, the problem is with the optionalFeatures array and running code against older browser versions. optionalFeatures is specified to ignore features it does not recognize, which allows for developers to request features as progressive enhancements.

For example, we could imagine using or whenever we want to add "advanced" features or even adding a new property to the dictionary.

If the spec were updated at some future point to change DOMString to, say, DOMString or XRSomeDictionary, then there would be a period of unknown duration in which passing the dictionary type as an optionalFeature would actually block the session's creation at the IDL bindings level in browsers that conform to the old standard, while allowing the feature in newer implementations. Certainly developers could find a way to feature detect around this, but it violates the contract of the API that unknown inputs will be ignored.

(This is not a problem for requiredFeatures, which is specified to reject on unrecognized inputs, and as such a bindings-level error would be within expected behavior. We changed both at the same time simply for consistency and better control over the error types that are thrown.)

Do we have example of any usage that isn't WebGL or storage related?

As described in the PR any here is used primarily as an alternative to object, which we avoided due to potential confusion in the WebIDL specification about whether or not object allowed for string values, because in Javascript string" instanceof object == false. It is my understanding that the JS object is distinct from the WebIDL object, but the spec does not make that distinction clear. (For the record, Chrome _does_ accept strings where the IDL specifies object, but I have not validated that that's consistent behavior across browsers.) In any case, for the purposes of this discussion I'll consider any and object to be roughly equivalent types, given that the only concrete difference I can see is that object is not nullable by default.

A non-exhaustive search for other Web APIs that use any or object turned up the following, so this doesn't appear to be as uncommon as suggested?

• WebBluetooth
• WebAudio
• WebRTC
• Encrypted Media Extensions
• And in addition to the aforementioned WebGL, the upcoming WebGPU spec also appears to intend on using any.

And, of course, the Permissions API's query method does seems like a reasonably good analog.

[T]he approach that was taken there was that most permissions would mostly be a name but a permission is still described as a dictionary because we knew that some permissions would be more complex.

This is, amusingly, an incredibly precise description of the reason why we made these changes.

Using any feels too generic.

If there is some implicit property of the any type that I'm unaware of that makes it unfit for this purpose, I'd be willing to swap it out for object as long as we can verify that it is indeed a distinct from the JavaScript object type and accepts strings.

That seems very open-ended :-)

I did not read this as a statement of concern about the use of any at the time. I apologize if I misunderstood your intent.

[cabanier]

I did not read this as a statement of concern about the use of any at the time. I apologize if I misunderstood your intent.

I was not too concerned about it. I would have preferred a slightly more restrictive interface (ie sequence<DOMString or XRFeatureInit>) but as long as we can describe it in prose, any is fine.

Thanks for writing up such a great explainer! In a way, it would be nice if the spec could link to this so we (or our successors) can remember why the interface looks this way.

[bzbarsky]

@cabanier The "as long as we can describe it in prose" precondition is being failed, as https://github.com/immersive-web/webxr/issues/860 shows...