Add "platforms" key in IDL schema compiler.

Add "platforms" key in IDL schema compiler.

Add analogue to JSON "platforms" key in IDL to restrict extension APIs'
availability on different OS.

The "platforms" key format like this:
  [platforms=("linux", "chromeos", "mac", "win", "chromeos_touch")]
  namespace Foo { ... };

BUG=119398
TEST=manual

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

[Haojian Wu, 2013-10-24 16:00:52]

Please review it, thanks.

[asargent_no_longer_on_chrome, 2013-10-24 16:06:46]

Code changes seem fine. Can you please add some unit tests? You can see existing
tests and add yours in tools/json_schema_compiler/idl_schema_test.py , putting
new data files in the tools/json_schema_compiler/test/ directory.

[Haojian Wu, 2013-10-25 09:19:32]

On 2013/10/24 16:06:46, Antony Sargent wrote:
> Code changes seem fine. Can you please add some unit tests? You can see
existing
> tests and add yours in tools/json_schema_compiler/idl_schema_test.py , putting
> new data files in the tools/json_schema_compiler/test/ directory.

Done. Also +kalman@ for c/r/r/e/binding.js review.

[asargent_no_longer_on_chrome, 2013-10-25 16:45:03]

lgtm

[not at google - send to devlin, 2013-10-25 17:00:08]

https://codereview.chromium.org/38573008/diff/40001/chrome/renderer/resources...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/38573008/diff/40001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:117: return
!schemaNode.platforms || schemaNode.platforms.length == 0 ||
if something specificies "platforms: []" then it should be an error, not work
for all platforms.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:317: platforms=[]):
don't set optional parameters to mutable objects, it means that whenever it
changes for one instance it'll change for all of them.

besides which, it should be None.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:375: platforms = []
None

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:386: internal = False
platforms = None here?

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
again, empty-platform-list should be error not all-platforms.

[Haojian Wu, 2013-10-26 03:15:51]

https://codereview.chromium.org/38573008/diff/40001/chrome/renderer/resources...
File chrome/renderer/resources/extensions/binding.js (right):

https://codereview.chromium.org/38573008/diff/40001/chrome/renderer/resources...
chrome/renderer/resources/extensions/binding.js:117: return
!schemaNode.platforms || schemaNode.platforms.length == 0 ||
On 2013/10/25 17:00:08, kalman wrote:
> if something specificies "platforms: []" then it should be an error, not work
> for all platforms.

Done.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:317: platforms=[]):
On 2013/10/25 17:00:08, kalman wrote:
> don't set optional parameters to mutable objects, it means that whenever it
> changes for one instance it'll change for all of them.
> 
> besides which, it should be None.

Done.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:375: platforms = []
On 2013/10/25 17:00:08, kalman wrote:
> None

Done.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/idl_schema.py:386: internal = False
On 2013/10/25 17:00:08, kalman wrote:
> platforms = None here?

Done. I forget to reset it here.

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/40001/tools/json_schema_compile...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
On 2013/10/25 17:00:08, kalman wrote:
> again, empty-platform-list should be error not all-platforms.

Done.

[hirono, 2013-10-28 02:31:56]

lgtm with nits! Thank you for fixing it!

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
File tools/json_schema_compiler/test/idl_namespace_all_platforms.idl (right):

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
tools/json_schema_compiler/test/idl_namespace_all_platforms.idl:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
nit: Could you replace 2012 with 2013 along the way? Thank you!

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
File tools/json_schema_compiler/test/idl_namespace_chromeos.idl (right):

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
tools/json_schema_compiler/test/idl_namespace_chromeos.idl:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
ditto

[Haojian Wu, 2013-10-28 03:12:51]

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
File tools/json_schema_compiler/test/idl_namespace_all_platforms.idl (right):

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
tools/json_schema_compiler/test/idl_namespace_all_platforms.idl:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
On 2013/10/28 02:31:56, hirono wrote:
> nit: Could you replace 2012 with 2013 along the way? Thank you!

Done.

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
File tools/json_schema_compiler/test/idl_namespace_chromeos.idl (right):

