Add additional restriction to policy schema internal (part2)

Add additional restriction to policy schema internal, including modification to
C++ verifier.

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

[binjin, 2014-01-20 15:39:49]

Hello Joao,

Here is the second CL to modify the C++ verifier. PTAL

Bin

[binjin, 2014-01-20 15:59:04]

Hello Joao,

I just fixed another failed unit tests due to modification to testschema, PTAL
at patchset 2 instead. Thanks

Bin

[Joao da Silva, 2014-01-20 20:22:17]

Great, this was exactly what we had in mind. Just a comment about moving the
validation code to the Schema class.

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
File components/policy/core/common/schema.cc (right):

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema.cc:131: bool ValidateStringRestriction(int
index, const char *str) const;
These should be private methods of Schema::, not of Schema::InternalStorage::.

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
File components/policy/core/common/schema_unittest.cc (right):

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema_unittest.cc:54: "      \"enum\": [1, 2, 3]"
This enumeration will get converted into a range (which is a good test to have).

Add another integer enum with a gap in the range, so that it doesn't get
converted to a range and test that too. For example, [ 10, 20, 30 ]

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema_unittest.cc:555: 
After adding the integer enum with gaps, test values here that are outside the
range and also between the gaps. For example, 0, 9, 10 (true), 11, 19, 20
(true), 21, 29, 30(true), 31, 100

[binjin, 2014-01-21 16:12:07]

Hello Joao,

Here are the fixes, PTAL

Bin

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
File components/policy/core/common/schema.cc (right):

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema.cc:131: bool ValidateStringRestriction(int
index, const char *str) const;
On 2014/01/20 20:22:18, Joao da Silva wrote:
> These should be private methods of Schema::, not of Schema::InternalStorage::.

Done.

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
File components/policy/core/common/schema_unittest.cc (right):

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema_unittest.cc:54: "      \"enum\": [1, 2, 3]"
On 2014/01/20 20:22:18, Joao da Silva wrote:
> This enumeration will get converted into a range (which is a good test to
have).
> 
> Add another integer enum with a gap in the range, so that it doesn't get
> converted to a range and test that too. For example, [ 10, 20, 30 ]

Done.

https://codereview.chromium.org/143413002/diff/40001/components/policy/core/c...
components/policy/core/common/schema_unittest.cc:555: 
On 2014/01/20 20:22:18, Joao da Silva wrote:
> After adding the integer enum with gaps, test values here that are outside the
> range and also between the gaps. For example, 0, 9, 10 (true), 11, 19, 20
> (true), 21, 29, 30(true), 31, 100

Done.

[Joao da Silva, 2014-01-21 16:30:05]

lgtm

https://codereview.chromium.org/143413002/diff/110001/components/policy/core/...
File components/policy/core/common/schema.cc (right):

https://codereview.chromium.org/143413002/diff/110001/components/policy/core/...
components/policy/core/common/schema.cc:734: // static
This method is not really static: it uses instance variables like storage_, so
this comment shouldn't be added.

https://codereview.chromium.org/143413002/diff/110001/components/policy/core/...
components/policy/core/common/schema.cc:751: // static
Same here

[commit-bot: I haz the power, 2014-01-22 10:20:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/binjin@chromium.org/143413002/190001

[commit-bot: I haz the power, 2014-01-22 14:48:40]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2014-01-22 15:01:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/binjin@chromium.org/143413002/190001

[commit-bot: I haz the power, 2014-01-22 15:03:01]

Change committed as 246328