generated_api.h should have its body generated into generated_api.cc

generated_api.h should have its body generated into generated_api.cc

BUG=172050
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181848

[Joe Thomas, 2013-02-02 08:33:43]

Please take a look. Thanks.

[not at google - send to devlin, 2013-02-04 16:57:24]

lg, thanks for doing this, but I want to hold off submitting until
https://codereview.chromium.org/12041098/ goes in.

https://codereview.chromium.org/12181005/diff/1/tools/json_schema_compiler/sc...
File tools/json_schema_compiler/schema_bundle_generator.py (right):

https://codereview.chromium.org/12181005/diff/1/tools/json_schema_compiler/sc...
tools/json_schema_compiler/schema_bundle_generator.py:44: def
GenerateHeader(self, file_base, body_code):
_GenerateHeader

https://codereview.chromium.org/12181005/diff/1/tools/json_schema_compiler/sc...
tools/json_schema_compiler/schema_bundle_generator.py:91: c.Append("//
TODO(miket): emit code for %s" % (namespace.unix_name))
I don't know what this TODO means and it has no bug link, let's delete it.

https://codereview.chromium.org/12181005/diff/1/tools/json_schema_compiler/sc...
tools/json_schema_compiler/schema_bundle_generator.py:96:
'ExtensionFunctionRegistry* registry);')
C++ indentation here needs fixing (and line above).

https://codereview.chromium.org/12181005/diff/1/tools/json_schema_compiler/sc...
tools/json_schema_compiler/schema_bundle_generator.py:151: def
GenerateFunctionRegistryRegisterAll(self):
_GenerateFunctionRegistryRegisterAll

[Joe Thomas, 2013-02-04 19:19:40]

On 2013/02/04 16:57:24, kalman wrote:
> lg, thanks for doing this, but I want to hold off submitting until
> https://codereview.chromium.org/12041098/ goes in.

Thanks for the review! 

I will rework on this once the above mentioned CL goes in.

[Joe Thomas, 2013-02-07 23:02:40]

kalman@

Please take a look. Updated the patch. Thanks,

