DOM/WebIDL Review Checklist

A review checklist for DOM Peers while doing WebIDL reviews. This is not exhaustive; things will be added as needed.

The general idea of WebIDL review is to review the interface, spec, and implementation of something before we expose it to the web.

IDL follows spec

• The IDL has a link to the relevant specification. If there isn't one, the API must not be exposed to the web in general.
• The IDL matches the IDL in the spec.
  • Double-check that our use of nullable sigils (?) matches the spec.
  • Double-check that our Exposed values match the spec.

Spec makes sense

• The API should be easy to understand and use, and consistent.
• ExposureGuidelines should be followed for the feature and the reviewer should review the intent to prototype/ship. E.g., this means checking whether other UAs plan to implement this spec, and if not why we're doing it.
• New readonly properties on global scopes should almost certainly be [Replaceable] to not break sites using variables or functions with a colliding name. This is especially salient for short names.
• Whether things are nullable or not needs to make sense.
• Whether arguments and dictionary members are optional or not needs to make sense.
• Exposed values should make sense.
• It uses SecureContext (see Secure Contexts Everywhere if not).
• The order in which checks that lead to exceptions being thrown happen should be defined in cases when the method can have multiple exceptional conditions.
• The spec should almost certainly not be using the term "active document". If it is, think extra carefully about what happens when that document is not same-origin with any of the other things the spec is talking about.
• The spec is being reasonable with its usage of incumbent/entry/current/relevant globals, settings objects, and so forth.
• The spec handles origins that stringify to "null" correctly.

Implementation is correct

• Check that the new implementation is exposed in the right places (e.g. test-only stuff not being exposed to the web, things that should be behind prefs actually being behind prefs, etc).
• Check that the implementation follows the spec, as much as is possible without domain expertise.
• All functions with non-nullable return values must never return null.
• All functions that accept nullable or optional values (as arguments, dictionary members, etc) must properly handle the null or "not present" cases.
• Exceptions thrown from functions should generally be specified. If there are non-specified exceptions being thrown, evaluate whether it's because Gecko has a state web specs assume can't arise (e.g. window without a document) or whether it's because the spec missed a case it should be handling.
• If the API has [Exposed=SomethingOtherThanWindow]:
  • Audit the API implementation to make sure it doesn't assume the existence of a Window or a Document
  • Require basic test coverage for each of the contexts in which the API is exposed.
• Implementation should use the argument types described in our binding layer documentation, not whatever internal types bindings are using (e.g. no types in the binding::detail namespace, no NonNull<T> arguments where T& would work, etc).
• Check that the GetParentObject() methods of various objects, combined with how those objects get constructed, associate them with the correct global.
• Any JSContexts being used by the implementation come from the right places (passed in, AutoJSAPI, AutoEntryScript, depending on the desired semantics).

Further reading

Please refer to the WebIDL spec or the documentation for our binding layer in order to double check your understanding of the IDL as needed.