try server build - http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/surkov.alexander@gmail.com-9a5301486a41/

I've updated perf tracking table - https://wiki.mozilla.org/Accessibility/EventCoalescing#Performance_improvements_tracking_table (tested on try server build I pointed above).

Comment on attachment 454345 [details] [diff] [review] patch r=me for the tests and the functionality part. Great job!

(In reply to comment #3) > I've updated perf tracking table - > https://wiki.mozilla.org/Accessibility/EventCoalescing#Performance_improvements_tracking_table > (tested on try server build I pointed above). Very nice.

Comment on attachment 454345 [details] [diff] [review] patch >+ >+ } else { Nit: strange place for a blank line >+ PRInt32 offset = mOffsets.SafeElementAt(index - 1, -1); >+ if (offset != -1) { >+ if (aInvalidateAfter) { >+ PRInt32 count = mOffsets.Length() - index; >+ if (count > 0) >+ mOffsets.RemoveElementsAt(index, count); >+ } >+ return offset; >+ } Rather than using SafeElementAt, I'd prefer checking count >= 0 like this: PRInt32 count = mOffsets.Length() - index; if (count >= 0) { if (count > 0 && aInvalidateAfter) mOffsets.RemoveElementsAt(index, count); return mOffsets[index - 1]; } This also makes things minusculely easier for the optimiser ;-) >+ PRUint32 childCount = GetChildCount(); >+ while (mOffsets.Length() < childCount && mOffsets.Length() < index) { index is always less than mChildren.Length() [or childCount], right? So you shouldn't have to compare against childCount.

(In reply to comment #6) > (From update of attachment 454345 [details] [diff] [review]) > >+ > >+ } else { > Nit: strange place for a blank line the empty line is intended to separate visually end of if block from else block, it seems to me it's a bit more readable, I would safe this if you don't mind. > Rather than using SafeElementAt, I'd prefer checking count >= 0 like this: > PRInt32 count = mOffsets.Length() - index; > if (count >= 0) { > if (count > 0 && aInvalidateAfter) > mOffsets.RemoveElementsAt(index, count); > return mOffsets[index - 1]; > } > This also makes things minusculely easier for the optimiser ;-) > > >+ PRUint32 childCount = GetChildCount(); > >+ while (mOffsets.Length() < childCount && mOffsets.Length() < index) { > index is always less than mChildren.Length() [or childCount], right? So you > shouldn't have to compare against childCount. I like this, thank you.

Neil's comments are fixed

Comment on attachment 455085 [details] [diff] [review] patch2 r=me. >+nsHyperTextAccessible::GetChildOffset(nsAccessible* aChild, >+ PRBool >+ EnsureChildren(); Good. We should probably add more tests (later).

landed on 1.9.3 (2.0) - http://hg.mozilla.org/mozilla-central/rev/7456a4bfba46