Summary: | Move resource buffering from SynchronousLoaderClient to NetworkResourceLoader | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pratik Solanki <psolanki> | ||||
Component: | WebKit2 | Assignee: | Pratik Solanki <psolanki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, beidson, kling, koivisto, psolanki | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 134560 | ||||||
Attachments: |
|
Description
Pratik Solanki
2014-07-08 10:39:02 PDT
Created attachment 234572 [details]
Patch
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. 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. Committed r170928: <http://trac.webkit.org/changeset/170928> |