Returns a bool for JSONValueConverter::Convert()

Returns a bool for JSONValueConverter::Convert()

As I see a real use scenario in chrome/browser/chromeos/gdata/
gdata_parser.cc, I realized that sometimes structural validity
matters.

It also introduce string16 whose code is almost equivalent to
std::string.

BUG=none
TEST=passed


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

[awong, 2012-01-03 22:02:09]

LGTM

FYI, Will's out on leave until late January so you should probably find another
base owner.

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter.h
File base/json/json_value_converter.h (right):

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:52: // Also note that Convert() will modify the
passed |message| even when fails
when fails -> when it fails

[satorux1, 2012-01-03 22:21:49]

adding evan to see if he's really back.

[Evan Martin, 2012-01-03 22:56:00]

Can you extend the test to cover the return-false cases?

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter.h
File base/json/json_value_converter.h (right):

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:226: DVLOG(1) << "failure at " << i << "-th
element";
Maybe this isn't necessary anymore?  A bit weird to log here but not in any of
the other false cases.

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:291: std::string StructType::* field) {
one more space here

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:297: string16 StructType::* field) {
one more space here

[Jun Mukai, 2012-01-04 16:03:52]

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter.h
File base/json/json_value_converter.h (right):

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:52: // Also note that Convert() will modify the
passed |message| even when fails
On 2012/01/03 22:02:09, awong wrote:
> when fails -> when it fails

Done.

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:226: DVLOG(1) << "failure at " << i << "-th
element";
On 2012/01/03 22:56:00, Evan Martin wrote:
> Maybe this isn't necessary anymore?  A bit weird to log here but not in any of
> the other false cases.

Other cases are simple 'key: "value"' style and its failure is reported inside
of Convert().  That's enough to notice the "value" is wrong.  On the other hand,
here is 'key: [list, of, values...]' style, so I imagine users may be interested
in which item is actually wrong.
But if you think it's not necessary, I will remove this.  Please tell me your
idea.

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:291: std::string StructType::* field) {
On 2012/01/03 22:56:00, Evan Martin wrote:
> one more space here

Done.

http://codereview.chromium.org/8952029/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:297: string16 StructType::* field) {
On 2012/01/03 22:56:00, Evan Martin wrote:
> one more space here

Done.

[Jun Mukai, 2012-01-04 23:17:45]

I also added tests of failure case too.  ptal.

[Evan Martin, 2012-01-04 23:24:38]

Awesome, thanks.  LGTM

[zel, 2012-01-04 23:41:18]

mukai, please also try to apply your JSONValueConverter class to
gdata_parser.cc. I would like to see how much simpler will the code become
there.

[commit-bot: I haz the power, 2012-01-05 08:38:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/8952029/6001

[commit-bot: I haz the power, 2012-01-05 09:46:44]

Change committed as 116473