Adding an origin trial for Budget API

Adding an origin trial for Budget API

The current plan for the Budget API in M59 is to release reserve() to stable, and experiment with getBudget and getCost.

BUG=617971

[harkness, 2017-04-12 15:29:53]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 15:30:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 15:34:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 15:34:13]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[harkness, 2017-04-12 16:29:56]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-12 16:30:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 18:09:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 18:09:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[harkness, 2017-04-13 12:49:30]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[harkness, 2017-04-13 12:49:35]

The CQ bit was unchecked by harkness@chromium.org

[harkness, 2017-04-13 12:50:51]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 12:51:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 12:54:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 12:54:09]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[harkness, 2017-04-13 13:09:15]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 13:09:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 13:12:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 13:12:35]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[harkness, 2017-04-13 13:22:58]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 13:23:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[harkness, 2017-04-13 14:03:43]

Description was changed from

==========
Adding an origin trial for Budget API

BUG=
==========

to

==========
Adding an origin trial for Budget API

The current plan for the Budget API in M59 is to release reserve() to stable,
and experiment with getBudget and getCost.

BUG=617971
==========

[harkness, 2017-04-13 14:03:43]

harkness@chromium.org changed reviewers:
+ peter@chromium.org

[commit-bot: I haz the power, 2017-04-13 15:25:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 15:25:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[harkness, 2017-04-20 16:22:31]

The CQ bit was checked by harkness@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-20 16:23:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 18:35:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-20 18:35:10]

Dry run: This issue passed the CQ dry run.

[harkness, 2017-04-21 08:28:21]

harkness@chromium.org changed reviewers:
+ iclelland@chromium.org

[harkness, 2017-04-21 08:28:22]

peter@ for Budget stuffs.
iclelland@ for Origin trial integration.

[iclelland, 2017-04-21 15:47:33]

On 2017/04/21 08:28:22, harkness wrote:
> peter@ for Budget stuffs.
> iclelland@ for Origin trial integration.

Origin trial integration LGTM -- note that the Budget API is still experimental,
so until that is flipped, the Budget API itself has to be enabled by flag before
the origin trial will have any additional effect.

[Peter Beverloo, 2017-04-24 14:24:02]

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html
(right):

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html:7:
<meta http-equiv="origin-trial"
content="AgFtR2Ps1Z9M/FW14Tgcwbajvq7kvzc/b1SPPSaaucG/P4ba6xC/69I9v8Pqx4wbsJINoqMabs9GE/LxOnPRfQIAAABTeyJvcmlnaW4iOiAiaHR0cDovLzEyNy4wLjAuMTo4MDAwIiwgImZlYXR1cmUiOiAiQnVkZ2V0UXVlcnkiLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0"
/>
Ian, how does this work? The Service Worker will be started as its own context,
but more importantly, can be started without the document existing at all.

  a) Are we satisfied with the test coverage for this vs. the equivalent HTTP
header?
  b) Do enabled origin trials always propagate to workers(/contexts) started
from a hosting document?

How would that work if two documents, one with the OT and one without, both
"start" the same shared/service worker?

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html:19:
let port;
nit: |port| can be local to the function on line 22.

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html:35:
return sendCommand(port, { command: 'checkInterfaces' });
Are we sure that the event listener on line 25 is being executed before the
returned promise resolves?

Since the sendCommand() function resolves the promise with the event's data, we
can do something like this:

return sendCommand(..)
  .then(data => {
    assert_equals(...);
  });

[iclelland, 2017-04-24 14:45:34]

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html
(right):

https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-origin-trial-interfaces-in-service-worker.html:7:
<meta http-equiv="origin-trial"
content="AgFtR2Ps1Z9M/FW14Tgcwbajvq7kvzc/b1SPPSaaucG/P4ba6xC/69I9v8Pqx4wbsJINoqMabs9GE/LxOnPRfQIAAABTeyJvcmlnaW4iOiAiaHR0cDovLzEyNy4wLjAuMTo4MDAwIiwgImZlYXR1cmUiOiAiQnVkZ2V0UXVlcnkiLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0"
/>
On 2017/04/24 14:24:01, Peter Beverloo wrote:
> Ian, how does this work? The Service Worker will be started as its own
context,
> but more importantly, can be started without the document existing at all.
> 
>   a) Are we satisfied with the test coverage for this vs. the equivalent HTTP
> header?

