Adding ArrayBuffer support to ServiceWorker Response and tests

Adding ArrayBuffer support to ServiceWorker Response and tests

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

[dmurph, 2014-09-08 20:14:58]

dmurph@chromium.org changed reviewers:
+ horo@chromium.org

[dmurph, 2014-09-08 20:14:59]

Horo,

PTAL, I'm adding support for the other construction types of response/request. 
This is a patch for Response.

[jsbell, 2014-09-08 20:32:47]

jsbell@chromium.org changed reviewers:
+ jsbell@chromium.org

[jsbell, 2014-09-08 20:32:48]

Some style/message/comment nits, so you don't have to wait for TOK to wake up.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/response-content-worker.js
(right):

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:5:
assert_equals(response.headers.get('Content-Type'), 'text/plain;charset=UTF-8');
style: wrap to 80 characters

See: http://www.chromium.org/blink/serviceworker/testing

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:5:
assert_equals(response.headers.get('Content-Type'), 'text/plain;charset=UTF-8');
Always have an assertion message; ideally, phrased as "... should ..."

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:6:
return response.body.asText().then(function(text) {
style: wrap before .then

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:7:
assert_equals(text, "test string");
Add message to assertion

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:16:
assert_false(response.headers.has('Content-Type'));
Add message to assertion

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:17:
return response.body.asArrayBuffer().then(function(buffer) {
style: wrap before the .then

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:18:
var resultIntView  = new Int32Array(buffer);
nit: extra space before the =

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:19:
assert_array_equals(resultIntView, [0,1,2,3,4,5,6,7,8,9]);
Add message to assertion

https://codereview.chromium.org/556563002/diff/20001/Source/modules/servicewo...
File Source/modules/serviceworkers/Response.idl (right):

https://codereview.chromium.org/556563002/diff/20001/Source/modules/servicewo...
Source/modules/serviceworkers/Response.idl:10: // FIXME: Add ctors for
ArrayBuffer, ArrayBufferView, FormData,
Nit: update the FIXME :)

[dmurph, 2014-09-08 20:59:30]

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/response-content-worker.js
(right):

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:5:
assert_equals(response.headers.get('Content-Type'), 'text/plain;charset=UTF-8');
On 2014/09/08 20:32:48, jsbell wrote:
> style: wrap to 80 characters
> 
> See: http://www.chromium.org/blink/serviceworker/testing

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:5:
assert_equals(response.headers.get('Content-Type'), 'text/plain;charset=UTF-8');
On 2014/09/08 20:32:48, jsbell wrote:
> Always have an assertion message; ideally, phrased as "... should ..."

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:6:
return response.body.asText().then(function(text) {
On 2014/09/08 20:32:48, jsbell wrote:
> style: wrap before .then

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:7:
assert_equals(text, "test string");
On 2014/09/08 20:32:48, jsbell wrote:
> Add message to assertion

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:16:
assert_false(response.headers.has('Content-Type'));
On 2014/09/08 20:32:48, jsbell wrote:
> Add message to assertion

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:17:
return response.body.asArrayBuffer().then(function(buffer) {
On 2014/09/08 20:32:48, jsbell wrote:
> style: wrap before the .then

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:18:
var resultIntView  = new Int32Array(buffer);
On 2014/09/08 20:32:48, jsbell wrote:
> nit: extra space before the =

Done.

https://codereview.chromium.org/556563002/diff/20001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:19:
assert_array_equals(resultIntView, [0,1,2,3,4,5,6,7,8,9]);
On 2014/09/08 20:32:48, jsbell wrote:
> Add message to assertion

Done.

[horo, 2014-09-09 01:38:50]

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/response-content-worker.js
(right):

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:4: var
response = new Response('test string');
+2 indent
http://www.chromium.org/blink/serviceworker/testing#TOC-Wrapping-and-Indentation
promise_test(function() {
    var response = new Response('test string');
    // ...
  }, 'Behavior of Response with string content.');

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:11:
assert_equals(text, 'test string',
+2 indent

  return response.body.asText()
    .then(function(text) {
        assert_equals(text, 'test string',
          'Response body text should match the string on construction.');
      });

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:17:
var intView  = new Int32Array([0, 1, 2, 3, 4, 55, 6, 7, 8, 9]);
ditto

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:25:
var resultIntView = new Int32Array(buffer);
ditto

https://codereview.chromium.org/556563002/diff/40001/Source/modules/servicewo...
File Source/modules/serviceworkers/Response.idl (right):

https://codereview.chromium.org/556563002/diff/40001/Source/modules/servicewo...
Source/modules/serviceworkers/Response.idl:10: // FIXME: Add ctors for
ArrayBufferView, FormData,
nit:
    // FIXME: Add ctors for ArrayBufferView, FormData and URLSearchParams
    // response bodies.

[dmurph, 2014-09-09 19:06:03]

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/response-content-worker.js
(right):

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:4: var
response = new Response('test string');
On 2014/09/09 01:38:50, horo wrote:
> +2 indent
>
http://www.chromium.org/blink/serviceworker/testing#TOC-Wrapping-and-Indentation
> promise_test(function() {
>     var response = new Response('test string');
>     // ...
>   }, 'Behavior of Response with string content.');

Done.

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:11:
assert_equals(text, 'test string',
On 2014/09/09 01:38:50, horo wrote:
> +2 indent
> 
>   return response.body.asText()
>     .then(function(text) {
>         assert_equals(text, 'test string',
>           'Response body text should match the string on construction.');
>       });

Done.

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:17:
var intView  = new Int32Array([0, 1, 2, 3, 4, 55, 6, 7, 8, 9]);
On 2014/09/09 01:38:50, horo wrote:
> ditto

Done.

https://codereview.chromium.org/556563002/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/response-content-worker.js:25:
var resultIntView = new Int32Array(buffer);
On 2014/09/09 01:38:50, horo wrote:
> ditto

Done.

https://codereview.chromium.org/556563002/diff/40001/Source/modules/servicewo...
File Source/modules/serviceworkers/Response.idl (right):

https://codereview.chromium.org/556563002/diff/40001/Source/modules/servicewo...
Source/modules/serviceworkers/Response.idl:10: // FIXME: Add ctors for
ArrayBufferView, FormData,
On 2014/09/09 01:38:50, horo wrote:
> nit:
>     // FIXME: Add ctors for ArrayBufferView, FormData and URLSearchParams
>     // response bodies.

Done.

[horo, 2014-09-10 00:47:26]

lgtm

[dmurph, 2014-09-10 17:32:02]

The CQ bit was checked by dmurph@chromium.org

[commit-bot: I haz the power, 2014-09-10 17:33:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmurph@chromium.org/556563002/60001

[commit-bot: I haz the power, 2014-09-10 18:35:55]

Committed patchset #4 (id:60001) as 181751