Align the event listener firing code with the latest DOM Specification and simplify it
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Aug 2016 00:48:16 +0000 (00:48 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Aug 2016 00:48:16 +0000 (00:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160828

Reviewed by Darin Adler.

Align the event listener firing code with the latest DOM specification:
- https://dom.spec.whatwg.org/#concept-event-listener-invoke
- https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
- https://dom.spec.whatwg.org/#concept-event-listener

The following changes were made:
1. RegisteredEventListener (equivalent to "event listener" in the spec)
   is now RefCounted
2. RegisteredEventListener now has a wasRemoved flag as in specification.
3. fireEventListeners() is now iterating over a copy of the event
   listeners, as in the specification. We rely on the wasRemoved removed
   flag to avoid executing event listeners that were removed while we
   iterate.

Previously, the code was modifying the event listeners vector we were
iterating on. Doing so safely lead to complicated and error prone code.
The new code is much simpler and easier to reason about.

This change seems to be perf-neutral on Speedometer.

No new tests, no web-exposed behavior change.

* CMakeLists.txt:
* WebCore.xcodeproj/project.pbxproj:
* bindings/js/JSCommandLineAPIHostCustom.cpp:
(WebCore::getJSListenerFunctions):
* dom/DOMAllInOne.cpp:
* dom/EventListenerMap.cpp:
(WebCore::EventListenerMap::assertNoActiveIterators):
(WebCore::EventListenerMap::containsCapturing):
(WebCore::EventListenerMap::containsActive):
(WebCore::findListener):
(WebCore::EventListenerMap::add):
(WebCore::removeListenerFromVector):
(WebCore::EventListenerMap::remove):
(WebCore::EventListenerMap::find):
(WebCore::removeFirstListenerCreatedFromMarkup):
(WebCore::copyListenersNotCreatedFromMarkupToTarget):
(WebCore::EventListenerMap::copyEventListenersNotCreatedFromMarkupToTarget):
(WebCore::EventListenerIterator::nextListener):
(WebCore::EventListenerMap::clear): Deleted.
* dom/EventListenerMap.h:
(WebCore::EventListenerMap::contains):
(WebCore::EventListenerMap::assertNoActiveIterators):
* dom/EventTarget.cpp:
(WebCore::EventTarget::removeEventListener):
(WebCore::EventTarget::getAttributeEventListener):
(WebCore::FiringEventListenersScope::FiringEventListenersScope):
(WebCore::FiringEventListenersScope::~FiringEventListenersScope):
(WebCore::EventTarget::fireEventListeners):
(WebCore::EventTarget::setAttributeEventListener): Deleted.
(WebCore::EventTarget::hasActiveEventListeners): Deleted.
(WebCore::EventTarget::dispatchEventForBindings): Deleted.
(WebCore::EventTarget::getEventListeners): Deleted.
* dom/EventTarget.h:
(WebCore::EventTarget::isFiringEventListeners):
* dom/RegisteredEventListener.cpp: Removed.
* dom/RegisteredEventListener.h:
(WebCore::RegisteredEventListener::Options::Options):
(WebCore::RegisteredEventListener::create):
(WebCore::RegisteredEventListener::listener):
(WebCore::RegisteredEventListener::useCapture):
(WebCore::RegisteredEventListener::isPassive):
(WebCore::RegisteredEventListener::isOnce):
(WebCore::RegisteredEventListener::wasRemoved):
(WebCore::RegisteredEventListener::markAsRemoved):
(WebCore::RegisteredEventListener::RegisteredEventListener):
* inspector/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::getEventListenersForNode):
(WebCore::InspectorDOMAgent::getEventListeners):
(WebCore::InspectorDOMAgent::buildObjectForEventListener):
* inspector/InspectorDOMAgent.h:
(WebCore::EventListenerInfo::EventListenerInfo):
* svg/SVGElement.cpp:
(WebCore::hasLoadListener):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204459 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/JSCommandLineAPIHostCustom.cpp
Source/WebCore/dom/DOMAllInOne.cpp
Source/WebCore/dom/EventListenerMap.cpp
Source/WebCore/dom/EventListenerMap.h
Source/WebCore/dom/EventTarget.cpp
Source/WebCore/dom/EventTarget.h
Source/WebCore/dom/RegisteredEventListener.cpp [deleted file]
Source/WebCore/dom/RegisteredEventListener.h
Source/WebCore/inspector/InspectorDOMAgent.cpp
Source/WebCore/inspector/InspectorDOMAgent.h
Source/WebCore/svg/SVGElement.cpp

index 21bf891..6661acd 100644 (file)
@@ -1503,7 +1503,6 @@ set(WebCore_SOURCES
     dom/QualifiedName.cpp
     dom/RadioButtonGroups.cpp
     dom/Range.cpp
-    dom/RegisteredEventListener.cpp
     dom/ScopedEventQueue.cpp
     dom/ScriptElement.cpp
     dom/ScriptExecutionContext.cpp
index e24ef87..e12bab6 100644 (file)
@@ -1,3 +1,86 @@
+2016-08-14  Chris Dumez  <cdumez@apple.com>
+
+        Align the event listener firing code with the latest DOM Specification and simplify it
+        https://bugs.webkit.org/show_bug.cgi?id=160828
+
+        Reviewed by Darin Adler.
+
+        Align the event listener firing code with the latest DOM specification:
+        - https://dom.spec.whatwg.org/#concept-event-listener-invoke
+        - https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke
+        - https://dom.spec.whatwg.org/#concept-event-listener
+
+        The following changes were made:
+        1. RegisteredEventListener (equivalent to "event listener" in the spec)
+           is now RefCounted
+        2. RegisteredEventListener now has a wasRemoved flag as in specification.
+        3. fireEventListeners() is now iterating over a copy of the event
+           listeners, as in the specification. We rely on the wasRemoved removed
+           flag to avoid executing event listeners that were removed while we
+           iterate.
+
+        Previously, the code was modifying the event listeners vector we were
+        iterating on. Doing so safely lead to complicated and error prone code.
+        The new code is much simpler and easier to reason about.
+
+        This change seems to be perf-neutral on Speedometer.
+
+        No new tests, no web-exposed behavior change.
+
+        * CMakeLists.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/js/JSCommandLineAPIHostCustom.cpp:
+        (WebCore::getJSListenerFunctions):
+        * dom/DOMAllInOne.cpp:
+        * dom/EventListenerMap.cpp:
+        (WebCore::EventListenerMap::assertNoActiveIterators):
+        (WebCore::EventListenerMap::containsCapturing):
+        (WebCore::EventListenerMap::containsActive):
+        (WebCore::findListener):
+        (WebCore::EventListenerMap::add):
+        (WebCore::removeListenerFromVector):
+        (WebCore::EventListenerMap::remove):
+        (WebCore::EventListenerMap::find):
+        (WebCore::removeFirstListenerCreatedFromMarkup):
+        (WebCore::copyListenersNotCreatedFromMarkupToTarget):
+        (WebCore::EventListenerMap::copyEventListenersNotCreatedFromMarkupToTarget):
+        (WebCore::EventListenerIterator::nextListener):
+        (WebCore::EventListenerMap::clear): Deleted.
+        * dom/EventListenerMap.h:
+        (WebCore::EventListenerMap::contains):
+        (WebCore::EventListenerMap::assertNoActiveIterators):
+        * dom/EventTarget.cpp:
+        (WebCore::EventTarget::removeEventListener):
+        (WebCore::EventTarget::getAttributeEventListener):
+        (WebCore::FiringEventListenersScope::FiringEventListenersScope):
+        (WebCore::FiringEventListenersScope::~FiringEventListenersScope):
+        (WebCore::EventTarget::fireEventListeners):
+        (WebCore::EventTarget::setAttributeEventListener): Deleted.
+        (WebCore::EventTarget::hasActiveEventListeners): Deleted.
+        (WebCore::EventTarget::dispatchEventForBindings): Deleted.
+        (WebCore::EventTarget::getEventListeners): Deleted.
+        * dom/EventTarget.h:
+        (WebCore::EventTarget::isFiringEventListeners):
+        * dom/RegisteredEventListener.cpp: Removed.
+        * dom/RegisteredEventListener.h:
+        (WebCore::RegisteredEventListener::Options::Options):
+        (WebCore::RegisteredEventListener::create):
+        (WebCore::RegisteredEventListener::listener):
+        (WebCore::RegisteredEventListener::useCapture):
+        (WebCore::RegisteredEventListener::isPassive):
+        (WebCore::RegisteredEventListener::isOnce):
+        (WebCore::RegisteredEventListener::wasRemoved):
+        (WebCore::RegisteredEventListener::markAsRemoved):
+        (WebCore::RegisteredEventListener::RegisteredEventListener):
+        * inspector/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::getEventListenersForNode):
+        (WebCore::InspectorDOMAgent::getEventListeners):
+        (WebCore::InspectorDOMAgent::buildObjectForEventListener):
+        * inspector/InspectorDOMAgent.h:
+        (WebCore::EventListenerInfo::EventListenerInfo):
+        * svg/SVGElement.cpp:
+        (WebCore::hasLoadListener):
+
 2016-08-14  Daniel Bates  <dabates@apple.com>
 
         Fix compiler errors when building iOS WebKit using the iOS 10 beta SDK
index fe45daf..5cb6382 100644 (file)
                85031B480A44EFC700F992E0 /* MouseRelatedEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 85031B320A44EFC700F992E0 /* MouseRelatedEvent.h */; settings = {ATTRIBUTES = (Private, ); }; };
                85031B490A44EFC700F992E0 /* MutationEvent.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 85031B330A44EFC700F992E0 /* MutationEvent.cpp */; };
                85031B4A0A44EFC700F992E0 /* MutationEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 85031B340A44EFC700F992E0 /* MutationEvent.h */; };
