Re: [FileAPI] FileReader.abort() and File[Saver|Writer].abort have different behaviors

On Wed, Sep 21, 2011 at 8:32 PM, Eric U <ericu@google.com> wrote:

> 5.6.3 is a "substep", not a "step"; all of 5's substeps are referred
> to as such in step 5: "Otherwise run these substeps:".  However, I
> believe we were actually discussing 5.5, loadend on the XHR itself.
> Either way, the below applies to both.
>
> Regarding what happens on abort:
>
> "This algorithm can be terminated by invoking the open() method. When
> it is terminated the user agent must terminate the algorithm after
> finishing the step it is on."
>
> I read that as saying that if you call open while in onabort [5.4], it
> completes step 5, including all of substep 5.6, including 5.5 and
> 5.6.3.  Do you think I'm mistaken?  I haven't run any test cases.
>

To my reading, a substep is a type of step.  The "after finishing..." sounds
like it's saying not to terminate the currently-executing step; that is, the
event dispatch isn't stopped in the middle.  Maybe "steps" vs. "substeps"
are precisely defined somewhere else, but either way this could probably be
clearer.  Anne?

I just tried this in FF6, and the behavior is inconsistent.  If the TCP
connection has been established, then it follows your interpretation, and
you get onloadstart, onabort, onloadstart, onloadend, onloadend [1][3].  If
the TCP connection hasn't yet been established [2][4], calling open() from
either onabort or onloadend throws NS_ERROR_IN_PROGRESS.

Chrome 14 is also inconsistent.  If the TCP connection has been established
[1][3], and it's still waiting for the HTTP response, abort() seems to do
nothing.  If the connection hasn't yet been established [2], abort() does
work, and open() can be called.  onloadend is unimplemented, so the above
question is untestable [4].

(IE9 doesn't implement any of these events.)

With the various inconsistencies, I wonder if the spec can't actually be
changed to disallow open() during the events, following FF6--that
NS_ERROR_IN_PROGRESS is probably unintentional, but it's in a shipping
browser nonetheless.  Wishful thinking, maybe...

Tests:
[1] https://zewt.org/~glenn/test-open-during-onabort.html#http/<https://zewt.org/%7Eglenn/test-open-during-onabort.html#timeout/loadend>
onabort<https://zewt.org/%7Eglenn/test-open-during-onabort.html#timeout/loadend>(HTTP
timeout)
[2] https://zewt.org/~glenn/test-open-during-onabort.html#tcp/onabort<https://zewt.org/%7Eglenn/test-open-during-onabort.html#timeout/loadend>(TCP
timeout)
[3] https://zewt.org/~glenn/test-open-during-onabort.html#http/onloadend<https://zewt.org/%7Eglenn/test-open-during-onabort.html#timeout/loadend>
[4] https://zewt.org/~glenn/test-open-during-onabort.html#tcp/onloadend<https://zewt.org/%7Eglenn/test-open-during-onabort.html#timeout/loadend>

My point was that if it doesn't throw an exception in XHR, they won't
> write their XHR handlers that way.  And so they haven't, and have
> legacy code.  And when they add FileWriter handlers, they'll copy the
> XHR code, and it will work most of the time, because an abort is an
> exceptional case, and then it will fail in the field.
>

It's exceptional, but not obscure--this couldn't go unnoticed through basic
testing, since it fails with an exception.  (If people don't test their
abort code path *at all*, it isn't going to work regardless of what we
do--the goal should be to make things reliably testable, not to allow people
to deploy code without testing anything.)  I think the bugs resulting from
out-of-order loadstart/loadend messages would be very easy to miss through
regular testing, more annoying to have to work around, and more likely to be
triggered by code written by another party (eg. the activity-monitor case).

On Wed, Sep 21, 2011 at 8:34 PM, Jonas Sicking <jonas@sicking.cc> wrote:

> > The spec says "The abort() algorithm can only be terminated by invoking
> > open() from an event handler."  If you call open() from 5.6.2's onabort,
> > then step 14 of the open() algorithm terminates abort(), and step 5.6.3
> > never happens, thus onloadend is never fired and loadstart and loadend
> > events are mismatched.
>
> This sounds like a bug in the XHR spec as it doesn't fulfill the
> invariants I listed.
>

I agree with those invariants, but they aren't normative anywhere, are they?

-- 
Glenn Maynard