Clean up GetRuleCascade (and the enumeration callbacks it uses) by changing all the things that use enumerators to use array access. This should make it both easier to understand and faster.

To do this, I'd want to add an |UnsafeElementAt| to nsVoidArray that assumes the element is within the bounds -- i.e., that neither has bounds-checking *nor* null-checking of mImpl.

This includes some stuff that I noticed while looking into bug 137247.

I'll future-proof these macros with PR_{BEGIN,END}_MACRO too...

OK, a summary of what this patch does so far (the bigger changes are at the end): * removes |EVENT_DEBUG| define that doesn't correspond to any code * removes heavily bitrotted DEBUG_RULES debugging code (makes no sense with the rule tree, and I see no need to rewrite it since nobody seems to need it) * macro-izes my RULE_HASH_STATS stuff so it takes up less room * Comments HasStateDependentStyle better, and makes a performance optimization to |IsStateSelector| since it was testing for many selectors for which HasStatetDependentStyle will never be called. This is the part that is a followup to bug 5693 (see comment 3). * Redoes GetRuleCascade a bit by: + sorting the final array in the reverse order to allow elimination of AppendRuleToArray in favor of nsISupportsArray::AppendElements. (This change is a little confusing: we build the final array by combining a bunch of order-sorted arrays for each weight. So the change is to sort the weight arrays in the reverse order and then combine the arrays by a simple append instead of appending through EnumerateBackwards. Then, of course, it also changes the enumeration order of this final array.) + merges BuildStateEnum and BuildHashEnum

- (pseudoClass->mAtom == nsCSSAtoms::dragPseudo) || - (pseudoClass->mAtom == nsCSSAtoms::selectionPseudo) || I assume that drag and selection are both supposed to fire notifications that cause these pseudos to be applied as needed, which is why we don't need them here? > /** > * Takes the hashtable of arrays (keyed by weight, in order sort) and > * puts them all in one big array which has a primary sort by weight > * and secondary sort by reverse order. > */ This comment could use some changing, methinks, to indicate that the primary sort is now by reverse weight and the secondary sort is by order. That seems fishy, by the way. I would expect the new way to have a sort by reverse weight and secondary sort by reverse order, so that the first thing in the list is the thing of highest weight that came latest. I'm guessing the comment is wrong to start with about the sort orders of the arrays? The last looks good.

Er, the _rest_ looks good. :)

Actually, drag and selection pseudos aren't implemented, and should probably be completely removed (perhaps as part of bug 3935). :selection isn't even a pseudo-class -- it's a pseudo-element! A corrected comment is: /** * Takes the hashtable of arrays (keyed by weight, in order sort) and * puts them all in one big array which has a primary sort by weight * and secondary sort by order. */ (change "reverse order" to "order") Now that I notice it, I could probably also change AppendRuleToTable to PrependRuleToTable at some point in the future, and fix the worst-case O(N^2) (cumulatively) insertion into the rule hash. Maybe I'll do that now...

Actually, no, that would be a bit of a pain unless I convert to pldhash, which would make this patch even more confusing than it is already. But I'll toss in an XXX comment.

Comment on attachment 79188 [details] [diff] [review] more/better comments, in response to bzbarsky r=bzbarsky

Comment on attachment 79188 [details] [diff] [review] more/better comments, in response to bzbarsky sr=waterson, although I'd have left the empty macros...well, empty.

Attachment 79188 [details] [diff] checked in to trunk, 2002-04-15 15:49 PDT. Leaving bug open for further work.

This crashes on startup in unrelated code, and I haven't figured out why.

The above patch should really go on bug 112318.

cc'ing myself

Comment on attachment 81286 [details] [diff] [review] failed attempt to convert to pldhash There is now a working patch on bug 112318 that replaces this one.