Use ScopedVector instead of std::vector in case of repeated messages.

Use ScopedVector instead of std::vector in case of repeated messages.

During using this library, I realized that we want to use DISALLOW_COPY_AND_ASSIGN
for the messages and RepeatedMessageConverter is inconsistent with such ones
because it uses a copy constructor as "push_back(NestedType())".  Then
I now think to use ScopedVector.

BUG=109375
TEST=passed locally


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

[awong, 2012-01-13 00:27:54]

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

http://codereview.chromium.org/9187047/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:214: const base::Value& value,
std::vector<Element>* field) const OVERRIDE {
Why do we still use a std::vector<> here?

[Jun Mukai, 2012-01-13 01:09:21]

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

http://codereview.chromium.org/9187047/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:214: const base::Value& value,
std::vector<Element>* field) const OVERRIDE {
On 2012/01/13 00:27:55, awong wrote:
> Why do we still use a std::vector<> here?

Either is fine but I don't have a strong motivation to rewrite here because this
is for basic types such like int, double, bool, and strings.  Also
ScopedVector<int> will return int* which seems not handy to use.

The problem here might be that I haven't yet met a real case scenario to use
this.  It could be okay to keep this now and remove this in another CL.

Any is fine to me.  I'll try rewriting to ScopedVector and see how it's going to
be.

[Jun Mukai, 2012-01-13 02:10:37]

Done the rewriting.  PTAL.

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

http://codereview.chromium.org/9187047/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:214: const base::Value& value,
std::vector<Element>* field) const OVERRIDE {
On 2012/01/13 00:27:55, awong wrote:
> Why do we still use a std::vector<> here?

Done.

[satorux1, 2012-01-13 02:13:51]

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

http://codereview.chromium.org/9187047/diff/1/base/json/json_value_converter....
base/json/json_value_converter.h:214: const base::Value& value,
std::vector<Element>* field) const OVERRIDE {
On 2012/01/13 02:10:37, mukai wrote:
> On 2012/01/13 00:27:55, awong wrote:
> > Why do we still use a std::vector<> here?
> 
> Done.

nice. ints and doubles are cheap to copy but strings aren't. making it
ScopedVector would also be good for consistency.

[awong, 2012-01-13 02:21:41]

LGTM

[Jun Mukai, 2012-01-13 11:35:41]

Evan, can you check this CL too for the OWNER's point of view?

[Evan Martin, 2012-01-13 16:25:17]

LGTM

[commit-bot: I haz the power, 2012-01-13 16:36:09]

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

[commit-bot: I haz the power, 2012-01-13 16:36:12]

Can't apply patch for file base/json/json_value_converter_unittest.cc.
While running patch -p1 --forward --force;
patching file base/json/json_value_converter_unittest.cc
Hunk #1 FAILED at 10.
Hunk #2 FAILED at 20.
Hunk #3 succeeded at 54 (offset 18 lines).
Hunk #4 succeeded at 169 with fuzz 1 (offset 101 lines).
Hunk #5 FAILED at 104.
Hunk #6 FAILED at 149.
4 out of 6 hunks FAILED -- saving rejects to file
base/json/json_value_converter_unittest.cc.rej

[Jun Mukai, 2012-01-13 17:16:00]

Oops... Did the git rebase and resolve the conflict.  Trying to make sure by git
try.

[awong, 2012-01-13 17:42:26]

FYI, if your'e going to commit queue it and all that your'e worried about
is compile correctness, you can skip the git try.  Both are going to do
just as good a job at scrubbing compiles.

On Fri, Jan 13, 2012 at 9:16 AM, <mukai@chromium.org> wrote:

> Oops... Did the git rebase and resolve the conflict.  Trying to make sure
> by git
> try.
>
>
http://codereview.chromium.**org/9187047/<http://codereview.chromium.org/9187...
>

[commit-bot: I haz the power, 2012-01-14 04:20:17]

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

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

Change committed as 117786