267040 - Remove obsolete xpcom component manager entry points

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P1)

Description

[Darin Fisher]

Remove nsIComponentManagerObsolete and associated xpcom entry points.

This code is only needed to support the 1.3 JRE, but Sun has already declared it
EOL.  It already doesn't work under Linux with our latest builds due to compiler
GCC C++ ABI changes, and I doubt it is worth supporting the 1.3 JRE in Mozilla
1.8 and beyond.

If you think we should keep this obsolete and deprecated API around longer, then
please let me know.

Comment 1

[Darin Fisher]

Attachment: eliminate usage of nsComponentManager:: methods in the tree [checked in]

this patch is a fairly straightforward translation from the obsolete
nsComponentManager:: methods to the corresponding methods on
nsIComponentManager and nsIComponentRegistrar.

i also added warnings to all the nsComponentManager:: methods that are now
unused in the tree.  (nsComponentManagerUtils.h still uses CreateInstance and
GetClassObject, so those don't have warnings.)

next steps after this patch are to teach CallCreateInstance and
CallGetClassObject how to not use this obsolete API.  then we should be ready
to drop nsComponentManager:: altogether unless we can show that there are still
some plugins that need it.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #1)
> Created an attachment (id=164148)
> next steps after this patch are to teach CallCreateInstance and
> CallGetClassObject how to not use this obsolete API.

this is bug 263360, I think.

Comment 3

[Benjamin Smedberg]

Comment on attachment 164148 [details] [diff] [review]
eliminate usage of nsComponentManager:: methods in the tree [checked in]

embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp
Weird whitespace?

Why can't all those nsComponentManager:: NS_WARNING()s be NS_ERROR()s?
Assertions are much more likely to get attention.

Comment 4

[Darin Fisher]

yeah, i'm not hung up on using NS_WARNING... NS_ERROR is fine by me.  i'll make
that change before landing this patch.  thanks for the quick review bsmedberg!

Comment 5

[Darin Fisher]

s/NS_ERROR/NS_NOTREACHED/ :-)

Comment 6

[Hiro]

