Currently, broadcaster/observers are implemented by maintaining the "listener" list in the broadcasting element. This causes two problems: 1. HTML elements cannot be broadcasters. 2. When a listener is removed from the content model, the broadcaster cannot reliably be notified that the element is "leaving" (i.e., relying on the values of attributes in the content model is fragile). See bug 18127. By promoting this functionality to the XUL document, we solve both problems.

spam, open xptoolkit qa contact moving over to jrgm

future projects.

Okay, attachment 54371 [details] [diff] [review] is ready for review: 1. I've moved the broadcaster APIs from nsIDOMXULElement to nsIDOMXULDocument (although nobody uses them). 2. I moved all of the broadcaster synchronization and maintenance from nsXULElement to nsXULDocument. Update happens via the nsIDocument::AttributeChanged notification. 3. I fixed the broadcaster mechanism so that you could actually observe a list of attributes. (The previous implementation was broken in this regard.) 4. Noted where ownership model may have a scary dangling pointer (cf. bug 18127). Didn't fix it, because I don't want to fight leaks and the case is obscure. I'll try to collect some footprint numbers later today.

shaver, you could review this one, too, with your new XUL element super-powers.

Mmm, 10KB requested on a single browser window.

Comment on attachment 54371 [details] [diff] [review] working patch >+ aNodeInfo->GetNameAtom(*getter_AddRefs(tagName)); >+ if ((tagName.get() == nsXULAtoms::broadcaster) || >+ (tagName.get() == nsXULAtoms::command) || >+ (tagName.get() == nsXULAtoms::key)) { >+ return NS_STYLE_HINT_NONE; Eschew .get()! >+ if (mBroadcasterMap) >+ PL_DHashTableDestroy(mBroadcasterMap); >+ How common are broadcasters? Better to save the extra allocation and inline the PLDHashTable? (Probably not, but I felt I should ask.) >+ entry->mListeners.~nsCheapVoidArray(); You don't see manual destructor invocation often enough, I say. >+ // We are observing the broadcaster, but is this the right >+ // attribute? >+ nsAutoString listeningToAttribute; >+ listener->GetAttr(kNameSpaceID_None, nsXULAtoms::attribute, listeningToAttribute); >+ >+ if (! listeningToAttribute.Equals(attrName)) >+ continue; Can we not observe all attributes, as well, with the "*"? >- NS_WARN_IF_FALSE(mNumCapturers >= 0, "Number of capturers has become negative"); >+ NS_ASSERTION(mNumCapturers >= 0, "Number of capturers has become negative"); (Thank you.) Fix those nits, and assuage my observing-* fears, and I'll stamp my r.

You are the wind beneath my wings, shaver! Attaching a get-eschewing, fancy dtor calling commented, star pushing patch. As best I can tell, there are only a few XUL files using broadcasters (navigator, editor, maybe some mail). I figure dialogs n' stuff didn't need 'em.

Comment on attachment 54443 [details] [diff] [review] all that and then some money. r=shaver

Eek, so I need to update the XUL spec, since the APIs moved to the document.

sr=hyatt

Changes checked in.