[webkit-reviews] review denied: [Bug 91920] [WIN] Implement WebMutableURLRequest::setHTTPBody() : [Attachment 153832] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 91920: [WIN] Implement WebMutableURLRequest::setHTTPBody()
https://bugs.webkit.org/show_bug.cgi?id=91920

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

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=153832&action=review


Looks good, but I think it could be clearer with some slight changes!

> Source/WebCore/platform/network/FormData.cpp:161
> +void FormData::appendUninitializedData(size_t size, char*& data)

I was initially confused about what this was supposed to do. I think it would
be clearer if this method was called something like "expandDataStore".
Also, couldn't it return the pointer to the start of the new allocation? This
would allow us to write something like:

void* target = expandDataStore(size);
memcpy(target, data, size);

> Source/WebKit/win/WebMutableURLRequest.cpp:303
> +    httpBody->appendUninitializedData(stat.cbSize.LowPart, formData);

... Then this would be:

void* formData = httpBody->expandDataStore(stat.cbSize.LowPart);