GTK: first cut at web contents as drag destination....

GTK: first cut at web contents as drag destination.

TEST=drag plain text into the web page in some place that will accept it, e.g. a text entry box. Also, no crashes when dragging from various places (tab strip, bookmark bar, desktop icons, omnibox, etc.)
BUG=http://crbug.com/15429


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

[Evan Stade, 2009-07-02 00:43:10]

square peg; round hole

[Evan Stade, 2009-07-06 18:29:49]

trying Evan

[Evan Martin, 2009-07-06 23:34:05]

i request more comments

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

http://codereview.chromium.org/150198/diff/1/3#newcode105
Line 105: class WebDragDest {
comment above this explaining what it does

http://codereview.chromium.org/150198/diff/1/3#newcode128
Line 128: void UpdateDragStatus(bool is_drop_target) {
comments on all functions

http://codereview.chromium.org/150198/diff/1/3#newcode143
Line 143: static gboolean OnDragMotionThunk(GtkWidget* widget,
but you can skip the comments on these static helpers ;)

http://codereview.chromium.org/150198/diff/1/3#newcode249
Line 249: GdkDragContext* context_;
Comments on these?  (Stuff like: when is context_ null?  Do we own widget_?)

[Evan Stade, 2009-07-06 23:49:15]

your wish is my command

(updated)

[Evan Martin, 2009-07-06 23:57:40]

Does this belong on the renderviewhost rather than the tabcontentshost?

http://codereview.chromium.org/150198/diff/3003/3005
File chrome/browser/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/150198/diff/3003/3005#newcode132
Line 132: void UpdateDragStatus(bool is_drop_target) {
is it intentional to ignore is_drop_target?

http://codereview.chromium.org/150198/diff/3003/3005#newcode217
Line 217: // |x| and |y| are seemingly arbitrary at this point.
I'd guess they're the mouse coordinates at either the point the request was made
or when last data request was received.  Are they not?

http://codereview.chromium.org/150198/diff/3003/3005#newcode224
Line 224: // The drag has left our widget; set |context_| to NULL to make sure
we will
I guess this comment belongs inside the function (describing what it's doing)
rather than outside (describing what it's for)

[Evan Stade, 2009-07-07 00:05:49]

http://codereview.chromium.org/150198/diff/3003/3005
File chrome/browser/tab_contents/tab_contents_view_gtk.cc (right):

http://codereview.chromium.org/150198/diff/3003/3005#newcode217
Line 217: // |x| and |y| are seemingly arbitrary at this point.
On 2009/07/06 23:57:40, Evan Martin wrote:
> I'd guess they're the mouse coordinates at either the point the request was
made
> or when last data request was received.  Are they not?

no, they are crazy, like (0, 2163) or so, when the actual coordinates relative
to the render view are 150, 150, and were never (0, 2163) relative to anything
(neither screen, source widget, nor dest widget).

http://codereview.chromium.org/150198/diff/3003/3005#newcode224
Line 224: // The drag has left our widget; set |context_| to NULL to make sure
we will
On 2009/07/06 23:57:40, Evan Martin wrote:
> I guess this comment belongs inside the function (describing what it's doing)
> rather than outside (describing what it's for)

Done.

[Evan Stade, 2009-07-07 00:10:16]

On 2009/07/06 23:57:40, Evan Martin wrote:
> Does this belong on the renderviewhost rather than the tabcontentshost?

this matches windows. I didn't give too much though to why it's part of the tab
contents rather than the render view host.

[Evan Martin, 2009-07-07 00:13:07]

LGTM