https://codereview.chromium.org/38573008/diff/130001/tools/json_schema_compil...
tools/json_schema_compiler/test/idl_namespace_chromeos.idl:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2013/10/28 02:31:56, hirono wrote:
> ditto

Done.

[not at google - send to devlin, 2013-10-28 17:04:47]

lgtm

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:317: platforms=None):
bad news; now these need to all be on different lines.

def __init__(self,
             namespace_node,
             ...)

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
if "platforms" can't be empty then this change doesn't do anything.

instead,

if 'platforms' not in json:
  return None
if not json['platforms']
  raise ValueError('"platforms" cannot be an empty list')

though just not changing this file at all is fine with me, too.

[Haojian Wu, 2013-10-29 02:09:31]

I also add more test cases and model test. Please review this part.

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:317: platforms=None):
On 2013/10/28 17:04:48, kalman wrote:
> bad news; now these need to all be on different lines.
> 
> def __init__(self,
>              namespace_node,
>              ...)

Done.

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
On 2013/10/28 17:04:48, kalman wrote:
> if "platforms" can't be empty then this change doesn't do anything.
> 
> instead,
> 
> if 'platforms' not in json:
>   return None
> if not json['platforms']
>   raise ValueError('"platforms" cannot be an empty list')
> 
> though just not changing this file at all is fine with me, too.

The statement "not json['platforms']"'s aim is to check whether the list is
None, instead empty list.

According to your comments, now I polish the code a little bit, and add empty
list check.

[not at google - send to devlin, 2013-10-29 14:36:51]

lgtm assuming you can delete that ==None check.

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
On 2013/10/29 02:09:32, Haojian Wu wrote:
> On 2013/10/28 17:04:48, kalman wrote:
> > if "platforms" can't be empty then this change doesn't do anything.
> > 
> > instead,
> > 
> > if 'platforms' not in json:
> >   return None
> > if not json['platforms']
> >   raise ValueError('"platforms" cannot be an empty list')
> > 
> > though just not changing this file at all is fine with me, too.
> 
> The statement "not json['platforms']"'s aim is to check whether the list is
> None, instead empty list.

How is platforms None? There are three false states here: platforms not in the
JSON, platforms empty, and platforms set to None (or False or whatever). I don't
understand how the last of those could happen apart from just a wrong schema.

What breaks if you remove the json['platforms'] == None check?

> 
> According to your comments, now I polish the code a little bit, and add empty
> list check.

Thanks.

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
tools/json_schema_compiler/model.py:491: # Check whether "platforms" is an empty
list.
More accurate would be to say "Sanity check: platforms should not be an empty
list."

[Haojian Wu, 2013-10-30 00:42:47]

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/190001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:489: if 'platforms' not in json or not
json['platforms']:
On 2013/10/29 14:36:52, kalman wrote:
> How is platforms None? There are three false states here: platforms not in the
> JSON, platforms empty, and platforms set to None (or False or whatever). I
don't
> understand how the last of those could happen apart from just a wrong schema.
> 
> What breaks if you remove the json['platforms'] == None check?
> 
> > 
> > According to your comments, now I polish the code a little bit, and add
empty
> > list check.
> 
> Thanks.

The third false states: platforms set to None, there are two small cases, see
below:

1) If an IDL file with non-specific platfroms, like:

namespace Foo {...}

The json['platforms'] will be None.

2) If an IDL file with an empty platforms, like:

[platforms=()]
namespace Foo {...}

The json['platforms'] will be an empty list.

In the first case, _GetPlatforms should return None, while in the second case,
should raise an error based on your suggestions.

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:354: 'platforms': self.platforms}
Here.

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/38573008/diff/200006/tools/json_schema_compil...
tools/json_schema_compiler/model.py:491: # Check whether "platforms" is an empty
list.
On 2013/10/29 14:36:52, kalman wrote:
> More accurate would be to say "Sanity check: platforms should not be an empty
> list."

Done.

[not at google - send to devlin, 2013-10-30 00:44:27]

ah I was forgetting about the IDL compiler case. I assumed it wasn't setting
platforms to None. anyway, lgtm.

[commit-bot: I haz the power, 2013-10-30 00:50:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/38573008/390001

[commit-bot: I haz the power, 2013-10-30 05:29:02]

Change committed as 231767