[webkit-reviews] review denied: [Bug 6642] Split XMLHttpRequest class into JS binding and implementation : [Attachment 5755] first cut

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at nypop.com>'s
request for review:
Bug 6642: Split XMLHttpRequest class into JS binding and implementation
http://bugzilla.opendarwin.org/show_bug.cgi?id=6642

Attachment 5755: first cut
http://bugzilla.opendarwin.org/attachment.cgi?id=5755&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Namespace should be WebCore::, not DOM:: for XMLHttpRequestImpl and everywhere.
DOM is now just a synonym for WebCore, and an obsolete one.

We're not going to use the "Impl" suffix any more going forward, so you should
omit it and name the class XMLHttpRequest.

xmlhttprequestimpl.h should be XMLHttpRequest.h (to match the class name) and
should not be in the WebCore/kjs/ecma directory. I suggest the WebCore/xml
directory (which wouldbe a new one).

Please leave the commented-out lines out altogether.

In the code for StatusText, I think you should just use jsStringOrNull. I think
it's OK for a null string to become a JavaScript null rather than undefined.
Same for GetAllResponseHeaders.

I'd like to see the listeners move into the DOM too, but I can see that might
be tricky. In general this still leaves a number of things in the bindings that
we should get into the DOM.

Looks good, the right direction. This is going to be great!