New issue
Advanced search Search tips

Issue 621641 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug-Regression

Blocking:
issue 585656



Sign in to add a comment

[OriginTrialEnabled] IDL attribute doesn't work on methods

Project Member Reported by cha...@chromium.org, Jun 20 2016

Issue description

Since the bindings refactor, we've lost the ability for the [OriginTrialEnabled] IDL attribute to be added to methods.

This isn't used currently by any origin trial features, but we should add it back
 
Status: Started (was: Assigned)
"Methods" also includes several classes of generated methods, which should also be supported. Those are triggered by the IDL declarations for:
  - iterable
  - maplike
  - setlike
  - stringifier
  - serializer

Comment 2 by cha...@chromium.org, Jun 24 2016

Blocking: 585656
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 24 2016

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

commit 0fa26a48c0341e8676aaafc1f33fc825d3581709
Author: iclelland <iclelland@chromium.org>
Date: Fri Jun 24 20:09:12 2016

[OriginTrials] Raise an exception at compile time for OT-enabled methods

We don't currently support origin trials on individual methods (methods which
are part of an origin-trial-enabled attribute are supported), and the
generated bindings code for these is incorrect. This patch causes an
exception to be raised at compile time to ensure that noone tries to add
the OriginTrialEnabled attribute to a method.

This also prevents trials from being defined on:
 - Iterators
 - Maplike and Setlike methods
 - Stringifiers and Serializers

As part of this patch, the methods defined in TestObject.idl which were
generating incorrect code have been commented out.

BUG= 621641 
R=chasej@chromium.org, haraken@chromium.org

Review-Url: https://codereview.chromium.org/2090773005
Cr-Commit-Position: refs/heads/master@{#401945}

[modify] https://crrev.com/0fa26a48c0341e8676aaafc1f33fc825d3581709/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
[modify] https://crrev.com/0fa26a48c0341e8676aaafc1f33fc825d3581709/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/0fa26a48c0341e8676aaafc1f33fc825d3581709/third_party/WebKit/Source/bindings/tests/idls/core/TestObject.idl
[modify] https://crrev.com/0fa26a48c0341e8676aaafc1f33fc825d3581709/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 29 2016

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

commit 81ca9f1a235637fffa49775ecf9d62286297dc95
Author: iclelland <iclelland@chromium.org>
Date: Wed Jun 29 14:19:43 2016

Allow origin trials to be declared on IDL operations (methods)

This change follows the changes to the V8 bindings generation code that allowed
origin trials to be defined on attributes and constants. Similarly, it
refactors the various lists of methods which were being passed into the template
context into a single list and several filters which may be applied to it.

This patch covers only the first stage of method support: Individual methods in
full or partial interfaces, with no overloads. Attempting to define an origin
trial on an overloaded method will result in an exception at compile time.

R=haraken@chromium.org, yukishiino@chromium.org
BUG= 621641 

Review-Url: https://codereview.chromium.org/2106983002
Cr-Commit-Position: refs/heads/master@{#402799}

[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origin_trials.js
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/templates/interface.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/templates/interface.h
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/templates/interface_base.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/templates/methods.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/templates/partial_interface.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/tests/idls/core/TestObject.idl
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/tests/idls/modules/TestInterfacePartial4.idl
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceOriginTrialEnabled.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterfacePartial.cpp
[modify] https://crrev.com/81ca9f1a235637fffa49775ecf9d62286297dc95/third_party/WebKit/Source/core/origin_trials/testing/InternalsFrobulateInterfaceBindings.idl

Project Member

Comment 5 by sheriffbot@chromium.org, Jul 4 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Fixed (was: Started)
Marking as fixed; there is still a regression, but the patches already applied cover all of the existing use cases. The only remaining work to be done is to support methods with overloads, but that does not currently apply to origin trials.

Sign in to add a comment