XMLHttpRequest::overrideMimeType should throw InvalidStateError

XMLHttpRequest::overrideMimeType should throw InvalidStateError

Before this CL, XMLHttpRequest accepted |overrideMimeType| regardless
of its state. This is not conformant to the current spec:
http://www.w3.org/TR/XMLHttpRequest2/#the-overridemimetype-method

This CL changes it to throw InvalidStateError when the state
is LOADING or DONE, which aligns to the spec.

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

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

[kouhei (in TOK), 2014-08-18 07:41:02]

Would you take a look?

[tyoshino (SeeGerritForStatus), 2014-08-18 07:56:46]

lgtm

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
File
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html
(right):

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html:14:
shouldThrow("xhr.overrideMimeType('text/asdf')");
i think we should use a safe input e.g. "text/plain" here so that the only odd
thing we have is readyState.

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html:29:
xhr.send(null);
check here too?

https://codereview.chromium.org/478223002/diff/40001/Source/core/xml/XMLHttpR...
File Source/core/xml/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/478223002/diff/40001/Source/core/xml/XMLHttpR...
Source/core/xml/XMLHttpRequest.cpp:1039:
exceptionState.throwDOMException(InvalidStateError, "MimeType cannot be overrode
when the state is LOADING or DONE.");
overrode -> overridden

[haraken, 2014-08-18 08:10:09]

LGTM

[kouhei (in TOK), 2014-08-18 09:29:21]

Thanks!

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
File
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html
(right):

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html:14:
shouldThrow("xhr.overrideMimeType('text/asdf')");
On 2014/08/18 07:56:46, tyoshino wrote:
> i think we should use a safe input e.g. "text/plain" here so that the only odd
> thing we have is readyState.

Done.

https://codereview.chromium.org/478223002/diff/40001/LayoutTests/http/tests/x...
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-overridemimetype-allowedState.html:29:
xhr.send(null);
On 2014/08/18 07:56:45, tyoshino wrote:
> check here too?

Done.

https://codereview.chromium.org/478223002/diff/40001/Source/core/xml/XMLHttpR...
File Source/core/xml/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/478223002/diff/40001/Source/core/xml/XMLHttpR...
Source/core/xml/XMLHttpRequest.cpp:1039:
exceptionState.throwDOMException(InvalidStateError, "MimeType cannot be overrode
when the state is LOADING or DONE.");
On 2014/08/18 07:56:46, tyoshino wrote:
> overrode -> overridden

Done.

[kouhei (in TOK), 2014-08-18 09:29:25]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-08-18 09:29:40]

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

[commit-bot: I haz the power, 2014-08-18 10:29:25]

Committed patchset #4 (60001) as 180440

[Mike West, 2014-08-18 13:12:23]

A revert of this CL (patchset #4) has been created in
https://codereview.chromium.org/486703002/ by mkwst@chromium.org.

The reason for reverting is: It looks like this broke a number of Chromium-side
browser tests: See
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%2...
for example.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
for all the broken tests..

[tyoshino (SeeGerritForStatus), 2014-08-19 02:24:55]

On 2014/08/18 13:12:23, Mike West wrote:
> A revert of this CL (patchset #4) has been created in
> https://codereview.chromium.org/486703002/ by mailto:mkwst@chromium.org.
> 
> The reason for reverting is: It looks like this broke a number of
Chromium-side
> browser tests: See
>
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%2...
> for example.
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
> for all the broken tests..

Sorry for the breakage. Thanks for investigation and revert.

[kouhei (in TOK), 2014-08-19 04:13:14]

On 2014/08/19 02:24:55, tyoshino wrote:
> On 2014/08/18 13:12:23, Mike West wrote:
> > A revert of this CL (patchset #4) has been created in
> > https://codereview.chromium.org/486703002/ by mailto:mkwst@chromium.org.
> > 
> > The reason for reverting is: It looks like this broke a number of
> Chromium-side
> > browser tests: See
> >
>
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%2...
> > for example.
> >
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
> > for all the broken tests..
> 
> Sorry for the breakage. Thanks for investigation and revert.

Updated test @ https://codereview.chromium.org/471363003/.
tyoshino-san: Can I try relanding?

[kouhei (in TOK), 2014-08-19 04:25:40]

On 2014/08/19 04:13:14, kouhei wrote:
> On 2014/08/19 02:24:55, tyoshino wrote:
> > On 2014/08/18 13:12:23, Mike West wrote:
> > > A revert of this CL (patchset #4) has been created in
> > > https://codereview.chromium.org/486703002/ by mailto:mkwst@chromium.org.
> > > 
> > > The reason for reverting is: It looks like this broke a number of
> > Chromium-side
> > > browser tests: See
> > >
> >
>
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%2...
> > > for example.
> > >
> >
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
> > > for all the broken tests..
> > 
> > Sorry for the breakage. Thanks for investigation and revert.
> 
> Updated test @ https://codereview.chromium.org/471363003/.
> tyoshino-san: Can I try relanding?

Chatted offline. Will reland.

[kouhei (in TOK), 2014-08-19 04:25:44]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2014-08-19 04:26:13]

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

[commit-bot: I haz the power, 2014-08-19 04:26:54]

Committed patchset #4 (60001) as 180519