ServiceWorker: support Request.{url,method,origin,headers} [chromium] (2/3)

ServiceWorker: support Request.{url,method,origin,headers} [chromium] (2/3)

Multi-sided patch: 
1/3: blink-side, https://codereview.chromium.org/304053003/
2/3: chromium-side, THIS PATCH
3/3: blink-side, cleanup Not Yet.

BUG=377373
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274450

[horo, 2014-05-29 11:42:46]

falken@
Could you please review this?

[falken, 2014-05-30 03:39:17]

I'm not sure I understand the test. Also, if we can just test in a Blink layout
test, that would be better. The FetchEvent browser tests were written before we
could have an end-to-end service worker.

https://codereview.chromium.org/305893003/diff/30001/content/renderer/service...
File content/renderer/service_worker/service_worker_script_context.cc (right):

https://codereview.chromium.org/305893003/diff/30001/content/renderer/service...
content/renderer/service_worker/service_worker_script_context.cc:127:
webRequest.setURL(blink::WebString(base::UTF8ToUTF16(request.url.spec())));
It seems more common in the codebase to use WebString::fromUTF8, is there a
reason to use UTF8ToUTF16 instead?

https://codereview.chromium.org/305893003/diff/30001/content/test/data/servic...
File content/test/data/service_worker/fetch_event.js (right):

https://codereview.chromium.org/305893003/diff/30001/content/test/data/servic...
content/test/data/service_worker/fetch_event.js:26:
getSortedHeaderString(event.request.headers));
I'm not sure I understand this. Why do we add request headers to the response?

[horo, 2014-05-30 04:55:17]

OK, I will write tests for Request in Blink layout tests.

https://codereview.chromium.org/305893003/diff/30001/content/renderer/service...
File content/renderer/service_worker/service_worker_script_context.cc (right):

https://codereview.chromium.org/305893003/diff/30001/content/renderer/service...
content/renderer/service_worker/service_worker_script_context.cc:127:
webRequest.setURL(blink::WebString(base::UTF8ToUTF16(request.url.spec())));
On 2014/05/30 03:39:17, falken wrote:
> It seems more common in the codebase to use WebString::fromUTF8, is there a
> reason to use UTF8ToUTF16 instead?

I changed to use WebURL in WebServiceWorkerRequest.

https://codereview.chromium.org/305893003/diff/30001/content/test/data/servic...
File content/test/data/service_worker/fetch_event.js (right):

https://codereview.chromium.org/305893003/diff/30001/content/test/data/servic...
content/test/data/service_worker/fetch_event.js:26:
getSortedHeaderString(event.request.headers));
On 2014/05/30 03:39:17, falken wrote:
> I'm not sure I understand this. Why do we add request headers to the response?

It was intended to make it possible to check the value of event.request in the
browser test.
But I reverted the changes for browser tests.

[falken, 2014-05-30 09:37:01]

https://codereview.chromium.org/305893003/diff/70001/content/renderer/service...
File content/renderer/service_worker/service_worker_script_context.cc (right):

https://codereview.chromium.org/305893003/diff/70001/content/renderer/service...
content/renderer/service_worker/service_worker_script_context.cc:129:
webRequest.setMethod(blink::WebString(base::UTF8ToUTF16(request.method)));
Why convert to UTF16? WebString::fromUTF8(string) to make a WebString from a
std::string looks more common in the codebase.

[horo, 2014-05-30 10:11:45]

https://codereview.chromium.org/305893003/diff/70001/content/renderer/service...
File content/renderer/service_worker/service_worker_script_context.cc (right):

https://codereview.chromium.org/305893003/diff/70001/content/renderer/service...
content/renderer/service_worker/service_worker_script_context.cc:129:
webRequest.setMethod(blink::WebString(base::UTF8ToUTF16(request.method)));
On 2014/05/30 09:37:01, falken wrote:
> Why convert to UTF16? WebString::fromUTF8(string) to make a WebString from a
> std::string looks more common in the codebase.

Done.

[falken, 2014-05-31 03:32:56]

lgtm!

[horo, 2014-06-02 22:11:46]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-06-02 22:12:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/horo@chromium.org/305893003/110001

[commit-bot: I haz the power, 2014-06-03 02:38:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-03 09:17:08]

Change committed as 274450