(In reply to comment #0)
> This code is only needed to support the 1.3 JRE, but Sun has already declared it
> EOL.  It already doesn't work under Linux with our latest builds due to compiler
> GCC C++ ABI changes, and I doubt it is worth supporting the 1.3 JRE in Mozilla
> 1.8 and beyond.

As standard, as for Mac OS X, only JRE1.3 is supported.
(See bug197813)
Don't any bad influences come out by this change?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Any idea which part of this would have increased Tdhtml a bit?  Do we need to
cache a component manager somewhere or something?

Comment 8

[Darin Fisher]

> Any idea which part of this would have increased Tdhtml a bit?  Do we need to
> cache a component manager somewhere or something?

It's most likely due to an increase in the number of calls to do_CreateInstance,
which has a lot of overhead (it uses nsCOMPtr_helper).  That's my guess anyways.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #6)

This change would break the MRJ Plugin Carbon, but not (I think)
otherwise impair Java 1.3.1 on Mac OS X.

http://www.mozilla.org/oji/MRJPluginCarbon.html

(See LiveConnectNativeMethods.cpp.)

I'm the author of something called the Java Embedding Plugin
(http://javaplugin.sourceforge.net/), which makes Java 1.4.X available
to other browsers than Safari on Mac OS X.  My program makes use of a
fixed-up version of the MRJ Plugin (which I distribute with the Java
Embedding Plugin) to work with Mozilla-family browsers.  Though you
haven't yet made this change, I've already (as of version 0.8.7 of the
JEP) included a workaround for it in "my" version of the MRJ Plugin.

So that everyone knows (the principals here should already be aware of
it), a similar design change that was incorporated into Firefox 1.0
also broke the MRJ Plugin ... and (indirectly) the Java Embedding
Plugin.  My current release (0.8.7) of the Java Embedding Plugin
includes a workaround for that problem, too.  See the following URLs:

http://sourceforge.net/tracker/index.php?func=detail&aid=1063222&group_id=107955&atid=649116
https://bugzilla.mozilla.org/show_bug.cgi?id=234169

Comment 10

[Darin Fisher]

Steven: can you be more specific about what parts of this bug will cause the MRJ
plugin to break?  does it use nsIComponentManagerObsolete?  or, does it call
some of the nsComponentManager:: static methods?

Comment 11

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #10)

LiveConnectNativeMethods.cpp (near the top) includes the following
code:

static nsresult getGlobalComponentManager(nsIComponentManagerObsolete* *result)
{
    return MRJPlugin::GetService(kComponentManagerCID,
NS_GET_IID(nsIComponentManagerObsolete), (void**)result);
}

So I guess you'd say that the MRJ Plugin "uses
nsIComponentManagerObsolete".

Comment 12

[Darin Fisher]

OK, then perhaps this bug should only be about eliminating the
nsComponentManager:: static methods.

Comment 13

[Benjamin Smedberg]

darin, let's get rid of the static methods first, then examine
nsIComponentManagerObsolete somewhere else. We might want to obsolete-freeze the
IID, but not stop supporting it until 2.0.

Comment 14

[Darin Fisher]

bsmedberg: that sounds like a good plan to me.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

maybe we should start putting obsolete/deprecated stuff inside an #ifndef
NS_DISABLE_DEPRECATED or something (or is this what MOZILLA_STRICT_API is?)

Comment 16

[Darin Fisher]

(In reply to comment #15)
> maybe we should start putting obsolete/deprecated stuff inside an #ifndef
> NS_DISABLE_DEPRECATED or something (or is this what MOZILLA_STRICT_API is?)

That's an interesting idea... I'm not sure how it would work with JS.  Anyways,
MOZILLA_STRICT_API is different.  As it turns out, components must be compiled
with MOZILLA_STRICT_API defined or else they may end up linking to private XPCOM
entry points.  In the future, MOZILLA_STRICT_API will be automatically defined
when anyone uses the Gecko SDK, but we aren't there yet.  Feel free to help make
that happen :-)

Comment 17

[Darin Fisher]

BTW, in case it isn't clear from the comments, attachment 164148 [details] [diff] [review] was checked in.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

I'm not very familiar with the "code flow" among the Mozilla-family
browsers.  Can you tell me where I'll find a binary that contains this
patch?  Which nightly or nightlies should I check, or should I be
looking somewhere else?  (It's OS X binaries that I need, of course.)

I don't anticipate any trouble ... but I figure it's a good idea to
test anyway, just to be sure.

Comment 19

[Darin Fisher]

Steven: try mozilla 1.8 alpha5, which was just released yesterday.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #19)

Thanks.

I've now tested Mozilla 1.8a5 on OS X 10.2.8 (with Java 1.4.1) and OS
X 10.3.6 (with Java 1.4.2 Update 2) with some of the usual LiveConnect
suspects, and had no trouble.  (The last "Weasel" test does often fail
on Java 1.4.1, but that's for completely unrelated reasons.)

http://www.simonstl.com/dynhtml/update/code/chap6/lc1.html
http://www.simonstl.com/dynhtml/update/code/chap6/lc2.html
http://information.overlaid.com/stable/weasel/installation/java.html

I only tested with my fixed-up version of the MRJ Plugin Carbon (what
I call the "MRJ Plugin JEP").  And in fact I _can't_ test with the
"original" MRJ Plugin Carbon -- which has a plethora of LiveConnect
bugs.  But, considering the nature of this patch, I think its safe to
say that it's compatible with both the MRJ Plugin JEP or the original
MRJ Plugin Carbon.

Comment 21

[Darin Fisher]

Steven: That's great news.  Thanks for doing that testing.

Comment 22

[Darin Fisher]

by next beta or bust i figure...

Comment 23

[Benjamin Smedberg]

We've already moved these symbols from xpcom.dll to xpcom_core, and haven't
noticed any significant problems, so let's just remove it now.

Comment 24

[Benjamin Smedberg]

Attachment: Remove exported functions

Comment 25

[Darin Fisher]

Comment on attachment 178259 [details] [diff] [review]
Remove exported functions

Ok, I'll bite.	But, your reasoning is way off.  The way dynamic symbols are
resolved on ELF at least, it does not necessarily matter in which DSO the
symbols are defined.  If a plugin is using these symbols, then what matters is
that they exist when the plugin is loaded.  So, we will only know if this
causes problems after this patch has been committed.

Comment 26

[Benjamin Smedberg]

I believe without certainty that symbols are lib-specific on windows. I was
blithely assuming that any such problems would show up on Windows, even if not
on ELF platforms.

Comment 27

[Doug Turner (:dougt)]

An old version of the JRE use to link against xpcom.  This will break that plugin.  

(not suggesting that breaking them is good or bad).

Comment 28

[Benjamin Smedberg]

Attachment 178259 [details] [diff] checked in on trunk. Morphing the bug a little bit, and
resolving. nsIComponentManagerObsolete can be another bug, if anyone cares
enough to risk removing it.

Comment 29

[Brendan Eich [:brendan]]

Yay, I may dance a jig over seeing this dead.

Doug, don't sweat it -- any JRE that old is not only obsolete, it has security
bugs we shouldn't support.

/be

Comment 30

[Patrick Schork]

This change appears to have broken pyXPCOM which still depends on
nsComponentManagerObsolete.h

c++ -o xpcom.o -c  -DMOZILLA_INTERNAL_API -DOSTYPE=\"Linux2.4.21-20\"
-DOSARCH=\"Linux\" -DBUILD_ID=0000000000  -I../../../../dist/include/xpcom
-I../../../../dist/include/string -I../../../../dist/include/pyxpcom
-I../../../../dist/include -I../../../../dist/include/nspr   
-I/usr/X11R6/include   -fPIC  -I/opt/python/include/python2.5 
-I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wconversion
-Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy
-Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar
-I/opt/python/include/python2.5 -pthread -pipe  -DNDEBUG -DTRIMMED -O 
-I/opt/python/include/python2.5  -I/usr/X11R6/include -DMOZILLA_CLIENT -include
../../../../mozilla-config.h -Wp,-MD,.deps/xpcom.pp
/opt/mozilla/extensions/python/xpcom/src/xpcom.cpp: In function `PyObject*
   PyXPCOMMethod_NS_GetGlobalComponentManager(PyObject*, PyObject*)':
/opt/mozilla/extensions/python/xpcom/src/xpcom.cpp:156: `
   NS_GetGlobalComponentManager' undeclared (first use this function)
/opt/mozilla/extensions/python/xpcom/src/xpcom.cpp:156: (Each undeclared
   identifier is reported only once for each function it appears in.)

Comment 31

[Christian :Biesinger (don't email me, ping me on IRC)]

you mean it worked before this checkin?

Comment 32

[Darin Fisher]

pyXPCOM should be fixed to use the newer (as of many years now) APIs.

Comment 33

[Patrick Schork]

pyXPCOM does not build from CVS.

There are missing #includes and MOZILLA_INTERNAL_API defs.

After fixing those problems though, you run into the
NS_GetGlobalComponentManager missing

Comment 34

[Darin Fisher]

pyXPCOM should use one of NS_GetComponentManager, NS_GetComponentRegistrar, or
NS_GetServiceManager instead of NS_GetGlobalComponentManager.

See:
http://developer.mozilla.org/xpcom/api/NS_GetComponentManager
http://developer.mozilla.org/xpcom/api/NS_GetComponentRegistrar
http://developer.mozilla.org/xpcom/api/NS_GetServiceManager

This discussion should probably move to a new bug.