New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 740865 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
OoO until Feb 4th
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task

Blocked on:
issue 654707
issue 740870
issue 740871
issue 740875



Sign in to add a comment

bindings: Remove support for WebIDL arrays

Project Member Reported by raphael....@intel.com, Jul 11 2017

Issue description

WebIDL arrays (e.g. "T[] bar") were removed from the spec back in 2015 with https://github.com/heycam/webidl/pull/52.

The bindings layer still accepts those, so it would be good to drop support and clean up the code. We need to drop arrays from //tools/idl_parser, the Python scripts, the Jinja2 templates as well as some C++ code.

But before any of that, we also need to remove existing uses of arrays in our IDL files, so I'll be filing other bugs blocking this one to track the removal.
 
Blockedon: 740870
Blockedon: 740871
Blockedon: 740875
Blockedon: 619665
Cc: foolip@chromium.org
We're now blocked only on bug 619665. I've repeatedly tried to get feedback there but never got an answer.

+foolip in case there are other ways to get noticed by the MSE people I'm not aware of.

Comment 6 by foolip@chromium.org, Aug 10 2017

If that's the only thing left blocking removing the syntax, then I'd just convert it to `[SameObject] FrozenArray<DOMString>` and assume the spec will follow.
> and assume the spec will follow

Given that the entire TrackDefault IDL was removed from the spec (or spec v1 at least), and the latest spec version to have it had a getKinds() instead (https://www.w3.org/TR/2016/CR-media-source-20160705/#widl-TrackDefault-getKinds-sequence-DOMString), using `[SameObject] FrozenArray<DOMString>' would solve the WebIDL problem, but make TrackDefault.idl deviate from any published version of the spec. I guess it's OK from an interop point of view in this case?

Comment 8 by foolip@chromium.org, Aug 11 2017

I spoke too quickly, not checking the spec. Since that now has getKinds(), it's better to make a non-observable change as you've done in https://chromium-review.googlesource.com/c/612250/ and make the change to getKinds() later. Having the FrozenArray<T> behavior in between would be risk with no upside, even if a small risk.
Blockedon: -619665
I've landed https://chromium-review.googlesource.com/c/612250/ which updates DefaultTrack#hints' type, so bug 619665 is no longer blocking this issue.
Status: Assigned (was: Available)
Owner: raphael....@intel.com
Blockedon: 654707
Status: Started (was: Assigned)
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 17 2017

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

commit 8794c1e7a28bf9a3299df18641df87332b8becd1
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Aug 17 08:27:12 2017

bindings: Remove support for WebIDL arrays.

WebIDL arrays (e.g. "T[] bar") were removed from the spec back in 2015 with
https://github.com/heycam/webidl/pull/52. Previous users should use
sequences or FrozenArrays most of the time.

Now that all existing WebIDL array users have been fixed, we can finally
adapt to the change and remove support for arrays from //tools/idl_parser as
well as the bindings layer and related tests.

Bug:  740865 
Change-Id: I9edfcb7b88963ea92836f7ed93945ad7edd48603
Reviewed-on: https://chromium-review.googlesource.com/616661
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#495108}
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/LayoutTests/fast/dom/idl-dictionary-unittest-expected.txt
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/LayoutTests/fast/dom/idl-dictionary-unittest.html
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/LayoutTests/fast/dom/idl-union-type-unittest-expected.txt
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/LayoutTests/fast/dom/idl-union-type-unittest.html
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/scripts/idl_definitions.py
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/scripts/idl_types.py
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/scripts/v8_types.py
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/templates/union_container.cpp.tmpl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/idls/core/TestObject.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/idls/core/TestTypedefs.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.h
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.h
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/DictionaryTest.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/DictionaryTest.h
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/InternalDictionary.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/UnionTypesTest.cpp
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/UnionTypesTest.h
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/third_party/WebKit/Source/core/testing/UnionTypesTest.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/tools/idl_parser/idl_parser.py
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/tools/idl_parser/test_parser/callback_web.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/tools/idl_parser/test_parser/interface_web.idl
[modify] https://crrev.com/8794c1e7a28bf9a3299df18641df87332b8becd1/tools/idl_parser/test_parser/typedef_web.idl

Labels: M-62
Status: Fixed (was: Started)
Victory!
That's great, thanks Raphael!

Sign in to add a comment