GTK: Download item as drag source....

GTK: Download item as drag source.

BUG=21656
TEST=Drag a completed download onto the desktop, or into the tabstrip, or wherever


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

[Evan Stade, 2009-09-17 00:42:19]

can you think of any reason the download shelf might disappear during the drag?

[Nico, 2009-09-18 01:05:42]

LG with comments (I seem to be a style stickler, sorry).

I can't think of a reason for the shelf to disappear during a drag (maybe if
extensions added an api for that at some point, but that's not very likely). If
that's possible in gtk, I'd add another callback for OnDragStart and then set a
bool drag_in_progress_ there and unset it in the "drag completion" handler, and
DCHECK() that boolean in the "close shelf" function. If you think that's stupid,
feel free to not do it.

http://codereview.chromium.org/215002/diff/1/4
File chrome/browser/gtk/download_item_gtk.cc (right):

http://codereview.chromium.org/215002/diff/1/4#newcode223
Line 223: 
is this newline intentional? i don't care either way, but i rarely see 2
consecutive newlines in chrome source, and the style guide says something along
the lines of "compress vertically" iirc.

http://codereview.chromium.org/215002/diff/1/4#newcode873
Line 873: item->get_download()->full_path());
This looks like it might fit into one line.

http://codereview.chromium.org/215002/diff/1/2
File chrome/browser/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/215002/diff/1/2#newcode250
Line 250: GtkDndUtil::GetAtomForTarget(GtkDndUtil::TEXT_URI_LIST)) {
Previous indentation looked more right to me (and more consistent with the "else
if" below).

[Evan Stade, 2009-09-18 02:43:41]

well, if the download shelf did disappear during the drag, it would either crash
or DCHECK the browser (depending on which gtk error we hit), so adding another
DCHECK won't do much. Let's just hope for the best!

http://codereview.chromium.org/215002/diff/1/4
File chrome/browser/gtk/download_item_gtk.cc (right):

http://codereview.chromium.org/215002/diff/1/4#newcode223
Line 223: 
On 2009/09/18 01:05:42, Nico wrote:
> is this newline intentional? i don't care either way, but i rarely see 2
> consecutive newlines in chrome source, and the style guide says something
along
> the lines of "compress vertically" iirc.

nope, fixed

http://codereview.chromium.org/215002/diff/1/4#newcode873
Line 873: item->get_download()->full_path());
On 2009/09/18 01:05:42, Nico wrote:
> This looks like it might fit into one line.

so it does

http://codereview.chromium.org/215002/diff/1/2
File chrome/browser/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/215002/diff/1/2#newcode250
Line 250: GtkDndUtil::GetAtomForTarget(GtkDndUtil::TEXT_URI_LIST)) {
On 2009/09/18 01:05:42, Nico wrote:
> Previous indentation looked more right to me (and more consistent with the
"else
> if" below).

changes to this file dropped