[webkit-reviews] review denied: [Bug 47423] [chromium] Update WebBlobData to adapt to BlobData change in terms of handling string data item : [Attachment 70268] Proposed Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Oct 9 14:11:33 PDT 2010
Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Jian Li
<jianli at chromium.org>'s request for review:
Bug 47423: [chromium] Update WebBlobData to adapt to BlobData change in terms
of handling string data item
https://bugs.webkit.org/show_bug.cgi?id=47423
Attachment 70268: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=70268&action=review
------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70268&action=review
> WebKit/chromium/public/WebBlobData.h:48
> +class WebRawData {
please put this in a separate header file. one class/struct per header please.
i would give this a better name because we already have WebData. what you have
here it seems is a container for a |const char*| and a length. that means this
class does not own the data. WebDependentData or WebDataPiece might be good
names.
> WebKit/chromium/public/WebBlobData.h:56
> + WEBKIT_API void reset()
note: these do not need WEBKIT_API since they are implemented inline. you
only need WEBKIT_API on methods that need to be exported from webkit.dll.
> WebKit/chromium/public/WebBlobData.h:86
> + WebRawData data;
have you considered using WebData here instead? why do you want this
class to not own the data?
More information about the webkit-reviews
mailing list