Allow trailing comma in JSON policy files.

Allow trailing comma in JSON policy files.

This is not really part of the JSON standard but a common practice and is
harmless in most of the cases so we can enable it for JSON policy files.

BUG=101141
TEST=none


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

[pastarmovj, 2011-11-03 10:13:55]

A tiny change to enable trailing comma in the JSON policy parser.

Btw the base/json folder has no OWNERS do I need buy in by some base owner or is
it fine if you lgtm this humongous change?

[Mattias Nissler (ping if slow), 2011-11-03 10:31:28]

LGTM

[pastarmovj, 2011-11-03 10:35:51]

Adding brettw for owners blessing.

[pastarmovj, 2011-11-08 15:35:31]

Hi Evan,
Initially I have added brettw as OWNER reviewer for my base changes because he
has touched/reviewed this file but he seem to be away for a while so can you
please review this small change?
Thanks!

[pastarmovj, 2011-11-10 13:12:24]

Hi Will,
I understand that Evan might also not be able to review that small CL so can you
please have a look at it as an OWNER of base?

[willchan no longer on Chromium, 2011-11-10 18:42:29]

On 2011/11/10 13:12:24, pastarmovj wrote:
> Hi Will,
> I understand that Evan might also not be able to review that small CL so can
you
> please have a look at it as an OWNER of base?

Please add a basic unittest for the base code.

[pastarmovj, 2011-11-14 16:10:26]

I implemented a suite of tests for JSONValueSerializer which was not present
until now and I test the new flag in there too.

[willchan no longer on Chromium, 2011-11-15 02:36:22]

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
File base/json/json_value_serializer_unittest.cc (right):

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:7: #include
"testing/gtest/include/gtest/gtest.h"
This one should get sorted among the rest below.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:11: #include
"base/json/json_value_serializer.h"
This one is supposed to go first, even ahead of the system include.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:19: std::string proper_json =
This is creating a static intiializer. Please use const char kProperJson[]
instead. Note also that it should be a const and CamelCased.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:60: namespace base {
You should probably just put everything within the base::(anonymous)::
namespace. There's no reason for any of this test code to have external
visibility.

[pastarmovj, 2011-11-17 10:35:35]

Made the proposed changes and added some more comments along the way. All other
changes between patchset 1 and 2 are result of rebasing on ToT.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
File base/json/json_value_serializer_unittest.cc (right):

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:7: #include
"testing/gtest/include/gtest/gtest.h"
On 2011/11/15 02:36:22, willchan wrote:
> This one should get sorted among the rest below.

Done.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:11: #include
"base/json/json_value_serializer.h"
On 2011/11/15 02:36:22, willchan wrote:
> This one is supposed to go first, even ahead of the system include.

Done.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:19: std::string proper_json =
On 2011/11/15 02:36:22, willchan wrote:
> This is creating a static intiializer. Please use const char kProperJson[]
> instead. Note also that it should be a const and CamelCased.

Done.

http://codereview.chromium.org/8423079/diff/7001/base/json/json_value_seriali...
base/json/json_value_serializer_unittest.cc:60: namespace base {
On 2011/11/15 02:36:22, willchan wrote:
> You should probably just put everything within the base::(anonymous)::
> namespace. There's no reason for any of this test code to have external
> visibility.

Done.

[willchan no longer on Chromium, 2011-11-17 14:34:43]

lgtm

[commit-bot: I haz the power, 2011-11-17 15:00:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pastarmovj@chromium.org/8423079/15001

[commit-bot: I haz the power, 2011-11-17 16:17:25]

Change committed as 110503