1450167 - Consider switching back to just using atoms for event types

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Right now we use atoms on mainthread and strings off-mainthread.  That requires us to have two variants of EventListenerAdded and EventListenerRemoved and adds a bunch of complexity in the ELM for the two cases.

It seems to me that we could just use atoms consistently, now that we have threadsafe atoms.  On background threads we wouldn't be able to use the nsContentUtils name-to-atom caches, but that should be ok, right?

Olli, thoughts?

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Sounds good. I thought I had filed a bug to make worker code to use atoms, but perhaps not.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Stop using atom-or-string for event names in the listener manager

Now that we support atoms off the the main thread, we can just use atoms.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8994382 [details] [diff] [review]
Stop using atom-or-string for event names in the listener manager

> EventListenerManager::AddEventListenerInternal(
>                         EventListenerHolder aListenerHolder,
>                         EventMessage aEventMessage,
>                         nsAtom* aTypeAtom,
>-                        const nsAString& aTypeString,
>                         const EventListenerFlags& aFlags,
>                         bool aHandler,
>                         bool aAllEvents)
> {
>-  MOZ_ASSERT(// Main thread
>-             (NS_IsMainThread() && aEventMessage && aTypeAtom) ||
>-             // non-main-thread
>-             (!NS_IsMainThread() && aEventMessage) ||
>-             aAllEvents, "Missing type"); // all-events listener
>+  MOZ_ASSERT(aEventMessage  ||
>+             aAllEvents, // all-events listener
(aEventMessage && aTypeAtom)
right?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/608abb3e16ab
Stop using atom-or-string for event names in the listener manager.  r=smaug

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/608abb3e16ab