Generate Chrome policy definition list from policy_templates.json.

Generate Chrome policy definition list from policy_templates.json.

The struct and static array is now generated into policy_constants.h.
Added policy::GetChromePolicyDefinitionList() and policy::GetPolicyName().
Removed ConfigurationPolicyProvider::PolicyDefinitionList.
Removed PolicyStates::GetPolicyName().
Removed ConfigurationPolicyPrefStore::GetChromePolicyDefinitionList().

BUG=None
TEST=Everything works as before, nothing breaks


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105388

[Joao da Silva, 2011-10-13 13:47:37]

Please review, thanks!

[Mattias Nissler (ping if slow), 2011-10-13 14:14:34]

couple of nits.

http://codereview.chromium.org/8258018/diff/1/chrome/app/policy/policy_templa...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8258018/diff/1/chrome/app/policy/policy_templa...
chrome/app/policy/policy_templates.json:78: #   The product name also affects in
which templates will the policy be included.
"templates" doesn't exactly suggest "generated code". Also, the line is one
character too long :)

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
(right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_provider_mac_unittest.cc:143:
GetChromePolicyDefinitionList(), prefs_);
fits previous line?

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
File chrome/browser/policy/file_based_policy_provider_unittest.cc (right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
chrome/browser/policy/file_based_policy_provider_unittest.cc:45:
GetChromePolicyDefinitionList(),
fits previous line?

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
chrome/browser/policy/file_based_policy_provider_unittest.cc:64:
GetChromePolicyDefinitionList(),
fits previous line?

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/policy_map.h
File chrome/browser/policy/policy_map.h (right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/policy_ma...
chrome/browser/policy/policy_map.h:15: class PolicyDefinitionList;
should be struct?

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:103: return [ x for (x,_,_,_) in
_GetPolicyList(template_file_contents) ]
space after comma?

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:107: return [ (x,y) for (x,y,z,w)
in _GetPolicyList(template_file_contents)
space after comma?

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:112: return [ (x,y) for (x,y,z,w)
in _GetPolicyList(template_file_contents)
space after comma?

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:155: '// Gets the policy name
indexed by the policy type.\n'
s/indexed by the/for the given/g

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:211: ' ++entry) {\n'
Please indent this line by two spaces if python allows it.

[Joao da Silva, 2011-10-13 14:40:43]

Reviewed, PTAL.

Hopefully also fixed the clang and chromeos build.

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/configura...
File chrome/browser/policy/configuration_policy_provider_mac_unittest.cc
(right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/configura...
chrome/browser/policy/configuration_policy_provider_mac_unittest.cc:143:
GetChromePolicyDefinitionList(), prefs_);
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> fits previous line?

Almost, needs 83 cols.

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
File chrome/browser/policy/file_based_policy_provider_unittest.cc (right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
chrome/browser/policy/file_based_policy_provider_unittest.cc:45:
GetChromePolicyDefinitionList(),
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> fits previous line?

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/file_base...
chrome/browser/policy/file_based_policy_provider_unittest.cc:64:
GetChromePolicyDefinitionList(),
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> fits previous line?

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/policy_map.h
File chrome/browser/policy/policy_map.h (right):

http://codereview.chromium.org/8258018/diff/1/chrome/browser/policy/policy_ma...
chrome/browser/policy/policy_map.h:15: class PolicyDefinitionList;
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> should be struct?

The clang bots are of the same opinion :-) Done.

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:103: return [ x for (x,_,_,_) in
_GetPolicyList(template_file_contents) ]
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> space after comma?

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:107: return [ (x,y) for (x,y,z,w)
in _GetPolicyList(template_file_contents)
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> space after comma?

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:112: return [ (x,y) for (x,y,z,w)
in _GetPolicyList(template_file_contents)
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> space after comma?

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:155: '// Gets the policy name
indexed by the policy type.\n'
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> s/indexed by the/for the given/g

Done.

http://codereview.chromium.org/8258018/diff/1/chrome/tools/build/generate_pol...
chrome/tools/build/generate_policy_source.py:211: ' ++entry) {\n'
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> Please indent this line by two spaces if python allows it.

Obsolete; this became an array lookup as discussed offline.

[Mattias Nissler (ping if slow), 2011-10-13 14:58:00]

LGTM

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:108: if "chrome" in z and not w ]
replace with whatever that discussion next to me comes up with :D

[pastarmovj, 2011-10-13 15:00:45]

LGTM. 
Great negative (LOC-wise) contribution!
Only a style nit.

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:103: return [ x for (x, _, _, _) in
_GetPolicyList(template_file_contents) ]
Drop the space.

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:107: return [ (x, y) for (x, y, z,
w) in _GetPolicyList(template_file_contents)
Just drop the space and we'll pretend there were not 5 people staring and
commenting on this multi-line comprehension. ;)

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:112: return [ (x, y) for (x, y, z,
w) in _GetPolicyList(template_file_contents)
ditto

[Joao da Silva, 2011-10-13 16:31:16]

One last round :-)

Please notice these additional changes:
- comment on policy_templates.json
- changes in cloud_policy_codegen.gyp (the initial CL broke the win64 build)
- python list comprehension: using more descriptive names instead of xyzw
- providers unittests: removed device_only policy test, ifdef'd chrome_os only
policy test

http://codereview.chromium.org/8258018/diff/1/chrome/app/policy/policy_templa...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8258018/diff/1/chrome/app/policy/policy_templa...
chrome/app/policy/policy_templates.json:78: #   The product name also affects in
which templates will the policy be included.
On 2011/10/13 14:14:34, Mattias Nissler wrote:
> "templates" doesn't exactly suggest "generated code". Also, the line is one
> character too long :)

Done.

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:103: return [ x for (x, _, _, _) in
_GetPolicyList(template_file_contents) ]
On 2011/10/13 15:00:46, pastarmovj wrote:
> Drop the space.

Done.

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:107: return [ (x, y) for (x, y, z,
w) in _GetPolicyList(template_file_contents)
On 2011/10/13 15:00:46, pastarmovj wrote:
> Just drop the space and we'll pretend there were not 5 people staring and
> commenting on this multi-line comprehension. ;)

Done :-)

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:108: if "chrome" in z and not w ]
On 2011/10/13 14:58:00, Mattias Nissler wrote:
> replace with whatever that discussion next to me comes up with :D

I added more descriptive names at the expense of shorter lines. wdyt?

http://codereview.chromium.org/8258018/diff/3003/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:112: return [ (x, y) for (x, y, z,
w) in _GetPolicyList(template_file_contents)
On 2011/10/13 15:00:46, pastarmovj wrote:
> ditto

Same as above.

[Mattias Nissler (ping if slow), 2011-10-13 16:45:03]

still LGTM, with even more creative list building proposals :)

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:109: if "chrome" in platforms and
not is_device_only]
now that this is three lines, I'd go for an actual loop :) Or you can actually
do something like:

map(lambda (x, y, _, _): (x, y),
    filter(lambda (_, _, p, d) : 'chrome' in p and not d,
           _GetPolicyList(template_file_contents))

In any case, please use single quotes.

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:117: and not is_device_only]
same here.

[Joao da Silva, 2011-10-13 17:38:17]

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
File chrome/tools/build/generate_policy_source.py (right):

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:109: if "chrome" in platforms and
not is_device_only]
On 2011/10/13 16:45:03, Mattias Nissler wrote:
> now that this is three lines, I'd go for an actual loop :) Or you can actually
> do something like:
> 
> map(lambda (x, y, _, _): (x, y),
>     filter(lambda (_, _, p, d) : 'chrome' in p and not d,
>            _GetPolicyList(template_file_contents))
> 
> In any case, please use single quotes.

Done (single quotes). I still find this more readable than a loop :-)

http://codereview.chromium.org/8258018/diff/9002/chrome/tools/build/generate_...
chrome/tools/build/generate_policy_source.py:117: and not is_device_only]
On 2011/10/13 16:45:03, Mattias Nissler wrote:
> same here.

Done.

[commit-bot: I haz the power, 2011-10-13 20:18:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/8258018/5004

[commit-bot: I haz the power, 2011-10-13 22:12:41]

Change committed as 105388