Bug 31826

Summary: [Qt] Make use of QNetworkReply::rawHeaderPairs
Product: WebKit Reporter: Holger Freyther <zecke>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Description Flags
Use the new API eric: review+, eric: commit-queue-

Description Holger Freyther 2009-11-24 04:51:49 PST
To avoid quadratic runtime behavior on transferring headers from Qt to WebCore use the new QNetworkReply method.
Comment 1 Holger Freyther 2009-11-24 04:56:40 PST
Created attachment 43758 [details]
Use the new API

Use the new API, first version.
Comment 2 Holger Freyther 2009-11-24 04:59:24 PST
Okay the ChangeLog misses the link to the bug, I will add it when landing (it is already added)
Comment 3 Eric Seidel (no email) 2009-11-24 10:13:37 PST
Comment on attachment 43758 [details]
Use the new API

Could you explain more about the quadratic runtime?  It would be helpful to have a better ChangeLog entry.  The code itself looks sane, but it would be much better to explain why this is a problem and how this is the right solution for the problem in the ChangeLog.

r+, but cq- because the ChangeLog should be improved before landing (I expect you plan to land this yourself anyway, but just to prevent any confusion that this was good-to-go as is.)
Comment 4 Holger Freyther 2009-11-24 14:23:44 PST
Thanks. The explanation is the following.

The internal Qt representation for headers is a QList (linked list) that contains a pair of headers (header name, value). And the current code is asking for a QStringList of header names (goes through the linked list to construct the temporary QStringList).

And then for each QString in this QStringList it is asking for the rawHeaderValue (which is doing a linear search in the internal linked list to find the value). So in the worse case you have two for loops in each other iterating over the number of elements to find the header value.

The new API is giving access to the header pairs directly so we end up with just one for loop.
Comment 5 Holger Freyther 2009-11-26 03:17:09 PST
Landed in r51411. I think I clarified the changelog.