Report body interfaces

[domenic]

Looking at Chromium's implementation, I see a lot of "report body" interfaces:

• ReportBody
• CSPViolationReportBody
• CoopAccessViolationReportBody
• DeprecationReportBody
• InterventionReportBody
• DocumentPolicyViolationReportBody
• FeaturePolicyViolationReportBody

However, most of these don't exist in specifications. And, many of them are using [NoInterfaceObject], which no longer exists on the web platform (it was removed from Web IDL).

I'm unsure how the spec is supposed to work here, and the specs definitely don't seem to match implementations.

• The Reporting API's "queue a violation report" algorithm takes an "object". But, the ReportingObserver API uses ReportBody, so is there an implicit requirement that such objects be subclasses of ReportBody?

• Some specs appear to fill this out using a generic "a new object" language, e.g. Fetch and COEP. This does not involve any interfaces. The proposed COOP reporting spec in Cross origin opener policy reporting whatwg/html#5518 does the same, although as noted above, the in-progress Chromium implementation seems to use an interface. How do these things show up in ReportingObserver?

• I can't figure out what the CSP spec is doing, but it certainly doesn't use CSPViolationReportBody.

• I can't find any spec for InterventionReportBody or DeprecationReportBody.

• DocumentPolicyViolationReportBody doesn't exist in Document Policy, and I can't find where that spec queues reports. Maybe that part isn't written yet?

• Permissions Policy seems to be fully on board the interface train: it declares a proper, non-NoInterfaceObject report body interface, and sends that through the "queue a violation report" algorithm.

This seems like a pretty big mess.

My recommendation if we were starting from scratch would be to not use interfaces for this, and have "queue a violation report" take either an object, or something more structured (like a record<USVString, any> or an Infra ordered map) to give guidance to callers. Implementations could unobservably use IDL dictionaries behind the scenes if they wanted, but specs would use the Fetch/COEP/COOP style of just supplying a table of keys/values. And we'd get rid of ReportBody entirely and make ReportingObserver's body just an object? instead of a ReportBody?.

However, we're not starting from scratch, so I'm unsure what the best path forward is.

/cc @clelland @camillelamy @mikewest

[jpchase]

Re not finding a spec for InterventionReportBody or DeprecationReportBody. In #189, crash, deprecation and intervention reports were moved to their own separate repos:
Crash reporting (spec)
Deprecation reporting (spec)
Intervention reporting (spec)

All three of these use the full interface approach, similar to Permissions Policy.

[domenic]

Thanks! That certainly tips the balance, at least in specs space, toward the full-interface approach. Although it feels a little unnecessary to create new constructor-less classes for things that are just data holders (see w3ctag/design-principles#11), it's not the end of the world. It's the current inconsistency that's troublesome, and in particular as HTML editor reviewing the COEP and COOP spec patches, I'm not sure what to do.

[clelland]

• Document Policy should define its report type; that's an omission.
• Chrome needs to remove the "NoInterfaceObject" from the various body types. I just filed crbug.com/1122656 for that.
• In ReportingObserver, these objects (as the 'body' property of the observed Report) are instances of their respective types, and are not simply ES dictionaries. The Reporting spec just says

Each report has a body, which is either null or an object which can be serialized into a JSON text. The fields contained in a report’s body are determined by the report’s type.

• Crash reports are necessarily not visible to reporting observers; it doesn't really matter what we do with those as long as the serialized report is correct.

[clelland]

We could switch to plain ES objects; there's really no benefit that I can see to using classes. It was added to support ReportingObserver; I suspect that it was originally so that we could use IDL to express the structure and the members that would be present.

[domenic]

You could still use dictionaries, either behind the scenes in implementations, or in specs if desired (although I'm not sure that would buy much).

[jpchase]

There was #164 where toJSON() wasn't working in Chromium, but there were also spec changes made. Just pointing it out, in case it matters here.

[domenic]

Yeah, using normal JS objects (dictionaries) would be a more natural way of solving that, instead of having the code generator generate a bunch of getters and a toJSON() method.

[RByers]

Following up from discussion of this on blink-dev.

The principle of using dictionaries when there's only data makes sense to me. But dictionaries can't be used as attributes and Report has a body attribute.

[domenic]

I think Report itself can be a dictionary, looking at how it's used?

[clelland]

Yes, Report could also be a dictionary; I don't believe that there are any constraints on dictionaries being dictionary members.

[RByers]

I think Report itself can be a dictionary, looking at how it's used?

Oh I see, make Report a dictionary and then ReportBody isn't an attribute anymore so all of them can be dictionaries too. Yeah that makes sense. I'm just looking over to try to understand if there would be any non-trivial web compat implications of this. @clelland thoughts?

[clelland]

I can't see any compat issues -- without the interface object currently being exposed, there doesn't seem to be any way to get the type of a Report or ReportBody -- so there shouldn't be any type-detection going on right now that would fail if we switched to dictionaries.

Reports (and their bodies) currently look like Objects to a script, and that wouldn't change. (The "Report" and e.g. "DeprecationReportBody" names are visible in DevTools' representation of the reports, so I'm not 100% sure that there's no way to discover them currently, but I don't believe it's likely.

[clelland]

(Not that anyone should be using something other than the type member to do report-type detection; I was just checking to see if it's currently even possible)

[RByers]

Yeah it looks like it is possible in theory, eg:

> window.speechSynthesis.getVoices()[0][Symbol.toStringTag]
'SpeechSynthesisVoice'

But poking around at this stuff (like __proto__ etc.) has always been somewhat implementation defined AFAIK, so I'm not worried about it as a real compat risk in practice.

[RByers]

Alright, sounds like we're converging on just moving all of these to dictionaries, right?

[clelland]

That's the one that does it... rpt.body[Symbol.toStringTag] reports the internal type.

That would change to Object for a dictionary, I suspect, if anyone were actually relying on it. It does seem substantially less likely to be a compatibility risk, for sure.

[clelland]

Moving to dictionaries makes sense to me -- I've put up a PR at #258 for that.

@bakulf, do you think this would work for Gecko?