[webkit-reviews] review denied: [Bug 7392] GMAIL: XMLHttpRequest does not correctly report "Interactive" state on receipt of load data : [Attachment 6627] simple fix (and a little cleanup)

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Eric Seidel <macdome at opendarwin.org>'s
request for review:
Bug 7392: GMAIL: XMLHttpRequest does not correctly report "Interactive" state
on receipt of load data
http://bugzilla.opendarwin.org/show_bug.cgi?id=7392

Attachment 6627: simple fix (and a little cleanup)
http://bugzilla.opendarwin.org/attachment.cgi?id=6627&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I think it's awkward to have changeState have a side effect of always calling
the listener if the passed state is Interactive. Seems like the wrong name for
a function that will both change state and have this particular side effect.

I'd suggest instead doing something different at the caller in
XMLHttpRequest::slotData; maybe code that detects the state and calls
callReadyStateChangeListener explicitly if we're already in "interactive"
state?

Also, I think that putting this code into a separate function makes the logic a
little weaker, because now the guarantee that the load event is sent exactly
once is pretty subtle. If anyone called callReadyStateChangeListener an extra
time when the state was already Completed, we'd fire a separate load event.

The fix looks good, but I think it can be done in a way that's slightly
clearer.