-               85031B4B0A44EFC700F992E0 /* RegisteredEventListener.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 85031B350A44EFC700F992E0 /* RegisteredEventListener.cpp */; };
                85031B4C0A44EFC700F992E0 /* RegisteredEventListener.h in Headers */ = {isa = PBXBuildFile; fileRef = 85031B360A44EFC700F992E0 /* RegisteredEventListener.h */; settings = {ATTRIBUTES = (Private, ); }; };
                85031B4D0A44EFC700F992E0 /* UIEvent.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 85031B370A44EFC700F992E0 /* UIEvent.cpp */; };
                85031B4E0A44EFC700F992E0 /* UIEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 85031B380A44EFC700F992E0 /* UIEvent.h */; settings = {ATTRIBUTES = (Private, ); }; };
                85031B320A44EFC700F992E0 /* MouseRelatedEvent.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = MouseRelatedEvent.h; sourceTree = "<group>"; };
                85031B330A44EFC700F992E0 /* MutationEvent.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = MutationEvent.cpp; sourceTree = "<group>"; };
                85031B340A44EFC700F992E0 /* MutationEvent.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = MutationEvent.h; sourceTree = "<group>"; };
-               85031B350A44EFC700F992E0 /* RegisteredEventListener.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = RegisteredEventListener.cpp; sourceTree = "<group>"; };
                85031B360A44EFC700F992E0 /* RegisteredEventListener.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = RegisteredEventListener.h; sourceTree = "<group>"; };
                85031B370A44EFC700F992E0 /* UIEvent.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = UIEvent.cpp; sourceTree = "<group>"; };
                85031B380A44EFC700F992E0 /* UIEvent.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = UIEvent.h; sourceTree = "<group>"; };
                                936DD03A09CEAC270056AE8C /* Range.idl */,
                                93D9D53B0DA27E180077216C /* RangeBoundaryPoint.h */,
                                A84D827B11D333ED00972990 /* RawDataDocumentParser.h */,
