[Webkit-unassigned] [Bug 132229] Coalesce responses on network process side

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=132229





--- Comment #13 from Alexey Proskuryakov <ap at webkit.org>  2014-04-29 12:30:38 PST ---
> > > Source/WebKit2/NetworkProcess/AsynchronousNetworkLoaderClient.cpp:88
> > > +            loader->send(Messages::WebResourceLoader::DidReceiveResponseWithCertificateInfo(m_coalescingResponse, CertificateInfo(m_coalescingResponse), false));
> > 
> > This looks like the best place to send a coalesced response - we already have all the data. What is the reason to not use this technique here?
> 
> I guess I don't understand what you mean here.

This is a missed optimization - a place where we could send a coalesced response, but don't.

> > > Source/WebKit2/NetworkProcess/AsynchronousNetworkLoaderClient.h:61
> > > +    long long m_coalescingResponseEncodedDataLength;
> > 
> > Is a signed value appropriate here?
> 
> It is the same type as used elsewhere for this data.

This is not sufficient. You are adding new code, and we don't have to copy old mistakes unless there is a reason to.

Using signed values for sizes causes all sorts of bugs.

> > Maybe it's OK because the loader protects both Frame and DocumentLoader. But it doesn't protect the Document, which can be closed and replaced.
> 
> This is the same protection other functions here have.

Do other functions make multiple calls to client? This new function is very different from others, as it's multi-step.

> > > Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp:143
> > > +        didReceiveData(data, encodedDataLength);
> > 
> > Seems like we shouldn't do this if client cancels the load while handling didReceiveResponse. Or does m_coreLoader become null in this case?
> 
> This case shouldn't be any different from the separate-messages case. You may have both didReceiveResponse and didReceiveData messages in-flight at the same time.

I don't understand this answer. There can be multiple messages, but each gets a lot of additional checks before it reaches WebResourceLoader, they don't share the same stack when executed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.