Add first sketch of the webmodules code generator.

Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true

Committed: https://crrev.com/e9b5677d326f0bd75e2a191d1e1316068518f039
Cr-Commit-Position: refs/heads/master@{#431696}

[dglazkov, 2016-10-29 01:23:14]

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

[dglazkov, 2016-10-29 01:23:28]

The CQ bit was unchecked by dglazkov@chromium.org

[commit-bot: I haz the power, 2016-10-29 01:23:30]

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

[dglazkov, 2016-10-29 02:07:57]

PTAL. Plan of attack:
1) Land the sketch
2) Iterate on code generators until fully implemented.
3) Make sure this is not crazy in terms of binary size, etc.
4) Hook up to compile.
5) Start migration.

[haraken, 2016-10-30 18:02:27]

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

[haraken, 2016-10-30 18:02:28]

If you are sure that the generated cpp bindings do not increase the binary size
(I hope so) and that they do not increase the build time when developers make
some change to an IDL file (I doubt it), I'm fine with generating the cpp
bindings for everything. Otherwise, it might make sense to generate the cpp
bindings only for IDL interfaces annotated with [GenerateWebModuleAPI]. (I'm
assuming that we won't need to add [GenerateWebModuleAPI] on many IDL interfaces
at least in short term.)

bashi@ and/or yukishiino@ might have thoughts.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py (right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:9:
TODO(dglazkov): Write proper description here.

I want to write this in this CL :)

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:23: class
CodeGeneratorCpp(CodeGeneratorBase):

Rename "Cpp" to "WebModule", "ServiceAPI" or something better.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/templates.gni (right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/templates.gni:14:
"cpp_interface.cpp.tmpl",

I'd rename this to web_module_interface.cpp.tmpl or service_interface.cpp.tmpl.

[dglazkov, 2016-10-31 17:30:00]

Thanks for your comments! Will work on the next patch!

[bashi, 2016-11-01 00:14:35]

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

[bashi, 2016-11-01 00:14:36]

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py (right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:1: # Copyright
2016 The Chromium Authors. All rights reserved.
I think we need to add code_generator_cpp.py in
Source/bindings/scripts/scripts.gni.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:51:
template_context = interface_context(
I'm wondering what kind of code CodeGeneratorCpp will/should generate. Could you
elaborate this a bit more?

[Yuki, 2016-11-01 12:58:53]

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

[Yuki, 2016-11-01 12:58:54]

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl
(right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl:3: {%
include 'copyright_block.txt' %}
copyright_block.txt includes // clang-format off, so you might want to enable it
again just after this line.  Given you're going to start to implement this from
scratch, you may want to follow clang-format from the beginning.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl:32: }
nit:
}  // namespace api

[Yuki, 2016-11-01 13:00:09]

Description was changed from

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true

Bindings tests should run the same code as prod code.

Removed and unified a bunch of code paths (more to go) so that
the bindings tests run more of the same code as what's used to
build Blink.

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

to

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true

Bindings tests should run the same code as prod code.

Removed and unified a bunch of code paths (more to go) so that
the bindings tests run more of the same code as what's used to
build Blink.

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

[Yuki, 2016-11-01 13:00:09]

yukishiino@chromium.org changed reviewers:
- bashi@google.com, yukishiino@chromium.or

[Yuki, 2016-11-01 13:05:26]

nit: CL description seems having a description for another CL.

[dglazkov, 2016-11-04 18:01:41]

On 2016/11/01 at 13:05:26, yukishiino wrote:
> nit: CL description seems having a description for another CL.

Oh yeah -- I rebased it on top of another not-yet landed CL :) Will remove.

[dglazkov, 2016-11-04 18:01:51]

Description was changed from

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true

Bindings tests should run the same code as prod code.

Removed and unified a bunch of code paths (more to go) so that
the bindings tests run more of the same code as what's used to
build Blink.

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

to

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true
==========

[dglazkov, 2016-11-04 18:04:05]

On 2016/10/30 at 18:02:28, haraken wrote:
> If you are sure that the generated cpp bindings do not increase the binary
size (I hope so) and that they do not increase the build time when developers
make some change to an IDL file (I doubt it), I'm fine with generating the cpp
bindings for everything. Otherwise, it might make sense to generate the cpp
bindings only for IDL interfaces annotated with [GenerateWebModuleAPI]. (I'm
assuming that we won't need to add [GenerateWebModuleAPI] on many IDL interfaces
at least in short term.)

Right. I think we'll need something like GenerateWebModuleAPI. That's somewhere
in step 3 of the plan.

> 
> bashi@ and/or yukishiino@ might have thoughts.
> 
>
https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py (right):
> 
>
https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:9:
TODO(dglazkov): Write proper description here.
> 
> I want to write this in this CL :)

Yes sir! :)

> 
>
https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:23: class
CodeGeneratorCpp(CodeGeneratorBase):
> 
> Rename "Cpp" to "WebModule", "ServiceAPI" or something better.

I'll go with WebModule for now.

> 
>
https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/templates/templates.gni (right):
> 
>
https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/templates/templates.gni:14:
"cpp_interface.cpp.tmpl",
> 
> I'd rename this to web_module_interface.cpp.tmpl or
service_interface.cpp.tmpl.

SG.

[dglazkov, 2016-11-04 20:58:39]

Comments addressed.

[dglazkov, 2016-11-04 21:03:08]

