1099588 - Make JS callers of ios.newChannel call ios.newChannel2 in chat/

Status: RESOLVED FIXED

(Chat Core :: General, defect)

Description

[Philip Chee]

+++ This bug was initially created as a clone of Bug #1099585 +++

(From Bug 1087720 comment #0)
> We have to change all JS callers of ioservice.newChannel to call
> ioservice.newChannel2. Since there are so many, we categorize and split them
> into different sub-bugs and use this bug as a tracking bug:

Comment 1

[:aceman]

Attachment: 1099588-IM.patch

Comment 2

[:aceman]

I'm not really sure what it is doing, but I have done what https://bugzilla.mozilla.org/attachment.cgi?id=8527395 does.

Comment 3

[:aceman]

Attachment: 1099588-IM.patch v2

Adding the missing 8th argument, see bug 1099587 and bug 1099585.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

I strongly encourage you to wait till Bug 1087442 lands which has information/documentation about all of these 8 arguments. You then also have to convert NetUtil.newChannel() calls. Bug 1087442 should hopefully make it into the codebase this week.

Please note, there should only be 8 arguments in JS.
"NSIChannel** result" is the resulting outgoing argument.

Comment 5

[Patrick Cloke [:clokep]]

Comment on attachment 8530980 [details] [diff] [review]
1099588-IM.patch v2

Review of attachment 8530980 [details] [diff] [review]:
-----------------------------------------------------------------

This fixes our current emoticon crash for me. aceman: does this need more work now that bug 1087442 has landed?

Comment 6

[:aceman]

I need to see if you use NetUtil.newChannel() somewhere. I plan to look at all the modules in a a few hours.

Comment 7

[:aceman]

Attachment: patch v3

Please try this as I do not run IM tests.

Comment 8

[Florian Quèze [:florian]]

Comment on attachment 8538024 [details] [diff] [review]
patch v3

Review of attachment 8538024 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me if someone can confirm that this works :).

Comment 9

[:Mook]

Comment on attachment 8538024 [details] [diff] [review]
patch v3

Review of attachment 8538024 [details] [diff] [review]:
-----------------------------------------------------------------

Added some drive-by comments.  I've only seriously looked at the smileProtocolHandler changes (since it causes crashes), and either form should fix the crashing part.

Thanks for working on this! :)

::: chat/components/src/smileProtocolHandler.js
@@ +34,5 @@
> +    let channel = Services.io.newChannel2(getSmileRealURI(smile), null, null, null,
> +                                          Services.scriptSecurityManager.getSystemPrincipal(),
> +                                          null,
> +                                          Ci.nsILoadInfo.SEC_NORMAL,
> +                                          Ci.nsIContentPolicy.TYPE_OTHER);

You probably want something like this instead:
    let uri = Services.io.newURI(getSmileRealURI(smile), null, null);
    let channel = Services.io.newChannelFromURIWithLoadInfo(uri, aLoadInfo);
(That method is new, but exists in today's nightly)

::: chat/protocols/xmpp/xmpp.jsm
@@ +1328,5 @@
> +    }).bind(this), null, null,
> +    Services.scriptSecurityManager.getSystemPrincipal(),
> +    null,
> +    Ci.nsILoadInfo.SEC_NORMAL,
> +    Ci.nsIContentPolicy.TYPE_OTHER);

Would that be TYPE_IMAGE rather than TYPE_OTHER here?

Comment 10

[:aceman]

Attachment: patch v4

OK, thanks.

Comment 11

[Patrick Cloke [:clokep]]

Thanks! Sorry for taking so long to check this in. I changed the spacing slightly before committing this.

https://hg.mozilla.org/comm-central/rev/a466603a0dde