Make passing of DownloadRequestHandle separate from DownloadCreateInfo.

Make passing of DownloadRequestHandle separate from DownloadCreateInfo.

This is to support mocking DownloadRequestHandle.

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

[Randy Smith (Not in Mondays), 2011-10-21 22:28:42]

Chris, could you take a look at this?  I'm going to be doing some cleanup (as
little as I can manage, but some) for doing the unit tests and I wouldn't mind
having your eye on them.  I think the review will involve more design judgement
than knowledge of the download code.  Andy, you're welcome to comment if you
want to.  

This is the first of a couple of small CLs, to get the DI in shape for having
unit tests written on it.  This one splits out DownloadRequestHandle from
DownloadCreateInfo.  The next one will shift DownloadItem and DownloadFile over
to taking an ABC in place of DownloadRequestHandle.  The one after that will
remove the portions of DownloadRequestHandle that only support testing (as we
can now support those sections with a mock class).

[cbentzel, 2011-10-24 12:13:04]

On the surface this seems fine, but I'm going to need to spend some time
re-reading the download data structures to see if this makes sense.

[Randy Smith (Not in Mondays), 2011-10-24 12:42:00]

On 2011/10/24 12:13:04, cbentzel wrote:
> On the surface this seems fine, but I'm going to need to spend some time
> re-reading the download data structures to see if this makes sense.

Ok.  Let me know if you want a whiteboard sketch of the whole trajectory of what
I'm doing, or to quiz me on those data structures.

[cbentzel, 2011-10-24 14:28:28]

Whiteboard would be great.

On Mon, Oct 24, 2011 at 8:42 AM, <rdsmith@chromium.org> wrote:

> On 2011/10/24 12:13:04, cbentzel wrote:
>
>> On the surface this seems fine, but I'm going to need to spend some time
>> re-reading the download data structures to see if this makes sense.
>>
>
> Ok.  Let me know if you want a whiteboard sketch of the whole trajectory of
> what
> I'm doing, or to quiz me on those data structures.
>
>
>
http://codereview.chromium.**org/8371009/<http://codereview.chromium.org/8371...
>

[cbentzel, 2011-10-25 12:54:22]

Did you want to convert this to pointer-ownership-plus-virtual-clone method in
this CL or a separate one?

[Randy Smith (Not in Mondays), 2011-10-25 13:09:28]

On 2011/10/25 12:54:22, cbentzel wrote:
> Did you want to convert this to pointer-ownership-plus-virtual-clone method in
> this CL or a separate one?

Neither.  I'm doing the shift to virtualization in my next CL, but the choice I
made (which I'll go over in that CL) was to continue to pass around
DownloadRequestHandle by value, and pass in a new copy by pointer at the point
of construction of DownloadItem and DownloadFile.  But regardless of how I solve
that problem, I need to pull DownloadRequestHandle out of DownloadCreateInfo, so
I figured I'd do that in this CL and do the thing that might cause some debate
in the next one.

[cbentzel, 2011-10-25 13:25:49]

LGTM

On Tue, Oct 25, 2011 at 9:09 AM, <rdsmith@chromium.org> wrote:

> On 2011/10/25 12:54:22, cbentzel wrote:
>
>> Did you want to convert this to pointer-ownership-plus-**virtual-clone
>> method in
>> this CL or a separate one?
>>
>
> Neither.  I'm doing the shift to virtualization in my next CL, but the
> choice I
> made (which I'll go over in that CL) was to continue to pass around
> DownloadRequestHandle by value, and pass in a new copy by pointer at the
> point
> of construction of DownloadItem and DownloadFile.  But regardless of how I
> solve
> that problem, I need to pull DownloadRequestHandle out of
> DownloadCreateInfo, so
> I figured I'd do that in this CL and do the thing that might cause some
> debate
> in the next one.
>
>
>
http://codereview.chromium.**org/8371009/<http://codereview.chromium.org/8371...
>

[commit-bot: I haz the power, 2011-10-25 16:03:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdsmith@chromium.org/8371009/1

[commit-bot: I haz the power, 2011-10-25 16:03:19]

Can't apply patch for file content/browser/download/download_file_manager.cc.
While running patch -p1 --forward --force;
patching file content/browser/download/download_file_manager.cc
Hunk #1 FAILED at 11.
Hunk #2 succeeded at 49 (offset -1 lines).
Hunk #3 succeeded at 60 (offset -1 lines).
Hunk #4 succeeded at 71 (offset -1 lines).
Hunk #5 succeeded at 115 (offset -1 lines).
Hunk #6 succeeded at 130 (offset -1 lines).
1 out of 6 hunks FAILED -- saving rejects to file
content/browser/download/download_file_manager.cc.rej

[commit-bot: I haz the power, 2011-10-25 20:30:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdsmith@chromium.org/8371009/7002

[commit-bot: I haz the power, 2011-10-25 22:49:43]

Change committed as 107219