[webkit-reviews] review requested: [Bug 40540] [chromium] Add new stubs for querying platform drag-and-drop and copy-and-paste data. : [Attachment 59101] Patch

[bugzilla-daemon at webkit.org]

Daniel Cheng <dcheng at chromium.org> has asked  for review:
Bug 40540: [chromium] Add new stubs for querying platform drag-and-drop and
copy-and-paste data.
https://bugs.webkit.org/show_bug.cgi?id=40540

Attachment 59101: Patch
https://bugs.webkit.org/attachment.cgi?id=59101&action=review

------- Additional Comments from Daniel Cheng <dcheng at chromium.org>
(In reply to comment #42)
> (From update of attachment 58937 [details])
> WebKit/chromium/public/WebClipboard.h:71
>  +	  // FIXME: Make this pure virtual after landing Chrome changes.
> no need.  since this is an interface implemented by chromium, we should make
all of
> the methods have default implementations.  that is what we do normally for
interfaces
> implemented by the embedder.	so instead of the FIXME, you can just change
all methods
> to have defualt implementations :)

Done.

> WebKit/chromium/public/WebDataTransfer.h:39
>  +	      ClipboardSource,
> nit: the convention for the API is to put the common name first.  so it
should
> be SourceClipboard, SourceSelection, SourceDrag instead.
> 

Done.

> WebKit/chromium/public/WebKitClient.h:90
>  +	  virtual WebVector<WebString>
getDataTransferTypes(WebDataTransfer::Source) { return WebVector<WebString>();
}
> it seems like it would be better to either put these methods on WebClipboard
> or make WebKitClient have a method that returns the current WebDataTransfer.
> that way we are not just dumping more stuff in WebKitClient.
> 

Done.

> WebKit/chromium/src/ChromiumBridge.cpp:220
>  +	  WebKit::WebString resultData;
> no need for WebKit:: prefix in this file.  there is a using decl up top.

Done.