New issue
Advanced search Search tips

Issue 650150 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug


Sign in to add a comment

Refactoring scripts for IDL bindings

Project Member Reported by peria@chromium.org, Sep 26 2016

Issue description

We should do refactoring of the scripts.
Now they have some concerns in both structure and script style.

For the structure, bashi@ has a design doc [1].
For the style, pylint warns to use relative imports, which does not work correctly.

We'd like to work for the latter one incrementally, and for the first one in a long way.


[1] https://docs.google.com/document/d/13yQkyEX0_y3n5Mgma6R1yEsKqPESddtzhjsvbArrE1Q/edit?usp=sharing
 

Comment 1 Deleted

Comment 3 by peria@chromium.org, Sep 28 2016

Owner: peria@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 2016

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

commit 8db1c398f8456c3d0342b97a15747d1cf2ae994c
Author: peria <peria@chromium.org>
Date: Wed Sep 28 08:28:33 2016

Remove a redundant option write-file-if-only-changed in bindings

It is always set on everywhere the option is used for >3 years,
and it means we have no hidden dependencies through generated files.

BUG=650150

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

[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/modules/BUILD.gn
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/compute_global_objects.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/compute_interfaces_info_individual.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/compute_interfaces_info_overall.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/generate_event_interfaces.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/generate_global_constructors.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/idl_compiler.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/scripts.gni
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Source/bindings/scripts/utilities.py
[modify] https://crrev.com/8db1c398f8456c3d0342b97a15747d1cf2ae994c/third_party/WebKit/Tools/Scripts/webkitpy/bindings/bindings_tests.py

Comment 6 by peria@chromium.org, Oct 21 2016

Summary: Refactoring scripts for IDL bindings (was: Need to update IDL bindings scripts)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 24 2016

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

commit 233364e61ae1b39f6b13bc1fda73926e43048e49
Author: peria <peria@chromium.org>
Date: Mon Oct 24 13:22:36 2016

Remove checking activity_logging_world_check in v8 method template.

It is not defined in implementation, and it is not needed to be checked.
https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md#LogActivity_m_a

BUG=650150

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

[modify] https://crrev.com/233364e61ae1b39f6b13bc1fda73926e43048e49/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 13 2016

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

commit 9b9858514e087acbcf12846f3cd77220ba964f14
Author: peria <peria@chromium.org>
Date: Tue Dec 13 08:25:54 2016

Remove attribute filters from .tmpl files.

We rarely handle all attributes in a loop.
Instead, we have filters to iterate only on accessors,
on data type attributes, so on.
It is very confusing to have all attributes in a
variable in .py and filter them in .tmpl files.

This CL applies such filters in pre-process in .py
and uses more descriptive names in template contexts.
It keeps 'attributes' for some usages, which are
to be replaced.

This is a refactoring in template engine, so it
does not change generated code.

BUG=650150

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

[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/scripts/code_generator.py
[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/9b9858514e087acbcf12846f3cd77220ba964f14/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 16 2016

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

commit 9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce
Author: peria <peria@chromium.org>
Date: Fri Dec 16 06:19:39 2016

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
Cr-Commit-Position: refs/heads/master@{#439046}

[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/scripts/code_generator.py
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/9c4bd1e5532fbf6b1e7cf87b2a75613f454ef3ce/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 27 2017

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

commit 5f2babfc8b63a2c91e656a5ac79b03bcc0f21377
Author: peria <peria@chromium.org>
Date: Mon Feb 27 09:32:54 2017

Move logics of preparePrototypeAndInterfaceObject from .tmpl to .py

Before this CL, we checked if an (partial) interface needed
preparePrototypeAndInterfaceObject() method and set two variables ("has_foobar"
and "foobar_func") in places of template files.
The condition is not so simple and we could easily introduce bugs if we had updates
around them.

This CL unifies all those template variables into one, and move the logical
part into python file.

BUG=650150

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

[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/templates/partial_interface.cpp.tmpl
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.h
[modify] https://crrev.com/5f2babfc8b63a2c91e656a5ac79b03bcc0f21377/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface2.h

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 28 2017

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

commit 70e38ec3001437528a4bf79a37ebdd9654ce9cb0
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Sep 28 04:44:08 2017

IDL parser: Use abspath to find test files

Before this CL, developers need to change their directory to src/tools/idl_parser
to run idl_parser_test.py. But it is not useful, and it is difficult to find
parse errors.
After this CL, they can find parse errors from any directories.


Bug: 650150
Change-Id: Id00649b2c594ad211d7b6e4cd45a9442cda12317
Reviewed-on: https://chromium-review.googlesource.com/688877
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504902}
[modify] https://crrev.com/70e38ec3001437528a4bf79a37ebdd9654ce9cb0/tools/idl_parser/idl_parser_test.py

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 28 2017

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

commit 1d9e6ea1b1d579d7818f285ccd025cc3fbee16c1
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Sep 28 04:52:25 2017

IDL parser: Add AST tests for Constructor and Exposed

These extended attributes have uncommon structures and they need
to be handled independently.
This CL adds those tests to show how IDL parser creates AST for them.


Bug: 650150
Change-Id: I041c1c105cc877354ac1b2149d91a9888eafdcf5
Reviewed-on: https://chromium-review.googlesource.com/688256
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504903}
[modify] https://crrev.com/1d9e6ea1b1d579d7818f285ccd025cc3fbee16c1/tools/idl_parser/test_parser/interface_web.idl

Comment 14 by peria@chromium.org, Nov 20 2017

Blocking: 781257 727971 672978 656517

Comment 15 by peria@chromium.org, Nov 20 2017

Blocking: 579896

Comment 16 by peria@chromium.org, Nov 20 2017

Blocking: 658098
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 7 2017

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

commit fb396f70fdafd36e61eeea88bb174caa95fd8513
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Thu Dec 07 07:34:43 2017

bindings: Generate a IDL information collection per component

This CL defines a new set of Web IDL based classes under web_idl/.
Collecitor class is the main API, which reads IDL files and
stacks converted information.

This CL also updates some BUILD.gn files to generate an
intermediate dump file per component, which includes all
information in IDL files in the component.


Bug: 650150, 727971, 579896
Change-Id: Idc98be4485855afc7797b7fc3af21fb7ee68f0ed
Reviewed-on: https://chromium-review.googlesource.com/648534
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522378}
[modify] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/core/BUILD.gn
[modify] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/modules/BUILD.gn
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/generate_web_idl_collection.py
[modify] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/scripts.gni
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/__init__.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/argument.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/attribute.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/callback_function.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/callback_interface.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/collection.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/collector.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/collector_test.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/constant.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/dictionary.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/ecma_script_types.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/enumeration.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/extended_attribute.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/idl_definition_builder.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/idl_types.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/implements.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/interface.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/literal_token.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/namespace.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/operation.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/typedef.py
[add] https://crrev.com/fb396f70fdafd36e61eeea88bb174caa95fd8513/third_party/WebKit/Source/bindings/scripts/web_idl/utilities.py

Comment 18 by peria@chromium.org, Jan 25 2018

Just a note. This refactoring will resolve a complex situation of IDL files in GN files.
For example, in https://chromium-review.googlesource.com/c/chromium/src/+/867915
it is difficult to find he needs to move 'Document.idl' from |core_idl_files| to |core_idl_with_modules_dependency_files|.

Now we have 15 GN variables to manage IDL files, and I think we can reduce them into 4-5 variables in clearer rules.
Blockedon: 839389

Comment 20 by peria@chromium.org, May 25 2018

Labels: Hotlist-Bindings-IDLCompiler

Sign in to add a comment