a code review: issues_1368_1610_r2558.patch

[Joel Webber]

Alex,

Please have a look at this patch (it's an evolution of the one we've been looking at together). The basic gist is that (as you discovered) setting onreadystatechange from within an onreadystatechange() callback crashes certain versions of jscript.dll. I also discovered that setting it to null sometimes throws an exception. So I preserved the 'nullFunc' stuff, added the setTimeout() trick around onreadystatechange sets within the callback, and updated the comments to reflect these issues. All the tests now pass as well.

Issues: 1368, 1610

Patch by: ajr, jgw

Files affected:

M      user/src/com/google/gwt/http/client/XMLHTTPRequest.java

M      user/src/com/google/gwt/xml/client/impl/XMLParserImplIE6.java

M      user/src/com/google/gwt/user/client/impl/HTTPRequestImpl.java

M      user/src/com/google/gwt/user/client/impl/HTTPRequestImplIE6.java

[Alex Rudnick]

LGTM! Shall I run it through drip as well, just to make sure?

--
-- Alex Rudnick
-- swe, gwt, atl

[Alex Rudnick]

Tested with the DynaTable example -- drip turns up no memory leaks
with either version of jscript.dll.

LGTM++.

[Joel Webber]

Thanks, committed as r2563.

[David Clément]

Couldn't we include this patch in a 1.4.63 version? it does not seems
to be long to port and it may be critical for a lot of users, at least
if 1.5 is not issued in the next months...

Thanks

David