[Webmodules]: Start handling types a bit more intelligently.

[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129
Committed: https://crrev.com/b5614aacc8b2836a85f9bb93cab08d9fe427684b
Cr-Commit-Position: refs/heads/master@{#439716}

[dglazkov, 2016-12-17 19:52:39]

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

[commit-bot: I haz the power, 2016-12-17 19:52:43]

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

[dglazkov, 2016-12-17 21:52:46]

PTAL

[commit-bot: I haz the power, 2016-12-17 22:47:05]

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

[commit-bot: I haz the power, 2016-12-17 22:47:06]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-12-18 13:44:03]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-12-18 13:44:03]

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py
(right):

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py:48:
class TypeResolver(object):

Even though I agree that web-modules will need a different type resolver from
v8_types.py, I'm a bit concerned that we might end up with inventing two type
resolvers that do similar things.

I'll defer the decision to bashi@ or yukishiino@.

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py:50: to
the generated and existing files."""

to to

[dglazkov, 2016-12-18 17:56:44]

Description was changed from

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria
BUG=654129
==========

to

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129
==========

[dglazkov, 2016-12-18 17:56:44]

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

[dglazkov, 2016-12-18 18:00:41]

+yukishiino.

Totally share your concern. WDYT about me making a pass at unifying the
resolvers (maybe a common base class?) after I flesh out how the web-module one
looks like?

[haraken, 2016-12-19 00:18:06]

On 2016/12/18 18:00:41, dglazkov wrote:
> +yukishiino.
> 
> Totally share your concern. WDYT about me making a pass at unifying the
> resolvers (maybe a common base class?) after I flesh out how the web-module
one
> looks like?

LGTM

[bashi, 2016-12-19 00:20:07]

bashi@chromium.org changed reviewers:
+ bashi@chromium.org

[bashi, 2016-12-19 00:20:08]

> Totally share your concern. WDYT about me making a pass at unifying the
> resolvers (maybe a common base class?) after I flesh out how the web-module
one
> looks like?

+1. haraken@'s concern is reasonable but I'd prefer to keep them separately at
this point. If they looks similar we can unify them later :)

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/scripts/code_generator_web_module_test.py
(right):

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module_test.py:69:
def test_includes_from_type_should_filter_date(self):
"Date" is removed from the WebIDL spec[1]. Our code generator still supports it
but if webmodule doesn't need it I'd prefer not to add further "Date" related
code. Does it make sense to remove this test?

[1] https://github.com/heycam/webidl/pull/69

[peria, 2016-12-19 01:05:05]

LGTM

> > Totally share your concern. WDYT about me making a pass at unifying the
> > resolvers (maybe a common base class?) after I flesh out how the web-module
> one
> > looks like?
> 
> +1. haraken@'s concern is reasonable but I'd prefer to keep them separately at
> this point. If they looks similar we can unify them later :)
> 

+1.

[Yuki, 2016-12-19 06:14:32]

On 2016/12/19 01:05:05, peria wrote:
> LGTM
> 
> > > Totally share your concern. WDYT about me making a pass at unifying the
> > > resolvers (maybe a common base class?) after I flesh out how the
web-module
> > one
> > > looks like?
> > 
> > +1. haraken@'s concern is reasonable but I'd prefer to keep them separately
at
> > this point. If they looks similar we can unify them later :)
> > 
> 
> +1.

+1.  LGTM.

[dglazkov, 2016-12-20 04:01:14]

Feedback addressed.

[dglazkov, 2016-12-20 04:01:26]

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py
(right):

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py:50: to
the generated and existing files."""
On 2016/12/18 at 13:44:03, haraken wrote:
> to to

Whoops, fixed.

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/scripts/code_generator_web_module_test.py
(right):

https://codereview.chromium.org/2581373004/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module_test.py:69:
def test_includes_from_type_should_filter_date(self):
On 2016/12/19 at 00:20:07, bashi1 wrote:
> "Date" is removed from the WebIDL spec[1]. Our code generator still supports
it but if webmodule doesn't need it I'd prefer not to add further "Date" related
code. Does it make sense to remove this test?
> 
> [1] https://github.com/heycam/webidl/pull/69

Done.

[dglazkov, 2016-12-20 04:01:32]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-12-20 04:01:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, peria@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2581373004/#ps20001
(title: "Feedback addressed.")

[commit-bot: I haz the power, 2016-12-20 04:02:14]

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-20 05:21:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482206492633540,
"parent_rev": "239cbd3401fcb2305f509902dd8c4d9c2ba96d4f", "commit_rev":
"7e98605bc1a8d5b8d6dd8784c52ed0a65f5c118e"}

[commit-bot: I haz the power, 2016-12-20 05:21:29]

Description was changed from

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129
==========

to

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129

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

[commit-bot: I haz the power, 2016-12-20 05:21:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-20 05:24:17]

Description was changed from

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129

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

to

==========
[Webmodules]: Start handling types a bit more intelligently.

* Introduce TypeResolver to manage conversion from WebIDL types
  to C++ types and paths
* Add is_void and is_date to IdlType
* Add tests

R=bashi,peria,yukishiino
BUG=654129

Committed: https://crrev.com/b5614aacc8b2836a85f9bb93cab08d9fe427684b
Cr-Commit-Position: refs/heads/master@{#439716}
==========

[commit-bot: I haz the power, 2016-12-20 05:24:18]

Patchset 2 (id:??) landed as
https://crrev.com/b5614aacc8b2836a85f9bb93cab08d9fe427684b
Cr-Commit-Position: refs/heads/master@{#439716}