New issue
Advanced search Search tips

Issue 871867 link

Starred by 1 user
Status: WontFix
Owner:
peria@chromium.org
Closed: Aug 9
Cc:
peria@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

[RuntimeEnabled] does not work with enum values

Project Member Reported by xhw...@chromium.org, Aug 7 
Chrome Version: M69
OS: All

Example CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1119565

enum MediaKeySessionType {
     "temporary",
     "persistent-license",
     [RuntimeEnabled=EncryptedMediaPersistentUsageRecordSession] "persistent-usage-record",
 };

MediaKeySession createSession(optional MediaKeySessionType sessionType = "temporary")

Now without enabling EncryptedMediaPersistentUsageRecordSession, if I run 

m.createSession('persistent-usage-record')

I get a DOMException (Unsupported session type), while ideally I should get a 

TypeError: ... The provided value 'persistent-usage-record' is not a valid enum valud of tyep MEdiaKeySessionType.

Also, the use of [RuntimeEnabled] on enum values is not documented at

https://chromium.googlesource.com/chromium/src/+/50.0.2661.2/third_party/WebKit/Source/bindings/idl-extended-attributes.md#RuntimeEnabled_i_m_a_c
-
without_flag.png
79.8 KB View Download
with_flag.png
71.5 KB View Download

Comment 1 by fs...@chromium.org, Aug 8

Components: -Blink Blink>Bindings
Status: Available (was: Untriaged)

Comment 2 by raphael....@intel.com, Aug 9

Cc: peria@chromium.org

Comment 3 by peria@chromium.org, Aug 9

Owner: peria@chromium.org
Status: WontFix (was: Available)
We are sorry, but the extended attributes on enumeration values are invalid syntax, and the IDL compiler should stop with an error.
https://heycam.github.io/webidl/#idl-enums

Our IDL parser just accepts them accidentally, and it should be fixed not to allow them.


Please check the value with the runtime enabled flag in the receiver's implementation, as you do in MediaKeys::createSession().

Comment 4 by xhw...@chromium.org, Aug 9 

Thanks for the clarification! I'll drop the [RuntimeEnabled] attribute on the new enum from my CL, but keep the C++ check.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10 

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

commit 36fcc8ace288e10163895764608c716b8eccd9f5
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Fri Aug 10 01:53:24 2018

IDL parser: Drop accepting extended attributes on enum values

For now, IDL parser allows to put extended attributes on enum values,
that is syntactically invalid in Web IDL spec [1].
In Blink, other IDL compiler parts ignore them, and no IDL files have them.

In order to avoid misleading developers, this CL makes IDL parser
to disallow extended attributes on enum values.

[1] https://heycam.github.io/webidl/#idl-enums


Bug:  871867 
Change-Id: Ic57406ce3ff94a0c1a4cf38da12b217c05667eb0
Reviewed-on: https://chromium-review.googlesource.com/1169482
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@{#582009}
[modify] https://crrev.com/36fcc8ace288e10163895764608c716b8eccd9f5/tools/idl_parser/idl_parser.py
►

Sign in to add a comment