Remove ScopedVector from tools/ipc_fuzzer/.

Remove ScopedVector from tools/ipc_fuzzer/.

BUG=554289
Review-Url: https://codereview.chromium.org/2972773004
Cr-Commit-Position: refs/heads/master@{#485337}
Committed: https://chromium.googlesource.com/chromium/src/+/ee095bcb2656c72ca3411b9a2c98cb19e789bdd5

[Avi (use Gerrit), 2017-07-06 20:27:14]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 20:27:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 20:44:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 20:44:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2017-07-06 21:15:27]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 21:16:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 21:40:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 21:40:47]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2017-07-06 21:46:01]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 21:46:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 21:56:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 21:56:16]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Avi (use Gerrit), 2017-07-06 22:03:12]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 22:03:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-07-06 22:23:40]

avi@chromium.org changed reviewers:
+ mbarbella@chromium.org

[commit-bot: I haz the power, 2017-07-06 22:40:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 22:40:58]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2017-07-10 17:34:44]

avi@chromium.org changed reviewers:
+ tsepez@chromium.org

[Avi (use Gerrit), 2017-07-10 17:34:45]

Tom, you're also an owner. PTAL; this is the last user of ScopedVector in
Chromium.

[Tom Sepez, 2017-07-10 17:45:47]

Hooray for the end of scopedvector.

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.cc:1152: p = std::move(ipc_message);
This doesn't look right even in the old code. But here, we make a message,
assign it to P, and then P goes out of scope and we delete it?

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.h (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.h:71: typedef std::unique_ptr<IPC::Message>
(*FuzzerFunction)(IPC::Message*, Fuzzer*);
nit: might as well move to |using| while were at it?

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
File tools/ipc_fuzzer/message_lib/message_file.h (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
tools/ipc_fuzzer/message_lib/message_file.h:17: typedef
std::vector<std::unique_ptr<IPC::Message>> MessageVector;
nit: move to |using| while were at it?

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
File tools/ipc_fuzzer/message_replay/replay_process.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
tools/ipc_fuzzer/message_replay/replay_process.cc:137: if
(!channel_->Send(message.release())) {
ah, pity Send() doesn't take unique_ptr itself ...

[Avi (use Gerrit), 2017-07-10 18:17:21]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-10 18:17:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-07-10 18:44:00]

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.cc:1152: p = std::move(ipc_message);
On 2017/07/10 17:45:47, Tom Sepez wrote:
> This doesn't look right even in the old code. But here, we make a message,
> assign it to P, and then P goes out of scope and we delete it?

My brain was mostly focused on getting this to compile, but you're right. I'd
been thinking that p was being used as an in/out param, but it's not.

What is going on here? How is this fuzzer even working?

Lemme try to change this to something reasonable. The FuzzTraits template takes
a pointer to the thing to fuzz, so this should be a pointer-to-unique_ptr, yes?

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.h (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.h:71: typedef std::unique_ptr<IPC::Message>
(*FuzzerFunction)(IPC::Message*, Fuzzer*);
On 2017/07/10 17:45:47, Tom Sepez wrote:
> nit: might as well move to |using| while were at it?

Done.

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
File tools/ipc_fuzzer/message_lib/message_file.h (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
tools/ipc_fuzzer/message_lib/message_file.h:17: typedef
std::vector<std::unique_ptr<IPC::Message>> MessageVector;
On 2017/07/10 17:45:47, Tom Sepez wrote:
> nit: move to |using| while were at it?

Done.

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
File tools/ipc_fuzzer/message_replay/replay_process.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/messag...
tools/ipc_fuzzer/message_replay/replay_process.cc:137: if
(!channel_->Send(message.release())) {
On 2017/07/10 17:45:47, Tom Sepez wrote:
> ah, pity Send() doesn't take unique_ptr itself ...

If we weren't moving to Mojo and obviating that, that would a refactor I would
have started years ago.

[Tom Sepez, 2017-07-10 18:45:10]

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.cc:1152: p = std::move(ipc_message);
On 2017/07/10 18:44:00, Avi (ping after 24h) wrote:
> On 2017/07/10 17:45:47, Tom Sepez wrote:
> > This doesn't look right even in the old code. But here, we make a message,
> > assign it to P, and then P goes out of scope and we delete it?
> 
> My brain was mostly focused on getting this to compile, but you're right. I'd
> been thinking that p was being used as an in/out param, but it's not.
> 
> What is going on here? How is this fuzzer even working?
> 
> Lemme try to change this to something reasonable. The FuzzTraits template
takes
> a pointer to the thing to fuzz, so this should be a pointer-to-unique_ptr,
yes?

SGTM.

[Avi (use Gerrit), 2017-07-10 18:46:44]

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
File tools/ipc_fuzzer/fuzzer/fuzzer.cc (right):

https://codereview.chromium.org/2972773004/diff/60001/tools/ipc_fuzzer/fuzzer...
tools/ipc_fuzzer/fuzzer/fuzzer.cc:1152: p = std::move(ipc_message);
On 2017/07/10 18:45:10, Tom Sepez wrote:
> On 2017/07/10 18:44:00, Avi (ping after 24h) wrote:
> > On 2017/07/10 17:45:47, Tom Sepez wrote:
> > > This doesn't look right even in the old code. But here, we make a message,
> > > assign it to P, and then P goes out of scope and we delete it?
> > 
> > My brain was mostly focused on getting this to compile, but you're right.
I'd
> > been thinking that p was being used as an in/out param, but it's not.
> > 
> > What is going on here? How is this fuzzer even working?
> > 
> > Lemme try to change this to something reasonable. The FuzzTraits template
> takes
> > a pointer to the thing to fuzz, so this should be a pointer-to-unique_ptr,
> yes?
> 
> SGTM.

This is done in the latest patch; PTAL.

[commit-bot: I haz the power, 2017-07-10 18:46:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-10 18:46:51]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2017-07-10 18:48:27]

lgtm

[Avi (use Gerrit), 2017-07-10 18:56:17]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2017-07-10 18:56:39]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-10 19:05:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499712977208630,
"parent_rev": "796ca9b1eb040c60bcfe44d84b7c55872802273c", "commit_rev":
"ee095bcb2656c72ca3411b9a2c98cb19e789bdd5"}

[commit-bot: I haz the power, 2017-07-10 19:05:30]

Description was changed from

==========
Remove ScopedVector from tools/ipc_fuzzer/.

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

to

==========
Remove ScopedVector from tools/ipc_fuzzer/.

BUG=554289

Review-Url: https://codereview.chromium.org/2972773004
Cr-Commit-Position: refs/heads/master@{#485337}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee095bcb2656c72ca3411b9a2c98...
==========

[commit-bot: I haz the power, 2017-07-10 19:05:32]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/ee095bcb2656c72ca3411b9a2c98...