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

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Daniel Cheng
<dcheng at chromium.org>'s request 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 58937: Patch
https://bugs.webkit.org/attachment.cgi?id=58937&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
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 :)
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.

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.

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