Storing headers as name-value pairs array information more accurate and allows to treat Set-Cookie headers (which become not parseable when joined by comma) correctly.
Created attachment 141695 [details] Patch
Comment on attachment 141695 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141695&action=review > Source/WebCore/inspector/front-end/HAREntry.js:75 > + headers: this._request.requestHeaders.slice(), I think slice() is redundant here, we do not modify HAR headers anywhere. > Source/WebCore/inspector/front-end/HAREntry.js:96 > + headers: this._request.responseHeaders.slice(), ditto > Source/WebCore/inspector/front-end/NetworkManager.js:164 > + if (values[i]) Why this check? What do we do about headers with empty value? > Source/WebCore/inspector/front-end/NetworkRequest.js:456 > + this._sortedRequestHeaders.sort(function(a,b) { return a.name.localeCompare(b.name) }); Shouldn't we sort according to a case-insensitive comparator()? > Source/WebCore/inspector/front-end/NetworkRequest.js:558 > + this._sortedResponseHeaders.sort(function(a,b) { return a.name.localeCompare(b.name) }); Ditto. > Source/WebCore/inspector/front-end/NetworkRequest.js:653 > + for (var i = 0; i < headers.length; ++i) { > + if (headers[i].name.toLowerCase() === headerName) I wonder if it makes sense to binary-search over sorted headers? > Source/WebCore/inspector/front-end/NetworkRequest.js:657 > + if (headerName.toLowerCase() === "Set-Cookie".toLowerCase()) /^set-cookie$/i.test()?
Created attachment 141708 [details] Patch
(In reply to comment #2) > (From update of attachment 141695 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=141695&action=review > > > Source/WebCore/inspector/front-end/HAREntry.js:75 > > + headers: this._request.requestHeaders.slice(), > > I think slice() is redundant here, we do not modify HAR headers anywhere. Done. > > Source/WebCore/inspector/front-end/NetworkManager.js:164 > > + if (values[i]) > > Why this check? What do we do about headers with empty value? Removed this check. > > Source/WebCore/inspector/front-end/NetworkRequest.js:456 > > + this._sortedRequestHeaders.sort(function(a,b) { return a.name.localeCompare(b.name) }); > > Shouldn't we sort according to a case-insensitive comparator()? Done. > > Source/WebCore/inspector/front-end/NetworkRequest.js:653 > > + for (var i = 0; i < headers.length; ++i) { > > + if (headers[i].name.toLowerCase() === headerName) > > I wonder if it makes sense to binary-search over sorted headers? I don't think it worth cluttering the code (the number of headers is usually about 10-15 and we will need two loops for multiple results binary search). > > Source/WebCore/inspector/front-end/NetworkRequest.js:657 > > + if (headerName.toLowerCase() === "Set-Cookie".toLowerCase()) > > /^set-cookie$/i.test()? headerName is already lower case, so I made it just (headerName === "set-cookie")
Comment on attachment 141708 [details] Patch LGTM
Committed r116952: <http://trac.webkit.org/changeset/116952>