[not at google - send to devlin, 2013-02-10 16:03:44]

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
File build/json_schema_bundle_compile.gypi (right):

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
build/json_schema_bundle_compile.gypi:61: '<(DEPTH)/skia/skia.gyp:skia',
why is this change needed?

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
File tools/json_schema_compiler/code.py (right):

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/code.py:136: def AdjustIndentLevel(self, adjustment):
please revert this change

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
File tools/json_schema_compiler/cpp_bundle_generator.py (right):

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/cpp_bundle_generator.py:142: c.AdjustIndentLevel(1) #
Increment indent level by 1.
There is no need for this, just don't use Sblock('class
GeneratedFunctionRegistry {'), rather

c.Append('class GeneratedFunctionRegistry {')
c.Sblock(' public:')

etc

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/cpp_bundle_generator.py:212: c.AdjustIndentLevel(-1)
# Decrement indent level by 1.
ditto

[Joe Thomas, 2013-02-11 22:58:23]

Please take a look at the updated patch. Thanks

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
File build/json_schema_bundle_compile.gypi (right):

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
build/json_schema_bundle_compile.gypi:61: '<(DEPTH)/skia/skia.gyp:skia',
On 2013/02/10 16:03:44, kalman wrote:
> why is this change needed?

This is required to solve the header file dependency while compiling
generated_api.cc. Below is the sequence from where the dependency comes from

SkTypes.h:14:26: fatal error: SkUserConfig.h: No such file or directory
./third_party/skia/include/core/SkFixed.h:13:0,
./third_party/skia/include/core/SkScalar.h:13,
./third_party/skia/include/core/SkColor.h:13,
./chrome/browser/extensions/extension_action.h:20,
./chrome/browser/extensions/api/extension_action/extension_action_api.h:10,
./chrome/browser/extensions/api/system_indicator/system_indicator_api.h:8,
out/Debug/obj/gen/chrome/common/extensions/api/generated_api.cc:10:

Same is the case with sync_proto below.

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
File tools/json_schema_compiler/code.py (right):

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/code.py:136: def AdjustIndentLevel(self, adjustment):
On 2013/02/10 16:03:44, kalman wrote:
> please revert this change

Done.

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
File tools/json_schema_compiler/cpp_bundle_generator.py (right):

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/cpp_bundle_generator.py:142: c.AdjustIndentLevel(1) #
Increment indent level by 1.
On 2013/02/10 16:03:44, kalman wrote:
> There is no need for this, just don't use Sblock('class
> GeneratedFunctionRegistry {'), rather
> 
> c.Append('class GeneratedFunctionRegistry {')
> c.Sblock(' public:')
> 
> etc

Done.

https://codereview.chromium.org/12181005/diff/5001/tools/json_schema_compiler...
tools/json_schema_compiler/cpp_bundle_generator.py:212: c.AdjustIndentLevel(-1)
# Decrement indent level by 1.
On 2013/02/10 16:03:44, kalman wrote:
> ditto

Done.

[not at google - send to devlin, 2013-02-11 23:00:42]

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
File build/json_schema_bundle_compile.gypi (right):

https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
build/json_schema_bundle_compile.gypi:61: '<(DEPTH)/skia/skia.gyp:skia',
On 2013/02/11 22:58:23, Joe Thomas wrote:
> On 2013/02/10 16:03:44, kalman wrote:
> > why is this change needed?
> 
> This is required to solve the header file dependency while compiling
> generated_api.cc. Below is the sequence from where the dependency comes from
> 
> SkTypes.h:14:26: fatal error: SkUserConfig.h: No such file or directory
> ./third_party/skia/include/core/SkFixed.h:13:0,
> ./third_party/skia/include/core/SkScalar.h:13,
> ./third_party/skia/include/core/SkColor.h:13,
> ./chrome/browser/extensions/extension_action.h:20,
> ./chrome/browser/extensions/api/extension_action/extension_action_api.h:10,
> ./chrome/browser/extensions/api/system_indicator/system_indicator_api.h:8,
> out/Debug/obj/gen/chrome/common/extensions/api/generated_api.cc:10:
> 
> Same is the case with sync_proto below.
> 

Hm, could you add the dependency where this rule is used then?

[Joe Thomas, 2013-02-11 23:20:15]

On 2013/02/11 23:00:42, kalman wrote:
>
https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
> File build/json_schema_bundle_compile.gypi (right):
> 
>
https://codereview.chromium.org/12181005/diff/5001/build/json_schema_bundle_c...
> build/json_schema_bundle_compile.gypi:61: '<(DEPTH)/skia/skia.gyp:skia',
> On 2013/02/11 22:58:23, Joe Thomas wrote:
> > On 2013/02/10 16:03:44, kalman wrote:
> > > why is this change needed?
> > 
> > This is required to solve the header file dependency while compiling
> > generated_api.cc. Below is the sequence from where the dependency comes from
> > 
> > SkTypes.h:14:26: fatal error: SkUserConfig.h: No such file or directory
> > ./third_party/skia/include/core/SkFixed.h:13:0,
> > ./third_party/skia/include/core/SkScalar.h:13,
> > ./third_party/skia/include/core/SkColor.h:13,
> > ./chrome/browser/extensions/extension_action.h:20,
> > ./chrome/browser/extensions/api/extension_action/extension_action_api.h:10,
> > ./chrome/browser/extensions/api/system_indicator/system_indicator_api.h:8,
> > out/Debug/obj/gen/chrome/common/extensions/api/generated_api.cc:10:
> > 
> > Same is the case with sync_proto below.
> > 
> 
> Hm, could you add the dependency where this rule is used then?

Done. Moved to api.gyp.

[not at google - send to devlin, 2013-02-11 23:22:49]

Thanks, lgtm.

[commit-bot: I haz the power, 2013-02-11 23:24:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12181005/5002

[commit-bot: I haz the power, 2013-02-11 23:56:33]

Retried try job too often on mac for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[Joe Thomas, 2013-02-12 00:01:27]

kalman@

With the CL https://codereview.chromium.org/11412211, all targets that depend on
syn should include sync.gyp:sync instead of inner sync targets (Note #3 in
https://codereview.chromium.org/11412211/diff/200040/sync/sync.gyp). Modified
this CL accordingly.

I suspect that this will solve the dependency errors that we encountered on some
of the bots with the previous version(patch set 4).

[Joe Thomas, 2013-02-12 00:08:56]

Could you please add win/mac trybots which failed with patch set 4?

[not at google - send to devlin, 2013-02-12 01:04:01]

What do you mean add?

[Joe Thomas, 2013-02-12 01:09:16]

On 2013/02/12 01:04:01, kalman wrote:
> What do you mean add?

Just wanted to make sure that PatchSet 5 builds on Mac and Win trybots before
submitting the CL. PatchSet 4 failed for these bots. I don't have the permission
to add trybots.

[not at google - send to devlin, 2013-02-12 01:11:16]

just CQ It?

[commit-bot: I haz the power, 2013-02-12 01:16:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12181005/12002

[Joe Thomas, 2013-02-12 01:16:48]

On 2013/02/12 01:11:16, kalman wrote:
> just CQ It?

Sure. I can do it. I thought it's better to add only the affected bots without
setting the COMMIT bit to avoid CQ trying on all the bots.

[commit-bot: I haz the power, 2013-02-12 04:45:43]

Change committed as 181848