529701 - nsCategoryCache could add a category observer twice in some situation.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Marco Bonardo [:mak]]

I'm still trying to completely understand what happens here, but this is what i can see so far.

I have a category called "history-observers".
i have a component that is registered as an "history-observers" entry through XPCOMUtils.
This component also implements nsIObserver.
History to notify changes enumerates observers through a categoryCache.

I have a test that executes some history stuff, my category observer is correctly enumerated and notified once.

then i Cc[my_component].getService(Ci.nsIObserver).notify(null, some_custom_notification, null);

at this point i see that nsCategoryCache receives an entry added notification from the category manager, the entry is then added again to the hash, History enumerates entries and notifies my component TWICE.

Doing some debugging i've seen that when nsCategoryCache starts it adds all entries that have been loaded so far (they come from compreg.dat at this point), but then when i run the above test it adds the entry again.

I'm trying to argue if i'm doing something wrong or there's a problem here.

Comment 1

[Marco Bonardo [:mak]]

actually, looks like i see the double addition even without the getService thing, i just execute a method that will cause a notification to be fired, the notification instantiates the component, that is registered again.

this is my test
http://mozilla.pastebin.com/m50a35910

i call removeAllPages, that will notify onClearHistory to "history-observers", expiration components should be instantiated at this point since it's in the category cache, and it will fire "places-expiration-finished".

Running this test i see that the entry is added twice, any history change from now on is notified twice to my component.

Comment 2

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

Attachment: patch

Comment 3

[Benjamin Smedberg]

I'd like a test as well... I think it can be done with xpcshell

Comment 4

[Benjamin Smedberg]

Urgh... if the category cache is exposed to xpcshell at all.

Comment 5

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

Nope, it's a C++-only API.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 413325 [details] [diff] [review]
patch

a192=beltzner mostly because I <3 biesi so much.

Comment 7

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

marking checkin-needed in case anybody gets around to do it tonight, otherwise I'll do it tomorrow

Comment 8

[Dietrich Ayala (:dietrich)]

http://hg.mozilla.org/mozilla-central/rev/231a65a2d273
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1114b55a5b52