1351443 - Can we only fallback to NewChannel() when NewChannel2() fails because it's unimplemented?

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Description

[Kris Maglione [:kmag]]

We currently fallback to NewChannel() when NewChannel2() fails for any reason. This is unexpected in a lot of cases. Since the fallback is only meant to handle cases where NewChannel2() is unimplemented, can we only do the fallback when the error code is NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED or NS_ERROR_NOT_IMPLEMENTED?

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Pat, I think that would make sense actually, what do you think?

Comment 2

[Patrick McManus [:mcmanus]]

sounds reasonable.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing

If the call to NewChannel2 returns NS_ERROR_NOT_IMPLEMENTED or NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED that means the implementation is actually missing, so it is OK to fall back to NewChannel.
If it fails with any other error code, we just return it.

Review commit: https://reviewboard.mozilla.org/r/125652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125652/

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8853585 [details]
Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing

Drive by feedback - thanks for providing the patch and the test.

Comment 5

[Patrick McManus [:mcmanus]]

Comment on attachment 8853585 [details]
Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing

https://reviewboard.mozilla.org/r/125652/#review129990

a LOG() or two might be useful..

Comment 6

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8853585 [details]
Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125652/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57b19b8018c7
Only fall back to NewChannel if implementation of NewChannel2 is missing r=mcmanus

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/57b19b8018c7