Closed Bug 1450167 Opened 2 years ago Closed Last year

Consider switching back to just using atoms for event types

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

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?
Flags: needinfo?(bugs)
Sounds good. I thought I had filed a bug to make worker code to use atoms, but perhaps not.
Flags: needinfo?(bugs)
Depends on: 1452741
Blocks: 1452743
No longer depends on: 1452741
Priority: -- → P3
Now that we support atoms off the the main thread, we can just use atoms.
Attachment #8994382 - Flags: review?(bugs)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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?
Attachment #8994382 - Flags: review?(bugs) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/608abb3e16ab
Stop using atom-or-string for event names in the listener manager.  r=smaug
https://hg.mozilla.org/mozilla-central/rev/608abb3e16ab
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.