[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 #12 from Antti Koivisto <koivisto at iki.fi>  2014-04-29 12:04:02 PST ---
(In reply to comment #11)
> > Source/WebKit2/NetworkProcess/AsynchronousNetworkLoaderClient.cpp:71
> > +        static const double responseCoalescingTime = 0.5;
> 
> How did you come up with this number?

It seemed reasonable. Only thing that it really should affect are progressively rendered images (the first display may be delayed up to 0.5s but will be more complete). Scripts and stylesheets are not processed until they finished in any case.

> This seems to mean that slow loading pages will now be committed half a second later than before, which I expect to be observable.

Coalescing is not enabled for the main document (or XHR).

> > 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.

> > 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.

> 
> > Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp:137
> > +    Ref<WebResourceLoader> protect(*this);
> 
> I'm somewhat worried that protecting only the loader is insufficient. I don't know to to tell what would be sufficient. Maybe run API tests with GuardMalloc? But the coverage is very sketchy.
> 
> 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.

> > 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.

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