1359269 - Add support for extended attributes on types in Web IDL

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Domenic Denicola]

Web IDL was recently revised to add extended attributes on types: https://github.com/heycam/webidl/pull/286. The existing extended attributes [EnforceRange], [Clamp], and [TreatNullAs] were moved to this model, so that they can no longer be used on attributes/arguments, but instead only on types. For example.

This is now making its way into specifications, see e.g. https://github.com/whatwg/html/pull/2580. It will also be crucial for SharedArrayBuffer API safelisting (although from what I understand Gecko has a workaround for that for now).

Gecko's IDL parser should be updated for this change.

Comment 1

[Andrew Overholt [:overholt]]

Peter, is this something you can take?

Comment 2

[Manish Goregaokar [:manishearth]]

I added support for this in Servo (which shares the webidl parser), might try to upstream it

https://github.com/servo/servo/pull/22874

Comment 3

[Boris Zbarsky [:bzbarsky]]

That would be much appreciated! There are some codegen bits that would need to be fixed too, but I think the parser is the bulk of the work.

Comment 4

[Manish Goregaokar [:manishearth]]

An interesting complication I'm hitting is that this should also remove the ability to use these attributes on dictionary members and attrs, however we use extra attributes like [ChromeOnly] in a bunch of places, so we'll have to special-case parsing for those (not something I needed to do on the servo side).

I'll probably make it such that they continue to work the same, but resolve the parser ambiguity for attributes on the inside.

Comment 5

[Manish Goregaokar [:manishearth]]

Do we have tests for ensuring that things like [Clamp] etc work? Or do we just rely on their usage in the rest of the code?

Comment 6

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 1: Add parser support for attributes on types in WebIDL;

Comment 7

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 2: Add codegen support for attributes on types in WebIDL;

Depends on D19733

Comment 8

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 3: Add a codegen test;

Depends on D19734

Comment 9

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 4: Update HTML spec idl files to use attributes on types;

See https://github.com/whatwg/html/pull/2580

Depends on D19735

Comment 10

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 5: Add support for attributes on types in arguments;

Depends on D19736

Comment 11

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 6: Add support for attributes on types in dictionaries;

Depends on D19737

Comment 12

[Manish Goregaokar [:manishearth]]

This should work.

A potential bit of cleanup I can do is to stop carrying around clamp/enforce/treatnullas info on arguments, attributes, and dictionaries, instead choosing to forward those to their child types. Not sure if I need to.

Comment 13

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 7: Add ArgumentRest entry to simplify parsing;

Depends on D19738

Comment 14

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 8: Make it a hard error to apply type attributes on arguments, attrs, and dict members when it's unambiguous, update webidls;

Depends on D20058

Comment 15

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 9: Clean up codegen now that Clamp/EnforceRange can only be applied to types;

Depends on D20059

Comment 16

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 10: Make it a hard error to apply TreatNullAs on non-types;

Depends on D20060

Comment 17

[Manish Goregaokar [:manishearth]]

Added patches making things hard errors, and updating specs.

Comment 18

[Boris Zbarsky [:bzbarsky]]

I'm sorry about the review lag here. I needed to sit down and carefully check the changes against the spec grammar... Future reviews should be much faster.

Comment 19

[Manish Goregaokar [:manishearth]]

Addressed review comments for everything except writing tests). Working on these now.

Comment 20

[Manish Goregaokar [:manishearth]]

Tests pushed

Comment 21

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1359269 - Part 11: Add a bunch of parser tests;

Depends on D20061

Comment 22

[Manish Goregaokar [:manishearth]]

Updated.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb694b612b1b
Part 1: Add parser support for attributes on types in WebIDL; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/c3f37539cb31
Part 2: Add codegen support for attributes on types in WebIDL; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/7fd8a3c1f827
Part 3: Add a codegen test; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/7705b1ebb154
Part 4: Update HTML spec idl files to use attributes on types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/4bb00dc53459
Part 5: Add support for attributes on types in arguments; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/1430913c5e9e
Part 6: Add support for attributes on types in dictionaries; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/bfb153c7f9c3
Part 7: Add ArgumentRest entry to simplify parsing; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/8f9509e82439
Part 8: Make it a hard error to apply type attributes on arguments, attrs, and dict members when it's unambiguous, update webidls; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/ae30401e7988
Part 9: Clean up codegen now that Clamp/EnforceRange can only be applied to types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/6c09eccc4bd1
Part 10: Make it a hard error to apply TreatNullAs on non-types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/00461a29f650
Part 11: Add a bunch of parser tests; r=bzbarsky

Comment 24

[Manish Goregaokar [:manishearth]]

passing try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6d348185d4417c62361c08294d7e9f6c572a418

Comment 25

[Dorel Luca [:dluca]]

Backed out 11 changesets (bug 1359269) for Linting failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=231399589&repo=autoland&lineNumber=944

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=00461a29f65058cb569c8a75e286d0c1ba6aff63&selectedJob=231399589

Backout:
https://hg.mozilla.org/integration/autoland/rev/0c70617150d9a41cb2f2f4fafcab9ae50c15ddd1

Comment 26

[Manish Goregaokar [:manishearth]]

Ah, some failures in the testsuite got introduced with the changes. Fixed.

Comment 27

[Manish Goregaokar [:manishearth]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a9c5def58739d411422d29ee7e9727fe62e9e32

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21deba7e36c5
Part 1: Add parser support for attributes on types in WebIDL; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/87d4c51be96f
Part 2: Add codegen support for attributes on types in WebIDL; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/ee5af9ad9e45
Part 3: Add a codegen test; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/3fccace0ea88
Part 4: Update HTML spec idl files to use attributes on types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/f4ec249fc52a
Part 5: Add support for attributes on types in arguments; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/a64f79c456c9
Part 6: Add support for attributes on types in dictionaries; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/b267c4012f94
Part 7: Add ArgumentRest entry to simplify parsing; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/eadc7a9fc56c
Part 8: Make it a hard error to apply type attributes on arguments, attrs, and dict members when it's unambiguous, update webidls; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/b26067ba0f78
Part 9: Clean up codegen now that Clamp/EnforceRange can only be applied to types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/759c00e21197
Part 10: Make it a hard error to apply TreatNullAs on non-types; r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/4166cae81546
Part 11: Add a bunch of parser tests; r=bzbarsky

Comment 29

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/21deba7e36c5
https://hg.mozilla.org/mozilla-central/rev/87d4c51be96f
https://hg.mozilla.org/mozilla-central/rev/ee5af9ad9e45
https://hg.mozilla.org/mozilla-central/rev/3fccace0ea88
https://hg.mozilla.org/mozilla-central/rev/f4ec249fc52a
https://hg.mozilla.org/mozilla-central/rev/a64f79c456c9
https://hg.mozilla.org/mozilla-central/rev/b267c4012f94
https://hg.mozilla.org/mozilla-central/rev/eadc7a9fc56c
https://hg.mozilla.org/mozilla-central/rev/b26067ba0f78
https://hg.mozilla.org/mozilla-central/rev/759c00e21197
https://hg.mozilla.org/mozilla-central/rev/4166cae81546