200431 - you can not and should not detect services by their name

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Embedding: APIs, defect)

Description

[timeless]

###!!! ASSERTION: You are calling CreateInstance when a service for this CID
already exist!: '!entry->mServiceObject', file
i:/build/mozilla/xpcom/components/nsComponentManager.cpp, line 1998

NTDLL! DbgBreakPoint address 0x77f9f9df
nsDebug::Assertion(const char * 0x002e692c, const char * 0x002e6914, const char
* 0x002e68d8, int 1998) line 270 + 13 bytes
nsDebug::WarnIfFalse(const char * 0x002e692c, const char * 0x002e6914, const
char * 0x002e68d8, int 1998) line 387 + 21 bytes
nsComponentManagerImpl::CreateInstanceByContractID(nsComponentManagerImpl *
const 0x0043db30, const char * 0x01061d88, nsISupports * 0x00000000, const nsID
& {...}, void * * 0x0012fbbc) line 1998 + 40 bytes
nsCreateInstanceByContractID::operator()(const nsID & {...}, void * *
0x0012fbbc) line 76 + 45 bytes
nsCOMPtr<nsIObserver>::assign_from_helper(const nsCOMPtr_helper & {...}, const
nsID & {...}) line 988 + 18 bytes
nsCOMPtr<nsIObserver>::operator=(const nsCOMPtr_helper & {...}) line 611
nsAppStartupNotifier::Observe(nsAppStartupNotifier * const 0x0105c780,
nsISupports * 0x00000000, const char * 0x00419ad4, const unsigned short *
0x00000000) line 103
main1(int 3, char * * 0x00444380, nsISupports * 0x00fe50d8) line 1106
main(int 3, char * * 0x00444380) line 1642 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! SetUnhandledExceptionFilter + 92 bytes

And people complain when i write js test apps that do this :)

+	cid	{"@mozilla.org/js/jsd/app-start-observer;2"}
+	categoryEntry	{"JSDebugger Startup Observer"}

and again
+	cid	{"@mozilla.org/scriptsecuritymanager;1"}
+	categoryEntry	{"service"}

Comment 1

[timeless]

.

Comment 2

[Adam Lock]

-> Doug

Comment 3

[Doug Turner (:dougt)]

I am not sure what you are asking for timeless.

First, lets be clear -- knowning how to use an component is purely a
documentation problem.  The ContractID isn't going to be enough for anyone to
know how exactly to use a component -- it wasn't designed for that, and we won't
enforce some uniform "is service" flag on the string.

The assertion that your seeing (note that he enabled a special #define) is for
catching bad design patterns (although alecf tells me that some places you need
to have the same object be a singleton in one context and a instance in another.
 that is all good, i guess, but I just don't want to see this kinda of crap:
http://lxr.mozilla.org/seamonkey/source/embedding/qa/testembed/BrowserView.cpp.mod#1179
)

if your asking for support to access a service by its user friendly Class Name
(one of the fields you pass into the nsIComponentRegistrar interface, file a
RFE.  Although I can tell you it borders on wont-fix.

Comment 4

[timeless]

so the code here:
mozilla/embedding/components/appstartup/src/nsAppStartupNotifier.cpp
is ok?
 86                        if (NS_SUCCEEDED(rv)) {
 87            

 88                            // If we see the word "service," in the beginning
 89                            // of the contractId then we create it as a service
 90                            // if not we do a createInstance
 91            
 92                            const char *pServicePrefix = "service,";
 93                            
 94                            nsCAutoString cid(contractId);
 95                            PRInt32 serviceIdx = cid.Find(pServicePrefix);

 96            
 97                            nsCOMPtr<nsIObserver> startupObserver;
 98                            if (serviceIdx == 0)

 99 cathleen 1.7                      startupObserver = do_GetService(cid.get()
+ strlen(pServicePrefix), &rv);

100 chak     1.1                  else

101                                   startupObserver =
do_CreateInstance(contractId, &rv);

Comment 5

[Doug Turner (:dougt)]

sure.  I agree.  over to default.

Comment 6

[Charles Rosendahl]

Comment 3:  Help me out here, what is the issue with 

http://lxr.mozilla.org/seamonkey/source/embedding/qa/testembed/BrowserView.cpp.mod#1179

The code seems to be littered with this syntax convention for all services.  Now
that I am the new owner of this 'stuff', I would like to make sure it is correct.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Charles, nsLocalFile is most emphatically not a service; that code should be
using do_CreateInstance (think about it this way -- there is only one instance
of a service, ever, but we may want to point to two different files at the same
time...)

Comment 8

[Adam Lock]

Can I clarify? The issue here is the dodgy assumption that a contract id
containing 'service' as a substring is a service?

I've never seen this code so I'm not 100% what it's even trying to do.

Comment 9

[Doug Turner (:dougt)]

hi adam.  I think the issue is that the code inspects the contract id string. 
contract id's should be treated, for the most part, as opaque strings.  

Comment 10

[Benjamin Smedberg]

Marking a bunch of bugs in the "Embedding: APIs" component INCOMPLETE in preparation to archive that component. If I have done this incorrectly, please reopen the bugs and move them to a more correct component as we don't have "embedding" APIs any more.