New issue
Advanced search Search tips

Issue 831346 link

Starred by 1 user
Status: Assigned
Owner:
cha...@chromium.org
Cc:
feature-...@grotations.appspotmail.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Origin trials should support the implied_by attribute on runtime flags

Project Member Reported by cha...@chromium.org, Apr 10 2018 
Origin trials are defined by adding configuration to a runtime flag (see runtime_enabled_features.json5). As a result, various code is generated to check if the defined trial is enabled, and also expose IDL members to JavaScript.

Runtime flags can define an "implied_by" relationship, which allows a feature to be enabled by other runtime flags.

The generated origin trials code includes a fallback to check the underlying runtime flag, but does not follow the "implied_by" relationship for a related trial.

The origin trials framework should add support for the implied_by relationship.

Comment 1 by cha...@chromium.org, Apr 12 2018 

The fix in CL 1008850 has a redundant check for implied trials. Leaving the extra check in that CL until WebXR is verified to be working. Plan is to remove after branch.

Comment 2 by cha...@chromium.org, Apr 12 2018 

Also need to add instructions to runtime_enabled_features.json5:
- Origin trials can only be implied by another flag if that flag is also an origin trial
- The reverse is not supported. If A (no origin trial) is implied by B (has origin trial), then enabling B via trial will not enable the flag for A. Depending on your perspective, that may or may not seem obvious.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 12 2018 

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

commit a34e731a1ac656550f5eb0709c5a6b535e160906
Author: Jason Chase <chasej@chromium.org>
Date: Thu Apr 12 20:53:40 2018

Add implied by relationships for origin trials

This CL adds support for origin trials to follow the implied_by
relationships defined for the runtime flags.

The implied_by relationship only applies if both of the runtime
flags are configured with an origin trial name.

The implied by relationship is actually implemented in two ways:
1) The generated OriginTrials::<name>Enabled methods will now
   check the defined trial, and any implied by trials
2) When a token enables a trial, any implied trials are also enabled
   at the same time.

Approach (1) doesn't handle the case of dynamically added tokens. In
theory, approach (2) makes (1) unnecessary. Keeping (1) and (2) for
now to ensure all cases are handled.

Bug: 831346
Change-Id: Ibab588f11c69d31f1acd9e599ee917fb036dd32b
Reviewed-on: https://chromium-review.googlesource.com/1008850
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Jason Chase <chasej@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550352}
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/implied-disabled-worker.js
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/implied-enabled-worker.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials-worker.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-disabled.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-enabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-disabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-enabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-script-added.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-script-added.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/make_origin_trials.py
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/templates/origin_trials.cc.tmpl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/templates/origin_trials.h.tmpl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/origin_trials/origin_trial_context.cc
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/origin_trials_test.h
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/origin_trials_test.idl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/v8/web_core_test_support.cc
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/platform/runtime_enabled_features.json5
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a34e731a1ac656550f5eb0709c5a6b535e160906

commit a34e731a1ac656550f5eb0709c5a6b535e160906
Author: Jason Chase <chasej@chromium.org>
Date: Thu Apr 12 20:53:40 2018

Add implied by relationships for origin trials

This CL adds support for origin trials to follow the implied_by
relationships defined for the runtime flags.

The implied_by relationship only applies if both of the runtime
flags are configured with an origin trial name.

The implied by relationship is actually implemented in two ways:
1) The generated OriginTrials::<name>Enabled methods will now
   check the defined trial, and any implied by trials
2) When a token enables a trial, any implied trials are also enabled
   at the same time.

Approach (1) doesn't handle the case of dynamically added tokens. In
theory, approach (2) makes (1) unnecessary. Keeping (1) and (2) for
now to ensure all cases are handled.

Bug: 831346
Change-Id: Ibab588f11c69d31f1acd9e599ee917fb036dd32b
Reviewed-on: https://chromium-review.googlesource.com/1008850
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Jason Chase <chasej@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550352}
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/implied-disabled-worker.js
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/implied-enabled-worker.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials-worker.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origintrials.js
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-disabled.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-enabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-disabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-enabled.html
[add] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-implied-script-added.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/WebKit/LayoutTests/http/tests/origin_trials/sample-api-script-added.html
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/make_origin_trials.py
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/templates/origin_trials.cc.tmpl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/build/scripts/templates/origin_trials.h.tmpl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/origin_trials/origin_trial_context.cc
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/origin_trials_test.h
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/origin_trials_test.idl
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/core/testing/v8/web_core_test_support.cc
[modify] https://crrev.com/a34e731a1ac656550f5eb0709c5a6b535e160906/third_party/blink/renderer/platform/runtime_enabled_features.json5
►

Sign in to add a comment