[Extension API Extern Generation] Auto-generate enums

[Extension API Extern Generation] Auto-generate enums

Enums used to simply be strings, but now we expose them (as string constants)
on the api object as part of the extension bindings. As such, it's appropriate
to have the externs generate an enum definition for them.

BUG=469920
Committed: https://crrev.com/d68755a4852b0aca4520df3a0ba7ceef76625d34
Cr-Commit-Position: refs/heads/master@{#322086}

[Devlin, 2015-03-24 17:10:50]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2015-03-24 17:10:50]

Now that we've got the base committed, we can start fixing those little things.
:)

[Tyler Breisacher (Chromium), 2015-03-24 17:25:53]

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

[Tyler Breisacher (Chromium), 2015-03-24 17:25:54]

Do we have any tests for this? It's simple enough that I don't think it needs to
be super extensively tested, but a simple test that just runs the externs
generator on a single IDL file (probably one for an API that is unlikely to
change much) and compares the output to a checked-in file from a previous run of
the generator, would be nice because it would be easy to see what the output
looks like without having to look at the Python code.

[Dan Beam, 2015-03-24 17:31:03]

On 2015/03/24 17:25:54, Tyler Breisacher (Chromium) wrote:
> Do we have any tests for this? It's simple enough that I don't think it needs
to
> be super extensively tested, but a simple test that just runs the externs
> generator on a single IDL file (probably one for an API that is unlikely to
> change much) and compares the output to a checked-in file from a previous run
of
> the generator, would be nice because it would be easy to see what the output
> looks like without having to look at the Python code.

i don't think we need to involve the file system or do any diffing.  just
testing all the code paths with:

  idl_file = """ ... fake IDL file ... """
  expecterns = """ ... expected externs ... """
  self.assertEquals(expecterns, Generate(idl_file))

otherwise I agree with tbreisacher@: this seems like something that can and
should be tested.

[Devlin, 2015-03-24 19:28:40]

Patchset #2 (id:20001) has been deleted

[Devlin, 2015-03-24 19:31:51]

Patchset #2 (id:40001) has been deleted

[Devlin, 2015-03-24 19:33:50]

Patchset #2 (id:60001) has been deleted

[Devlin, 2015-03-24 19:36:43]

Tests added.

[Tyler Breisacher (Chromium), 2015-03-24 20:02:36]

On 2015/03/24 19:36:43, Devlin wrote:
> Tests added.

+1. I suggested a separate file because I'm used to Java where multi-line string
literals are annoying. With Python """ strings this is great.

[Tyler Breisacher (Chromium), 2015-03-24 20:07:52]

lgtm

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
File tools/json_schema_compiler/js_externs_generator.py (right):

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:73: value_lines.append("  %s:
'%s'," % (value, value))
It seems a bit strange to have a list comp and then iterate over it. Would this
work?

for v in js_type.enum_values:
  value_lines.append("  %s: '%s'," % (v.name, v.name))

or perhaps just do it all in one list comp:

value_lines = ["  %s: '%s'," % (v.name, v.name) for v in js_type.enum_values]

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:75: # TODO(devlin): This will
be unneeded, if when
Actually I think you can drop line 77 now, because this is code, not JsDoc, and
it should be parsed according to the ES5 grammar.

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:110: if is_constructor: var
+= ' = function()'
I think you need the body too:

if is_constructor: var += ' = function() {}'

[Devlin, 2015-03-24 20:31:55]

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
File tools/json_schema_compiler/js_externs_generator.py (right):

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:73: value_lines.append("  %s:
'%s'," % (value, value))
On 2015/03/24 20:07:52, Tyler Breisacher (Chromium) wrote:
> It seems a bit strange to have a list comp and then iterate over it. Would
this
> work?
> 
> for v in js_type.enum_values:
>   value_lines.append("  %s: '%s'," % (v.name, v.name))
> 
> or perhaps just do it all in one list comp:
> 
> value_lines = ["  %s: '%s'," % (v.name, v.name) for v in js_type.enum_values]

Done.

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:75: # TODO(devlin): This will
be unneeded, if when
On 2015/03/24 20:07:52, Tyler Breisacher (Chromium) wrote:
> Actually I think you can drop line 77 now, because this is code, not JsDoc,
and
> it should be parsed according to the ES5 grammar.

Done.

https://codereview.chromium.org/1015033003/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/js_externs_generator.py:110: if is_constructor: var
+= ' = function()'
On 2015/03/24 20:07:52, Tyler Breisacher (Chromium) wrote:
> I think you need the body too:
> 
> if is_constructor: var += ' = function() {}'
> 

I would have thought so, too, but figured maybe there some was weirdness with
constructors (since it was this way before).  Anyway, added the body.

[Dan Beam, 2015-03-24 21:19:18]

lgtm++

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
File tools/json_schema_compiler/js_externs_generator_test.py (right):

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:11: 
nit: \n\n

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:100: 
nit: \n\n

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:109: 
nit: \n\n

[Devlin, 2015-03-24 21:26:27]

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
File tools/json_schema_compiler/js_externs_generator_test.py (right):

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:11: 
On 2015/03/24 21:19:18, Dan Beam wrote:
> nit: \n\n

Done.

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:100: 
On 2015/03/24 21:19:18, Dan Beam wrote:
> nit: \n\n

Done.

https://codereview.chromium.org/1015033003/diff/100001/tools/json_schema_comp...
tools/json_schema_compiler/js_externs_generator_test.py:109: 
On 2015/03/24 21:19:18, Dan Beam wrote:
> nit: \n\n

Done.

[Devlin, 2015-03-24 21:26:30]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-03-24 21:26:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
tbreisacher@chromium.org, dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1015033003/#ps120001
(title: " ")

[commit-bot: I haz the power, 2015-03-24 21:27:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1015033003/120001

[commit-bot: I haz the power, 2015-03-24 23:03:15]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-03-24 23:04:41]

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