CaptureController.setFocusBehavior() while promise pending or after it is rejected
eladalon1983 opened this issue · 8 comments
The use cases we had in mind for CaptureController.setFocusBehavior() were:
- Calling setFocusBehavior() before getDisplayMedia() is called.
- Calling setFocusBehavior() immediately after getDisplayMedia()'s promise is resolved.
There are two additional options which we did not explicitly consider:
3) Calling setFocusBehavior() after the promise is rejected.
4) Calling setFocusBehavior() while the promise is pending.
I think we should:
- Throw if (3).
- Make (4) equivalent to (1).
Call to action: Do we agree on this?
Assuming we do agree - what's the most elegant way of getting this into the spec? Ideally, we'd not need to replace all of gDM's "return a promise rejected with X" steps with "if there is a controller, set some internal slot to a value, and regardless of the presence of a controller, return a promise rejected with X."
- Throw if (3).
That seems consistent with throwing in case source is stopped.
- Make (4) equivalent to (1).
This makes sense, I assumed until now this was already the case.
Assuming we do agree - what's the most elegant way of getting this into the spec? Ideally, we'd not need to replace all of gDM's "return a promise rejected with X" steps with "if there is a controller, set some internal slot to a value, and regardless of the presence of a controller, return a promise rejected with X."
One possibility is to replace the [IsBound] by [GetDisplayPromise].
Then in setFocusBehavior, check whether [GetDisplayPromise] is a rejected promise.
If so, throw.
An alternative is to add a line like:
- Upon rejection of p, set controller [FocusDecisionFinalized] (or another dedicated slot) to true.
Thanks, Youenn. I think we're aligned on what the spec should say, then, and are working out how to say it. I like all options presented, but let's devil's advocate away.
One possibility is to replace the [IsBound] by [GetDisplayPromise]. Then in setFocusBehavior, check whether [GetDisplayPromise] is a rejected promise. If so, throw.
To do this, the getDisplayMedia algorithm should be modified to say "let p be a new promise" earlier, set it in the internal slot, then whenever it currently throws, it'd need to modify the local variable p before returning it. It's similar to the approach in my original post, where we'd have to modify every "reject" line. I am OK with this option, but I prefer the next one.
An alternative is to add a line like:
- Upon rejection of p, set controller [FocusDecisionFinalized] (or another dedicated slot) to true.
I think this would be much preferable. But I don't think it can refer to p, which is only defined at step 10 (as of the time of writing). It'd be more along the lines of "if the algorithm returns a rejected promise, and if controller is not null, then set controller.[FocusDecisionFinalized] to true."
Another option occurs to me - adding an exit-step at the end of the algorithm, and all steps that read "return x" will read "go to exit step with x as retval." Wdyt?
We could do like mediacapture-main and use something like jump to the step labeled...
We would create p upfront, and the step would reject p with the provided error.
That's what I mean with an exit-step, yes.
Overall agree. Regarding how to spec it, the two options have slightly different timing: Upon rejection of p queues a microtask, which might be an observable side-effect for spec-writing convenience, so I'd prefer the exit-step.
It might be slightly better to set the slot value in the micro task happening just before the micro task where the web application will know the promise is rejected.
Otherwise, it allows the web application to synchronously know whether getDisplayMedia was synchronously rejected, which seems unusual.
Good observation, though maybe it's a natural consequence of our unusual pattern? But more importantly, do we want setFocusBehavior throw or not here?
const controller = new CaptureController();
const p = navigator.mediaDevices.getDisplayMedia({controller}); // rejects e.g. over lack of user gesture
controller.setFocusBehavior("focus-captured-surface"); // to throw or not to throw...If we do, then that is detecting failure synchronously. Why is that problematic? I think I'd prefer not throwing at all over throwing based on timing.