This now depends on https://codereview.chromium.org/2481513002. PTAL.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py (right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:1: # Copyright
2016 The Chromium Authors. All rights reserved.
On 2016/11/01 at 00:14:36, bashi1 (slow) wrote:
> I think we need to add code_generator_cpp.py in
Source/bindings/scripts/scripts.gni.

Done.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:9:
TODO(dglazkov): Write proper description here.
On 2016/10/30 at 18:02:28, haraken wrote:
> I want to write this in this CL :)

Done.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:23: class
CodeGeneratorCpp(CodeGeneratorBase):
On 2016/10/30 at 18:02:28, haraken wrote:
> Rename "Cpp" to "WebModule", "ServiceAPI" or something better.

Done.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_cpp.py:51:
template_context = interface_context(
On 2016/11/01 at 00:14:36, bashi1 (slow) wrote:
> I'm wondering what kind of code CodeGeneratorCpp will/should generate. Could
you elaborate this a bit more?

I wrote up a brief description of the class. Hope it helps :)

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl
(right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl:3: {%
include 'copyright_block.txt' %}
On 2016/11/01 at 12:58:54, Yuki wrote:
> copyright_block.txt includes // clang-format off, so you might want to enable
it again just after this line.  Given you're going to start to implement this
from scratch, you may want to follow clang-format from the beginning.

Done.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/cpp_interface.cpp.tmpl:32: }
On 2016/11/01 at 12:58:54, Yuki wrote:
> nit:
> }  // namespace api

Done.

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/templates.gni (right):

https://codereview.chromium.org/2458913003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/templates.gni:14:
"cpp_interface.cpp.tmpl",
On 2016/10/30 at 18:02:28, haraken wrote:
> I'd rename this to web_module_interface.cpp.tmpl or
service_interface.cpp.tmpl.

Done.

[dglazkov, 2016-11-04 21:04:19]

I apologize, the https://codereview.chromium.org/2481513002 and this patche are
intermingled here in the latest patch. Let me try to split up...

[dglazkov, 2016-11-04 21:33:38]

Changed generated file prefix to Web.

[dglazkov, 2016-11-04 21:38:33]

Let's just get https://codereview.chromium.org/2481513002 landed and then I'll
rebase :)

[dglazkov, 2016-11-04 23:08:19]

Implemented WebModuleAPI extended attribute.

[bashi, 2016-11-06 23:58:20]

lgtm on my side with one (future) nit.

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

https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py:71:
template_context = interface_context(interface,
v8_interface.interface_context() creates a jinja2 context for c++ <=> v8
bindings. Reading the description I think we don't need v8 stuff in the context?
If so, it would be better to create a separate function called
webmodule_context() which returns a context for web modules.

(My concern is that interface_context() may end up having both v8 bindings
specific and web modules specific logic)

[haraken, 2016-11-07 02:20:25]

LGTM

But don't we need web_module_interface.h.tmpl?

[Yuki, 2016-11-07 02:55:53]

lgtm

https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/templates/web_module_interface.cpp.tmpl
(right):

https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/templates/web_module_interface.cpp.tmpl:34: }
nit:  }  // namespace blink

[haraken, 2016-11-10 04:09:01]

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

[haraken, 2016-11-10 04:09:02]

+Elliott

One feedback I've recently heard is that the name "Web modules" is confusing. We
have Source/modules/. We're going to support ES6 modules and HTML modules. And
we're adding Web modules. A lot of modules that have totally different meanings
:)

We might want to think about a better name for Web modules at least. FWIW,
"Service" is also confusing with the servicification project in Chromium.

[dglazkov, 2016-11-12 00:11:14]

On 2016/11/07 at 02:55:53, yukishiino wrote:
> lgtm
> 
>
https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/bindings/templates/web_module_interface.cpp.tmpl
(right):
> 
>
https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/templates/web_module_interface.cpp.tmpl:34:
}
> nit:  }  // namespace blink

Done.

[dglazkov, 2016-11-12 00:11:35]

On 2016/11/07 at 02:20:25, haraken wrote:
> LGTM
> 
> But don't we need web_module_interface.h.tmpl?

Not yet. I am not generating a header yet.

[dglazkov, 2016-11-12 00:12:15]

On 2016/11/06 at 23:58:20, bashi wrote:
> lgtm on my side with one (future) nit.
> 
>
https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py
(right):
> 
>
https://codereview.chromium.org/2458913003/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/scripts/code_generator_web_module.py:71:
template_context = interface_context(interface,
> v8_interface.interface_context() creates a jinja2 context for c++ <=> v8
bindings. Reading the description I think we don't need v8 stuff in the context?
If so, it would be better to create a separate function called
webmodule_context() which returns a context for web modules.
> 
> (My concern is that interface_context() may end up having both v8 bindings
specific and web modules specific logic)

This is a good point, will make sure to keep this clean.

[dglazkov, 2016-11-12 00:14:04]

Ready to land.

[dglazkov, 2016-11-12 00:14:49]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-11-12 00:14:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, bashi@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2458913003/#ps100001
(title: "Ready to land.")

[commit-bot: I haz the power, 2016-11-12 00:15:34]

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-11-12 00:22:21]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-11-12 00:26:45]

Description was changed from

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true
==========

to

==========
Add first sketch of the webmodules code generator.

This is the beginning of the C++ code generator that will generate
the replacemenet for the Web* interfaces (aka WebKit API).

There's not much to see here at the moment, just the inklings of
the future.

R=yukishiino,bashi
BUG=654129
NOTRY=true

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

[commit-bot: I haz the power, 2016-11-12 00:26:46]

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