[webkit-reviews] review granted: [Bug 134732] Move resource buffering from SynchronousLoaderClient to NetworkResourceLoader : [Attachment 234572] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Pratik Solanki <psolanki at apple.com>'s
request for review:
Bug 134732: Move resource buffering from SynchronousLoaderClient to
NetworkResourceLoader
https://bugs.webkit.org/show_bug.cgi?id=134732

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

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


> Source/WebKit2/NetworkProcess/SynchronousNetworkLoaderClient.cpp:104
> +    ASSERT(loader);

Assertion that an argument is non-null is a clue that the argument should be a
reference rather than a pointer.

> Source/WebKit2/NetworkProcess/SynchronousNetworkLoaderClient.cpp:116
> +    Vector<char> responseData;
> +    SharedBuffer* buffer = loader->bufferedData();
> +    if (buffer && buffer->size())
> +	   responseData.append(buffer->data(), buffer->size());
> +
> +    m_delayedReply->send(m_error, m_response, responseData);

It’s annoying to have to put this into a Vector. What does send do with the
Vector? Can we somehow avoid the copy by using either move or getting the bits
directly from the SharedBuffer.

> Source/WebKit2/NetworkProcess/SynchronousNetworkLoaderClient.h:63
> +    void sendDelayedReply(NetworkResourceLoader*);

This should take a reference rather than a pointer.