base: Remove some unnecessary const scoped_refptr<>&.

base: Remove some unnecessary const scoped_refptr<>&.

This patch removes some unnecessary const scoped_refptr<>&. It replaces
some instances with a copy of scoped_refptr, followed by a move. Other
instances, it passes a raw non-owning pointer.

R=danakj
BUG=589044
Committed: https://crrev.com/82b0c16d66f85881e084ae16ef366ee4e6f4cffc
Cr-Commit-Position: refs/heads/master@{#382043}

[vmpstr, 2016-03-14 23:42:40]

Please take a look.

[Primiano Tucci (use gerrit), 2016-03-15 15:58:11]

primiano@chromium.org changed reviewers:
+ primiano@chromium.org

[Primiano Tucci (use gerrit), 2016-03-15 15:58:12]

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager_unittest.cc (right):

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager_unittest.cc:73: const char* kMDPName =
"TestDumpProvider";
Why are we re-duplicating the code here? The point of that bool dumps_on_s_t_t_r
was to avoid having two copy/pasted Register methods.

[danakj, 2016-03-15 18:27:36]

LGTM % primiano

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager_unittest.cc (right):

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager_unittest.cc:74:
mdm->RegisterDumpProviderWithSequencedTaskRunner(mdp, kMDPName, task_runner,
move the task runner?

[vmpstr, 2016-03-15 18:57:38]

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager_unittest.cc (right):

https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager_unittest.cc:73: const char* kMDPName =
"TestDumpProvider";
On 2016/03/15 15:58:11, Primiano wrote:
> Why are we re-duplicating the code here? The point of that bool
dumps_on_s_t_t_r
> was to avoid having two copy/pasted Register methods.

The part that was a bit awkward was on lines 61-62. It would take a
scoped_refptr, convert it to a raw pointer, downcast it to a derived class, and
wrap that back in a scoped_refptr. It's a bit subtle and it wouldn't work
properly if ever move to std::shared_ptr. 

Duplicating some of the code seemed not too bad. Do you think the previous
approach was better? This is test code, so I don't mind reverting these changes.

[vmpstr, 2016-03-17 21:30:22]

On 2016/03/15 18:57:38, vmpstr wrote:
>
https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
> File base/trace_event/memory_dump_manager_unittest.cc (right):
> 
>
https://codereview.chromium.org/1800743003/diff/1/base/trace_event/memory_dum...
> base/trace_event/memory_dump_manager_unittest.cc:73: const char* kMDPName =
> "TestDumpProvider";
> On 2016/03/15 15:58:11, Primiano wrote:
> > Why are we re-duplicating the code here? The point of that bool
> dumps_on_s_t_t_r
> > was to avoid having two copy/pasted Register methods.
> 
> The part that was a bit awkward was on lines 61-62. It would take a
> scoped_refptr, convert it to a raw pointer, downcast it to a derived class,
and
> wrap that back in a scoped_refptr. It's a bit subtle and it wouldn't work
> properly if ever move to std::shared_ptr. 
> 
> Duplicating some of the code seemed not too bad. Do you think the previous
> approach was better? This is test code, so I don't mind reverting these
changes.

primiano@, can you please review and see if you agree with my reasoning or let
me know if you want me to change it back

[Primiano Tucci (use gerrit), 2016-03-18 00:17:06]

I have been very  busy all the day and I will be traveling tomorrow and I have
just a phone now. 
since we are talking about few lines of a test, it is not worthing blocking this
cl. lgtm, you seem to have a rationale there,  so I trust your judgment. sorry
for the delay...  busy days.

[vmpstr, 2016-03-18 00:21:34]

On 2016/03/18 00:17:06, Primiano wrote:
> I have been very  busy all the day and I will be traveling tomorrow and I have
> just a phone now. 
> since we are talking about few lines of a test, it is not worthing blocking
this
> cl. lgtm, you seem to have a rationale there,  so I trust your judgment. sorry
> for the delay...  busy days.

Thanks! Feel free to comment here after it lands. I'm happy to do some
follow-ups to get this to a state where we're both happy.

[vmpstr, 2016-03-18 00:21:45]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-03-18 00:22:16]

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

[commit-bot: I haz the power, 2016-03-18 01:16:10]

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

[commit-bot: I haz the power, 2016-03-18 01:16:11]

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

[vmpstr, 2016-03-18 18:15:55]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-03-18 18:16:28]

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

[commit-bot: I haz the power, 2016-03-18 19:17:37]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-18 19:18:32]

Description was changed from

==========
base: Remove some unnecessary const scoped_refptr<>&.

This patch removes some unnecessary const scoped_refptr<>&. It replaces
some instances with a copy of scoped_refptr, followed by a move. Other
instances, it passes a raw non-owning pointer.

R=danakj
BUG=589044
==========

to

==========
base: Remove some unnecessary const scoped_refptr<>&.

This patch removes some unnecessary const scoped_refptr<>&. It replaces
some instances with a copy of scoped_refptr, followed by a move. Other
instances, it passes a raw non-owning pointer.

R=danakj
BUG=589044

Committed: https://crrev.com/82b0c16d66f85881e084ae16ef366ee4e6f4cffc
Cr-Commit-Position: refs/heads/master@{#382043}
==========

[commit-bot: I haz the power, 2016-03-18 19:18:34]

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