ServiceWorker: Fix conversion from Request headers to WebServiceWorkerRequest headers

ServiceWorker: Fix conversion from Request headers to WebServiceWorkerRequest headers

The headers need to be coalesced when converting from one type to another.

A similar CL for WebServiceWorkerResponse headers is here: https://codereview.chromium.org/564773004/

BUG=415157
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182314

[jkarlin, 2014-09-17 15:36:27]

jkarlin@chromium.org changed reviewers:
+ horo@chromium.org, jochen@chromium.org

[jkarlin, 2014-09-17 15:36:27]

horo@chromium.org: Please review changes in everything

jochen@chromium.org: Please review changes in platform

Thanks!

[jochen (gone - plz use gerrit), 2014-09-17 19:21:29]

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
File Source/platform/exported/WebServiceWorkerRequest.cpp (right):

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
Source/platform/exported/WebServiceWorkerRequest.cpp:76:
result.storedValue->value = result.storedValue->value + ", " + String(value);
would it make sense to add a check that we only do this for headers where this
is valid?

or what happens if the same key/value pair is set multiple times? should we
still append?

[jochen (gone - plz use gerrit), 2014-09-17 19:28:00]

instead of requiring lower case names, you could also use a CaseFoldingHash

i'm not sure whether appending headers like this is the right thing to do.
ResourceResponse seems to not do this, why is it required here?

[horo, 2014-09-17 21:13:09]

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
File Source/platform/exported/WebServiceWorkerRequest.cpp (right):

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
Source/platform/exported/WebServiceWorkerRequest.cpp:63: ASSERT(String(key) ==
String(key).lower());
The header key in content::ServiceWorkerFetchRequest is not lowercased.
So blink::WebServiceWorkerRequest::setHeader() called from
content::ServiceWorkerScriptContext::OnFetchEvent() will crash here.

[jkarlin, 2014-09-18 13:43:43]

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
File Source/platform/exported/WebServiceWorkerRequest.cpp (right):

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
Source/platform/exported/WebServiceWorkerRequest.cpp:63: ASSERT(String(key) ==
String(key).lower());
On 2014/09/17 21:13:09, horo wrote:
> The header key in content::ServiceWorkerFetchRequest is not lowercased.
> So blink::WebServiceWorkerRequest::setHeader() called from
> content::ServiceWorkerScriptContext::OnFetchEvent() will crash here.

Done. Removed lowercase requirement.  HTTPHeaderMap uses a casefolding hash so
lower is unnecessary.  I'll use a casefolding hash equivalent in the browser as
well.

https://codereview.chromium.org/578763003/diff/60001/Source/platform/exported...
Source/platform/exported/WebServiceWorkerRequest.cpp:76:
result.storedValue->value = result.storedValue->value + ", " + String(value);
On 2014/09/17 19:21:28, jochen wrote:
> would it make sense to add a check that we only do this for headers where this
> is valid?

Good question, and I don't have a good answer.  This CL makes Fetch behave like
Blink's WebURLLoaderImpl: 
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
which seems like a good starting place.

> or what happens if the same key/value pair is set multiple times? should we
> still append?

They'll be duplicated, which seems reasonable.  This is also the behavior of
WebURLLoader.

[jochen (gone - plz use gerrit), 2014-09-18 18:35:57]

lgtm

[horo, 2014-09-19 02:26:23]

lgtm

[jkarlin, 2014-09-19 11:19:21]

The CQ bit was checked by jkarlin@chromium.org

[commit-bot: I haz the power, 2014-09-19 11:20:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/578763003/80001

[commit-bot: I haz the power, 2014-09-19 11:24:33]

Committed patchset #5 (id:80001) as 182314