Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(100)

Issue 42592: Linux: write images to clipboard.... (Closed)

Created:
11 years, 9 months ago by Evan Stade
Modified:
9 years, 6 months ago
Reviewers:
Evan Martin
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Linux: write images to clipboard. Writing a bitmap to the clipboard is a rather slow operation, as it involves piping it over IPC and then converting it to a PNG. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12482

Patch Set 1 #

Total comments: 10

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -18 lines) Patch
M base/clipboard.cc View 1 chunk +1 line, -1 line 0 comments Download
M base/clipboard_linux.cc View 1 3 chunks +24 lines, -5 lines 0 comments Download
M base/scoped_clipboard_writer.h View 1 chunk +0 lines, -4 lines 0 comments Download
M base/scoped_clipboard_writer.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 1 chunk +5 lines, -5 lines 0 comments Download
M tools/gtk_clipboard_dump/gtk_clipboard_dump.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M webkit/glue/webclipboard_impl.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
Evan Stade
http://codereview.chromium.org/42592/diff/1/6 File base/clipboard_linux.cc (right): http://codereview.chromium.org/42592/diff/1/6#newcode166 Line 166: // gtk_clipboard_request_image() and might change in the future. ...
11 years, 9 months ago (2009-03-25 04:53:53 UTC) #1
Evan Stade
On 2009/03/25 04:53:53, estade wrote: > http://codereview.chromium.org/42592/diff/1/6 > File base/clipboard_linux.cc (right): > > http://codereview.chromium.org/42592/diff/1/6#newcode166 > ...
11 years, 9 months ago (2009-03-25 04:54:20 UTC) #2
Evan Martin
LGTM, comments follow http://codereview.chromium.org/42592/diff/1/6 File base/clipboard_linux.cc (right): http://codereview.chromium.org/42592/diff/1/6#newcode166 Line 166: // gtk_clipboard_request_image() and might change ...
11 years, 9 months ago (2009-03-25 15:44:30 UTC) #3
Evan Stade
11 years, 9 months ago (2009-03-25 18:30:00 UTC) #4
http://codereview.chromium.org/42592/diff/1/6
File base/clipboard_linux.cc (right):

http://codereview.chromium.org/42592/diff/1/6#newcode166
Line 166: // gtk_clipboard_request_image() and might change in the future.
http://bugzilla.gnome.org/show_bug.cgi?id=576749

http://codereview.chromium.org/42592/diff/1/4
File base/scoped_clipboard_writer.cc (left):

http://codereview.chromium.org/42592/diff/1/4#oldcode123
Line 123: #if defined(OS_WIN)
On 2009/03/25 15:44:30, Evan Martin wrote:
> What about OS_MAC?  Should we still protect against that?

AFAIK this code is valid on the mac. (Although it just so happens that it's
never called.)

http://codereview.chromium.org/42592/diff/1/5
File base/scoped_clipboard_writer.h (left):

http://codereview.chromium.org/42592/diff/1/5#oldcode57
Line 57: // memcpy the pixels into GDI and the blit the bitmap to the clipboard.
On 2009/03/25 15:44:30, Evan Martin wrote:
> Is this comment no longer true?

Not really, because it doesn't apply to other platforms, and also it's the
*only* way to copy a bitmap to the clipboard (in this class). On windows it's
overridden in renderer_glue to use shared memory, in which case it's not slow.

http://codereview.chromium.org/42592/diff/1/8
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/42592/diff/1/8#newcode506
Line 506: // on Windows and it only applies to bitmaps. (On other platforms,
bitmaps
On 2009/03/25 15:44:30, Evan Martin wrote:
> s/other platforms/Linux/  maybe?

Done.

Powered by Google App Engine
This is Rietveld 408576698