[webkit-reviews] review denied: [Bug 122516] Avoid resizing the internal buffer of SharedBuffer when creating a PurgeableBuffer : [Attachment 213708] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 122516: Avoid resizing the internal buffer of SharedBuffer when creating a
PurgeableBuffer
https://bugs.webkit.org/show_bug.cgi?id=122516

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213708&action=review


I like the direction this patch is going, but there are some details I think
you should fix.

> Source/WebCore/platform/PurgeableBuffer.h:38
> +	   static PassOwnPtr<PurgeableBuffer> create(size_t);

I think we should follow the pattern for String::createUninitialized here. Use
an out argument for the pointer and use the word “uninitialized” in the name of
the function. I also suggest including the makePurgeable(false) in that
function instead of doing it at the call site.

> Source/WebCore/platform/PurgeableBuffer.h:44
> +	   char* data();

I think it would be better to pass the non-const char* out of the create
function rather than always exposing a non-const data pointer after the fact on
all purgeable buffers. So please change this back.

> Source/WebCore/platform/SharedBuffer.cpp:383
> +void SharedBuffer::copySegmentsOrDataArrayAndClear(char* destination,
unsigned bytesToCopy) const

Is there any caller of copyDataArrayAndClear that does not want this behavior?
Why do we need a separate function for this?