Bug 1834370 - Keep listeners for each event type in a separate array, and use binary search on the outer list. r=smaug

This considerably improves the testcase in bug 1834003, because it
reduces the amount of memory we need to look at when checking the
listeners at the nsWindowRoot. At the moment, nsWindowRoot has 156
listeners for 94 different event types, all from JSWindowActor event
listeners.

Having a separate array per event type also matches what Blink and Webkit do.

Differential Revision: https://phabricator.services.mozilla.com/D183431

dom/base/nsContentUtils.cpp

@@ -956,7 +956,8 @@ bool nsContentUtils::IsExternalProtocol(nsIURI* aURI) {
   return NS_SUCCEEDED(rv) && doesNotReturnData;
 }
 
-static nsAtom* GetEventTypeFromMessage(EventMessage aEventMessage) {
+/* static */
+nsAtom* nsContentUtils::GetEventTypeFromMessage(EventMessage aEventMessage) {
   switch (aEventMessage) {
 #define MESSAGE_TO_EVENT(name_, message_, type_, struct_) \
   case message_:                                          \

dom/base/nsContentUtils.h

@@ -1692,6 +1692,11 @@ class nsContentUtils {
    */
   static EventMessage GetEventMessage(nsAtom* aName);
 
+  /**
+   * Return the event type atom for a given event message.
+   */
+  static nsAtom* GetEventTypeFromMessage(EventMessage aEventMessage);
+
   /**
    * Returns the EventMessage and nsAtom to be used for event listener
    * registration.

dom/events/EventListenerManager.h

@@ -271,9 +271,6 @@ class EventListenerManager final : public EventListenerManagerBase {
       }
     }
 
-    MOZ_ALWAYS_INLINE bool MatchesEventMessage(
-        const WidgetEvent* aEvent, EventMessage aEventMessage) const;
-
     MOZ_ALWAYS_INLINE bool MatchesEventGroup(const WidgetEvent* aEvent) const {
       return mFlags.mInSystemGroup == aEvent->mFlags.mInSystemGroup;
     }
@@ -291,6 +288,66 @@ class EventListenerManager final : public EventListenerManagerBase {
     }
   };
 
+  /**
+   * A reference counted subclass of a listener observer array.
+   */
+  struct ListenerArray final : public nsAutoTObserverArray<Listener, 1> {
+    NS_INLINE_DECL_REFCOUNTING(EventListenerManager::ListenerArray);
+    size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const;
+
+   protected:
+    ~ListenerArray() = default;
+  };
+
+  /**
+   * An entry in the event listener map for a certain event type, carrying the
+   * array of listeners for that type.
+   */
+  struct EventListenerMapEntry {
+    size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;
+
+    // The event type. Null if this entry is for "all events" listeners.
+    RefPtr<nsAtom> mTypeAtom;
+    // The array of listeners. New listeners are always added at the end.
+    // This is a RefPtr rather than an inline member for two reasons:
+    //  - It needs to be a separate heap allocation so that, if the array of
+    //    entries is mutated during iteration, the ListenerArray remains in a
+    //    stable place.
+    //  - It's a RefPtr rather than a UniquePtr so that iteration can share
+    //    ownership of it and make sure that the listener array remains alive
+    //    even if the entry is removed during iteration.
+    RefPtr<ListenerArray> mListeners;
+  };
+
+  /**
+   * The map of event listeners, keyed by event type atom.
+   */
+  struct EventListenerMap {
+    bool IsEmpty() const { return mEntries.IsEmpty(); }
+    void Clear() { mEntries.Clear(); }
+
+    Maybe<size_t> EntryIndexForType(nsAtom* aTypeAtom) const;
+    Maybe<size_t> EntryIndexForAllEvents() const;
+
+    // Returns null if no entry is present for the given type.
+    RefPtr<ListenerArray> GetListenersForType(nsAtom* aTypeAtom) const;
+    RefPtr<ListenerArray> GetListenersForAllEvents() const;
+
+    // Never returns null, creates a new empty entry if needed.
+    RefPtr<ListenerArray> GetOrCreateListenersForType(nsAtom* aTypeAtom);
+    RefPtr<ListenerArray> GetOrCreateListenersForAllEvents();
+
+    size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;
+
+    // The array of entries, ordered by event type atom (specifically by the
+    // nsAtom* address). If mEntries contains an entry for "all events"
+    // listeners, that entry will be the first entry, because its atom will be
+    // null so it will be ordered to the front.
+    // All entries have non-empty listener arrays. If a non-empty listener
+    // entry becomes empty, it is removed immediately.
+    AutoTArray<EventListenerMapEntry, 2> mEntries;
+  };
+
   explicit EventListenerManager(dom::EventTarget* aTarget);
 
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(EventListenerManager)
@@ -405,7 +462,7 @@ class EventListenerManager final : public EventListenerManagerBase {
       return;
     }
 
-    if (mListeners.IsEmpty() || aEvent->PropagationStopped()) {
+    if (mListenerMap.IsEmpty() || aEvent->PropagationStopped()) {
       return;
     }
 
@@ -513,7 +570,7 @@ class EventListenerManager final : public EventListenerManagerBase {
 
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const;
 
-  uint32_t ListenerCount() const { return mListeners.Length(); }
+  uint32_t ListenerCount() const;
 
   void MarkForCC();
 
@@ -543,6 +600,17 @@ class EventListenerManager final : public EventListenerManagerBase {
                            dom::EventTarget* aCurrentTarget,
                            nsEventStatus* aEventStatus, bool aItemInShadowTree);
 
+  /**
+   * Iterate the listener array and calls the matching listeners.
+   *
+   * Returns true if any listener matching the event group was found.
+   */
+  MOZ_CAN_RUN_SCRIPT
+  bool HandleEventWithListenerArray(
+      ListenerArray* aListeners, nsAtom* aTypeAtom, EventMessage aEventMessage,
+      nsPresContext* aPresContext, WidgetEvent* aEvent, dom::Event** aDOMEvent,
+      dom::EventTarget* aCurrentTarget, bool aItemInShadowTree);
+
   /**
    * Call the listener.
    *
@@ -588,7 +656,7 @@ class EventListenerManager final : public EventListenerManagerBase {
   /**
    * Find the Listener for the "inline" event listener for aTypeAtom.
    */
-  Listener* FindEventHandler(EventMessage aEventMessage, nsAtom* aTypeAtom);
+  Listener* FindEventHandler(nsAtom* aTypeAtom);
 
   /**
    * Set the "inline" event listener for aName to aHandler.  aHandler may be
@@ -691,7 +759,7 @@ class EventListenerManager final : public EventListenerManagerBase {
 
   // BE AWARE, a lot of instances of EventListenerManager will be created.
   // Therefor, we need to keep this class compact.  When you add integer
-  // members, please add them to EventListemerManagerBase and check the size
+  // members, please add them to EventListenerManagerBase and check the size
   // at build time.
 
   already_AddRefed<nsIScriptGlobalObject> GetScriptGlobalAndDocument(
@@ -699,7 +767,7 @@ class EventListenerManager final : public EventListenerManagerBase {
 
   void MaybeMarkPassive(EventMessage aMessage, EventListenerFlags& aFlags);
 
-  nsAutoTObserverArray<Listener, 2> mListeners;
+  EventListenerMap mListenerMap;
   dom::EventTarget* MOZ_NON_OWNING_REF mTarget;
   RefPtr<nsAtom> mNoListenerForEventAtom;
 

dom/events/test/test_bug448602.html

@@ -78,7 +78,16 @@ function runTests() {
   root.addEventListener("click", bubblingListener);
   root.addEventListener("fooevent", capturingListener, true);
   root.addEventListener("fooevent", bubblingListener);
-  infos = els.getListenerInfoFor(root);
+
+  // We now have both "click" and "fooevent" listeners.
+  // Get the new set of listener infos, because we'll want to flip certain
+  // "click" event listeners on and off in the tests below.
+  // The order of event types is not guaranteed by getListenerInfoFor; but the
+  // order of listeners for a single event is guaranteed. So we filter the infos
+  // by event type.
+  const combinedListenerInfos = [...els.getListenerInfoFor(root)];
+  const clickInfos = combinedListenerInfos.filter((info) => info.type == "click");
+
   // Use a child node to dispatch events so that both capturing and bubbling
   // listeners get called.
   l2 = document.getElementById("testlevel2");
@@ -88,29 +97,29 @@ function runTests() {
   ok(bubblingListenerCalled);
   clearListenerStates();
 
-  infos[0].enabled = false;
+  clickInfos[0].enabled = false;
   l2.click();
   ok(!handlerCalled);
   ok(capturingListenerCalled);
   ok(bubblingListenerCalled);
   clearListenerStates();
-  infos[0].enabled = true;
+  clickInfos[0].enabled = true;
 
-  infos[1].enabled = false;
+  clickInfos[1].enabled = false;
   l2.click();
   ok(handlerCalled);
   ok(!capturingListenerCalled);
   ok(bubblingListenerCalled);
   clearListenerStates();
-  infos[1].enabled = true;
+  clickInfos[1].enabled = true;
 
-  infos[2].enabled = false;
+  clickInfos[2].enabled = false;
   l2.click();
   ok(handlerCalled);
   ok(capturingListenerCalled);
   ok(!bubblingListenerCalled);
   clearListenerStates();
-  infos[2].enabled = true;
+  clickInfos[2].enabled = true;
 
   root.removeEventListener("click", capturingListener, true);
   root.removeEventListener("click", bubblingListener);

testing/web-platform/tests/dom/events/remove-all-listeners.html

@@ -0,0 +1,95 @@
+<!doctype html>
+<title>Various edge cases where listeners are removed during iteration</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="log"></div>
+<script>
+
+test(function() {
+  var type = "foo";
+  var target = document.createElement("div");
+
+  var listener1CallCount = 0;
+  var listener2CallCount = 0;
+  var listener3CallCount = 0;
+  function listener1() {
+    listener1CallCount++;
+    target.removeEventListener(type, listener1);
+    target.removeEventListener(type, listener2);
+    target.addEventListener(type, listener3);
+  }
+  function listener2() {
+    listener2CallCount++;
+  }
+  function listener3() {
+    listener3CallCount++;
+  }
+
+  target.addEventListener(type, listener1);
+  target.addEventListener(type, listener2);
+
+  // Dispatch the event. Only listener1 should be called because
+  // it removes listener2. And listener3 is added when we've already
+  // started iterating, so it shouldn't be called either.
+  target.dispatchEvent(new Event(type));
+  assert_equals(listener1CallCount, 1);
+  assert_equals(listener2CallCount, 0);
+  assert_equals(listener3CallCount, 0);
+
+  // Now that only listener3 is set, dispatch another event. Only
+  // listener3 should be called.
+  target.dispatchEvent(new Event(type));
+  assert_equals(listener1CallCount, 1);
+  assert_equals(listener2CallCount, 0);
+  assert_equals(listener3CallCount, 1);
+}, "Removing all listeners and then adding a new one should work.");
+
+test(function() {
+  var type = "foo";
+  var target = document.createElement("div");
+
+  var listener1CallCount = 0;
+  var listener2CallCount = 0;
+  var listener3CallCount = 0;
+  function listener1() {
+    listener1CallCount++;
+    // Recursively dispatch another event from this listener.
+    // This will only call listener2 because listener1 is a "once" listener.
+    target.dispatchEvent(new Event(type));
+    assert_equals(listener1CallCount, 1);
+    assert_equals(listener2CallCount, 1);
+    assert_equals(listener3CallCount, 0);
+
+    // Now all listeners are removed - the two "once" listeners have already both
+    // been called once. Add another listener.
+    target.addEventListener(type, listener3);
+  }
+  function listener2() {
+    listener2CallCount++;
+  }
+  function listener3() {
+    listener3CallCount++;
+  }
+
+  // Add two "once" listeners.
+  target.addEventListener(type, listener1, { once: true });
+  target.addEventListener(type, listener2, { once: true });
+
+  // Dispatch the event.
+  target.dispatchEvent(new Event(type));
+
+  // The listener call counts should still match what they were
+  // at the end of listener1.
+  assert_equals(listener1CallCount, 1);
+  assert_equals(listener2CallCount, 1);
+  assert_equals(listener3CallCount, 0);
+
+  // Now that only listener3 is set, dispatch another event. Only
+  // listener3 should be called.
+  target.dispatchEvent(new Event(type));
+  assert_equals(listener1CallCount, 1);
+  assert_equals(listener2CallCount, 1);
+  assert_equals(listener3CallCount, 1);
+}, "Nested usage of once listeners should work.");
+
+</script>