Remove ScopedVector from net/socket tests

Remove ScopedVector from net/socket tests

Some scoped_ptrs are made by make_scoped_ptr function because
the bare pointer is also used after putting scoped_ptr to vector.

BUG=554289
Committed: https://crrev.com/9d66b7d291c6a6797308abced82895fa531b268f
Cr-Commit-Position: refs/heads/master@{#361078}

[Olli Raula, 2015-11-19 16:01:35]

Description was changed from

==========
Remove ScopedVector from net/socket

Some scoped_ptrs are made by make_scoped_ptr function because
the bare pointer is also used after putting scoped_ptr to vector.

BUG=554289
==========

to

==========
Remove ScopedVector from net/socket

Some scoped_ptrs are made by make_scoped_ptr function because
the bare pointer is also used after putting scoped_ptr to vector.

BUG=554289
==========

[Olli Raula, 2015-11-19 16:01:35]

olli.raula@intel.com changed reviewers:
+ mmenke@chromium.org

[Olli Raula, 2015-11-19 16:01:45]

Could you review?

[mmenke, 2015-11-19 16:26:12]

LGTM.  Two nits about pre-existing issues.

Also, could you update your description to say "Remove ScopedVector from
net/socket tests" (This removes them from net/sockets in general as well, but
think it's relevant this is a test-only change)

https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util.cc
File net/socket/socket_test_util.cc (right):

https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util...
net/socket/socket_test_util.cc:1906: job_list_.push_back(make_scoped_ptr(job));
Other comment applies here, too.

https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util.h
File net/socket/socket_test_util.h (right):

https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util...
net/socket/socket_test_util.h:1034:
requests_.push_back(make_scoped_ptr(request));
nit:  Think it's cleaner (And safer across refactors) to either make it a
scoped_ptr<TestSocketRequest> and use pass, or just inline its construction in
the make_scoped_ptr call.

[Olli Raula, 2015-11-20 09:19:06]

Description was changed from

==========
Remove ScopedVector from net/socket

Some scoped_ptrs are made by make_scoped_ptr function because
the bare pointer is also used after putting scoped_ptr to vector.

BUG=554289
==========

to

==========
Remove ScopedVector from net/socket tests

Some scoped_ptrs are made by make_scoped_ptr function because
the bare pointer is also used after putting scoped_ptr to vector.

BUG=554289
==========

[Olli Raula, 2015-11-20 09:22:06]

>
https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util...
> net/socket/socket_test_util.h:1034:
> requests_.push_back(make_scoped_ptr(request));
> nit:  Think it's cleaner (And safer across refactors) to either make it a
> scoped_ptr<TestSocketRequest> and use pass, or just inline its construction in
> the make_scoped_ptr call.

Hmm, that was the thing what I was saying in the commit message. We need that 
bare pointer later in the code so we cannot construct scoped_ptr directly. Any
succestions?

[mmenke, 2015-11-20 16:21:35]

On 2015/11/20 09:22:06, Olli Raula wrote:
> >
>
https://codereview.chromium.org/1459633003/diff/1/net/socket/socket_test_util...
> > net/socket/socket_test_util.h:1034:
> > requests_.push_back(make_scoped_ptr(request));
> > nit:  Think it's cleaner (And safer across refactors) to either make it a
> > scoped_ptr<TestSocketRequest> and use pass, or just inline its construction
in
> > the make_scoped_ptr call.
> 
> Hmm, that was the thing what I was saying in the commit message. We need that 
> bare pointer later in the code so we cannot construct scoped_ptr directly. Any
> succestions?

Sorry, I missed that.  I'm not sure any of the alternatives are much better
(Grab a raw pointer before passing ownership, using job_list_.back(), calling
connect before adding it to the list).  Of those, I think the last is probably
the best, since ConnectJobs don't invoke their callback synchronously.  Think we
can just keep it as-is, though.

LGTM

[Olli Raula, 2015-11-23 07:14:50]

The CQ bit was checked by olli.raula@intel.com

[commit-bot: I haz the power, 2015-11-23 07:14:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1459633003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1459633003/1

[commit-bot: I haz the power, 2015-11-23 08:30:49]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-11-23 08:31:46]

Patchset 1 (id:??) landed as
https://crrev.com/9d66b7d291c6a6797308abced82895fa531b268f
Cr-Commit-Position: refs/heads/master@{#361078}