-                               85031B350A44EFC700F992E0 /* RegisteredEventListener.cpp */,
                                85031B360A44EFC700F992E0 /* RegisteredEventListener.h */,
                                A76E5F7E135E0DCF00A69837 /* RenderedDocumentMarker.h */,
                                4998AEC413F9D0EA0090B1AA /* RequestAnimationFrameCallback.h */,
                                2EC41DE41C0410A300D294FE /* RealtimeMediaSourceSupportedConstraints.cpp in Sources */,
                                FD45A95A175D417100C21EC8 /* RectangleShape.cpp in Sources */,
                                BCAB418113E356E800D8AAF3 /* Region.cpp in Sources */,
-                               85031B4B0A44EFC700F992E0 /* RegisteredEventListener.cpp in Sources */,
                                CDFC360518CA61C20026E56F /* RemoteCommandListener.cpp in Sources */,
                                CDFC360718CA696C0026E56F /* RemoteCommandListenerIOS.mm in Sources */,
                                CD8ACA881D237AA200ECC59E /* RemoteCommandListenerMac.mm in Sources */,
index a0ca83e..5751962 100644 (file)
@@ -73,7 +73,7 @@ static JSArray* getJSListenerFunctions(ExecState& state, Document* document, con
         return nullptr;
     size_t handlersCount = listenerInfo.eventListenerVector.size();
     for (size_t i = 0, outputIndex = 0; i < handlersCount; ++i) {
-        const JSEventListener* jsListener = JSEventListener::cast(listenerInfo.eventListenerVector[i].listener.get());
+        const JSEventListener* jsListener = JSEventListener::cast(&listenerInfo.eventListenerVector[i]->listener());
         if (!jsListener) {
             ASSERT_NOT_REACHED();
             continue;
@@ -89,7 +89,7 @@ static JSArray* getJSListenerFunctions(ExecState& state, Document* document, con
 
         JSObject* listenerEntry = constructEmptyObject(&state);
         listenerEntry->putDirect(vm, Identifier::fromString(&state, "listener"), function);
-        listenerEntry->putDirect(vm, Identifier::fromString(&state, "useCapture"), jsBoolean(listenerInfo.eventListenerVector[i].useCapture));
+        listenerEntry->putDirect(vm, Identifier::fromString(&state, "useCapture"), jsBoolean(listenerInfo.eventListenerVector[i]->useCapture()));
         result->putDirectIndex(&state, outputIndex++, JSValue(listenerEntry));
     }
     return result;
index d3d5ea3..c0bf4fc 100644 (file)
 // #include "QualifiedName.cpp"
 #include "RadioButtonGroups.cpp"
 #include "Range.cpp"
-#include "RegisteredEventListener.cpp"
 #include "ScopedEventQueue.cpp"
 #include "ScriptElement.cpp"
 #include "ScriptExecutionContext.cpp"
index 7e79aeb..9b46694 100644 (file)
@@ -45,7 +45,7 @@ using namespace WTF;
 namespace WebCore {
 
 #ifndef NDEBUG
-void EventListenerMap::assertNoActiveIterators()
+void EventListenerMap::assertNoActiveIterators() const
 {
     ASSERT(!m_activeIteratorCount);
 }
@@ -55,37 +55,28 @@ EventListenerMap::EventListenerMap()
 {
 }
 
-bool EventListenerMap::contains(const AtomicString& eventType) const
-{
-    for (auto& entry : m_entries) {
-        if (entry.first == eventType)
-            return true;
-    }
-    return false;
-}
-
 bool EventListenerMap::containsCapturing(const AtomicString& eventType) const
 {
-    for (auto& entry : m_entries) {
-        if (entry.first == eventType) {
-            for (auto& eventListener : *entry.second) {
-                if (eventListener.useCapture)
-                    return true;
-            }
-        }
+    auto* listeners = find(eventType);
+    if (!listeners)
+        return false;
+
+    for (auto& eventListener : *listeners) {
+        if (eventListener->useCapture())
+            return true;
     }
     return false;
 }
 
 bool EventListenerMap::containsActive(const AtomicString& eventType) const
 {
-    for (auto& entry : m_entries) {
-        if (entry.first == eventType) {
-            for (auto& eventListener : *entry.second) {
-                if (!eventListener.isPassive)
-                    return true;
-            }
-        }
+    auto* listeners = find(eventType);
+    if (!listeners)
+        return false;
+
+    for (auto& eventListener : *listeners) {
+        if (!eventListener->isPassive())
+            return true;
     }
     return false;
 }
@@ -108,47 +99,51 @@ Vector<AtomicString> EventListenerMap::eventTypes() const
     return types;
 }
 
-static bool addListenerToVector(EventListenerVector* vector, Ref<EventListener>&& listener, const RegisteredEventListener::Options& options)
+static inline size_t findListener(const EventListenerVector& listeners, EventListener& listener, bool useCapture)
 {
-    RegisteredEventListener registeredListener(WTFMove(listener), options);
-
-    if (vector->find(registeredListener) != notFound)
-        return false; // Duplicate listener.
-
-    vector->append(registeredListener);
-    return true;
+    for (size_t i = 0; i < listeners.size(); ++i) {
+        auto& registeredListener = listeners[i];
+        if (registeredListener->listener() == listener && registeredListener->useCapture() == useCapture)
+            return i;
+    }
+    return notFound;
 }
 
 bool EventListenerMap::add(const AtomicString& eventType, Ref<EventListener>&& listener, const RegisteredEventListener::Options& options)
 {
     assertNoActiveIterators();
 
-    for (auto& entry : m_entries) {
-        if (entry.first == eventType)
-            return addListenerToVector(entry.second.get(), WTFMove(listener), options);
+    if (auto* listeners = find(eventType)) {
+        if (findListener(*listeners, listener, options.capture) != notFound)
+            return false; // Duplicate listener.
+        listeners->append(RegisteredEventListener::create(WTFMove(listener), options));
+        return true;
     }
 
-    m_entries.append(std::make_pair(eventType, std::make_unique<EventListenerVector>()));
-    return addListenerToVector(m_entries.last().second.get(), WTFMove(listener), options);
+    auto listeners = std::make_unique<EventListenerVector>();
+    listeners->uncheckedAppend(RegisteredEventListener::create(WTFMove(listener), options));
+    m_entries.append({ eventType, WTFMove(listeners) });
+    return true;
 }
 
-static bool removeListenerFromVector(EventListenerVector* listenerVector, EventListener& listener, bool useCapture, size_t& indexOfRemovedListener)
+static bool removeListenerFromVector(EventListenerVector& listeners, EventListener& listener, bool useCapture)
 {
-    RegisteredEventListener registeredListener(listener, useCapture);
-    indexOfRemovedListener = listenerVector->find(registeredListener);
-    if (indexOfRemovedListener == notFound)
+    size_t indexOfRemovedListener = findListener(listeners, listener, useCapture);
+    if (UNLIKELY(indexOfRemovedListener == notFound))
         return false;
-    listenerVector->remove(indexOfRemovedListener);
+
+    listeners[indexOfRemovedListener]->markAsRemoved();
+    listeners.remove(indexOfRemovedListener);
     return true;
 }
 
-bool EventListenerMap::remove(const AtomicString& eventType, EventListener& listener, bool useCapture, size_t& indexOfRemovedListener)
+bool EventListenerMap::remove(const AtomicString& eventType, EventListener& listener, bool useCapture)
 {
     assertNoActiveIterators();
 
     for (unsigned i = 0; i < m_entries.size(); ++i) {
         if (m_entries[i].first == eventType) {
-            bool wasRemoved = removeListenerFromVector(m_entries[i].second.get(), listener, useCapture, indexOfRemovedListener);
+            bool wasRemoved = removeListenerFromVector(*m_entries[i].second, listener, useCapture);
             if (m_entries[i].second->isEmpty())
                 m_entries.remove(i);
             return wasRemoved;
@@ -158,7 +153,7 @@ bool EventListenerMap::remove(const AtomicString& eventType, EventListener& list
     return false;
 }
 
-EventListenerVector* EventListenerMap::find(const AtomicString& eventType)
+EventListenerVector* EventListenerMap::find(const AtomicString& eventType) const
 {
     assertNoActiveIterators();
 
@@ -172,8 +167,12 @@ EventListenerVector* EventListenerMap::find(const AtomicString& eventType)
 
 static void removeFirstListenerCreatedFromMarkup(EventListenerVector& listenerVector)
 {
-    bool foundListener = listenerVector.removeFirstMatching([] (const RegisteredEventListener& listener) {
-        return listener.listener->wasCreatedFromMarkup();
+    bool foundListener = listenerVector.removeFirstMatching([] (const auto& registeredListener) {
+        if (registeredListener->listener().wasCreatedFromMarkup()) {
+            registeredListener->markAsRemoved();
+            return true;
+        }
+        return false;
     });
     ASSERT_UNUSED(foundListener, foundListener);
 }
@@ -192,13 +191,13 @@ void EventListenerMap::removeFirstEventListenerCreatedFromMarkup(const AtomicStr
     }
 }
 
-static void copyListenersNotCreatedFromMarkupToTarget(const AtomicString& eventType, EventListenerVector* listenerVector, EventTarget* target)
+static void copyListenersNotCreatedFromMarkupToTarget(const AtomicString& eventType, EventListenerVector& listenerVector, EventTarget* target)
 {
-    for (auto& listener : *listenerVector) {
+    for (auto& registeredListener : listenerVector) {
         // Event listeners created from markup have already been transfered to the shadow tree during cloning.
-        if (listener.listener->wasCreatedFromMarkup())
+        if (registeredListener->listener().wasCreatedFromMarkup())
             continue;
-        target->addEventListener(eventType, *listener.listener, listener.useCapture);
+        target->addEventListener(eventType, registeredListener->listener(), registeredListener->useCapture());
     }
 }
 
@@ -207,7 +206,7 @@ void EventListenerMap::copyEventListenersNotCreatedFromMarkupToTarget(EventTarge
     assertNoActiveIterators();
 
     for (auto& entry : m_entries)
-        copyListenersNotCreatedFromMarkupToTarget(entry.first, entry.second.get(), target);
+        copyListenersNotCreatedFromMarkupToTarget(entry.first, *entry.second, target);
 }
 
 EventListenerIterator::EventListenerIterator(EventTarget* target)
@@ -236,16 +235,16 @@ EventListenerIterator::~EventListenerIterator()
 EventListener* EventListenerIterator::nextListener()
 {
     if (!m_map)
-        return 0;
+        return nullptr;
 
     for (; m_entryIndex < m_map->m_entries.size(); ++m_entryIndex) {
         EventListenerVector& listeners = *m_map->m_entries[m_entryIndex].second;
         if (m_index < listeners.size())
-            return listeners[m_index++].listener.get();
+            return &listeners[m_index++]->listener();
         m_index = 0;
     }
 
-    return 0;
+    return nullptr;
 }
 
 } // namespace WebCore
index 9f532ad..2499f72 100644 (file)
@@ -30,8 +30,7 @@
  *
  */
 
-#ifndef EventListenerMap_h
-#define EventListenerMap_h
+#pragma once
 
 #include "RegisteredEventListener.h"
 #include <atomic>
@@ -43,22 +42,22 @@ namespace WebCore {
 
 class EventTarget;
 
-typedef Vector<RegisteredEventListener, 1> EventListenerVector;
+using EventListenerVector = Vector<RefPtr<RegisteredEventListener>, 1>;
 
 class EventListenerMap {
 public:
     EventListenerMap();
 
     bool isEmpty() const { return m_entries.isEmpty(); }
-    WEBCORE_EXPORT bool contains(const AtomicString& eventType) const;
+    bool contains(const AtomicString& eventType) const { return find(eventType); }
     bool containsCapturing(const AtomicString& eventType) const;
     bool containsActive(const AtomicString& eventType) const;
 
     void clear();
 
     bool add(const AtomicString& eventType, Ref<EventListener>&&, const RegisteredEventListener::Options&);
-    bool remove(const AtomicString& eventType, EventListener&, bool useCapture, size_t& indexOfRemovedListener);
-    EventListenerVector* find(const AtomicString& eventType);
+    bool remove(const AtomicString& eventType, EventListener&, bool useCapture);
+    WEBCORE_EXPORT EventListenerVector* find(const AtomicString& eventType) const;
     Vector<AtomicString> eventTypes() const;
 
     void removeFirstEventListenerCreatedFromMarkup(const AtomicString& eventType);
@@ -67,7 +66,7 @@ public:
 private:
     friend class EventListenerIterator;
 
-    void assertNoActiveIterators();
+    void assertNoActiveIterators() const;
 
     Vector<std::pair<AtomicString, std::unique_ptr<EventListenerVector>>, 2> m_entries;
 
@@ -79,7 +78,6 @@ private:
 class EventListenerIterator {
     WTF_MAKE_NONCOPYABLE(EventListenerIterator);
 public:
-    EventListenerIterator();
     explicit EventListenerIterator(EventTarget*);
 #ifndef NDEBUG
     ~EventListenerIterator();
@@ -94,9 +92,7 @@ private:
 };
 
 #ifdef NDEBUG
-inline void EventListenerMap::assertNoActiveIterators() { }
+inline void EventListenerMap::assertNoActiveIterators() const { }
 #endif
 
 } // namespace WebCore
-
-#endif // EventListenerMap_h
index 0bd5b80..b7a32d4 100644 (file)
@@ -43,6 +43,7 @@
 #include <wtf/NeverDestroyed.h>
 #include <wtf/Ref.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/TemporaryChange.h>
 #include <wtf/Vector.h>
 
 using namespace WTF;
@@ -101,28 +102,7 @@ bool EventTarget::removeEventListener(const AtomicString& eventType, EventListen
     if (!d)
         return false;
 
-    size_t indexOfRemovedListener;
-
-    if (!d->eventListenerMap.remove(eventType, listener, options.capture, indexOfRemovedListener))
-        return false;
-
-    // Notify firing events planning to invoke the listener at 'index' that
-    // they have one less listener to invoke.
-    if (!d->firingEventIterators)
-        return true;
-    for (auto& firingIterator : *d->firingEventIterators) {
-        if (eventType != firingIterator.eventType)
-            continue;
-
-        if (indexOfRemovedListener >= firingIterator.size)
-            continue;
-
-        --firingIterator.size;
-        if (indexOfRemovedListener <= firingIterator.iterator)
-            --firingIterator.iterator;
-    }
-
-    return true;
+    return d->eventListenerMap.remove(eventType, listener, options.capture);
 }
 
 bool EventTarget::setAttributeEventListener(const AtomicString& eventType, RefPtr<EventListener>&& listener)
@@ -136,10 +116,10 @@ bool EventTarget::setAttributeEventListener(const AtomicString& eventType, RefPt
 EventListener* EventTarget::getAttributeEventListener(const AtomicString& eventType)
 {
     for (auto& eventListener : getEventListeners(eventType)) {
-        if (eventListener.listener->isAttribute())
-            return eventListener.listener.get();
+        if (eventListener->listener().isAttribute())
+            return &eventListener->listener();
     }
-    return 0;
+    return nullptr;
 }
 
 bool EventTarget::hasActiveEventListeners(const AtomicString& eventType) const
@@ -215,57 +195,51 @@ bool EventTarget::fireEventListeners(Event& event)
     ASSERT_WITH_SECURITY_IMPLICATION(!NoEventDispatchAssertion::isEventDispatchForbidden());
     ASSERT(event.isInitialized());
 
-    EventTargetData* d = eventTargetData();
-    if (!d)
+    EventTargetData* data = eventTargetData();
+    if (!data)
         return true;
 
-    EventListenerVector* legacyListenersVector = nullptr;
-    const AtomicString& legacyTypeName = legacyType(event);
-    if (!legacyTypeName.isEmpty())
-        legacyListenersVector = d->eventListenerMap.find(legacyTypeName);
-
-    EventListenerVector* listenersVector = d->eventListenerMap.find(event.type());
-
-    if (listenersVector)
-        fireEventListeners(event, d, *listenersVector);
-    else if (legacyListenersVector) {
-        AtomicString typeName = event.type();
-        event.setType(legacyTypeName);
-        fireEventListeners(event, d, *legacyListenersVector);
-        event.setType(typeName);
+    TemporaryChange<bool> firingEventListenersScope(data->isFiringEventListeners, true);
+
+    if (auto* listenersVector = data->eventListenerMap.find(event.type())) {
+        fireEventListeners(event, *listenersVector);
+        return !event.defaultPrevented();
     }
 
+    const AtomicString& legacyTypeName = legacyType(event);
+    if (!legacyTypeName.isEmpty()) {
+        if (auto* legacyListenersVector = data->eventListenerMap.find(legacyTypeName)) {
+            AtomicString typeName = event.type();
+            event.setType(legacyTypeName);
+            fireEventListeners(event, *legacyListenersVector);
+            event.setType(typeName);
+        }
+    }
     return !event.defaultPrevented();
 }
 
-void EventTarget::fireEventListeners(Event& event, EventTargetData* d, EventListenerVector& entry)
+// Intentionally creates a copy of the listeners vector to avoid event listeners added after this point from being run.
+// Note that removal still has an effect due to the removed field in RegisteredEventListener.
+void EventTarget::fireEventListeners(Event& event, EventListenerVector listeners)
 {
     Ref<EventTarget> protectedThis(*this);
-
-    // Fire all listeners registered for this event. Don't fire listeners removed during event dispatch.
-    // Also, don't fire event listeners added during event dispatch. Conveniently, all new event listeners will be added
-    // after or at index |size|, so iterating up to (but not including) |size| naturally excludes new event listeners.
-
-    size_t i = 0;
-    size_t size = entry.size();
-    if (!d->firingEventIterators)
-        d->firingEventIterators = std::make_unique<FiringEventIteratorVector>();
-    d->firingEventIterators->append(FiringEventIterator(event.type(), i, size));
+    ASSERT(!listeners.isEmpty());
 
     ScriptExecutionContext* context = scriptExecutionContext();
     Document* document = nullptr;
     InspectorInstrumentationCookie willDispatchEventCookie;
     if (is<Document>(context)) {
         document = downcast<Document>(context);
-        willDispatchEventCookie = InspectorInstrumentation::willDispatchEvent(*document, event, size > 0);
+        willDispatchEventCookie = InspectorInstrumentation::willDispatchEvent(*document, event, true);
     }
 
-    for (; i < size; ++i) {
-        RegisteredEventListener registeredListener = entry[i];
+    for (auto& registeredListener : listeners) {
+        if (UNLIKELY(registeredListener->wasRemoved()))
+            continue;
 
-        if (event.eventPhase() == Event::CAPTURING_PHASE && !registeredListener.useCapture)
+        if (event.eventPhase() == Event::CAPTURING_PHASE && !registeredListener->useCapture())
             continue;
-        if (event.eventPhase() == Event::BUBBLING_PHASE && registeredListener.useCapture)
+        if (event.eventPhase() == Event::BUBBLING_PHASE && registeredListener->useCapture())
             continue;
 
         // If stopImmediatePropagation has been called, we just break out immediately, without
@@ -274,24 +248,22 @@ void EventTarget::fireEventListeners(Event& event, EventTargetData* d, EventList
             break;
 
         // Do this before invocation to avoid reentrancy issues.
-        if (registeredListener.isOnce)
-            removeEventListener(event.type(), *registeredListener.listener, ListenerOptions(registeredListener.useCapture));
+        if (registeredListener->isOnce())
+            removeEventListener(event.type(), registeredListener->listener(), ListenerOptions(registeredListener->useCapture()));
 
-        if (registeredListener.isPassive)
+        if (registeredListener->isPassive())
             event.setInPassiveListener(true);
 
         InspectorInstrumentationCookie cookie = InspectorInstrumentation::willHandleEvent(context, event);
         // To match Mozilla, the AT_TARGET phase fires both capturing and bubbling
         // event listeners, even though that violates some versions of the DOM spec.
-        registeredListener.listener->handleEvent(context, &event);
+        registeredListener->listener().handleEvent(context, &event);
         InspectorInstrumentation::didHandleEvent(cookie);
 
-        if (registeredListener.isPassive)
+        if (registeredListener->isPassive())
             event.setInPassiveListener(false);
     }
 
-    d->firingEventIterators->removeLast();
-
     if (document)
         InspectorInstrumentation::didDispatchEvent(willDispatchEventCookie);
 }
@@ -310,15 +282,6 @@ void EventTarget::removeAllEventListeners()
     if (!d)
         return;
     d->eventListenerMap.clear();
-
-    // Notify firing events planning to invoke the listener at 'index' that
-    // they have one less listener to invoke.
-    if (d->firingEventIterators) {
-        for (auto& firingEventIterator : *d->firingEventIterators) {
-            firingEventIterator.iterator = 0;
-            firingEventIterator.size = 0;
-        }
-    }
 }
 
 } // namespace WebCore
index 43982a6..8a3b00a 100644 (file)
@@ -97,7 +97,7 @@ public:
     ~EventTargetData();
 
     EventListenerMap eventListenerMap;
-    std::unique_ptr<FiringEventIteratorVector> firingEventIterators;
+    bool isFiringEventListeners { false };
 };
 
 enum EventTargetInterface {
@@ -182,7 +182,7 @@ private:
     virtual void refEventTarget() = 0;
     virtual void derefEventTarget() = 0;
     
-    void fireEventListeners(Event&, EventTargetData*, EventListenerVector&);
+    void fireEventListeners(Event&, EventListenerVector);
 
     friend class EventListenerIterator;
 };
@@ -207,7 +207,7 @@ inline bool EventTarget::isFiringEventListeners()
     EventTargetData* d = eventTargetData();
     if (!d)
         return false;
-    return d->firingEventIterators && !d->firingEventIterators->isEmpty();
+    return d->isFiringEventListeners;
 }
 
 inline bool EventTarget::hasEventListeners() const
diff --git a/Source/WebCore/dom/RegisteredEventListener.cpp b/Source/WebCore/dom/RegisteredEventListener.cpp
deleted file mode 100644 (file)
index 3adaca3..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2001 Peter Kelly (pmk@post.com)
- * Copyright (C) 2001 Tobias Anton (anton@stud.fbi.fh-darmstadt.de)
- * Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
- * Copyright (C) 2003, 2005, 2006, 2008 Apple Inc. All rights reserved.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB.  If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#include "config.h"
-#include "RegisteredEventListener.h"
-
-
-namespace WebCore {
-
-} // namespace WebCore
index d55d9e2..c2cd4cf 100644 (file)
 
 namespace WebCore {
 
-    class RegisteredEventListener {
-    public:
-        struct Options {
-            Options(bool capture = false, bool passive = false, bool once = false)
-                : capture(capture)
-                , passive(passive)
-                , once(once)
-            { }
+// https://dom.spec.whatwg.org/#concept-event-listener
+class RegisteredEventListener : public RefCounted<RegisteredEventListener> {
+public:
+    struct Options {
+        Options(bool capture = false, bool passive = false, bool once = false)
+            : capture(capture)
+            , passive(passive)
+            , once(once)
+        { }
 
-            bool capture;
-            bool passive;
-            bool once;
-        };
+        bool capture;
+        bool passive;
+        bool once;
+    };
+
+    static Ref<RegisteredEventListener> create(Ref<EventListener>&& listener, const Options& options)
+    {
+        return adoptRef(*new RegisteredEventListener(WTFMove(listener), options));
+    }
 
-        RegisteredEventListener(Ref<EventListener>&& listener, const Options& options)
-            : listener(WTFMove(listener))
-            , useCapture(options.capture)
-            , isPassive(options.passive)
-            , isOnce(options.once)
-        {
-        }
+    EventListener& listener() const { return const_cast<EventListener&>(m_listener.get()); }
+    bool useCapture() const { return m_useCapture; }
+    bool isPassive() const { return m_isPassive; }
+    bool isOnce() const { return m_isOnce; }
+    bool wasRemoved() const { return m_wasRemoved; }
 
-        RegisteredEventListener(Ref<EventListener>&& listener, bool useCapture)
-            : listener(WTFMove(listener))
-            , useCapture(useCapture)
-        {
-        }
+    void markAsRemoved() { m_wasRemoved = true; }
 
-        RefPtr<EventListener> listener;
-        bool useCapture { false };
-        bool isPassive { false };
-        bool isOnce { false };
-    };
-    
-    inline bool operator==(const RegisteredEventListener& a, const RegisteredEventListener& b)
+private:
+    RegisteredEventListener(Ref<EventListener>&& listener, const Options& options)
+        : m_listener(WTFMove(listener))
+        , m_useCapture(options.capture)
+        , m_isPassive(options.passive)
+        , m_isOnce(options.once)
     {
-        // Other data members are purposefully not checked. The DOM specification says that upon adding / removing
-        // EventListeners, we should only check the type and the capture flag.
-        return *a.listener == *b.listener && a.useCapture == b.useCapture;
     }
 
+    Ref<EventListener> m_listener;
+    bool m_useCapture { false };
+    bool m_isPassive { false };
+    bool m_isOnce { false };
+    bool m_wasRemoved { false };
+};
+
 } // namespace WebCore
 
 #endif // RegisteredEventListener_h
index d94d84b..6443b65 100644 (file)
@@ -822,8 +822,8 @@ void InspectorDOMAgent::getEventListenersForNode(ErrorString& errorString, int n
     size_t eventInformationLength = eventInformation.size();
     for (auto& info : eventInformation) {
         for (auto& listener : info.eventListenerVector) {
-            if (listener.useCapture)
-                listenersArray->addItem(buildObjectForEventListener(listener, info.eventType, info.node, objectGroup));
+            if (listener->useCapture())
+                listenersArray->addItem(buildObjectForEventListener(*listener, info.eventType, info.node, objectGroup));
         }
     }
 
@@ -831,8 +831,8 @@ void InspectorDOMAgent::getEventListenersForNode(ErrorString& errorString, int n
     for (size_t i = eventInformationLength; i; --i) {
         const EventListenerInfo& info = eventInformation[i - 1];
         for (auto& listener : info.eventListenerVector) {
-            if (!listener.useCapture)
-                listenersArray->addItem(buildObjectForEventListener(listener, info.eventType, info.node, objectGroup));
+            if (!listener->useCapture())
+                listenersArray->addItem(buildObjectForEventListener(*listener, info.eventType, info.node, objectGroup));
         }
     }
 }
@@ -858,13 +858,13 @@ void InspectorDOMAgent::getEventListeners(Node* node, Vector<EventListenerInfo>&
         for (auto& type : d->eventListenerMap.eventTypes()) {
             const EventListenerVector& listeners = ancestor->getEventListeners(type);
             EventListenerVector filteredListeners;
-            filteredListeners.reserveCapacity(listeners.size());
+            filteredListeners.reserveInitialCapacity(listeners.size());
             for (auto& listener : listeners) {
-                if (listener.listener->type() == EventListener::JSEventListenerType)
-                    filteredListeners.append(listener);
+                if (listener->listener().type() == EventListener::JSEventListenerType)
+                    filteredListeners.uncheckedAppend(listener);
             }
             if (!filteredListeners.isEmpty())
-                eventInformation.append(EventListenerInfo(ancestor, type, filteredListeners));
+                eventInformation.append(EventListenerInfo(ancestor, type, WTFMove(filteredListeners)));
         }
     }
 }
@@ -1477,7 +1477,7 @@ RefPtr<Inspector::Protocol::Array<Inspector::Protocol::DOM::Node>> InspectorDOMA
 
 Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEventListener(const RegisteredEventListener& registeredEventListener, const AtomicString& eventType, Node* node, const String* objectGroupId)
 {
-    RefPtr<EventListener> eventListener = registeredEventListener.listener;
+    Ref<EventListener> eventListener = registeredEventListener.listener();
 
     JSC::ExecState* state = nullptr;
     JSC::JSObject* handler = nullptr;
@@ -1486,7 +1486,7 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
     int columnNumber = 0;
     String scriptID;
     String sourceName;
-    if (auto scriptListener = JSEventListener::cast(eventListener.get())) {
+    if (auto scriptListener = JSEventListener::cast(eventListener.ptr())) {
         JSC::JSLockHolder lock(scriptListener->isolatedWorld().vm());
         state = execStateFromNode(scriptListener->isolatedWorld(), &node->document());
         handler = scriptListener->jsFunction(&node->document());
@@ -1507,7 +1507,7 @@ Ref<Inspector::Protocol::DOM::EventListener> InspectorDOMAgent::buildObjectForEv
 
     auto value = Inspector::Protocol::DOM::EventListener::create()
         .setType(eventType)
-        .setUseCapture(registeredEventListener.useCapture)
+        .setUseCapture(registeredEventListener.useCapture())
         .setIsAttribute(eventListener->isAttribute())
         .setNodeId(pushNodePathToFrontend(node))
         .setHandlerBody(body)
index 3744cb1..cc9715c 100644 (file)
@@ -80,10 +80,10 @@ typedef String ErrorString;
 typedef int BackendNodeId;
 
 struct EventListenerInfo {
-    EventListenerInfo(Node* node, const AtomicString& eventType, const EventListenerVector& eventListenerVector)
+    EventListenerInfo(Node* node, const AtomicString& eventType, EventListenerVector&& eventListenerVector)
         : node(node)
         , eventType(eventType)
-        , eventListenerVector(eventListenerVector)
+        , eventListenerVector(WTFMove(eventListenerVector))
     {
     }
 
index a1d77e6..fb79bd9 100644 (file)
@@ -584,11 +584,8 @@ static bool hasLoadListener(Element* element)
         return true;
 
     for (element = element->parentOrShadowHostElement(); element; element = element->parentOrShadowHostElement()) {
-        const EventListenerVector& entry = element->getEventListeners(eventNames().loadEvent);
-        for (auto& listener : entry) {
-            if (listener.useCapture)
-                return true;
-        }
+        if (element->hasCapturingEventListeners(eventNames().loadEvent))
+            return true;
     }
 
     return false;