Cleanup: ResourceDispatcherHost::BeginDownload takes a scoped_ptr<URLRequest> argument.

Cleanup: ResourceDispatcherHost::BeginDownload takes a scoped_ptr<URLRequest> argument.

This used to take an URLRequest*, with a comment about ownership. Changing to a scoped_ptr argument makes the ownership explicit and enforceable by the compiler.

Also, changed the name of the child_id argument to be process_unique_id to be consistent with the rest of the file.


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

[cbentzel, 2012-01-19 13:07:48]

jam: resource_dispatcher_host
ben: downloads

[benjhayden, 2012-01-19 14:15:38]

lgtm

[jam, 2012-01-19 18:09:33]

actually, process_unique_id is not the convention we use (child_id is)*. can you
switch the file to use child_id instead?

*search cs for child_id finds 132 hits, while process_unique_id finds 4

[jam, 2012-01-19 18:17:36]

On 2012/01/19 18:09:33, John Abd-El-Malek wrote:
> actually, process_unique_id is not the convention we use (child_id is)*. can
you
> switch the file to use child_id instead?
> 
> *search cs for child_id finds 132 hits, while process_unique_id finds 4

btw i just found that in resource_dispatcher_host.cc, there are 117 matches of
either child_id or child_id()!

[cbentzel, 2012-01-19 18:17:41]

Huh, resource_dispatcher_host.h shows 23 cases of process_unique_id, versus
4 for child_id.

Perhaps cs is lagging behind?

http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host...


Happy to change to child_id - that's what I've seen in the past - but
thought this was an effort to be more specific about the id.

On Thu, Jan 19, 2012 at 1:09 PM, <jam@chromium.org> wrote:

> actually, process_unique_id is not the convention we use (child_id is)*.
> can you
> switch the file to use child_id instead?
>
> *search cs for child_id finds 132 hits, while process_unique_id finds 4
>
>
http://codereview.chromium.**org/9150028/<http://codereview.chromium.org/9150...
>

[cbentzel, 2012-01-19 18:50:54]

Moved to child_id, per our discussion.

[jam, 2012-01-19 18:54:21]

lgtm

[commit-bot: I haz the power, 2012-01-19 19:38:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cbentzel@chromium.org/9150028/3003

[commit-bot: I haz the power, 2012-01-19 20:56:07]

Change committed as 118351