217766 - use nsIObserverService in place of nsINetModuleMgr (eliminate nsIHttpNotify)

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Darin Fisher]

use nsIObserverService in place of nsINetModuleMgr (eliminate nsIHttpNotify)

the nsINetModuleMgr mechanism is really unnecessary.  for what we do with it,
nsIObserverService is perfectly fine.  we can eliminate nsINetModuleMgr if we
dump nsIHttpNotify (OnModifyRequest & OnExamineResponse) in favor of simple
NotifyObserver calls.  now that cookies no longer implements nsIHttpNotify,
there are no consumers of it in the tree, and the only thing that will be
effected is the livehttpheaders extension from mozdev (and maybe other add-ons
like it).  i will make announcements in n.p.m.netlib and send mail directly to
the livehttpheaders maintainer(s).

patch coming up...

Comment 1

[Darin Fisher]

Attachment: v1 patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 130611 [details] [diff] [review]
v1 patch

>Index: base/src/nsFileStreams.h

What does this change have to do with this bug?

sr=bzbarsky; I assume you will be cvs removing the netmodulemgr idl and
implementation?

Comment 3

[Darin Fisher]

thanks for the quick review bz.. i needed to add prio.h to nsFileStreams.h
because otherwise there is a compiler error in nsNetModule.cpp.  as it turns out
nsNetModuleMgr.h #include'd nspr.h and it was #include'd in nsNetModule.cpp
before nsFileStreams.h.  nsFileStreams.h references PRFileDesc without a forward
declaration.  instead of adding my own forward declaration, i just #include'd
prio.h since a forward declaration of a typedef is not fun.

and yes, i will be cvs removing all the dead files once i land this patch ;-)

Comment 4

[[e-mail]]

Comment on attachment 130611 [details] [diff] [review]
v1 patch

>Index: protocol/http/public/nsIHttpProtocolHandler.idl
>===================================================================
>+/**
>+ * Before an HTTP request is sent to the server, this observer topic is
>+ * notified.  The observer of this topic can then chose to set any additional

s/chose/choose/ :)

nice! r=dwitte

Comment 5

[Darin Fisher]

Attachment: v1.1 patch - revised patch

same patch + dwitte sp nit + tiny compilation fix

thx for the speedy reviews guys!

Comment 6

[Daniel Savard]

I will look at how to modify LiveHTTPHeaders to be able to use
nsIObserverService.  Does this works now with the latest nightly build so I can
try my modification ?
Or do I need to wait for a particular nightly build ?

Comment 7

[Darin Fisher]

Daniel: the patch has not been checked into the mozilla source tree yet.  when
that happens, this bug will be marked fixed, and then the changes will appear in
the next daily build.

Comment 8

[Darin Fisher]

ok, this patch has been checked in on the trunk.  mozilla 1.6 will no longer
support nsIHttpNotify or the nsINetModuleMgr interfaces.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using LXR.