Open youennf opened 2 years ago
cropTo()
should also be allowed to fail for resource-limitation, while we're at it.Barring optimizations, the role of produceCropTarget is to create an association between an interface that cannot be serialized with one that can.¹ There is no reason for this to fail. If I busy-spin on creating these, resource exhaustion seems on par with every other trivial platform object, and we don't go about annotating these with failure paths.
1. "Calling produceCropTarget on an Element of a supported type associates that Element with a CropTarget. This CropTarget may be used as input to cropTo."
I agree with @jan-ivar here.
- I think we should allow it to fail if the implementation does not yet support a specific Element type, for a start. Ideally all implementations will support all Element types. But we don't live in an ideal world.
It seems out of scope of this particular issue, WebIDL should solve this without the spec saying anything.
- It sounds quite reasonable to also allow failure for reasons of resource-limitations in the browser.
Why? This is a pattern we do not see in any spec. As already said, this is a potential privacy issue as implemented in Chrome and I do not see why we should specify behaviours with such hazards.
- I think
cropTo()
should also be allowed to fail for resource-limitation, while we're at it.
That sounds ok to me. cropTo is an expensive method, it already has an error code path, reusing it is not harmful.
It seems out of scope of this particular issue, WebIDL should solve this without the spec saying anything.
Element
should be received, and one browser accepts (IFrame or Div)
, that's an interop issue. Better document it in the spec.(ElementType1 or ElementType2 or ... ElementType2950)
?Why? This is a pattern we do not see in any spec. As already said, this is a potential privacy issue as implemented in Chrome and I do not see why we should specify behaviours with such hazards.
This is not identical, but it's similar:
The set of effective Capabilities may be platform dependent.
For example, on a resource-limited device it may not be possibl
to set properties P1 and P2 both to 'high', while on another
less limited device, this may be possible.
- If we standardize that
Element
should be received, and one browser accepts(IFrame or Div)
Do we have such case? I would hope not. In Chrome's case, my understanding is that the WebIDL might accept HTMLElement and not Element, this is fine and is handled by WebIDL just fine.
- Do we now specify in the IDL that we support
(ElementType1 or ElementType2 or ... ElementType2950)
?
We could if we think this makes sense. Some API support BufferSource, some only ArrayBuffer. If we have to go down there, we should probably rethink our API shape though.
This is not identical, but it's similar:
How is it similar? There are hardware limitations which do not allow to support more than a maximum number of pixels, a UA must have to deal with these limitations. In our case, failing a CropTarget is due to a UA optimisation to reduce latency. The UA could have decided to go with another approach.
Additionally, Chrome's implementation AIUI is failing CropTarget generation once a fixed number of generated CropTarget is reached. This allows observing GC. We usually try to avoid designs that allow observing GC.
Do we have such case? I would hope not.
Sorry, I don't understand this question. What is it attached to?
In Chrome's case, my understanding is that the WebIDL might accept HTMLElement and not Element, this is fine and is handled by WebIDL just fine.
I think you're recalling a different discussion here which is not, IMHO, relevant to the current discussion. Chrome has never implemented HTMLElement OR Element. Chrome has so far only implemented cropping to HTMLDivElement and HTMLIFrameElement.
Implementing cropping to a given set of related Element subclasses, and ensuring it works correctly, is significant work. This work must precede updating the IDL files. We cannot expose untested functionality to unsuspecting Web applications.
Assume we invest this work in all current elements right out of the gate. Great. Now what happens when a new element type is introduced?
It is best if we allow gradual rolling out of features. Web developers clamoring to crop to a DIV now, should not be forced to wait until we implement HTMLSuperObscureElement. They'll do just fine wrapping their obscure element inside of HTMLDivElement or a similar work-around.
In our case, failing a CropTarget is due to a UA optimisation to reduce latency.
In this particular thread (#48), we're also discussing failure due to incomplete implementation. (This particular type of failure ~can~ should be synchronous.)
We usually try to avoid designs that allow observing GC.
Chrome might implement as only freeing up tokens when the (sub-)document is navigated, and not when things are garbaged-collected. At any rate, this is not part of the specification.
we're also discussing failure due to incomplete implementation. (This particular type of failure ~can~ should be synchronous.)
Incomplete implementations are not defined in the spec, you can implement it however you like. Throwing an exception/rejecting a promise might be fine, using WebIDL or synchronous checks. Maybe it might be easier for web developers to generate a valid CropTarget attached to the nearest parent element for which cropping is supported (div or body/iframe). When rolling out support of a new element type, the cropping will become more accurate. Or failing at cropTo time by considering that this was not a valid CropTarget.
In any case, a TypeError synchronous failure is very different from the kind of failures https://github.com/w3c/mediacapture-region/pull/47 is trying to add to the spec. The scope of this issue is about the latter: should we allow crop target production to fail in case of surface state propagation failure?
Chrome has so far only implemented cropping to HTMLDivElement and HTMLIFrameElement.
I am surprised to hear that. I remember stating that kind of possibility in favour of putting this API in element and not mediaDevices as a convenient way to improve feature detection. At the time, I remember you saying this was out of scope since we were going with Element.
Throwing an exception/rejecting a promise might be fine, using WebIDL or synchronous checks.
So let's go for it and have one less outstanding issue. If I send a PR that says that a synchronous error should be raised if the element is not of a type supported by the implementation, will you approve it? We can discuss other failures separately.
At the time, I remember you saying this was out of scope since we were going with Element.
I only have a vague memory of this discussion. The current implementation in Chrome, as of the time of this writing, is here - always has been HTMLDivElement and HTMLIFrameElement
.
Long-term, I expect everyone to support any Element, and feature-discovery to be unimportant. But for the short-term, with only some Elements being supported by implementations, I think the spec should make an affordance. I hope that clarifies my position?
This work must precede updating the IDL files.
@eladalon1983 that's backwards. Specs inform implementation, not the other way around. It seems perfectly fine to standardize what we want at this stage, which is FPWD. @youennf suggested ways an imperfect implementation can handle this, or failing that it's not like WebIDL per se physically prevents an implementation from inventing is own validation on top (it violates the spec, but it's not a technical limitation).
In any case, I agree it is out of scope here, so please open a separate issue.
Specs inform implementation, not the other way around.
It is useful for specifications to reflect abiding reality. This serves Web developers. They often read these specs too, and can regard them as authoritative.
As for "abiding":
In any case, I agree it is out of scope here, so please open a separate issue.
I think the two issues are highly correlated, and revolve around whether the spec should specify that, in reality, the user agent may fail the process, and that spec authors accept this reality.
It is useful for specifications to reflect abiding reality. This serves Web developers. Developers often read these specs too, and who often treat these specs as authoritative.
@eladalon1983 that's not how "authoritative" works. What you appear to be saying is Chrome's implementation is authoritative and the spec is not and should be updated to match. Am I misreading that?
I oppose this view. I think Chrome's implementation is buggy so I've filed crbug 1328836. Let's discuss there.
I think the two issues are highly correlated, and revolve around whether the spec should specify that, in reality, the user agent may fail the process, and that spec authors accept this reality.
No they're not related, unless we conflate "Should generation of CropTarget from elements be able to fail?" with input validation, which is already present. Even Chrome performs input validation:
await navigator.mediaDevices.produceCropId("");
TypeError: Failed to execute 'produceCropId' on 'MediaDevices': The provided value
is not of type '(HTMLDivElement or HTMLIFrameElement)'.
But this is built into WebIDL bindings and happens synchronously. This issue is about whether generation of a valid target can fail.
What you appear to be saying is Chrome's implementation is authoritative and the spec is not and should be updated to match. Am I misreading that?
You are indeed misreading that.
I oppose this view.
We have that in common.
I think Chrome's implementation is buggy
It will be "buggy" once shipped. Right now it's "incomplete", and that's not at all surprising, given how the spec continued to evolve past the point where the Origin Trial started - which is par for the course.
But this is built into WebIDL bindings and happens synchronously. This issue is about whether generation of a valid target can fail.
That the IDL bindings are going to make the implementation reject synchronously, is an implementation detail. The spec says that any Element should be accepted. But this is not necessarily going to happen with Chrome; not at every single given time point as new Elements are specced and implemented. I think it's very likely that Safari and Firefox will behave similarly. Specs are read by both implementers and developers. It's good if it alerts developers to risks such as partial implementations.
It's good if it alerts developers to risks such as partial implementations.
I do not know any spec that does this. I do not see what is so special that would require doing something like this here. MDN does this, maybe this is the place that you are looking for.
If Chrome is shipping a partial implementation, it is important to think of how to complete the implementation in following updates. Implementors do that exercise regularly when implementing specs without requesting spec changes.
If the issue is about supporting element is hard and might not be feasible in some environments, let's discuss this in a separate issue.
What we are discussing here is related to some of the changes being proposed in https://github.com/w3c/mediacapture-region/pull/47. Let's focus on this.
In an out-of-band chat with Harald, he has raised the interesting point of HTMLAudioElement
and similar elements. In other words, some elements simply do not make sense as crop-targets. That means we now have three loosely related issues:
We can discuss these either separately or altogether.
In other words, some elements simply do not make sense as crop-targets.
HTMLAudioElement makes sense to me, but let's move this discussion elsewhere if you wan to have it.
All elements have a bounding box afaik so we are good in terms of spec definition after https://github.com/w3c/mediacapture-region/issues/20. Some bounding boxes will probably be more used than others, that is fine.
We can discuss these either separately or altogether.
This is clearly a separate discussion, please file a separate issue.
Creating a {}
crop target should not fail, because it would be premature to allocate cropping resources at this point for anything other than as a UA optimization. Any page can create cropTargets at any point, with zero knowledge of whether they'll ever be captured, making for a highly unreliable signal that cropping is about to happen.
UA optimizations are encouraged, but carry inherent complexity. I sense a general understanding in the web platform that:
Creating a {} crop target should not fail, because it would be premature to allocate cropping resources at this point
Regardless of whether anything is allocated, it is usually good to fail earlier rather than later, and for the failure to happen close to where things went off-track. Failing in cropTo()
means failing in a different document, maybe even another tab one day [*]. When the failure is really associated with cropTo()
, that's all well and good. But for failures of the type described in bulletpoint 2 of this message, there is no real association with cropTo(), and that allows failing earlier.
-- [*] See #63.
We've already agreed to discuss bulletpoint 2 in https://github.com/w3c/mediacapture-region/issues/55 not here, so let's stick to that. Input type validation is irrelevant here. This issue is about failing generation due to resource allocation, as described in PR https://github.com/w3c/mediacapture-region/pull/47, from which this issue was opened.
Creating a
{}
crop target should not fail, because it would be premature to allocate cropping resources at this point
Allowing random JS in would-be-captured documents to exhaust cropping resources seems highly problematic:
DoS is easily avoided by simply doing IPC and resource allocation in cropTo. With that baseline, any earlier resource allocation is purely UA optimization, whose cost and complexity should be contained to said UA, within the existing API.
In addition to DoS, it might be a potential fingerprinting issue, implementations should really avoid these issues. We should be careful to not introduce such potential pitfalls in specs. Failing later at cropTo time has very limited privacy concerns given user gave permission to do capture.
Also, as discussed before, there are ways to avoid these issues while retaining the benefits of early allocation in non pathological cases. I do not see what prevents Chrome to switch to this kind of implementation.
I also disagree with failing earlier being better. I believe web developers will not account for this and this might lead to very mysterious reasons/hard to debug reasons.
As discussed in https://github.com/w3c/mediacapture-region/issues/17, it has been clarified that Chrome is currently failing CropTarget generation in some cases, in particular if a renderer process generates too many of them.
We should understand two things: