Add glue for supporting custom MIME types in DataTransfer.

Add glue for supporting custom MIME types in DataTransfer.

BUG=31037
TEST=none in this patch, will be landed in WebKit as layout tests.

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

[dcheng, 2011-12-01 23:13:33]

This patch depends on https://bugs.webkit.org/show_bug.cgi?id=73594, but to
enable all the layout tests added in that patch, this Chrome revision will need
to be rolled into WebKit. I'm planning on breaking this up into 3 changes:

1) Land WebKit patch with the glue and layout tests, but mark clipboard layout
test as failing for now.
2) Land Chromium patch once WebKit has rolled past the revision.
3) Roll Chromium revision in WebKit and remove the expectation.

[tony, 2011-12-02 00:13:29]

http://codereview.chromium.org/8775025/diff/1/content/browser/renderer_host/c...
File content/browser/renderer_host/clipboard_message_filter.h (right):

http://codereview.chromium.org/8775025/diff/1/content/browser/renderer_host/c...
content/browser/renderer_host/clipboard_message_filter.h:49: string16* data);
Nit: The other messages seem to use |result| rather than |data|.

http://codereview.chromium.org/8775025/diff/1/content/renderer/renderer_clipb...
File content/renderer/renderer_clipboard_client.h (right):

http://codereview.chromium.org/8775025/diff/1/content/renderer/renderer_clipb...
content/renderer/renderer_clipboard_client.h:35: string16* data);
Nit: OVERRIDE like the other methods?

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_aur...
File ui/base/clipboard/clipboard_aurax11.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_aur...
ui/base/clipboard/clipboard_aurax11.cc:246: // TODO(dcheng): Implement this.
Nit: Should we add a NOTIMPLEMENTED()?

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_gtk.cc
File ui/base/clipboard/clipboard_gtk.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_gtk...
ui/base/clipboard/clipboard_gtk.cc:494: // TODO(dcheng): Implement this.
Nit: NOTIMPLEMENTED()?

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_mac.mm
File ui/base/clipboard/clipboard_mac.mm (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_mac...
ui/base/clipboard/clipboard_mac.mm:290: // TODO(dcheng): Implement this.
ditto

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_win.cc
File ui/base/clipboard/clipboard_win.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_win...
ui/base/clipboard/clipboard_win.cc:550: // TODO(dcheng): Implement this.
ditto

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc
File webkit/glue/webdropdata.cc (right):

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc#newc...
webkit/glue/webdropdata.cc:39: for (size_t i = 0; i < custom_data_copy.size();
++i)
Nit: The for loop should use {} since the body is 2 lines.

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc#newc...
webkit/glue/webdropdata.cc:68: custom_data_vector[i++] = data;
Nit: I think it's a bit easier to read with i as part of the for loop (e.g.,:
++it, ++i).

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.h
File webkit/glue/webdropdata.h (right):

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.h#newco...
webkit/glue/webdropdata.h:59: std::map<string16, string16> custom_data;
Hmm, I guess since the API is from javascript, there's no easy way to put bytes
into the clipboard for other apps to consume.  That seems like a pretty big
limitation of custom data types, but I guess that can be addressed in future
versions of the spec?

[dcheng, 2011-12-02 00:29:17]

http://codereview.chromium.org/8775025/diff/1/content/browser/renderer_host/c...
File content/browser/renderer_host/clipboard_message_filter.h (right):

http://codereview.chromium.org/8775025/diff/1/content/browser/renderer_host/c...
content/browser/renderer_host/clipboard_message_filter.h:49: string16* data);
On 2011/12/02 00:13:29, tony wrote:
> Nit: The other messages seem to use |result| rather than |data|.

Done.

http://codereview.chromium.org/8775025/diff/1/content/renderer/renderer_clipb...
File content/renderer/renderer_clipboard_client.h (right):

http://codereview.chromium.org/8775025/diff/1/content/renderer/renderer_clipb...
content/renderer/renderer_clipboard_client.h:35: string16* data);
On 2011/12/02 00:13:29, tony wrote:
> Nit: OVERRIDE like the other methods?

Done.

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_aur...
File ui/base/clipboard/clipboard_aurax11.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_aur...
ui/base/clipboard/clipboard_aurax11.cc:246: // TODO(dcheng): Implement this.
On 2011/12/02 00:13:29, tony wrote:
> Nit: Should we add a NOTIMPLEMENTED()?

Done.

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_gtk.cc
File ui/base/clipboard/clipboard_gtk.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_gtk...
ui/base/clipboard/clipboard_gtk.cc:494: // TODO(dcheng): Implement this.
On 2011/12/02 00:13:29, tony wrote:
> Nit: NOTIMPLEMENTED()?

Done.

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_gtk...
ui/base/clipboard/clipboard_gtk.cc:494: // TODO(dcheng): Implement this.
On 2011/12/02 00:13:29, tony wrote:
> Nit: NOTIMPLEMENTED()?

Done.

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_mac.mm
File ui/base/clipboard/clipboard_mac.mm (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_mac...
ui/base/clipboard/clipboard_mac.mm:290: // TODO(dcheng): Implement this.
On 2011/12/02 00:13:29, tony wrote:
> ditto

Done.

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_win.cc
File ui/base/clipboard/clipboard_win.cc (right):

http://codereview.chromium.org/8775025/diff/1/ui/base/clipboard/clipboard_win...
ui/base/clipboard/clipboard_win.cc:550: // TODO(dcheng): Implement this.
On 2011/12/02 00:13:29, tony wrote:
> ditto

Done.

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc
File webkit/glue/webdropdata.cc (right):

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc#newc...
webkit/glue/webdropdata.cc:39: for (size_t i = 0; i < custom_data_copy.size();
++i)
On 2011/12/02 00:13:29, tony wrote:
> Nit: The for loop should use {} since the body is 2 lines.

Done.

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.cc#newc...
webkit/glue/webdropdata.cc:68: custom_data_vector[i++] = data;
On 2011/12/02 00:13:29, tony wrote:
> Nit: I think it's a bit easier to read with i as part of the for loop (e.g.,:
> ++it, ++i).

Done.

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.h
File webkit/glue/webdropdata.h (right):

http://codereview.chromium.org/8775025/diff/1/webkit/glue/webdropdata.h#newco...
webkit/glue/webdropdata.h:59: std::map<string16, string16> custom_data;
On 2011/12/02 00:13:29, tony wrote:
> Hmm, I guess since the API is from javascript, there's no easy way to put
bytes
> into the clipboard for other apps to consume.  That seems like a pretty big
> limitation of custom data types, but I guess that can be addressed in future
> versions of the spec?

Given the APIs available on Windows/Linux, we can't safely expose those APIs to
the web, even with attempts to set a limit of x per domain, since these
resources aren't reclaimable. It's definitely not ideal, hopefully the native
APIs improve.

[tony, 2011-12-02 00:33:13]

LGTM

[dcheng, 2011-12-02 06:41:31]

jam for content/* changes and sky for ui/base/clipboard/* changes.

[sky, 2011-12-02 16:06:26]

LGTM

http://codereview.chromium.org/8775025/diff/7002/ui/base/clipboard/clipboard.h
File ui/base/clipboard/clipboard.h (right):

http://codereview.chromium.org/8775025/diff/7002/ui/base/clipboard/clipboard....
ui/base/clipboard/clipboard.h:159: void ReadCustomData(Buffer buffer, const
string16& type,
when you wrap each param should be on its own line.

http://codereview.chromium.org/8775025/diff/7002/ui/base/clipboard/clipboard_...
File ui/base/clipboard/clipboard_aurax11.cc (right):

http://codereview.chromium.org/8775025/diff/7002/ui/base/clipboard/clipboard_...
ui/base/clipboard/clipboard_aurax11.cc:244: const string16& data,
nit: data->type

[jam, 2011-12-02 17:14:39]

content lgtm