New issue
Advanced search Search tips

Issue 819112 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Cleanup IDL files for type annotations

Project Member Reported by peria@chromium.org, Mar 6 2018

Issue description

Web IDL defines "annotated types" and those extended attributes can't be set on others.
Beside it, because of the historical reason, some .idl files still have those extended attributes on members.
Let's update IDL files and IDL compilers to check if type annotations are valid.
 

Comment 1 by peria@chromium.org, Mar 6 2018

Invalid use cases of [TreatNullAs]. All specs for them are fixed to use annotated types.
- core/html/HTMLBodyElement.idl
- core/html/HTMLFontElement.idl
- core/html/HTMLTableCellElement.idl
- core/html/HTMLTableElement.idl
- core/html/HTMLFrameElement.idl
- core/html/HTMLImageElement.idl
- core/html/HTMLIFrameElement.idl
- core/html/HTMLObjectElement.idl
- core/html/forms/HTMLTextAreaElement.idl
- core/html/HTMLTableRowElement.idl
- core/html/forms/HTMLInputElement.idl

Comment 2 by bashi@chromium.org, Mar 6 2018

What do you mean by "invalid"? It would be helpful to refer the spec and what it should be. We have some entries for TreatNullAs like issue 391194. Should we obsolete these or they are different issues?

Comment 3 by peria@chromium.org, Mar 7 2018

Invalid use cases of [EnforceRange]. Those specs do not have type annotation for the cases.

- core/offscreencanvas/OffscreenCanvas.idl
- core/css/cssom/CSSUnitValue.idl

Comment 4 by peria@chromium.org, Mar 7 2018

I used 'invalid use cases' to point the cases that "applicable to types" extended attribute is used on other than types; e.g.
  [TreatNullAs=EmptyString] attribute DOMString foo;

Comment 5 by peria@chromium.org, Mar 7 2018

Few tests also have [EnforceRange] on attribute.
And I think WebCryptoAPI and OffscreenCanvas need spec updates.
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93164ab38bce1dd72dcb931e2156e1590be99598

commit 93164ab38bce1dd72dcb931e2156e1590be99598
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Wed Mar 07 04:57:57 2018

idl: Move TreatNullAs annotation to types

An extended attribute [TreatNullAs] annotates string types in IDL, so
it should be placed just before type names, like
  [TreatNullAs=EmptyString] DOMString
and HTML spec says to so.
This CL changes the IDL style to replace [TreatNullAs] annotations,
and has no behavior changes.


Bug: 819112
Change-Id: I7e9c2a9f9f995a9627d62529e71465f238399587
Reviewed-on: https://chromium-review.googlesource.com/952724
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541349}
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLBodyElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLFontElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLFrameElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLIFrameElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLImageElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLObjectElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLTableCellElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLTableElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/HTMLTableRowElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/forms/HTMLInputElement.idl
[modify] https://crrev.com/93164ab38bce1dd72dcb931e2156e1590be99598/third_party/WebKit/Source/core/html/forms/HTMLTextAreaElement.idl

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a61c77321fa4fd5305c470f766b8bc0ab201132

commit 5a61c77321fa4fd5305c470f766b8bc0ab201132
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Sat Mar 10 16:08:01 2018

idl: Fix [EnforceRange] annotations

This CL fixes usages of [EnforceRange] annotations in IDL files.
No changes on behaviors.

CSSUnitValue:
 [EnforceRange] can't annotate non-integer numbers, and the spec
 doesn't have the annotation.

OffscreenCanvas:
 The spec doesn't have [EnforceRange], but it seems the spec is wrong.
 (requesting to update the spec; https://github.com/whatwg/html/issues/3540)

IDBFactory and IDBIndex:
 Follows spec.

Test IDLs:
 This CL works for tests in Chromium repository, and following tests
 need to be upstream for WPT.
 - wpt/interfaces/IndexedDB.idl
 - wpt/interfaces/WebCryptoAPI.idl


Bug: 819112
Change-Id: If7082cba0251a83e269fea428340b3b61d4c17cf
Reviewed-on: https://chromium-review.googlesource.com/952589
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542375}
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/LayoutTests/external/wpt/interfaces/IndexedDB.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/LayoutTests/external/wpt/interfaces/WebCryptoAPI.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/bindings/tests/idls/core/TestObject.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/core/css/cssom/CSSUnitValue.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/core/testing/TypeConversions.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/modules/indexeddb/IDBFactory.idl
[modify] https://crrev.com/5a61c77321fa4fd5305c470f766b8bc0ab201132/third_party/WebKit/Source/modules/indexeddb/IDBIndex.idl

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44fad229dff3d629d6dea9472ce4f3faa7dd59de

commit 44fad229dff3d629d6dea9472ce4f3faa7dd59de
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Tue Jun 26 06:58:22 2018

bindings: Drop InterfaceGarbageCollected from IDL compiler tests

Now all interfaces are on Oilpan heap, and we don't support [GarbageCollected].
Hence we don't have to have tests for InterfaceGarbageCollected
separately from Interface.


