[Origin Trials] Allow static attributes to be defined on origin trials

[Origin Trials] Allow static attributes to be defined on origin trials

BUG=614352
Committed: https://crrev.com/7ba930f9c87cf8b2fca5e561a9ba6ec6e0e3c1d2
Cr-Commit-Position: refs/heads/master@{#404408}

[iclelland, 2016-07-01 01:55:56]

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

[iclelland, 2016-07-01 01:55:56]

+r Yuki, can you PTAL?
I'm still trying to work out how best to write tests for this; the internals
object doesn't actually get a visible interface on window, so the usual
origin_trials layout tests can't see a static member there.

[haraken, 2016-07-01 02:04:10]

On 2016/07/01 01:55:56, iclelland wrote:
> +r Yuki, can you PTAL?
> I'm still trying to work out how best to write tests for this; the internals
> object doesn't actually get a visible interface on window, so the usual
> origin_trials layout tests can't see a static member there.

Can we probably introduce window.internals.OriginTrialTest, where
OriginTrialTest provides some static members? We already have DictionaryTest,
UnionTypesTest etc on window.internals.

[iclelland, 2016-07-01 02:11:52]

On 2016/07/01 02:04:10, haraken wrote:
> On 2016/07/01 01:55:56, iclelland wrote:
> > +r Yuki, can you PTAL?
> > I'm still trying to work out how best to write tests for this; the internals
> > object doesn't actually get a visible interface on window, so the usual
> > origin_trials layout tests can't see a static member there.
> 
> Can we probably introduce window.internals.OriginTrialTest, where
> OriginTrialTest provides some static members? We already have DictionaryTest,
> UnionTypesTest etc on window.internals.

That's a good idea; I'll add that. I suppose that the OriginTrialTest interface
itself can't also be enabled by the SampleAPI trial, but all of its members
should be.

[haraken, 2016-07-01 02:16:07]

On 2016/07/01 02:11:52, iclelland wrote:
> On 2016/07/01 02:04:10, haraken wrote:
> > On 2016/07/01 01:55:56, iclelland wrote:
> > > +r Yuki, can you PTAL?
> > > I'm still trying to work out how best to write tests for this; the
internals
> > > object doesn't actually get a visible interface on window, so the usual
> > > origin_trials layout tests can't see a static member there.
> > 
> > Can we probably introduce window.internals.OriginTrialTest, where
> > OriginTrialTest provides some static members? We already have
DictionaryTest,
> > UnionTypesTest etc on window.internals.
> 
> That's a good idea; I'll add that. I suppose that the OriginTrialTest
interface
> itself can't also be enabled by the SampleAPI trial, but all of its members
> should be.

Yes, it will work.

[Yuki, 2016-07-04 06:04:36]

On 2016/07/01 02:16:07, haraken wrote:
> On 2016/07/01 02:11:52, iclelland wrote:
> > On 2016/07/01 02:04:10, haraken wrote:
> > > On 2016/07/01 01:55:56, iclelland wrote:
> > > > +r Yuki, can you PTAL?
> > > > I'm still trying to work out how best to write tests for this; the
> internals
> > > > object doesn't actually get a visible interface on window, so the usual
> > > > origin_trials layout tests can't see a static member there.
> > > 
> > > Can we probably introduce window.internals.OriginTrialTest, where
> > > OriginTrialTest provides some static members? We already have
> DictionaryTest,
> > > UnionTypesTest etc on window.internals.
> > 
> > That's a good idea; I'll add that. I suppose that the OriginTrialTest
> interface
> > itself can't also be enabled by the SampleAPI trial, but all of its members
> > should be.
> 
> Yes, it will work.

Sorry for being late.

window.internals.OriginTrialTest sounds good to me, and the change itself looks
fine.

[iclelland, 2016-07-07 18:35:21]

On 2016/07/04 06:04:36, Yuki wrote:
> On 2016/07/01 02:16:07, haraken wrote:
> > On 2016/07/01 02:11:52, iclelland wrote:
> > > On 2016/07/01 02:04:10, haraken wrote:
> > > > On 2016/07/01 01:55:56, iclelland wrote:
> > > > > +r Yuki, can you PTAL?
> > > > > I'm still trying to work out how best to write tests for this; the
> > internals
> > > > > object doesn't actually get a visible interface on window, so the
usual
> > > > > origin_trials layout tests can't see a static member there.
> > > > 
> > > > Can we probably introduce window.internals.OriginTrialTest, where
> > > > OriginTrialTest provides some static members? We already have
> > DictionaryTest,
> > > > UnionTypesTest etc on window.internals.
> > > 
> > > That's a good idea; I'll add that. I suppose that the OriginTrialTest
> > interface
> > > itself can't also be enabled by the SampleAPI trial, but all of its
members
> > > should be.
> > 
> > Yes, it will work.
> 
> Sorry for being late.
> 
> window.internals.OriginTrialTest sounds good to me, and the change itself
looks
> fine.

I realized, while writing a CL to add the OriginTrialTest interface, that we can
also access the Internals interface as internals.constructor, even if the
interface name "Internals" isn't exposed. I've written the test to use that, and
as a separate CL, I'm adding a new full interface to use for new layout tests.
It requires us to be able to install origin trials on an interface with no
existing instances, though, (which is a third CL), so I need to wait before
submitting that one.

If it's okay with you, I'm going to submit this as-is, testing a static
attribute on window.internals, and then I will upload the next two CLs.

Thanks

[haraken, 2016-07-08 01:53:12]

LGTM

[Yuki, 2016-07-08 08:35:11]

LGTM.

https://codereview.chromium.org/2116533003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origin_trials.js
(right):

https://codereview.chromium.org/2116533003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origin_trials.js:105:
var internals_interface = window.internals.constructor;
nit: IIRC, our JS coding style is |internalsInterface|.
https://google.github.io/styleguide/javascriptguide.xml?showone=Naming#Naming

[iclelland, 2016-07-08 13:51:17]

https://codereview.chromium.org/2116533003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origin_trials.js
(right):

https://codereview.chromium.org/2116533003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/origin_trials/resources/origin_trials.js:105:
var internals_interface = window.internals.constructor;
On 2016/07/08 08:35:11, Yuki wrote:
> nit: IIRC, our JS coding style is |internalsInterface|.
> https://google.github.io/styleguide/javascriptguide.xml?showone=Naming#Naming

Thanks, fixed!

[iclelland, 2016-07-08 13:51:23]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-07-08 13:51:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2116533003/#ps60001
(title: "Fix JS variable name")

[commit-bot: I haz the power, 2016-07-08 13:52:37]

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

[iclelland, 2016-07-08 14:51:19]

The CQ bit was unchecked by iclelland@chromium.org

[iclelland, 2016-07-08 14:56:52]

The CQ bit was checked by iclelland@chromium.org

[iclelland, 2016-07-08 14:56:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2116533003/#ps80001
(title: "Fix it harder")

[commit-bot: I haz the power, 2016-07-08 14:57:11]

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, 2016-07-08 16:43:22]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-08 16:45:03]

Description was changed from

==========
[Origin Trials] Allow static attributes to be defined on origin trials

BUG=614352
==========

to

==========
[Origin Trials] Allow static attributes to be defined on origin trials

BUG=614352

Committed: https://crrev.com/7ba930f9c87cf8b2fca5e561a9ba6ec6e0e3c1d2
Cr-Commit-Position: refs/heads/master@{#404408}
==========

[commit-bot: I haz the power, 2016-07-08 16:45:04]

Patchset 5 (id:??) landed as
https://crrev.com/7ba930f9c87cf8b2fca5e561a9ba6ec6e0e3c1d2
Cr-Commit-Position: refs/heads/master@{#404408}