104168 - xpcom-autoregistration notification never fires

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Robert Ginda]

I need to register the js debugger as an observer of the xpcom-autoregistration
topic so that I can start keeping track of scripts right away.  The problem is
twofold.  First, the component manager only enters the notification code if it
fails to create the notifier service (doh!), and second, the notification list
comes from a table built at runtime.  We havn't provided any hooks proir to this
point in the app, so there is no way for a component to add itself to the list.

The upcoming patch fixes what I assume was a NS_FAILED vs NS_SUCCEDED mixup, and
uses the nsAppStartNotifier (which is poorly named, it's actually a generic
"enumerate some category containing nsIObservers, and tell them something"
component) to take care of the notifications.  This allows clients to register
as an "xpcom-autoregistration" observer via the category manager.

This does introduce an asymmetrical relationship between the
"xpcom-autoregistration" topic, which respects categories, and "xpcom-shutdown
" which still uses a notify list built at runtime... but don't let that bother you.

Comment 1

[Robert Ginda]

Attachment: proposed patch

Comment 2

[Doug Turner (:dougt)]

hey, your lucky it fires at all! :-) :-)

I do not like the patch.  It creates a depend on embedding.  I will take a look
at this more tomorrow.  been up since 4am and I missed a meal or two.  damn
coworker bought 200 bucks worth of candy ;-)

Comment 3

[Robert Ginda]

Actually, the point of this bug is that it *doesn't* fire at all, but I'll let
it slip because you're tired ;)

I was afraid someone would complain about the new REQUIRES, so I am prepared to
offer a solution.  How's about we rename nsAppStartNotifier to
nsCategoryNotifier, and move it to xpcom/components.  No one has a new depend,
and the component gets the name it deserves.

Comment 4

[Robert Ginda]

Here comes a new patch, fire at will.

Comment 5

[Robert Ginda]

Attachment: new patch, move appstartnotifier to xpcom/components, and give it a better name

Comment 6

[Robert Ginda]

Alternatley, we could add a notifyCategory to the category manager, and avoid
the new component and interface.  That is if the interface isn't frozen (I don't
see any comment about it.)

Comment 7

[Doug Turner (:dougt)]

robert, the component manager is going to get an overhaul in the next week or
two.  I would like to roll this into the new interfaces.  I will keep you posted.  

Comment 8

[Robert Ginda]

Can I get a status check on this one?

The debugger code would make use of this patch has just been checked in, and
this is now the only bug blocking the ability to debug javascript components
that have changed since the last run.

Comment 9

[Robert Ginda]

Attachment: NS_CreateServicesFromCategory, who knew?

Comment 10

[Robert Ginda]

thanks to dougt for pointing this out.  The latest patch is *much* simpler, and
does the same thing.

I'll file a seperate bug to remove nsAppStartupNotifier.

Comment 11

[Doug Turner (:dougt)]

Comment on attachment 55773 [details] [diff] [review]
NS_CreateServicesFromCategory, who knew?

looks fine. We should eventually rename this utility function.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 55773 [details] [diff] [review]
NS_CreateServicesFromCategory, who knew?

Yeah, the NS_CreateServicesFromCategory name is slightly off, isn't it?

/be

Comment 13

[Robert Ginda]

I checked this in a few days ago, forgot to mark as such.