Bug 134732 – Move resource buffering from SynchronousLoaderClient to NetworkResourceLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Pratik Solanki]

We want to buffer JS and CSS and resources as well - see bug 134560. In order to share code, we should move the buffering code SynchronousLoaderClient to NetworkResourceLoader.

[Pratik Solanki]

Created attachment 234572 [details]
Patch

[Darin Adler]

Comment on attachment 234572 [details]
Patch

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.

[Pratik Solanki]

Comment on attachment 234572 [details]
Patch

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.

True. I'll make this a reference.

>> Source/WebKit2/NetworkProcess/SynchronousNetworkLoaderClient.cpp:116
>> +    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.

I didn't like this as well but it didn't seem much worse than we had before. send() seems to be generated code and it looks like it just passes the Vector down to the encoder. I think we should be able to modify it to take SharedBuffer but it's not straightforward. I'll ask Anders.

>> Source/WebKit2/NetworkProcess/SynchronousNetworkLoaderClient.h:63
>> +    void sendDelayedReply(NetworkResourceLoader*);
> 
> This should take a reference rather than a pointer.

Ok.

[Pratik Solanki]

Committed r170928: <http://trac.webkit.org/changeset/170928>