Add the responseType parameter to InspectorTest.makeXHR

Add the responseType parameter to InspectorTest.makeXHR

We are planning to refine XHR with responseType='blob'.
To test that the Inspector works with the new implementation,
we add the ability of specifying responseType to InspectorTest.makeXHR.

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

[vsevik, 2013-09-04 11:38:58]

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
File
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html
(right):

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:14:
InspectorTest.makeXHR("GET", "resources/resource.php", true, undefined,
undefined, [], false, undefined, 'blob', step2);
Should we test a synchronous one as well?

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:22:
InspectorTest.addResult("resource.content before requesting resource size: " +
request1.resourceSize);
So what do you want to output here, resource.content or resource size?

This test does not currently test response content at all, is there a way to
test it?

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:31:
Tests XHR network resource type and content for asynchronous requests.
The message should match the test case.

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:32:
<a href="https://bugs.webkit.org/show_bug.cgi?id=61205">Bug 61205</a>
Please remove this.

[yusukesuzuki, 2013-09-05 02:22:46]

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
File
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html
(right):

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:14:
InspectorTest.makeXHR("GET", "resources/resource.php", true, undefined,
undefined, [], false, undefined, 'blob', step2);
On 2013/09/04 11:38:59, vsevik wrote:
> Should we test a synchronous one as well?

In the Document context, responseType='blob' doesn't work with synchronous
XHR[1].
Since I think makeXHR is intended to be used for testing XHR in the Document
context, I didn't test a synchronous one.
Is it correct?

[1]: http://www.w3.org/TR/XMLHttpRequest2/#the-responsetype-attribute

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:22:
InspectorTest.addResult("resource.content before requesting resource size: " +
request1.resourceSize);
On 2013/09/04 11:38:59, vsevik wrote:
> So what do you want to output here, resource.content or resource size?
> 
> This test does not currently test response content at all, is there a way to
> test it?

In the issue[2], we're working on downloading a Blob directly in the browser
process.

Since it prevents transferring the data to the renderer process and as a result,
the resource created by XHR with 'blob' type doesn't have the content.
So I think we should read the data from a Blob downloaded by XHR if we need the
content. Is there a way to access the XHR.response directly from the resource?

[2]: http://crbug.com/269055

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:31:
Tests XHR network resource type and content for asynchronous requests.
On 2013/09/04 11:38:59, vsevik wrote:
> The message should match the test case.

Done. Sorry, I missed it.

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:32:
<a href="https://bugs.webkit.org/show_bug.cgi?id=61205">Bug 61205</a>
On 2013/09/04 11:38:59, vsevik wrote:
> Please remove this.

Done.

[yusukesuzuki, 2013-09-05 02:24:17]

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
File
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html
(right):

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:22:
InspectorTest.addResult("resource.content before requesting resource size: " +
request1.resourceSize);
On 2013/09/05 02:22:47, yusukesuzuki wrote:
> On 2013/09/04 11:38:59, vsevik wrote:
> > So what do you want to output here, resource.content or resource size?
> > 
> > This test does not currently test response content at all, is there a way to
> > test it?
> 
> In the issue[2], we're working on downloading a Blob directly in the browser
> process.
> 
> Since it prevents transferring the data to the renderer process and as a
result,
> the resource created by XHR with 'blob' type doesn't have the content.
> So I think we should read the data from a Blob downloaded by XHR if we need
the
> content. Is there a way to access the XHR.response directly from the resource?
> 
> [2]: http://crbug.com/269055

Oops, I've misread it. I'll change "resource.content" to "resource.size".

[yusukesuzuki, 2013-09-05 04:45:24]

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
File
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html
(right):

https://codereview.chromium.org/23464028/diff/3001/LayoutTests/http/tests/ins...
LayoutTests/http/tests/inspector/network/network-xhr-async-response-type-blob.html:22:
InspectorTest.addResult("resource.content before requesting resource size: " +
request1.resourceSize);
To fix the issue[1], I have 2 plans.

1.
Split into 3 patches. The 1st patch disables the requestContent for XHR with
'blob' response type(returns null). The 2nd patch fixes the issue of XHR blob.
And 3rd patch recovers the requestContent for XHR with 'blob' response type.

2.
Do it with 1 patch. It fixes the issue and keeps inspector working.

Which plan do you prefer? I'll follow your decision.

[1]: http://crbug.com/269055

[vsevik, 2013-09-05 14:34:23]

lgtm

I am fine with landing this patch as is since it adds an otherwise missing test
for blob XHRs.
I would still prefer that you kept loading content in this test just like it is
done for normal XHRs, so that when you change the way blob XHRs are loaded you
didn't regress this functionality.
I am not sure if loading blob content from inspector backend will be hard after
your changes or is going to be fairly straightforward though.

[vsevik, 2013-09-05 14:35:59]

Could you please cc me on the change when you finally switch to the new blob xhr
implementation?

[yusukesuzuki, 2013-09-06 03:20:28]

On 2013/09/05 14:35:59, vsevik wrote:
> Could you please cc me on the change when you finally switch to the new blob
xhr
> implementation?

Sure. I'll cc you when I create the CL of switching to the new implementation.

[commit-bot: I haz the power, 2013-09-06 03:20:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusukesuzuki@chromium.org/23464028/15001

[commit-bot: I haz the power, 2013-09-06 04:38:46]

Retried try job too often on linux_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[vsevik, 2013-09-06 06:30:20]

You need to rebaseline

http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/console-xhr-logging.html

[yusukesuzuki, 2013-09-06 06:47:31]

On 2013/09/06 06:30:20, vsevik wrote:
> You need to rebaseline
> 
> http/tests/inspector/console-xhr-logging-async.html
> http/tests/inspector/console-xhr-logging.html

Ah! Because of the network-test.js changes, the line numbers of the result of
the console.log is changed!
Thanks, I've modified them.

[commit-bot: I haz the power, 2013-09-06 07:02:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusukesuzuki@chromium.org/23464028/40001

[yusukesuzuki, 2013-09-06 07:18:13]

On 2013/09/05 14:34:23, vsevik wrote:
> lgtm
> 
> I am fine with landing this patch as is since it adds an otherwise missing
test
> for blob XHRs.
> I would still prefer that you kept loading content in this test just like it
is
> done for normal XHRs, so that when you change the way blob XHRs are loaded you
> didn't regress this functionality.
> I am not sure if loading blob content from inspector backend will be hard
after
> your changes or is going to be fairly straightforward though.

Understood. So not to regress this functionality, first I'll create the CL that
keeps the ability of loading blob XHR content after my changes.

[commit-bot: I haz the power, 2013-09-06 08:42:19]

Change committed as 157357