297598 - bring pyxpcom up to speed

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

split from 267040 comment 33:

PyXPCOM doesn't compile anymore since the removal of NS_GetGlobalComponentManager.

Comment 1

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: port to non-obsolete component manager/registrar, and various cleanups

I ported all uses of nsIComponentManagerObsolete over to nsICompnentManager
resp. nsIComponentRegistrar (except in one place where I couldn't find the
equivalent functionality). For backward compatibility, the
NS_GetGlobalComponentManager python function is retained, wrapping
nsIComponentManagerObsolete. The not backward-compatible changes are:
* _xpcom.IID_nsIComponentManager previously was nsICMObsolete'd IID, but now
really is nsICM's IID, and
* xpcom.components.manager now is nsICM, not nsICMObsolete
I hope that's not going to be a problem?

Because it would have been hard to separate it out, the patch also includes
various gcc4 fixes (mostly ";" removed in inappropriate places), and Shane
Hathaway's patch from bug 281156 for python using UTF-32 on linux.

There is one FIXME added for a pre-existing problem in
Py_nsISupports::PyObjectFromInterface, which I want to take in another round
since this patch is already getting too big.

Comment 2

[José Aliste]

With patch id=187714 actually I can compile pyxpcom but when I try to do 
import xpcom.components I get the following error. 
>>> import xpcom.components
Failed to get info for IID '{a88e5a60-205a-4bb1-94e1-2628daf51eae}'
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "/usr/local/lib/python2.3/site-packages/xpcom/components.py", line 55, in ?
    manager = xpcom.client.Component(_xpcom.GetComponentManager(),
_xpcom.IID_nsIComponentManager)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
229, in __init__
    self.QueryInterface(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
299, in QueryInterface
    self._remember_interface_info(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
272, in _remember_interface_info
    method_infos, getters, setters, constants = BuildInterfaceInfo(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
147, in BuildInterfaceInfo
    interface = xpt.Interface(iid)
  File "/usr/local/lib/python2.3/site-packages/xpcom/xpt.py", line 83, in __init__
    item = iim.GetInfoForIID(iid)
xpcom.Exception: 0x80004005 (NS_ERROR_FAILURE)

Comment 3

[Gustavo Gama]

(In reply to comment #2)
> With patch id=187714 actually I can compile pyxpcom but when I try to do 
> import xpcom.components I get the following error. 
> >>> import xpcom.components
> Failed to get info for IID '{a88e5a60-205a-4bb1-94e1-2628daf51eae}'
> Traceback (most recent call last):
>   File "<stdin>", line 1, in ?
>   File "/usr/local/lib/python2.3/site-packages/xpcom/components.py", line 55, in ?
>     manager = xpcom.client.Component(_xpcom.GetComponentManager(),
> _xpcom.IID_nsIComponentManager)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 229, in __init__
>     self.QueryInterface(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 299, in QueryInterface
>     self._remember_interface_info(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 272, in _remember_interface_info
>     method_infos, getters, setters, constants = BuildInterfaceInfo(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/client/__init__.py", line
> 147, in BuildInterfaceInfo
>     interface = xpt.Interface(iid)
>   File "/usr/local/lib/python2.3/site-packages/xpcom/xpt.py", line 83, in __init__
>     item = iim.GetInfoForIID(iid)
> xpcom.Exception: 0x80004005 (NS_ERROR_FAILURE)

I tested the patch with Firefox trunk (Jul 25, 2005) and python-2.3.5 but
couldn't reproduce the problem. I followed the steps described in
http://live.gnome.org/Epiphany/EphyPython/PyXPCOM (replacing the patch with the
updated version, of course) and everything worked just fine.

Comment 4

[Christian Persch (GNOME) (away; not receiving bug mail)]

Also this message on the PyXPCOM mailing list suggests that the patch works fine
on win and macosx too: http://aspn.activestate.com/ASPN/Mail/Message/pyxpcom/2753221

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: Updated to trunk and test fix

Updating the patch to the trunk, plus:
* It appears Windows now uses "xpcom_trunk.dll" rather than 'xpcom.dll'.  I am
building the 'suite' so I have a slight concern that different builds use
different names for this DLL - can anyone confirm this?

* The test_streams test was not being run via regrtest.  The test needed
reworking, so at the same time I converted it to use unittest.

My CVS is all working again so I'm able to steer this through.

Comment 6

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

> * It appears Windows now uses "xpcom_trunk.dll" rather than 'xpcom.dll'.

xpcom.dll exists. it contains only frozen functions, though. depending on
whether libxul is enabled, the rest of the functions live in
xul.dll/xpcom_core.dll. I'm not sure what happens in static builds.

Comment 7

[Alec Flett]

Comment on attachment 191153 [details] [diff] [review]
Updated to trunk and test fix

ok, I have a few questions before I stamp this r=alecf

1) in PyCreateInstanceByContractID, why must the 2nd parameter be None? There
should at least be comments documenting why this is the case (i.e. what future
API feature are we expecting)

2)
+#if defined(NSCAP_FEATURE_TEST_DONTQUERY_CASES)
+	nsISupports *query_result = nsnull;
+	pis->QueryInterface (riid, (void**)&query_result);
+	NS_ASSERTION(query_result == pis, "QueryInterface needed");
+	NS_IF_RELEASE(query_result);
+#endif

This should be in #ifdef NS_DEBUG, since the only point of the code is to test
the assertion

(I'm curious why pyxpcom doesn't use nsCOMPtr<> but that is probably a much
bigger question!)


If you want to just give me the explanation in this bug for 1) or how to fix
it, I will fix both locally before checking in.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Attachment: Updated patch

Thanks Alec,

1) The missing arg is an "outer" interface.  pyxpcom has never supported this
param - the patch has just reorganized the old code.  I've updated a new patch
with a brief comment about why we ignore it (basically as I'm not sure of the
semantics yet and I've never seen anyone actually use it.

2) I've come to the conclusion that the new debug test is actually invalid. 
Some cases where the assertion fails are valid - it is the responsibility of
the caller to ensure things are sane.  All such callers are in pyxpcom, and I
believe they are sane.	This may help trap errors in other components, but as
it has false-positives, it should go.  My new patch has this removed.

Regarding nsCOMPtr<> - there are probably some places where it should be used
(and that new block was an example).  However, in most cases a pyxpcom C++
function is called which steals the reference from the caller - ie, most places
that get a new reference pass ownership on instead of releasing it.  Although
nsCOMPtr does have facilities for this, its primary use (ensuring references
are released) is not needed, so it is not used.

This patch also changes "xpcom_core.dll" to "nspr4.dll" as we discussed in
mail, including a new comment.

Comment 9

[Christopher Aillon (sabbatical, not receiving bugmail)]

What is the status here?  Is this ready for review?

Comment 10

[Alec Flett]

Comment on attachment 191674 [details] [diff] [review]
Updated patch

oh yeah, heh :)

I'm giving this r=alecf, and asking for a super-reviewer now.

Shaver, do you mind doing the review here? This brings PyXPCOM back from the
dead (again) and does some significant work to get Unicode working correctly
between XPCOM and Python...

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 191674 [details] [diff] [review]
Updated patch

rs=shaver on the strength of it being confined to extensions/python, and that
stuff already being savagely busted on the trunk

Comment 12

[Mark Hammond [:markh] [:mhammond]]

Checked in the trunk - thanks everyone.

Comment 13

[Christian Persch (GNOME) (away; not receiving bug mail)]

(In reply to comment #12)
> Checked in the trunk - thanks everyone.

IMHO this should be merged to 1.8 branch too. Since it's no part of the default
build, there's no risk associated. What do you think?

Comment 14

[Shane Caraveo]

(In reply to comment #13)
> IMHO this should be merged to 1.8 branch too. 

FYI I'm using cvs trunk of extension/python with the 1.8 branch, it's working
fine.  It would be nice to have pyxpcom buildable in the 1.8 branch.

Comment 15

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 191674 [details] [diff] [review]
Updated patch

Not part of the build, no risk. This patch fixes pyxpcom to work at all on 1.8
branch.

Comment 16

[Mark Hammond [:markh] [:mhammond]]

FYI, checked in on the 1.8 branch.

Comment 17

[Christian Persch (GNOME) (away; not receiving bug mail)]

It looks like the new file
mozilla/extensions/python/xpcom/src/PyIComponentManagerObsolete.cpp wasn't added
on 1.8 branch, making the build fail on a fresh checkout.

Comment 18

[Mark Hammond [:markh] [:mhammond]]

thanks - added that missing file to the branch.