[Extensions API] Remove inline enums

[Extensions API] Remove inline enums

Remove all inlined enums from extension APIs.

BUG=472279
TBR=derat@chromium.org (mechanical changes to chrome/browser/chromeos/app_mode/kiosk_app_update_service.cc and chrome/browser/ui/ash/ash_keyboard_controller_proxy.cc)

Committed: https://crrev.com/00f1fc22bfcc653b47d62778170af3eee9855720
Cr-Commit-Position: refs/heads/master@{#323912}

[Devlin, 2015-04-02 23:54:34]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-04-02 23:54:43]

Patchset #1 (id:20001) has been deleted

[Devlin, 2015-04-02 23:54:52]

Patchset #1 (id:40001) has been deleted

[Devlin, 2015-04-02 23:55:01]

Patchset #1 (id:60001) has been deleted

[Devlin, 2015-04-03 00:00:54]

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

[Devlin, 2015-04-03 00:00:56]

Hey Ben - so this one sucks.  No other way to cut it.  But it's a one-time cost.

It's mostly a mechanical change, with the exception of when I found something
that made me cringe too badly to pass up (context menus stuff comes to mind).

https://codereview.chromium.org/1055673002/diff/80001/chrome/test/data/extens...
File
chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/assertions.js
(right):

https://codereview.chromium.org/1055673002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/assertions.js:34:
name == 'substr' ||
Without these, we hit String.substr and Object.hasOwnProperty in the bindings
when creating the enums
(https://code.google.com/p/chromium/codesearch#chromium/src/extensions/rendere...).
 This strikes me as probably wrong... but this patch is just making that code
path trigger, not causing the bindings to execute anywhere differently. 
Thoughts?

[not at google - send to devlin, 2015-04-03 17:04:38]

lgtm, but double check that enums aren't automatically inlined in the
documentation. I can't see a change that would prevent that from happening.

https://code.google.com/p/chromium/codesearch#search/&q=_DetectInlineableType...
might be a good start.

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/m...
File extensions/common/api/management.json (right):

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/m...
extensions/common/api/management.json:26: "id": "ExtensionInfoDisabledReason",
I'd just have "ExtensionDisabledReason..." etc, not
"ExtensionInfoDisabledReason". For all the enums obviously.

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/r...
File extensions/common/api/runtime.json (right):

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/r...
extensions/common/api/runtime.json:47: "id": "PlatformInfoOs",
Likewise, might as well leave out the "Info" here.

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

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:154: real_t.namespace !=
self._namespace else '',
So, the docserver code is all over the place when it comes to styling ternary
ifs, mostly because there is no good way to format python ternary ifs at all -
but the correct way (that I can tell) according to Google conventions is:

prop.unix_name,
('%s::' % real_t.namespace.unix_name
 if real_t.namespace != self._namespace
 else '')

And it would look better if you didn't inline it TBH:

namespace_prefix = ('%s::' % real_t.namespace.unix_name
                    if real_t.namespace != self._namespace
                    else '')
items.append(...)

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:296: (('%s::' %
underlying_type.namespace.unix_name) if
Ditto. This is even harder to dissect.

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:1037: ('%s::' %
underlying_type.namespace.unix_name) if
Ditto.

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

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:205: if not namespace.allow_inline_enums and
not isinstance(parent, Namespace):
Is there a reason to make this a parameter rather than just the default
behavior?

[Devlin, 2015-04-03 19:38:51]

https://codereview.chromium.org/1055673002/diff/80001/chrome/test/data/extens...
File
chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/assertions.js
(right):

https://codereview.chromium.org/1055673002/diff/80001/chrome/test/data/extens...
chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/assertions.js:34:
name == 'substr' ||
On 2015/04/03 00:00:56, Devlin wrote:
> Without these, we hit String.substr and Object.hasOwnProperty in the bindings
> when creating the enums
>
(https://code.google.com/p/chromium/codesearch#chromium/src/extensions/rendere...).
>  This strikes me as probably wrong... but this patch is just making that code
> path trigger, not causing the bindings to execute anywhere differently. 
> Thoughts?

Resolved.

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/m...
File extensions/common/api/management.json (right):

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/m...
extensions/common/api/management.json:26: "id": "ExtensionInfoDisabledReason",
On 2015/04/03 17:04:38, kalman wrote:
> I'd just have "ExtensionDisabledReason..." etc, not
> "ExtensionInfoDisabledReason". For all the enums obviously.

Done.

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/r...
File extensions/common/api/runtime.json (right):

https://codereview.chromium.org/1055673002/diff/80001/extensions/common/api/r...
extensions/common/api/runtime.json:47: "id": "PlatformInfoOs",
On 2015/04/03 17:04:38, kalman wrote:
> Likewise, might as well leave out the "Info" here.

Done.

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

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:154: real_t.namespace !=
self._namespace else '',
On 2015/04/03 17:04:38, kalman wrote:
> So, the docserver code is all over the place when it comes to styling ternary
> ifs, mostly because there is no good way to format python ternary ifs at all -
> but the correct way (that I can tell) according to Google conventions is:
> 
> prop.unix_name,
> ('%s::' % real_t.namespace.unix_name
>  if real_t.namespace != self._namespace
>  else '')
> 
> And it would look better if you didn't inline it TBH:
> 
> namespace_prefix = ('%s::' % real_t.namespace.unix_name
>                     if real_t.namespace != self._namespace
>                     else '')
> items.append(...)

Done.

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:296: (('%s::' %
underlying_type.namespace.unix_name) if
On 2015/04/03 17:04:38, kalman wrote:
> Ditto. This is even harder to dissect.

Done.

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/cc_generator.py:1037: ('%s::' %
underlying_type.namespace.unix_name) if
On 2015/04/03 17:04:38, kalman wrote:
> Ditto.

Done.

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

https://codereview.chromium.org/1055673002/diff/80001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:205: if not namespace.allow_inline_enums and
not isinstance(parent, Namespace):
On 2015/04/03 17:04:38, kalman wrote:
> Is there a reason to make this a parameter rather than just the default
> behavior?

The docserver inlines types, and then feeds them back into the compiler, so we
need to still allow for that functionality.

[not at google - send to devlin, 2015-04-03 20:01:23]

lgtm

[Devlin, 2015-04-03 20:40:50]

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

[commit-bot: I haz the power, 2015-04-03 20:41:27]

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

[commit-bot: I haz the power, 2015-04-03 20:57:01]

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

[commit-bot: I haz the power, 2015-04-03 20:57:03]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Devlin, 2015-04-06 17:13:13]

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

[Devlin, 2015-04-06 17:13:25]

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

[Devlin, 2015-04-06 17:13:25]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1055673002/#ps120001
(title: "Latest master")

[commit-bot: I haz the power, 2015-04-06 17:13:50]

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

[commit-bot: I haz the power, 2015-04-06 17:19:44]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2015-04-06 17:36:27]

Patchset 3 (id:??) landed as
https://crrev.com/00f1fc22bfcc653b47d62778170af3eee9855720
Cr-Commit-Position: refs/heads/master@{#323912}