Report feature policy violations

[clelland]

This adds support for reporting violations of the fullscreen feature policy, which should only be triggered when a script actually tries to enter fullscreen in a document where that is not allowed. (Checking fullscreenEnabled, for instance, also involves a feature policy check, but would not trigger a violation report)

---

Preview | Diff

[annevk]

This only reports if the second check fails, but presumably it should also report when the first check fails?

Could we bake reporting into "allowed to use" and suppress it the few times it should not happen? That might make it less likely to be overlooked.

[foolip]

It seems best to only create reports when it the error was because of a policy violation. Doing that in https://html.spec.whatwg.org/multipage/iframe-embed-object.html#allowed-to-use makes sense to me too.

[clelland]

I think there may be cases where the call to 'allowed-to-use' and the actual determination of a policy violation are separated in the spec algorithms. For those cases, are we still going to need to have a separate "report a violation now" entry point, or is it okay to just call allowed-to-use again? -- We could mention somewhere that allowed-to-use should be idempotent, so the result of the first call can be cached.

[annevk]

I guess doing a survey of sorts across callers would be good, to find out the best API contract with the least amount of hassle for callers.

[foolip]

@clelland how will you implement this? Will the checking and the reporting be entirely separate, will there be a flag to the check that says whether to report, or something else? Unless it's convoluted by implementation details that don't matter at the spec level, a similar structure in the spec might make sense.

[clelland]

The current implementation in Chrome has an IsFeatureEnabled() method which takes a bool that determines whether to report immediately on failure or not. There's also a separate ReportFeaturePolicyViolation method that can be called on its own if that's needed. Then it's up to the call sites for each feature to decide whether a failure is a reportable violation or not.

[annevk]

Sounds like the spec should match that pattern, provided it works well for the call sites.

[foolip]

Sounds like a reasonable structure, yeah. @clelland, in this case, would the reporting be implicit or explicit? Seems to me given the structure that making it implicit in the check is the way to make sure a violation is only reported when that's the real reason fullscreen failed?