27017 - nsINetModMgr should go away

Status: VERIFIED WONTFIX

(Core :: Networking, defect, P3)

Description

[Judson Valeski]

To minimize GetProxyObject() calls we should just convert the module manager 
stuff to nsIObserver.

Comment 1

[Doug Turner (:dougt)]

[e-mail] is there any reason that this should be beta1?

Comment 2

[Judson Valeski]

no, did I put beta in there. sorry if I did.

Comment 3

[Doug Turner (:dougt)]

warren, jud, sorry guys.

m15 --> m17

Comment 4

[Judson Valeski]

*** Bug 28042 has been marked as a duplicate of this bug. ***

Comment 5

[Doug Turner (:dougt)]

gagan,  can you look into doing this?

Comment 6

[Warren Harris]

Maybe Ruslan should own this. I filed a bug last night about http thread-safety 
issues, and this class was involved. It would be good to tackle both at once.

Comment 7

[ruslan]

-> myself

Comment 8

[Doug Turner (:dougt)]

Fixing this should reduce the chance that we crash during activation. 

http://bugzilla.mozilla.org/show_bug.cgi?id=34718

Comment 9

[Jay Patel [:jay]]

Making [nsbeta2-] and Putting on [NEED INFO] radar. PDT needs to know impact to 
user and risk of fix to make a call on this bug. Please let us know when you 
have a fix in hand so we can discuss whether to land this thing or not.

Comment 10

[ruslan]

module manager is a complete nightmare in case when you try to run the handler 
on a non-UI thread. Unfortunately it's being used all over the place. I'll see 
what's involved into fixing this completely.

Comment 11

[ruslan]

So far it can't really go away, as we do need to proxy calls back to the 
originating thread. On top of everything I had to do the same for 
OnProgress/etc. events as well. Once Warren finalizes notification object we can 
replaced this and status notification events with it. Removing nsbeta2 and 
pushing the milestone forward.

Comment 12

[ruslan]

I can't go away, since we need to proxy calls back to UI thread and the proxy 
code itself is already checking if it's on the same thread and makes a direct 
call.

Comment 13

[Doug Turner (:dougt)]

ruslan, are the calls to getProxy minimized?  do we create proxies more than we 
absolutely have to?

Comment 14

[Judson Valeski]

Right now for every HTTP request and response we create a proxy for each 
listener (one for the request and one for the response). It's done every time in 
the interest of handling a runtime change in the listeners. We could require 
that listeners register at module load time and cache each listener.

Comment 15

[Doug Turner (:dougt)]

that would be a good optimization.  reopen bug?

Comment 16

[Judson Valeski]

sure. as long as we're ok w/ losing the dynamic runtime checking for new
listeners.

Comment 17

[ruslan]

We HAVE to create proxies for the cases we use. The only solution in the future 
is to come up with only one interceptor (at the expense of the beaty of the 
code) and each consumer will have to recognize what's going on. When the handler 
is running on the same thread (which is the case for almost all cases with http) 
- there's no overhead at all, because proxy code will recognize that fact and 
will not do anything.

Comment 18

[Judson Valeski]

ruslan, you mis-understood. we're still talking about using proxies, just 
caching them and keeping them in a list rather than creating them twice for 
*every* http transaction.

Comment 19

[Warren Harris]

Also, I thought the point of this bug was to eliminate nsINetModMgr in favor of 
nsIObserver -- nothing to do with when and if the observer gets proxied or not.

Comment 20

[ruslan]

I still don't understand what's the problem. The proxy object even in cases when 
it gets created is getting cached in netmodule entry. The performance impact 
should be minimal.

Comment 21

[Daniel Veditz [:dveditz]]

Adding nsbeta2 keyword to bugs with nsbeta2 triage value in status field so the 
queries don't get screwed up

Comment 22

[ruslan]

wontfix for now.

Comment 23

[Tom Everingham]

verified Wontfix