New issue
Advanced search Search tips

Issue 788196 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[OriginTrialEnabled] does not work for Dictionary members

Project Member Reported by cha...@chromium.org, Nov 23 2017

Issue description

Currently, adding [OriginTrialEnabled] to the IDL definition of a Dictionary member is ignored, and has no affect on the generated bindings code. This should work similarly to [RuntimeEnabled], which does generate conditional logic in the bindings.

In most cases, dictionary members are not visible to JavaScript. However, they are visible when returned from a method, e.g. in navigator.mediaDevices.getSupportedConstraints().

Support for [OriginTrialEnabled] on dictionary members allow for trials when they are exposed to script.
 

Comment 1 by mek@chromium.org, Nov 27 2017

Even when a dictionary is passed to a method it is still script visible if a particular dictionary member is read or not, right? At least I thought that's how in some cases websites are expected to do feature detection for dictionary members.

Comment 2 by cha...@chromium.org, Nov 27 2017

Yes, I believe you're correct. I was thinking in terms of there's no way to simply construct a dictionary, and inspect the members.

I guess it's more accurate to say that dictionary members are script visible when passed to a consuming method, or when a method constructs and returns a dictionary.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2017

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

commit a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b
Author: Jason Chase <chasej@chromium.org>
Date: Thu Nov 30 23:20:25 2017

Support [OriginTrialEnabled] on IDL Dictionary members

This CL adds support for applying the [OriginTrialEnabled] attribute to
individual members of a dictionary in IDL. The resulting behaviour is
similar to the existing support for [RuntimeEnabled].

For [RuntimeEnabled], the existing behaviour is that a controlled member on a
dictionary will be ignored in the input to a method, unless the flag is enabled.
Specifically, the member value provided in JavaScript will not be copied to the
internal C++ data representation. This same behaviour has been implemented for
[OriginTrialEnabled].

The other change is to apply both [RuntimeEnabled] and [OriginTrialEnabled] to
dictionaries created internally, and output to JavaScript. Previously, all
dictionary members were returned, regardless if [RuntimeEnabled] was applied.
NOTE: This means that there could be some dictionary members that were always
visible to JavaScript, but will be no longer. e.g. MediaTrackSupportedConstraints:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/mediastream/MediaDevices.idl?type=cs&l=14

Bug:  788196 
Change-Id: I110aa286fa4ff237b9716a6bce136093e25d9e4e
Reviewed-on: https://chromium-review.googlesource.com/794670
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Jason Chase <chasej@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520739}
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials.js
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/scripts/code_generator.py
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/scripts/v8_dictionary.py
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp.tmpl
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/core/core_idl_files.gni
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/core/testing/OriginTrialsTest.h
[modify] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/core/testing/OriginTrialsTest.idl
[add] https://crrev.com/a4364d123a9c13e3ed6e67011ea7a73e5c4ae79b/third_party/WebKit/Source/core/testing/OriginTrialsTestDictionary.idl

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 1 2017

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

commit 12280690f8935c430978eb1b19749b41944d3f94
Author: Jason Chase <chasej@chromium.org>
Date: Fri Dec 01 04:15:58 2017

Minor refactor to origin trials bindings

The bindings for origin trials had two utility functions to detect the
presence of the [OriginTrialEnabled] attribute on a definition. Both of
these functions were being used semi-interchangeably to check for origin
trials.

This CL merges the two utility functions to simplify the logic.

Also, there was some basic support for a [FeaturePolicy] attribute in
the utility function. This logic was removed, as the [FeaturePolicy]
was never used, and is not intended to be used in the future.

Bug:  788196 
Change-Id: I2eda8e25147d30e251a1d9d076abe9a4f4b29c8a
Reviewed-on: https://chromium-review.googlesource.com/802015
Commit-Queue: Jason Chase <chasej@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520851}
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/code_generator.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/v8_dictionary.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp.tmpl
[modify] https://crrev.com/12280690f8935c430978eb1b19749b41944d3f94/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp

Status: Fixed (was: Available)

Sign in to add a comment