Bug: 819112
Change-Id: I7ef1d9e373944836e61194419d6d4c12300d4610
Reviewed-on: https://chromium-review.googlesource.com/1114398
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570341}
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/idls/core/test_dictionary.idl
[delete] https://crrev.com/ed9e7a0392386f78fab6144bc2da6f5e4bbdcd5c/third_party/blink/renderer/bindings/tests/idls/core/test_interface_garbage_collected.idl
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/idls/core/test_object.idl
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/results/core/test_dictionary.cc
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/results/core/test_dictionary.h
[delete] https://crrev.com/ed9e7a0392386f78fab6144bc2da6f5e4bbdcd5c/third_party/blink/renderer/bindings/tests/results/core/test_interface_garbage_collected_or_string.cc
[delete] https://crrev.com/ed9e7a0392386f78fab6144bc2da6f5e4bbdcd5c/third_party/blink/renderer/bindings/tests/results/core/test_interface_garbage_collected_or_string.h
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/results/core/v8_test_dictionary.cc
[delete] https://crrev.com/ed9e7a0392386f78fab6144bc2da6f5e4bbdcd5c/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_garbage_collected.cc
[delete] https://crrev.com/ed9e7a0392386f78fab6144bc2da6f5e4bbdcd5c/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_garbage_collected.h
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
[modify] https://crrev.com/44fad229dff3d629d6dea9472ce4f3faa7dd59de/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.h

Update status.
third_party/WebKit/LayoutTests/external/wpt/interfaces/DOM-Parsing.idl uses [EnforceRange] for attribute.
third_party/WebKit/LayoutTests/external/wpt/interfaces/FileAPI.idl uses [Clamp] before 'optional', in Blob.slice().


Status: Assigned (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a5ebcdc6650fa2cbfd3894c45e9d4564e933316

commit 8a5ebcdc6650fa2cbfd3894c45e9d4564e933316
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Mon Oct 15 12:23:57 2018

IDL: Fix IDL description of IDBObjectStore

An extended attribute [EnforceRange] is applicable to integer types,
and not applicable to others, e.g. arguments.
This CL fixes the folloing wrong usage of [EnforceRange], and it
follows the spec of IDBOjbectStore.
https://w3c.github.io/IndexedDB/#object-store-interface


Bug: 819112
Change-Id: I65fa19cfd4d1119213390a54dba3247ee0cd2b4a
Reviewed-on: https://chromium-review.googlesource.com/c/1280622
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599616}
[modify] https://crrev.com/8a5ebcdc6650fa2cbfd3894c45e9d4564e933316/third_party/blink/renderer/modules/indexeddb/idb_object_store.idl

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebf32031547a6d3a7b9f29ebb9bd3296ed0f0d66

commit ebf32031547a6d3a7b9f29ebb9bd3296ed0f0d66
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Tue Oct 16 08:30:53 2018

IDL: Fix IDL grammar in CSSStyleDeclaration

[TreatNullAs] is applicable on DOMString, so the previous declaration
  [TreatNullAs] optional DOMString
violates WebIDL grammer.
This CL fixes it.
Its spec also needs to be updated.


Bug: 819112
Change-Id: Ifabca102c3969f3421817e41ad0d3f726643d047
Reviewed-on: https://chromium-review.googlesource.com/c/1281462
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599913}
[modify] https://crrev.com/ebf32031547a6d3a7b9f29ebb9bd3296ed0f0d66/third_party/blink/renderer/core/css/css_style_declaration.idl

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fdd99242518b07d1d48e6f8453cbb7963a27cb6a

commit fdd99242518b07d1d48e6f8453cbb7963a27cb6a
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Oct 25 07:10:48 2018

IDL: Fix wrong usages of [TreatNullAs]

[TreatNullAs] is an extended attribute which is applicable to
DOMString type.
This CL fixes following wrong use cases of it in Blink,
but does not change renderer behaviors.

TestDictionary: applied to ByteString
CSSStyleDeclaration: applied to attribute, spec conformance
Window: applied to an union type, and spec does not have it on |url| parameter.


Bug: 819112
Change-Id: I8769248ee48f7c1fad1562ac5c017a49e79cbb1f
Reviewed-on: https://chromium-review.googlesource.com/c/1298813
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602629}
[modify] https://crrev.com/fdd99242518b07d1d48e6f8453cbb7963a27cb6a/third_party/blink/renderer/bindings/tests/idls/core/test_dictionary.idl
[modify] https://crrev.com/fdd99242518b07d1d48e6f8453cbb7963a27cb6a/third_party/blink/renderer/bindings/tests/results/core/test_dictionary.h
[modify] https://crrev.com/fdd99242518b07d1d48e6f8453cbb7963a27cb6a/third_party/blink/renderer/bindings/tests/results/core/v8_test_dictionary.cc
[modify] https://crrev.com/fdd99242518b07d1d48e6f8453cbb7963a27cb6a/third_party/blink/renderer/core/css/css_style_declaration.idl
[modify] https://crrev.com/fdd99242518b07d1d48e6f8453cbb7963a27cb6a/third_party/blink/renderer/core/frame/window.idl

IDL files should have been fixed, and we've removed supports of extended attributes applicable to types that are not in annotated types.
I'll close this after introducing a verifier.

Sign in to add a comment