[Bindings][Refactoring] Remove filters for origin trial features

Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]

BUG=650150
Committed: https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce
Cr-Commit-Position: refs/heads/master@{#439046}

[peria, 2016-12-13 06:58:47]

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

[commit-bot: I haz the power, 2016-12-13 06:58:59]

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, 2016-12-13 08:31:12]

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

[commit-bot: I haz the power, 2016-12-13 08:31:12]

Dry run: This issue passed the CQ dry run.

[peria, 2016-12-13 13:25:56]

Patchset #1 (id:1) has been deleted

[peria, 2016-12-13 13:35:36]

Description was changed from

==========
Remove filters for origin trial features

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

to

==========
Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]


BUG=650150
==========

[peria, 2016-12-13 13:36:04]

peria@chromium.org changed reviewers:
+ bashi@chromium.org, haraken@chromium.org, yukishiino@chromium.org

[peria, 2016-12-13 13:36:05]

PTL

[peria, 2016-12-13 13:37:43]

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

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

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, 2016-12-13 14:58:18]

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

[commit-bot: I haz the power, 2016-12-13 14:58:19]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-12-14 02:01:35]

LGTM

[Yuki, 2016-12-14 05:35:04]

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

[Yuki, 2016-12-14 05:35:04]

+iclelland@ FYI

LGTM.

https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
(right):

https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl:538: {% if
feature.attributes or feature.methods | sort %}
Why do we need "| sort"?

[peria, 2016-12-14 06:05:30]

will commit after iclelland@'s review.

https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
(right):

https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl:538: {% if
feature.attributes or feature.methods | sort %}
On 2016/12/14 05:35:04, Yuki wrote:
> Why do we need "| sort"?

Not needed. Removed.

[peria, 2016-12-15 05:21:52]

Did a refactoring, and revived conditions to filter origin trial methods.

PTAL.

[Yuki, 2016-12-15 06:22:59]

LGTM.

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:110: key =
'origin_trial_feature_name'
Constants should be all capital letters.
s/key/KEY/g

[peria, 2016-12-16 02:48:12]

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:110: key =
'origin_trial_feature_name'
On 2016/12/15 06:22:59, Yuki wrote:
> Constants should be all capital letters.
> s/key/KEY/g

Done.

[peria, 2016-12-16 02:49:47]

On 2016/12/14 06:05:30, peria wrote:
> will commit after iclelland@'s review.
> 
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
> (right):
> 
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl:538: {%
if
> feature.attributes or feature.methods | sort %}
> On 2016/12/14 05:35:04, Yuki wrote:
> > Why do we need "| sort"?
> 
> Not needed. Removed.

I had a small concern about some conditions in filtering
methods, but now with PS5, it is same with the original one.
So let me land this.


iclelland@
If you have any concern, please let me know.

[peria, 2016-12-16 02:49:55]

The CQ bit was checked by peria@chromium.org

[peria, 2016-12-16 02:49:55]

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/2573713002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-16 02:51:25]

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

[iclelland, 2016-12-16 04:19:22]

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:116: return [member
for member in members if member[key] == name]
Very minor nit -- These two methods look very similar, but actually operate on
different data (member_filter_by_name requires that the data previously have
been run through member_filter). I feel like there there should probably be a
comment here to that effect, or else use member.get(key) for consistency.

[iclelland, 2016-12-16 04:19:39]

On 2016/12/16 02:49:47, peria wrote:
> On 2016/12/14 06:05:30, peria wrote:
> > will commit after iclelland@'s review.
> > 
> >
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
> > (right):
> > 
> >
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl:538: {%
> if
> > feature.attributes or feature.methods | sort %}
> > On 2016/12/14 05:35:04, Yuki wrote:
> > > Why do we need "| sort"?
> > 
> > Not needed. Removed.
> 
> I had a small concern about some conditions in filtering
> methods, but now with PS5, it is same with the original one.
> So let me land this.
> 
> 
> iclelland@
> If you have any concern, please let me know.

No real concerns; I had originally switched to using filters in the template
because I thought it made the logic clearer, but this looks fine. (It would be
good to have a style guide for the Jjnja templates, if we want to avoid any more
back-and-forth on this :) )

[peria, 2016-12-16 05:01:56]

On 2016/12/16 04:19:39, iclelland wrote:
> On 2016/12/16 02:49:47, peria wrote:
> > On 2016/12/14 06:05:30, peria wrote:
> > > will commit after iclelland@'s review.
> > > 
> > >
> >
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2573713002/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl:538:
{%
> > if
> > > feature.attributes or feature.methods | sort %}
> > > On 2016/12/14 05:35:04, Yuki wrote:
> > > > Why do we need "| sort"?
> > > 
> > > Not needed. Removed.
> > 
> > I had a small concern about some conditions in filtering
> > methods, but now with PS5, it is same with the original one.
> > So let me land this.
> > 
> > 
> > iclelland@
> > If you have any concern, please let me know.
> 
> No real concerns; I had originally switched to using filters in the template
> because I thought it made the logic clearer, but this looks fine. (It would be
> good to have a style guide for the Jjnja templates, if we want to avoid any
more
> back-and-forth on this :) )

Agreed to have a style guide.

IMO, I prefer to put logical filters into context engines and make templates
clean.
Because it is difficult to have indents for code generators in templates, so
that
its logic can be unclear at a glance.

[peria, 2016-12-16 05:02:10]

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/v8_interface.py (right):

https://codereview.chromium.org/2573713002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/v8_interface.py:116: return [member
for member in members if member[key] == name]
On 2016/12/16 04:19:22, iclelland wrote:
> Very minor nit -- These two methods look very similar, but actually operate on
> different data (member_filter_by_name requires that the data previously have
> been run through member_filter). I feel like there there should probably be a
> comment here to that effect, or else use member.get(key) for consistency.

I got it. I'll put a comment in a follow-up CL.

[commit-bot: I haz the power, 2016-12-16 05:23:14]

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

[commit-bot: I haz the power, 2016-12-16 05:23:15]

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_...)

[peria, 2016-12-16 05:27:43]

The CQ bit was checked by peria@chromium.org

[commit-bot: I haz the power, 2016-12-16 05:28:24]

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-12-16 06:19:42]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481866063552530,
"parent_rev": "0af86204db08ca3114835ad4ee647dd851d15c42", "commit_rev":
"3f6c6db5084ff0031e6fac966fcdec07c1a5a5cb"}

[commit-bot: I haz the power, 2016-12-16 06:20:10]

Description was changed from

==========
Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]


BUG=650150
==========

to

==========
Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]


BUG=650150

Review-Url: https://codereview.chromium.org/2573713002
==========

[commit-bot: I haz the power, 2016-12-16 06:20:12]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-12-16 06:23:52]

Description was changed from

==========
Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]


BUG=650150

Review-Url: https://codereview.chromium.org/2573713002
==========

to

==========
Remove filters for origin trial features

It was very confusing to filter all features in .tmpl file.
This CL make the context to have origin trial feature contexts
together with names and needs_instance.

Before:
features = [{name, needs_instance}]

After:
features = [{name, needs_instance,
             [constants], [attributes], [methods]}]

BUG=650150

Committed: https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce
Cr-Commit-Position: refs/heads/master@{#439046}
==========

[commit-bot: I haz the power, 2016-12-16 06:23:53]

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