If this test is passing, and it appears that the trial is enabled when it should
not be, I would suggest that we shouldn't be satisfied with the test coverage :)

The tests in LayoutTests/http/tests/origin_trials will *always* see the budget
feature available, because the runtime flag for experimental features is turned
on, I believe. The tests there are useful for ensuring that the properties are
bound correctly, because Chrome still uses the origin trials mechanism to attach
them to the JS objects.

Tests in LayoutTests/virtual/origin-trials-runtimeflags-disabled/ run with the
runtime flag disabled, but you should still be able to turn the feature on with
a trial token. The service worker tests should pass in this directory if we add
the token as an HTTP header (see
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/test...
as an example)

>   b) Do enabled origin trials always propagate to workers(/contexts) started
> from a hosting document?

Origin trials propagate to dedicated worker threads from the document which
creates them, but not to shared workers or service workers. Those need to have
the trial enabled by an HTTP header (we could probably add a JS interface, but
have not done that yet for worker scripts). The token is cached with the service
worker version, so that if the service worker is restarted without a network
request, the token will still be enabled.


> 
> How would that work if two documents, one with the OT and one without, both
> "start" the same shared/service worker?

In both cases, the presence of the token in the HTTP response which delivers the
worker script will determine whether the trial is enabled in that context.

[Peter Beverloo, 2017-04-24 15:00:44]

On Mon, Apr 24, 2017 at 3:45 PM, <iclelland@chromium.org> wrote:

>
> https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/
> LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html
> File
> third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html
> (right):
>
> https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/
> LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html#newcode7
> third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html:7:
> <meta http-equiv="origin-trial"
> content="AgFtR2Ps1Z9M/FW14Tgcwbajvq7kvzc/b1SPPSaaucG/P4ba6xC/
> 69I9v8Pqx4wbsJINoqMabs9GE/LxOnPRfQIAAABTeyJvcmlnaW4iOiAi
> aHR0cDovLzEyNy4wLjAuMTo4MDAwIiwgImZlYXR1cmUiOiAiQnVkZ2V0UXVl
> cnkiLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0"
> />
> On 2017/04/24 14:24:01, Peter Beverloo wrote:
> > Ian, how does this work? The Service Worker will be started as its own
> context,
> > but more importantly, can be started without the document existing at
> all.
> >
> > a) Are we satisfied with the test coverage for this vs. the
> equivalent HTTP
> > header?
>
> If this test is passing, and it appears that the trial is enabled when
> it should not be, I would suggest that we shouldn't be satisfied with
> the test coverage :)
>
> The tests in LayoutTests/http/tests/origin_trials will *always* see the
> budget feature available, because the runtime flag for experimental
> features is turned on, I believe. The tests there are useful for
> ensuring that the properties are bound correctly, because Chrome still
> uses the origin trials mechanism to attach them to the JS objects.
>

Right, I see. That means that the new test isn't actually verifying Origin
Trials behaviour, it's verifying that marking features as experimental
works. That's unfortunate.

I guess what we want to test is where (1) the feature has not been enabled
because it's marked as experimental, but (2) the feature is enabled because
the OT token is passed. Which relates to your next paragraph...

Tests in LayoutTests/virtual/origin-trials-runtimeflags-disabled/ run
> with the runtime flag disabled, but you should still be able to turn the
> feature on with a trial token. The service worker tests should pass in
> this directory if we add the token as an HTTP header (see
> https://cs.chromium.org/chromium/src/third_party/
> WebKit/LayoutTests/http/tests/origin_trials/webexposed/
> resources/foreignfetch-origin-trial-interfaces-worker-enabled.php
> as an example)
>

Yes, this!

Jen, the important thing to do is to verify that the feature can be enabled
through OT in the origin-trials-runtimeflags-disabled virtual test suite.
The Foreign Fetch test Ian links to shows how to do that - run the Service
Worker test once w/ the header, and once without the header. In the
non-virtual test it should be available in both, in the virtual test only
when the header is provided. Some PHP will be needed for that.

> b) Do enabled origin trials always propagate to workers(/contexts)
> started
> > from a hosting document?
>
> Origin trials propagate to dedicated worker threads from the document
> which creates them, but not to shared workers or service workers. Those
> need to have the trial enabled by an HTTP header (we could probably add
> a JS interface, but have not done that yet for worker scripts). The
> token is cached with the service worker version, so that if the service
> worker is restarted without a network request, the token will still be
> enabled.
>

