Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(37)

Issue 668125: Basic DragImage implementation.... (Closed)

Created:
11 years ago by Evan Stade
Modified:
9 years, 8 months ago
CC:
chromium-reviews, jam+cc_chromium.org, brettw+cc_chromium.org, ben+cc_chromium.org, Erik does not do reviews, Aaron Boodman, pam+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Basic DragImage implementation. Only the chromium part is for review. The webkit part shows how that will look when I create the patch for webkit later. This can be landed without the change to webkit. TODO later: - use the image on windows, mac - implement the other DragImageRef functions TEST=drag an image from the render view in GTK BUG=11457 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=41799

Patch Set 1 #

Total comments: 19

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 1

Patch Set 6 : '' #

Patch Set 7 : typo #

Patch Set 8 : '' #

Patch Set 9 : no webkit #

Total comments: 1

Patch Set 10 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+144 lines, -33 lines) Patch
M chrome/browser/cocoa/notifications/balloon_view_host_mac.h View 5 6 7 8 9 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_host.h View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_host.cc View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/gtk/notifications/balloon_view_host_gtk.h View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/gtk/tab_contents_drag_source.h View 1 2 3 4 5 6 7 8 9 3 chunks +10 lines, -1 line 0 comments Download
M chrome/browser/gtk/tab_contents_drag_source.cc View 1 2 3 4 5 6 7 8 9 4 chunks +14 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/render_view_host.h View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/render_view_host.cc View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/render_view_host_delegate.h View 1 2 3 4 5 6 7 8 9 2 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/interstitial_page.cc View 1 2 3 4 5 6 7 8 9 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents_view_gtk.h View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/tab_contents_view_gtk.cc View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -2 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents_view_mac.h View 3 4 5 6 7 8 9 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/tab_contents/tab_contents_view_mac.mm View 3 4 5 6 7 8 9 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/views/notifications/balloon_view_host.h View 5 6 7 8 9 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/views/tab_contents/tab_contents_view_gtk.h View 3 4 5 6 7 8 9 3 chunks +10 lines, -4 lines 0 comments Download
M chrome/browser/views/tab_contents/tab_contents_view_gtk.cc View 3 4 5 6 7 8 9 1 chunk +5 lines, -2 lines 0 comments Download
M chrome/browser/views/tab_contents/tab_contents_view_win.h View 3 4 5 6 7 8 9 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/views/tab_contents/tab_contents_view_win.cc View 3 4 5 6 7 8 9 1 chunk +4 lines, -1 line 0 comments Download
M chrome/common/render_messages_internal.h View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/renderer/render_view.h View 1 2 3 4 5 6 7 8 9 2 chunks +8 lines, -1 line 0 comments Download
M chrome/renderer/render_view.cc View 1 2 3 4 5 6 7 8 9 3 chunks +21 lines, -3 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Evan Stade
I don't think I can break this up into two half patches that don't depend ...
11 years ago (2010-03-05 00:58:17 UTC) #1
Evan Stade
> I don't think I can break this up into two half patches that don't ...
11 years ago (2010-03-05 01:33:00 UTC) #2
tony
http://codereview.chromium.org/668125/diff/1/7 File chrome/browser/gtk/tab_contents_drag_source.cc (right): http://codereview.chromium.org/668125/diff/1/7#newcode312 chrome/browser/gtk/tab_contents_drag_source.cc:312: gtk_drag_set_icon_pixbuf(drag_context, pixbuf, Does gtk_drag_set_icon_pixbuf take ownership of |pixbuf|? http://codereview.chromium.org/668125/diff/1/8 ...
11 years ago (2010-03-05 03:13:08 UTC) #3
darin (slow to review)
can you do something clever like what jam did here: http://codereview.chromium.org/669145/diff/1002/1003 the macro is only ...
11 years ago (2010-03-05 08:09:41 UTC) #4
Avi (use Gerrit)
http://codereview.chromium.org/668125/diff/1/22 File third_party/WebKit/WebCore/platform/chromium/DragImageRef.h (right): http://codereview.chromium.org/668125/diff/1/22#newcode36 third_party/WebKit/WebCore/platform/chromium/DragImageRef.h:36: typedef NativeImageSkia* DragImageRef; On 2010/03/05 03:13:08, tony wrote: > ...
11 years ago (2010-03-05 14:51:08 UTC) #5
Evan Stade
http://codereview.chromium.org/668125/diff/1/7 File chrome/browser/gtk/tab_contents_drag_source.cc (right): http://codereview.chromium.org/668125/diff/1/7#newcode312 chrome/browser/gtk/tab_contents_drag_source.cc:312: gtk_drag_set_icon_pixbuf(drag_context, pixbuf, On 2010/03/05 03:13:08, tony wrote: > Does ...
11 years ago (2010-03-05 19:53:20 UTC) #6
tony
Chrome side code LGTM http://codereview.chromium.org/668125/diff/1/21 File third_party/WebKit/WebCore/platform/chromium/DragImageChromium.cpp (right): http://codereview.chromium.org/668125/diff/1/21#newcode71 third_party/WebKit/WebCore/platform/chromium/DragImageChromium.cpp:71: return dragImage; On 2010/03/05 19:53:20, ...
11 years ago (2010-03-08 01:15:40 UTC) #7
Evan Stade
webkit side is https://bugs.webkit.org/show_bug.cgi?id=35811
11 years ago (2010-03-08 19:40:09 UTC) #8
Evan Stade
updated to use WebImage instead of WebDragImageRef. It should be possible to land the chrome ...
11 years ago (2010-03-09 03:14:12 UTC) #9
Evan Stade
ping
10 years, 12 months ago (2010-03-11 17:01:29 UTC) #10
tony
Sorry for missing this. LGTM!
10 years, 12 months ago (2010-03-12 00:39:12 UTC) #11
darin (slow to review)
http://codereview.chromium.org/668125/diff/22002/6015 File chrome/renderer/render_view.cc (right): http://codereview.chromium.org/668125/diff/22002/6015#newcode1903 chrome/renderer/render_view.cc:1903: Send(new ViewHostMsg_StartDragging(routing_id_, instead of replicating the Send and the ...
10 years, 12 months ago (2010-03-12 00:51:13 UTC) #12
Evan Stade
On 2010/03/12 00:51:13, darin wrote: > http://codereview.chromium.org/668125/diff/22002/6015 > File chrome/renderer/render_view.cc (right): > > http://codereview.chromium.org/668125/diff/22002/6015#newcode1903 > ...
10 years, 11 months ago (2010-03-16 00:04:52 UTC) #13
darin (slow to review)
10 years, 11 months ago (2010-03-16 03:55:39 UTC) #14
Thanks!  LGTM

On Mon, Mar 15, 2010 at 5:04 PM, <estade@chromium.org> wrote:

> On 2010/03/12 00:51:13, darin wrote:
>
>> http://codereview.chromium.org/668125/diff/22002/6015
>> File chrome/renderer/render_view.cc (right):
>>
>
>  http://codereview.chromium.org/668125/diff/22002/6015#newcode1903
>> chrome/renderer/render_view.cc:1903: Send(new
>> ViewHostMsg_StartDragging(routing_id_,
>> instead of replicating the Send and the message creation, how about just
>> initializing an SkBitmap local variable only in the WEBKIT_USING_SKIA
>> case?
>>
>
> done.
>
>
>
> http://codereview.chromium.org/668125
>

Powered by Google App Engine
This is Rietveld 408576698