This changes the base clipboard class.

This changes the base clipboard class, as accomplishing paste requires that Clipboard::FormatType be IPC-able. The lowest-common denominator of unsigned int, NSString*, and GdkAtom is string, so string it is. (Linux changes by estade.)


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

[Avi (use Gerrit), 2009-03-10 18:25:07]

Evan: Linux stuff
Ricarco: Windows stuff
Pink: Mac stuff

[pink (ping after 24hrs), 2009-03-10 18:32:26]

Mac stuff LGTM

[Evan Martin, 2009-03-10 18:52:05]

http://codereview.chromium.org/41012/diff/3022/3026
File base/clipboard_linux.cc (right):

http://codereview.chromium.org/41012/diff/3022/3026#newcode28
Line 28: return gdk_atom_intern(str.c_str(), false);
So this requires a round-trip to the X server (blocking IPC call) and is kind of
expensive.

http://codereview.chromium.org/41012/diff/3022/3026#newcode188
Line 188: bool Clipboard::IsFormatAvailable(std::string format) const {
const ref here?

http://codereview.chromium.org/41012/diff/3022/3026#newcode202
Line 202: if (targets[i] == StringToGdkAtom(format.c_str())) {
since that function is expensive, it should be only computed once for format. 
And you can pass the string through, you don't need the .c_str().

[Avi (use Gerrit), 2009-03-10 19:32:01]

Fixed; please review.

http://codereview.chromium.org/41012/diff/3022/3026
File base/clipboard_linux.cc (right):

http://codereview.chromium.org/41012/diff/3022/3026#newcode28
Line 28: return gdk_atom_intern(str.c_str(), false);
On 2009/03/10 18:52:06, Evan Martin wrote:
> So this requires a round-trip to the X server (blocking IPC call) and is kind
of
> expensive.

Alas, yes.

http://codereview.chromium.org/41012/diff/3022/3026#newcode188
Line 188: bool Clipboard::IsFormatAvailable(std::string format) const {
On 2009/03/10 18:52:06, Evan Martin wrote:
> const ref here?

Hmmm. Yes.

[Evan Martin, 2009-03-10 19:39:16]

linux looks good.

you win evan's weekly gold star for fixing this.

[rvargas (doing something else), 2009-03-10 20:27:24]

LGTM