[webkit-reviews] review cancelled: [Bug 13141] XMLHttpRequest should set readyState to 0 after abort() : [Attachment 16423] Proposed patch and testcase

[bugzilla-daemon at webkit.org]

Julien Chaffraix <julien.chaffraix at gmail.com> has cancelled Julien Chaffraix
<julien.chaffraix at gmail.com>'s request for review:
Bug 13141: XMLHttpRequest should set readyState to 0 after abort()
http://bugs.webkit.org/show_bug.cgi?id=13141

Attachment 16423: Proposed patch and testcase
http://bugs.webkit.org/attachment.cgi?id=16423&action=edit

------- Additional Comments from Julien Chaffraix <julien.chaffraix at gmail.com>
> 1. I don't understand why you're removing the state change in open() I can
see
> that affecting behavior (a second open call on a completed xhr?  what about a

> call which fails due to a domain check?)  Maybe I'm being over cautious, but
if
> we're to remove that call, I think there should be a test to show that
desired
> result.

The call to changeState is not necessary. The changeState is done in abort()
and not in open() now.
The instructions between the new call to changeReadyState and the old one are
related to the cancellation of the previous XMLHttpRequest (clearing document
and creating a new ResourceResponse()). They are thus unaffected by the change
and cannot return unexpectedly.
For the check, they were done after the call to changeState and it is also the
case with the patch.
 
> 2.  I'm not sure I fully understand your test case, and why it needs to be
> recursive as such.  Perhaps it's just to clever for me...

The aim is to try to abort the XMLHttpRequest for all the readyState. The
recursive is just a choice I have made. It implies an order in which we execute
the tests (from state 1 to 4).
I am not sure that this order is guaranteed in the test case attached which is
not recursive. Tell me which one should be the better (I would the output of
the previous one so that it is more understandable ;)) and I will create a new
patch.