Split EventTargetFactory.in and auto-generate modules-related files.

Split EventTargetFactory.in into two files that contain core or modules only event targets.

Auto-generate modules-related event target files and make it ready for moving 
all modules-related files to modules directory.

BUG=371581
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174218

[c.shu, 2014-05-09 21:56:14]

This CL moves all modules-related EventTargets out of core. However, some files
need to be auto-generated and will be taken care of later.

[abarth-chromium, 2014-05-10 00:43:09]

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
File Source/modules/EventTargetModulesHeaders.h (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModulesHeaders.h:29: */
Please use a new-style copyright header:

http://dev.chromium.org/blink/coding-style#TOC-License

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModulesHeaders.h:30: 
Extra blank line here.

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModulesHeaders.h:66: #if ENABLE(WEB_AUDIO)
Not for this CL, but can we remove this ifdef?

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
File Source/modules/EventTargetModulesInterfaces.h (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModulesInterfaces.h:75: #endif //
EventTargetModulesInterfaces_h
This file should be autogenerated.  Maybe that's what you meant when you said
you'd do the autogeneration separately.

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
File Source/modules/EventTargetModulesNames.cpp (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModulesNames.cpp:100: StringImpl* AudioNodeImpl =
StringImpl::createStatic("AudioNode", 9, 5081561);
We need to autogenerate these files.  I realize that will make the CL more
complex, but we really don't want to hard-code these hash values...

It shouldn't be too hard to teach the gyp how to run make_names.py and such.

[c.shu, 2014-05-10 01:03:37]

>
https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
> Source/modules/EventTargetModulesNames.cpp:100: StringImpl* AudioNodeImpl =
> StringImpl::createStatic("AudioNode", 9, 5081561);
> We need to autogenerate these files.  I realize that will make the CL more
> complex, but we really don't want to hard-code these hash values...
> 
> It shouldn't be too hard to teach the gyp how to run make_names.py and such.

Thanks, Adam. I was thinking to separate the effort and do the auto-generation
in the 2nd step. But it looks like better to get the job all at once.

[abarth-chromium, 2014-05-10 01:12:55]

Yeah, normally that would be fine, but the hard-code hash values are a real
problem.  If someone wants to add a new event target tomorrow, they'll be in a
tough spot trying to figure out the hash value by hand.

[c.shu, 2014-05-10 01:25:43]

On 2014/05/10 01:12:55, abarth wrote:
> Yeah, normally that would be fine, but the hard-code hash values are a real
> problem.  If someone wants to add a new event target tomorrow, they'll be in a
> tough spot trying to figure out the hash value by hand.

No problem, I will work on that. I did the hard-code first hoping the approach
works. It seems the build-bot failed on tests. I will take a look.

[Nils Barth (inactive), 2014-05-12 01:31:40]

https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
File Source/modules/InitModules.cpp (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
Source/modules/InitModules.cpp:22: //    EventNames::init(); TODO: split core
and modules names under bindings
Could you link to a bug number? http://crbug.com/358074 is fine as the tracking
bug, unless you want to open a separate one.

[Nils Barth (inactive), 2014-05-12 01:40:14]

On 2014/05/10 01:03:37, c.shu wrote:
> Thanks, Adam. I was thinking to separate the effort and do the auto-generation
> in the 2nd step. But it looks like better to get the job all at once.

I think you can do the autogeneration separately as a first step:
generate new EventTargetsModules files but still include them in core/Init.cpp,
and then move to InitModules.cpp or ModulesInit.cpp in followup.
I.e.:
1. Split generation, don't change C++ usage.
2. Change C++ usage.
?

[Nils Barth (inactive), 2014-05-12 01:41:44]

On 2014/05/10 01:25:43, c.shu wrote:
> On 2014/05/10 01:12:55, abarth wrote:
> > Yeah, normally that would be fine, but the hard-code hash values are a real
> > problem.  If someone wants to add a new event target tomorrow, they'll be in
a
> > tough spot trying to figure out the hash value by hand.
> 
> No problem, I will work on that. I did the hard-code first hoping the approach
> works. It seems the build-bot failed on tests. I will take a look.

Don't you need to update web/WebKit.cpp initializeWithoutV8() to call
initModules()?

[Nils Barth (inactive), 2014-05-12 01:41:56]

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
File Source/modules/EventTargetModules.h (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModules.h:14: class AudioContext;
Shouldn't you be removing these from core/events/EventTarget.h?

https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
File Source/modules/InitModules.cpp (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
Source/modules/InitModules.cpp:13: void initModules()
abarth:
Do we have a naming preference about initModules() vs. modulesInit()?

Ditto EventTargetModules vs. ModulesEventTarget etc.

(No preference personally, but consistency with existing practices
worthwhile...)

[c.shu, 2014-05-12 14:34:23]

On 2014/05/12 01:41:44, Nils Barth wrote:
> On 2014/05/10 01:25:43, c.shu wrote:
> > On 2014/05/10 01:12:55, abarth wrote:
> > > Yeah, normally that would be fine, but the hard-code hash values are a
real
> > > problem.  If someone wants to add a new event target tomorrow, they'll be
in
> a
> > > tough spot trying to figure out the hash value by hand.
> > 
> > No problem, I will work on that. I did the hard-code first hoping the
approach
> > works. It seems the build-bot failed on tests. I will take a look.
> 
> Don't you need to update web/WebKit.cpp initializeWithoutV8() to call
> initModules()?

Yeah, it's also what I thought. :)

[c.shu, 2014-05-12 14:46:20]

On 2014/05/12 01:40:14, Nils Barth wrote:
> On 2014/05/10 01:03:37, c.shu wrote:
> > Thanks, Adam. I was thinking to separate the effort and do the
auto-generation
> > in the 2nd step. But it looks like better to get the job all at once.
> 
> I think you can do the autogeneration separately as a first step:
> generate new EventTargetsModules files but still include them in
core/Init.cpp,
> and then move to InitModules.cpp or ModulesInit.cpp in followup.
> I.e.:
> 1. Split generation, don't change C++ usage.
> 2. Change C++ usage.
> ?

Thanks, Nils. How about I do the following two steps?
1. Split generation. But have the core header files include generated modules
files so I don't have to update tons of module files.
2. Separate header files, init functions, update module files.

[c.shu, 2014-05-12 19:02:21]

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
File Source/modules/EventTargetModules.h (right):

https://codereview.chromium.org/275283002/diff/20001/Source/modules/EventTarg...
Source/modules/EventTargetModules.h:14: class AudioContext;
On 2014/05/12 01:41:56, Nils Barth wrote:
> Shouldn't you be removing these from core/events/EventTarget.h?

Done.

[c.shu, 2014-05-12 19:03:35]

On 2014/05/12 01:31:40, Nils Barth wrote:
>
https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
> File Source/modules/InitModules.cpp (right):
> 
>
https://codereview.chromium.org/275283002/diff/20001/Source/modules/InitModul...
> Source/modules/InitModules.cpp:22: //    EventNames::init(); TODO: split core
> and modules names under bindings
> Could you link to a bug number? http://crbug.com/358074 is fine as the
tracking
> bug, unless you want to open a separate one.

will do. I have crbug.com/371581 already, which blocks 358074.

[c.shu, 2014-05-12 21:27:10]

This CL auto-generates the modules-related event target files but does not touch
the modules directory yet.

[Nils Barth (inactive), 2014-05-13 02:14:58]

On 2014/05/12 14:46:20, c.shu wrote:
> Thanks, Nils. How about I do the following two steps?
> 1. Split generation. But have the core header files include generated modules
> files so I don't have to update tons of module files.
> 2. Separate header files, init functions, update module files.

Sounds great!
Will review latest patch set in a second.

[Nils Barth (inactive), 2014-05-13 08:28:47]

LGTM build-wise: code is clear and well-structured, and avoids duplication.
abarth, want to take a look at it?

[c.shu, 2014-05-13 21:03:19]

On 2014/05/13 08:28:47, Nils Barth wrote:
> LGTM build-wise: code is clear and well-structured, and avoids duplication.
> abarth, want to take a look at it?

Would you double-check the patch, abarth?

[eseidel, 2014-05-16 21:15:14]

lgtm

[commit-bot: I haz the power, 2014-05-16 21:15:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/c.shu@samsung.com/275283002/80001

[commit-bot: I haz the power, 2014-05-16 23:44:38]

Change committed as 174218