Timing of initial inputsourceschange event

[Manishearth]

https://immersive-web.github.io/webxr/#list-of-active-xr-input-sources

Each XRSession has a list of active XR input sources (a list of XRInputSource) which MUST be initially an empty list.

This means that if the device already has input sources connected from the start, it needs to immediately fire an inputsourceschange event after the session is created. Threejs relies on this happening.

However, there's a period of time in which the session exists but is not yet available to user code (when the promise hasn't been resolved yet, or the promise has resolved and the resolution microtasks haven't triggered yet). In such a scenario, firing an inputsourceschange event is not useful since nothing can catch it. The current design of the spec seems to have a bit of a pit of failure where codebases can reasonably assume that all input sources will arrive through an inputsourceschange event, but it is technically legal for implementors to fire these events before JS code is practically able to catch it.

I feel like we should either:

• Change the spec wording to allow the input sources list to be non empty at the beginning, and urge framework authors to not make this assumption
• Add spec text that explicitly triggers the add input source algorithm for existing input sources right after the "resolve" line in the requestSession() algorithm

Servo currently does the former, but that's currently incorrect. Chrome seems to be doing the latter, but it's not explicit, so it might be possible that there's actually a race there.

cc @alcooper91

[Manishearth]

Add spec text that explicitly triggers the add input source algorithm for existing input sources right after the "resolve" line in the requestSession() algorithm

This might not be correct either, the correct thing to do here may be to spawn a task doing this at this point.

Looks like we already queue a task within "add input source", so this is fine. But it's important to highlight that that task has to be queued for this to work right, we cannot immediately fire the event.

Honestly this feels like it hinges too much on the precise scheduling of tasks, no matter what. I think given that resolve() will use a microtask, using "queue a task" to ensure something happens after it is correct, but it also gives me the spec heebie jeebies, I do not like relying on this.

It also means that you can only rely on this if you're running requestSession(...).resolve(....), if you're resolving later it may not work out and you will have to check the initial array anyway. It might be better to pick and document the former approach, as opposed to choosing the latter approach where applications still should check the initial array but now it's much less obvious why.

[Manishearth]

It's been pointed out to me that we can even have the resolve happen after the "queue a task to fire events" because of how microtasks work (the microtask for resolve() callbacks will be fulfilled before any new task). This is nice in that it's typical to not do anything in a task after resolving a promise. However, this gives me even more spec heebie jeebies.

[Manishearth]

cc @cabanier oculus browser may also have this race, worth looking into

[cabanier]

cc @cabanier oculus browser may also have this race, worth looking into

Thanks! I will take a look

[cabanier]

@Manishearth Is the issue that the inputsourceschange event can be missed because the author didn't get the session yet?

[Manishearth]

Yes, because there's a gap between when the session sets up its resources and when resolve(..) has its callbacks run

[cabanier]

If the event was missed, the list of controllers should not be empty.
Maybe authors should check for controllers when the session resolves (in addition to when the event is generated) and not rely on it being empty.

There should be a note in the spec to clarify this behavior.

[Manishearth]

Right, threejs is currently incorrect by making this assumption. Our options are:

• document this possible issue very strongly and hope nobody else makes the same mistake as Three
• change the spec so this isn't an issue anymore
• do both: harden up implementations, but document the behavior as still possible, so the related issues of attaching listeners even later than resolve isn't a problem

I slightly prefer the second option because the failure mode of (1) is a very subtle race and I'm not certain that authors will notice it.

But I'm down for any of these options, I just want us to do something

[cabanier]

I don't quite follow. Can you elaborate what would be the spec change?
Also, why would we make a change if it doesn't always solve it? It seems just leaving the normative part and just adding a note would be better...

[Manishearth]

The spec change would be to:

• Tweak https://immersive-web.github.io/webxr/#dom-xrsystem-requestsession so that after the "resolve promise" step, we have an "if there are any existing input sources on session, queue a task to fire an XRInputSourceChange event with these sources" step
• Tweak https://immersive-web.github.io/webxr/#xrsession-add-input-source to clarify that we only start monitoring for input sources after the "resolve promise" step in the requestSession query has executed.

The reason I'd like to make the change is that given that Chrome never starts off with input sources attached based on their architecture (but may have a rare race condition around it), authors may assume this to always be the case, and I'd like to avoid the pit of failure by asking user agents to implement this robustly.

[Manishearth]

@cabanier any thoughts? should we go with allowing the array to start empty, or tweaking the spec as listed above to be more robust? I can try and write a PR for it.

[cabanier]

I think we should tweak the spec.

[Manishearth]

#1002