Ack, perfect!


>
>
>
> >
> > How would that work if two documents, one with the OT and one without,
> both
> > "start" the same shared/service worker?
>
> In both cases, the presence of the token in the HTTP response which
> delivers the worker script will determine whether the trial is enabled
> in that context.
>

Thanks,
Peter


>
> https://codereview.chromium.org/2812223004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Peter Beverloo, 2017-04-24 15:00:44]

On Mon, Apr 24, 2017 at 3:45 PM, <iclelland@chromium.org> wrote:

>
> https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/
> LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html
> File
> third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html
> (right):
>
> https://codereview.chromium.org/2812223004/diff/140001/third_party/WebKit/
> LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html#newcode7
> third_party/WebKit/LayoutTests/http/tests/origin_trials/webexposed/budget-
> origin-trial-interfaces-in-service-worker.html:7:
> <meta http-equiv="origin-trial"
> content="AgFtR2Ps1Z9M/FW14Tgcwbajvq7kvzc/b1SPPSaaucG/P4ba6xC/
> 69I9v8Pqx4wbsJINoqMabs9GE/LxOnPRfQIAAABTeyJvcmlnaW4iOiAi
> aHR0cDovLzEyNy4wLjAuMTo4MDAwIiwgImZlYXR1cmUiOiAiQnVkZ2V0UXVl
> cnkiLCAiZXhwaXJ5IjogMjAwMDAwMDAwMH0"
> />
> On 2017/04/24 14:24:01, Peter Beverloo wrote:
> > Ian, how does this work? The Service Worker will be started as its own
> context,
> > but more importantly, can be started without the document existing at
> all.
> >
> > a) Are we satisfied with the test coverage for this vs. the
> equivalent HTTP
> > header?
>
> If this test is passing, and it appears that the trial is enabled when
> it should not be, I would suggest that we shouldn't be satisfied with
> the test coverage :)
>
> The tests in LayoutTests/http/tests/origin_trials will *always* see the
> budget feature available, because the runtime flag for experimental
> features is turned on, I believe. The tests there are useful for
> ensuring that the properties are bound correctly, because Chrome still
> uses the origin trials mechanism to attach them to the JS objects.
>

Right, I see. That means that the new test isn't actually verifying Origin
Trials behaviour, it's verifying that marking features as experimental
works. That's unfortunate.

I guess what we want to test is where (1) the feature has not been enabled
because it's marked as experimental, but (2) the feature is enabled because
the OT token is passed. Which relates to your next paragraph...

Tests in LayoutTests/virtual/origin-trials-runtimeflags-disabled/ run
> with the runtime flag disabled, but you should still be able to turn the
> feature on with a trial token. The service worker tests should pass in
> this directory if we add the token as an HTTP header (see
> https://cs.chromium.org/chromium/src/third_party/
> WebKit/LayoutTests/http/tests/origin_trials/webexposed/
> resources/foreignfetch-origin-trial-interfaces-worker-enabled.php
> as an example)
>

Yes, this!

Jen, the important thing to do is to verify that the feature can be enabled
through OT in the origin-trials-runtimeflags-disabled virtual test suite.
The Foreign Fetch test Ian links to shows how to do that - run the Service
Worker test once w/ the header, and once without the header. In the
non-virtual test it should be available in both, in the virtual test only
when the header is provided. Some PHP will be needed for that.

> b) Do enabled origin trials always propagate to workers(/contexts)
> started
> > from a hosting document?
>
> Origin trials propagate to dedicated worker threads from the document
> which creates them, but not to shared workers or service workers. Those
> need to have the trial enabled by an HTTP header (we could probably add
> a JS interface, but have not done that yet for worker scripts). The
> token is cached with the service worker version, so that if the service
> worker is restarted without a network request, the token will still be
> enabled.
>

Ack, perfect!


>
>
>
> >
> > How would that work if two documents, one with the OT and one without,
> both
> > "start" the same shared/service worker?
>
> In both cases, the presence of the token in the HTTP response which
> delivers the worker script will determine whether the trial is enabled
> in that context.
>

Thanks,
Peter


>
> https://codereview.chromium.org/2812223004/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.