Use EnumerateHeaderLines to parse added headers in webRequest API

Use EnumerateHeaderLines to parse added headers in webRequest API

The responseHeaders that are passed to the webRequest.onHeadersReceived
event are generated by the GetResponseHeadersList method. This method
enumerates the headers using HttpResponseHeaders::EnumerateHeaderLines,
which skips over commas in header values.
But the return value of the webRequest.onHeadersReceived handler is
parsed by HttpResponseHeaders:EnumerateHeader, which stops at commas.
So, if the original response header contains a comma, and the extension
returns the response headers in unmodified form, it was still mistakenly
flagged as modified. This results in duplication of headers.

To fix this, the return value is now parsed using EnumerateHeaderLines.

BUG=526367
TEST=unit_tests --gtest_filter=ExtensionWebRequestHelpersTest.*

Committed: https://crrev.com/4109859acc8f1c514154f167e1251b15f96d6e1e
Cr-Commit-Position: refs/heads/master@{#347896}

[robwu, 2015-09-06 21:30:54]

rob@robwu.nl changed reviewers:
+ battre@chromium.org, kalman@chromium.org

[robwu, 2015-09-06 21:30:54]

Dominic: Please review.

Ben: Need a rubber stamp for web_request_api_helpers.cc after Dominic's
approval.

[not at google - send to devlin, 2015-09-08 22:55:34]

I can rubberstamp it, but may as well make a couple of comments first

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
File extensions/browser/api/web_request/web_request_api_helpers.cc (right):

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
extensions/browser/api/web_request/web_request_api_helpers.cc:366: void* iter =
nullptr;
I presume you don't need |iter| for anything?

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
extensions/browser/api/web_request/web_request_api_helpers.cc:371: if
(base::LowerCaseEqualsASCII(name, name_lowercase) &&
Seems like you could use EqualsCaseInsensitiveASCII and not need to lowercase
|i.first|?

[robwu, 2015-09-08 23:00:21]

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
File extensions/browser/api/web_request/web_request_api_helpers.cc (right):

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
extensions/browser/api/web_request/web_request_api_helpers.cc:366: void* iter =
nullptr;
On 2015/09/08 22:55:34, kalman wrote:
> I presume you don't need |iter| for anything?

iter is just used to keep track of the header iterator state.

https://codereview.chromium.org/1315643005/diff/1/extensions/browser/api/web_...
extensions/browser/api/web_request/web_request_api_helpers.cc:371: if
(base::LowerCaseEqualsASCII(name, name_lowercase) &&
On 2015/09/08 22:55:34, kalman wrote:
> Seems like you could use EqualsCaseInsensitiveASCII and not need to lowercase
> |i.first|?

Possibly, but transforming once to lowercase and storing the result in a
variable seems better than turning the characters to lowercase multiple times
for each header line.

[not at google - send to devlin, 2015-09-08 23:01:25]

rs lgtm, both my comments were made before realising there was another nested
loop in there.

[battre, 2015-09-09 03:32:17]

LGTM, sorry for the delay.

[robwu, 2015-09-09 08:58:52]

The CQ bit was checked by rob@robwu.nl

[commit-bot: I haz the power, 2015-09-09 08:59:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1315643005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1315643005/1

[commit-bot: I haz the power, 2015-09-09 10:03:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-09-09 10:04:35]

Patchset 1 (id:??) landed as
https://crrev.com/4109859acc8f1c514154f167e1251b15f96d6e1e
Cr-Commit-Position: refs/heads/master@{#347896}