[webkit-reviews] review granted: [Bug 41426] Add WKArrayRef API to WebKit2 : [Attachment 60144] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 30 14:11:29 PDT 2010
Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 41426: Add WKArrayRef API to WebKit2
https://bugs.webkit.org/show_bug.cgi?id=41426
Attachment 60144: Patch
https://bugs.webkit.org/attachment.cgi?id=60144&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> + * Shared/WebArray.cpp: Added.
> + (WebKit::WebArray::WebArray):
> + (WebKit::WebArray::~WebArray):
> + * Shared/WebArray.h: Added.
> + (WebKit::WebArray::create):
> + (WebKit::WebArray::adopt):
> + (WebKit::WebArray::at):
> + (WebKit::WebArray::size):
> + (WebKit::WebArray::):
> + * UIProcess/API/C/WKAPICast.h:
> + (WebKit::):
> + * UIProcess/API/C/WKArray.cpp: Added.
> + (WKArrayGetItemAtIndex):
> + (WKArrayGetSize):
> + (WKArrayRetain):
> + (WKArrayRelease):
I don’t see any reason to leave in these lists of added functions. I normally
just delete them for added files.
Also, that broken "(WebKit::)" definitely should not be left in.
> +WebArray::WebArray(const void** entries, size_t size, const
WebArrayCallbacks* callbacks)
> + : m_entries((void**)fastMalloc(size * sizeof(void*)))
How about using "new" instead of fastMalloc and avoiding the type casts? How
about using static_cast if you need a typecast?
> + enum AdoptTag { Adopt };
> + WebArray(AdoptTag, void** entries, size_t size, const WebArrayCallbacks*
callbacks);
Maybe the AdoptTag should be the last argument. Might generate better code
even.
I suggest leaving out the argument name "callbacks".
> +WK_EXPORT const void* WKArrayGetItemAtIndex(WKArrayRef array, size_t index);
What is behavior when you use a bad index? Guarantee of returning 0? Undefined?
Seems OK, r=me
More information about the webkit-reviews
mailing list