1087720 - (tracking bug) Make JS callers of ioservice.newChannel call NewChannel2

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

We have to change all JS calles 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:

addon-sdk
b2g
browser_base
browser_components
browser_devtools
browser_extensions
browser_metro
browser_modules
content
dom
extensions
image
intl
layout
mobile
modules
netwerk
services
testing
toolkit

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

I am going to assign the tracking bug to myself; dependents of this bug can be assigned to various people, but I would like to drive this one.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Once all the dependencies have been cleared we should be able to add assertions to AsyncOpen() and also Open() of all channels making sure that we have a loadInfo on all channels (within our codebase). Whether we are going to land those assertions is a different story, but at least we can make sure we haven't forgotten to add the LoadInfo to any channels.

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

Jonas,

sometimes when using
> Services.scriptSecurityManager.getSystemPrincipal(),
then we get
> ReferenceError: Services is not defined

Here is the question, which of the two should we prefer?

> XPCOMUtils.defineLazyModuleGetter(this, 'Services', 'resource://gre/modules/Services.jsm'); [1]
> Components.utils.import("resource://gre/modules/Services.jsm"); [2]

I would rather use the lazyGetter, but wanna make sure we are on the same page!

[1] http://mxr.mozilla.org/mozilla-central/search?string=XPCOMUtils.defineLazyModuleGetter%28this%2C%2B%27Services%27%2C
[2] http://mxr.mozilla.org/mozilla-central/search?string=import%28%22resource%3A//gre/modules/Services.jsm%22

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't know. Try asking on #developers or dev-platform. I don't know how the Services property is implemented.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, sorry, I misunderstood the question. I think the lazy-getter is better if we're not always hitting the codepath that uses Services. But if a .jsm always uses Services.something, then you might as well just do a simpler import.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Just a quick updates on facts. This bug depends on 14 individual bugs having a total of 241 callsites for newChannel(). The 'X' indicates whether the patches call (touch) NetUtil.newChannel instead of directly calling ioService.newChannel. This is important because we want be able to land patches individually. So I guess it makes sense to add NewChannel2 to NetUtil.jsm in addition to the current NewChannel. Once all dependcies for this bug landed, we should remove NewChannel from NetUtil.jsm so people do not call it accidentaly.


bugs                | callsites | Netutil. |
============================================
addon-sdk           |        10 | X        |
b2g                 |         1 |          |
browser             |        22 | X        |
dom                 |        19 | X        |
extensions          |         5 | X        |
image               |         2 |          |
intl                |         4 |          |
layout              |         1 |          |
mobile              |         3 | X        |
modules             |         9 | X        |
netwerk             |       129 | X        |
services            |         5 | X        |
testing             |         1 |          |
toolit              |        30 | X        |
============================================

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1087720.patch

Before closing this bug we should make sure that we land the attached patch removing:
* NetUtil.newChannel, and
* NetUtil.asyncFetch

Once all the dependencies for this bug landed, we should call newChannel2 and asyncFetch2 everywhere within our codebase.

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Attachment: js_17_shrink_netutiljsm.patch

Just rebased the patch that removes NetUtil.NewChannel() and NetUtil.asyncFetch() once all JS callsites are converted to call newChannel2 and asyncFetch2().

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

While landing all these patches where we converted newChannel to newChannel2, we got new callsites, which we now have to update again:
http://mxr.mozilla.org/mozilla-central/search?string=.newChannel%28

Going to create new bugs and adding patches for these new callsites.

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

All of the dependencies we care about before implementing the shim (channel wrapper) in bug 1120487 cleared by now. Resetting the assignee to nobody but leaving the bug open for now.

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8535774 [details] [diff] [review]
js_17_shrink_netutiljsm.patch

This patch became obsolete over time.

Comment 12

[Christoph Kerschbaumer [:ckerschb]]

All of the blocking bugs have been resolved - time to close this one!