13201 - use nsIAtoms or nsAutoStrings in nsXULKeyListener

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Chris Waterson]

I haven't profiled any of the editor/keytyping stuff, but code in
nsXULKeyListener seems like an obvious candidate for replacing nsString usage
with nsIAtoms (or at worst, nsAutoStrings for automatic variables).

Comment 1

[saari (gone)]

Actually, I'm replacing most of that string crap with booleans.



What is the advantage to an nsIAtom?

Comment 2

[Chris Waterson]

An "atom" is a string that has been hashed to a unique value. Everytime you ask
for the same string, you get back the same atom. So, this is a big win if you
do a lot of string comparison: you can atomize the stings you routinely compare
against, and do a single pointer compare with "==" instead of a strcmp().

Comment 3

[saari (gone)]

Oh, it is a flyweight.

Why must we think of new names for everything?

Comment 4

[Chris Waterson]

Atom tables have been around since people started writing compiles, methinks.
:-)

Comment 5

[saari (gone)]

Eh... OK. You win.

Comment 6

[Akkana Peck]

Regardless of how long the term "atom" has been in use (quite a while, not that
it matters), for things like "true" and "false" it makes sense just to convert
to booleans inside the key listener code (though it wouldn't hurt to use atoms
for those boolean values when initially parsing the code).  No particular reason
to store atoms representing a string when all it does is codify a boolean value
...

Comment 7

[saari (gone)]

Booleans are the plan

Comment 8

[saari (gone)]

Converted to nsAutoStrings, and fewer of them. May go back and make them atoms
later.

Comment 9

[ckritzer (gone)]

Assigning to originator per new organizational structure.

Comment 10

[ckritzer (gone)]

Changing QA Contact back to [e-mail]

Comment 11

[Paul MacQuiddy]

marking verified

Comment 12

[leger]

BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL.  XUL 
component will be deleted.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3961cc21ccac
[fenix] Renamed Deps.mozilla_ui_publicsuffixlist to Deps.mozilla_lib_publicsuffixlist (https://github.com/mozilla-mobile/fenix/pull/13201)