[webkit-reviews] review requested: [Bug 44914] [chromium] Add an interface for platform copy/paste drag/drop data objects : [Attachment 65989] Patch

[bugzilla-daemon at webkit.org]

Daniel Cheng <dcheng at chromium.org> has asked  for review:
Bug 44914: [chromium] Add an interface for platform copy/paste drag/drop data
objects
https://bugs.webkit.org/show_bug.cgi?id=44914

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

------- Additional Comments from Daniel Cheng <dcheng at chromium.org>
(In reply to comment #2)
> (From update of attachment 65985 [details])
> > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> > +	     [chromium] Add an interface for platform copy/paste drag/drop data
objects
> > +	     https://bugs.webkit.org/show_bug.cgi?id=44914
> 
> Nit: Please add a sentence explaining the general goal (ie., something about
so we can eventually support arbitrary data types).

Done. Updating the other patch as well.

> 
> > diff --git a/WebCore/platform/chromium/ChromiumDataObjectNew.h
b/WebCore/platform/chromium/ChromiumDataObjectNew.h
> > +	 virtual String getData(const String& type, bool& succeeded) const = 0;

> > +	 virtual bool setData(const String& type, const String& data) = 0;
> 
> It seems like these methods should use the same style.  Two possibilities:
both return a bool (on success) or just remove the bool param complete from
getdata (do we need to differentiate between empty and fail?).	The other
getters below don't seem to use a bool.

This is inherited from Clipboard's getData() / setData() signature. I agree
it's not ideal, but I don't feel comfortable changing Clipboard.