Move DragDownload() out of download_util.h

Move DragDownload() out of download_util.h

With this change download_util.h is gone, the remained function is moved
into a new header file called drag_download_item.h.

Which makes more sense since the remained function is related to
drag and drop functionality.

- Renamed DragDownload() to DragDownloadItem().

BUG=104549
TEST=unit_tests
R=benjhayden@chromium.org,asanka@chromium.org
TBR=thestig

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219481

[benjhayden, 2013-08-18 21:23:17]

Is there a particular reason to name the file download_item_drag.h? I'd expect a
function named DragDownloadItem to be in a file named drag_download_item.h. I
think the reason that all the other files start with download_ is because
they're noun phrases like DownloadHistory, DownloadService, DownloadItemModel,
etc., not because filenames should generally start with download_.

[tfarina, 2013-08-18 21:32:31]

Ben, I just followed asanka's suggestion. See
https://code.google.com/p/chromium/issues/detail?id=104549#c46

[tfarina, 2013-08-19 18:33:48]

On Sun, Aug 18, 2013 at 6:32 PM,  <tfarina@chromium.org> wrote:
> Ben, I just followed asanka's suggestion. See
> https://code.google.com/p/chromium/issues/detail?id=104549#c46
>
Ben, I'm fine to name it whatever you and asanka prefer ;) (if that
wasn't clear).

> https://codereview.chromium.org/23271005/



-- 
Thiago

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[benjhayden, 2013-08-19 18:37:01]

Yep, thanks, I'll let Asanka say which he'd prefer. It looks like he isn't at
work today.

[asanka, 2013-08-20 01:37:15]

On 2013/08/19 18:37:01, benjhayden_chromium wrote:
> Yep, thanks, I'll let Asanka say which he'd prefer. It looks like he isn't at
> work today.

I'm on leave this week and won't be able to check email / do reviews regularly.
So I'll defer back to Ben :)

When I suggested download_item_drag / DownloadItemDrag I was thinking of a class
that would being consistent naming for the GTK implementation
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...)
as well as the others. My understanding is that the GTK code can't be
implemented as a single function. Also, it seemed inconsistent to have a
DownloadItemDrag and a DragDownload{,Item} in the codebase.

[benjhayden, 2013-08-20 17:29:59]

On 2013/08/20 01:37:15, asanka (OOO till Aug-26) wrote:
> On 2013/08/19 18:37:01, benjhayden_chromium wrote:
> > Yep, thanks, I'll let Asanka say which he'd prefer. It looks like he isn't
at
> > work today.
> 
> I'm on leave this week and won't be able to check email / do reviews
regularly.
> So I'll defer back to Ben :)
> 
> When I suggested download_item_drag / DownloadItemDrag I was thinking of a
class
> that would being consistent naming for the GTK implementation
>
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...)
> as well as the others. My understanding is that the GTK code can't be
> implemented as a single function. Also, it seemed inconsistent to have a
> DownloadItemDrag and a DragDownload{,Item} in the codebase.

My understanding is that the desired cross-platform interface is a static
DragDownloadItem function, and that DownloadItemDrag is an unavoidable gtk
implementation detail that arguably should be hidden in an anonymous namespace.
It looks to me like DragDownloadItem on gtk could just instantiate a
DownloadItemDrag directly instead of bouncing through
DownloadItemDrag::BeginDrag() if DownloadItemDrag::DownloadItemDrag() were made
public.
I'm not sure if I'd rather have the gtk implementation of DragDownloadItem()
moved to chrome/browser/ui/gtk/custom_drag where it's currently actually
implemented, or if I'd rather have DownloadItemDrag moved to
chrome/browser/download/drag_download_item.cc (for consistency with mac/views).
I'm also not sure what I'd do with DownloadItemDrag::SetSource() and DragData. I
would rather keep them with DownloadItemDrag than move them to
download_item_gtk.cc. If DownloadItemDrag moves to an anonymous namespace, then
perhaps either drag_download_item_gtk.h/custom_drag.h could declare
SetDownloadDragSource() outside of any namespaces.

Unfortunately, I'm battling a cold (in August! That's the last time I camp in
New Hampshire) so I don't quite have the energy to form a coherent and complete
vision for DragDownloadItem(). This cl doesn't appear to actually break
anything, and I think Asanka and I have articulated our ideas about the naming,
so I'll leave you to decide what scheme makes the most sense and go ahead and
commit it. If Asanka or I care enough, we can always move stuff around again
later.

LGTM

Thanks again for all your help!

[tfarina, 2013-08-25 01:56:20]

TBRing owner for the trival updates in the rest of chrome/

Lei -> chrome

[tfarina, 2013-08-25 01:57:16]

Ben, I renamed the file to drag_download_item*

I'll follow up with the gtk consolidation in another CL.

[commit-bot: I haz the power, 2013-08-25 01:57:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/23271005/36001

[commit-bot: I haz the power, 2013-08-25 05:03:40]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-08-25 13:31:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/23271005/36001

[commit-bot: I haz the power, 2013-08-25 15:37:52]

Change committed as 219481

[Lei Zhang, 2013-08-26 05:09:22]

Bits outside of download directories lgtm